GIT: unionfs2-2.6.27.y: Unionfs: remove for_writepages nfs workaround

Erez Zadok ezk at fsl.cs.sunysb.edu
Thu Aug 12 23:17:18 EDT 2010


commit e7d8205acf3b00deb84c96d3a98f944eba263f21
Author: Erez Zadok <ezk at cs.sunysb.edu>
Date:   Thu Oct 18 17:16:51 2007 -0400

    Unionfs: remove for_writepages nfs workaround
    
    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 at cs.sunysb.edu>

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)


More information about the unionfs-cvs mailing list