Linux: Drop PageReclaim AOP_WRITEPAGE_ACTIVATE case
authorMarc Dionne <marc.dionne@your-file-system.com>
Mon, 5 May 2014 17:33:10 +0000 (13:33 -0400)
committerD Brashear <shadow@your-file-system.com>
Mon, 2 Jun 2014 14:33:51 +0000 (10:33 -0400)
The exit case here seems to have been added to avoid recursion into
the writeback code and eventual deadlock (see RT #15239).  One issue
is that the PageReclaim check can trigger in code paths that don't
deal with the AOP_WRITEPAGE_ACTIVATE code correctly, leading to EIO
errors when multiple threads are doing large mmap writes and memory
pressure is sufficient to trigger reclaim.

The check could be improved to check wbc.for_reclaim which seems to
indicate more reliably when it is safe to return ACTIVATE, but given
that the CPageWrite flag already provides more targeted recursion
prevention, it seems safer to just drop this special case.

Note that many kernel filesystems used to have a similar check mainly
to prevent excessive stack usage, but as more recent kernels have
moved away from doing any writeback during direct reclaim this is a
case that should no longer occur.  Partly as a result of this there
are very few users of AOP_WRITEPAGE_ACTIVATE left in the kernel,
which may be a motivation to find a better mechanism for OpenAFS
eventually.

This has been shown to help avoid EIO errors with multiple processes
doing intensive mmap writing.

Thanks to Yadav Yadavendra for identifying the issue and providing
extensive analysis and testing.

Change-Id: I88d9ef6e6e7a8f666f82c5ca481254839c2ba1e5
Reviewed-on: http://gerrit.openafs.org/11125
Reviewed-by: D Brashear <shadow@your-file-system.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

src/afs/LINUX/osi_vnodeops.c

index 30e6f30..6f4000b 100644 (file)
@@ -2665,11 +2665,6 @@ afs_linux_writepage(struct page *pp)
     int code = 0;
     int code1 = 0;
 
-    if (PageReclaim(pp)) {
-       return AOP_WRITEPAGE_ACTIVATE;
-       /* XXX - Do we need to redirty the page here? */
-    }
-
     page_cache_get(pp);
 
     inode = mapping->host;