From 5e99d56cf2a8c4c9789bc7ace04a804d07e6238f Mon Sep 17 00:00:00 2001 From: Simon Wilkinson Date: Wed, 21 Oct 2009 23:17:15 +0100 Subject: [PATCH 1/1] Use set_page_writeback and end_page_writeback Calling set_page_writeback and end_page_writeback is necessary to ensure that the dirty page radix tree and the page dirty flags tally. The results of end_page_writeback are also used by the bdi code to prioritise writeback. The Linux kernel documentation contains further warnings of doom for what may happen due to not calling them. Adding set_page_writeback and end_page_writeback also allows us to unlock the page earlier (the page can be locked any time after the writeback flag is set). This means that we're not calling the backing filesystem's ->write function with our pages locked, and should help reduce contention and the potential for deadlocks there. Change-Id: I9130b2ad9a09c6b9b16a0f63d7b4a614a93de8d3 Reviewed-on: http://gerrit.openafs.org/819 Reviewed-by: Marc Dionne Tested-by: Marc Dionne Reviewed-by: Derrick Brashear --- src/afs/LINUX/osi_vnodeops.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-) diff --git a/src/afs/LINUX/osi_vnodeops.c b/src/afs/LINUX/osi_vnodeops.c index 553e070..1c5cd0a 100644 --- a/src/afs/LINUX/osi_vnodeops.c +++ b/src/afs/LINUX/osi_vnodeops.c @@ -2127,17 +2127,23 @@ afs_linux_writepage(struct page *pp) isize = i_size_read(inode); /* Don't defeat an earlier truncate */ - if (page_offset(pp) > isize) + if (page_offset(pp) > isize) { + set_page_writeback(pp); + unlock_page(pp); goto done; + } AFS_GLOCK(); ObtainWriteLock(&vcp->lock, 537); code = afs_linux_prepare_writeback(vcp); - if (code) { + if (code == AOP_WRITEPAGE_ACTIVATE) { + /* WRITEPAGE_ACTIVATE is the only return value that permits us + * to return with the page still locked */ ReleaseWriteLock(&vcp->lock); AFS_GUNLOCK(); return code; } + /* Grab the creds structure currently held in the vnode, and * get a reference to it, in case it goes away ... */ credp = vcp->cred; @@ -2145,6 +2151,16 @@ afs_linux_writepage(struct page *pp) ReleaseWriteLock(&vcp->lock); AFS_GUNLOCK(); + set_page_writeback(pp); + + SetPageUptodate(pp); + + /* We can unlock the page here, because it's protected by the + * page_writeback flag. This should make us less vulnerable to + * deadlocking in afs_write and afs_DoPartialWrite + */ + unlock_page(pp); + /* If this is the final page, then just write the number of bytes that * are actually in it */ if ((isize - page_offset(pp)) < to ) @@ -2171,12 +2187,7 @@ afs_linux_writepage(struct page *pp) afs_maybe_unlock_kernel(); done: - SetPageUptodate(pp); - if ( code != AOP_WRITEPAGE_ACTIVATE ) { - /* XXX - do we need to redirty the page here? */ - unlock_page(pp); - } - + end_page_writeback(pp); page_cache_release(pp); if (code1) -- 1.9.4