[PATCH 7/9] Unionfs: remove for_writepages nfs workaround

[Erez Zadok]

This is no longer necessary since struct writeback_control no longer has a
fs_private field which lower file systems (esp. nfs) use.  Plus, unionfs now
defines its own ->writepages method.

Signed-off-by: Erez Zadok <ezk@xxxxxxxxxxxxx>
---
 fs/unionfs/mmap.c |   39 ---------------------------------------
 1 files changed, 0 insertions(+), 39 deletions(-)

diff --git a/fs/unionfs/mmap.c b/fs/unionfs/mmap.c
index b43557e..bed11c3 100644
--- a/fs/unionfs/mmap.c
+++ b/fs/unionfs/mmap.c
@@ -19,39 +19,6 @@
 
 #include "union.h"
 
-/*
- * Unionfs doesn't implement ->writepages, which is OK with the VFS and
- * keeps our code simpler and smaller.  Nevertheless, somehow, our own
- * ->writepage must be called so we can sync the upper pages with the lower
- * pages: otherwise data changed at the upper layer won't get written to the
- * lower layer.
- *
- * Some lower file systems (e.g., NFS) expect the VFS to call its writepages
- * only, which in turn will call generic_writepages and invoke each of the
- * lower file system's ->writepage.  NFS in particular uses the
- * wbc->fs_private field in its nfs_writepage, which is set in its
- * nfs_writepages.  So if we don't call the lower nfs_writepages first, then
- * NFS's nfs_writepage will dereference a NULL wbc->fs_private and cause an
- * OOPS.  If, however, we implement a unionfs_writepages and then we do call
- * the lower nfs_writepages, then we "lose control" over the pages we're
- * trying to write to the lower file system: we won't be writing our own
- * new/modified data from the upper pages to the lower pages, and any
- * mmap-based changes are lost.
- *
- * This is a fundamental cache-coherency problem in Linux.  The kernel isn't
- * able to support such stacking abstractions cleanly.  One possible clean
- * way would be that a lower file system's ->writepage method have some sort
- * of a callback to validate if any upper pages for the same file+offset
- * exist and have newer content in them.
- *
- * This whole NULL ptr dereference is triggered at the lower file system
- * (NFS) because the wbc->for_writepages is set to 1.  Therefore, to avoid
- * this NULL pointer dereference, we set this flag to 0 and restore it upon
- * exit.  This probably means that we're slightly less efficient in writing
- * pages out, doing them one at a time, but at least we avoid the oops until
- * such day as Linux can better support address_space_ops in a stackable
- * fashion.
- */
 static int unionfs_writepage(struct page *page, struct writeback_control *wbc)
 {
 	int err = -EIO;
@@ -59,7 +26,6 @@ static int unionfs_writepage(struct page *page, struct writeback_control *wbc)
 	struct inode *lower_inode;
 	struct page *lower_page;
 	char *kaddr, *lower_kaddr;
-	int saved_for_writepages = wbc->for_writepages;
 
 	inode = page->mapping->host;
 	lower_inode = unionfs_lower_inode(inode);
@@ -101,14 +67,9 @@ static int unionfs_writepage(struct page *page, struct writeback_control *wbc)
 
 	BUG_ON(!lower_inode->i_mapping->a_ops->writepage);
 
-	/* workaround for some lower file systems: see big comment on top */
-	if (wbc->for_writepages && !wbc->fs_private)
-		wbc->for_writepages = 0;
-
 	/* call lower writepage (expects locked page) */
 	clear_page_dirty_for_io(lower_page); /* emulate VFS behavior */
 	err = lower_inode->i_mapping->a_ops->writepage(lower_page, wbc);
-	wbc->for_writepages = saved_for_writepages; /* restore value */
 
 	/* b/c find_lock_page locked it and ->writepage unlocks on success */
 	if (err)
-- 
1.5.2.2

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/