Use set_page_writeback and end_page_writeback
authorSimon Wilkinson <sxw@inf.ed.ac.uk>
Wed, 21 Oct 2009 22:17:15 +0000 (23:17 +0100)
committerDerrick Brashear <shadow|account-1000005@unknown>
Fri, 13 Nov 2009 03:27:07 +0000 (19:27 -0800)
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 <marc.c.dionne@gmail.com>
Tested-by: Marc Dionne <marc.c.dionne@gmail.com>
Reviewed-by: Derrick Brashear <shadow@dementia.org>

src/afs/LINUX/osi_vnodeops.c

index 553e070..1c5cd0a 100644 (file)
@@ -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)