GIT: unionfs2-2.6.27.y: Unionfs: rewrite do_unionfs_readpage to use vfs_read (bugfix)

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


commit 92a93b62b4b558b7e9e30e7c34181d4db2fa29e5
Author: Erez_Zadok <ezk at cs.sunysb.edu>
Date:   Sat Jul 14 03:36:15 2007 -0400

    Unionfs: rewrite do_unionfs_readpage to use vfs_read (bugfix)
    
    In do_unionfs_readpage, we used to call the lower file system's ->readpage.
    However, some file systems (e.g., tmpfs) don't implement ->readpage, causing
    a NULL pointer dereference under certain conditions, especially under severe
    memory pressure.  This patch reimplements do_unionfs_readpage using
    vfs_read, which makes the code simpler and more reliable, as we depend on
    the VFS to do most of the hard work (even if this implementation might be a
    bit slower).
    
    This fix also makes sense because it makes the mmap code in unionfs more
    symmetric with unionfs_commit_write --- which uses vfs_write().
    
    Signed-off-by: Erez Zadok <ezk at cs.sunysb.edu>

diff --git a/fs/unionfs/mmap.c b/fs/unionfs/mmap.c
index 56937b7..f97f6cf 100644
--- a/fs/unionfs/mmap.c
+++ b/fs/unionfs/mmap.c
@@ -114,85 +114,53 @@ out:
 static int unionfs_do_readpage(struct file *file, struct page *page)
 {
 	int err = -EIO;
-	struct dentry *dentry;
-	struct file *lower_file = NULL;
-	struct inode *inode, *lower_inode;
-	char *page_data;
-	struct page *lower_page;
-	char *lower_page_data;
+	struct file *lower_file;
+	struct inode *inode;
+	mm_segment_t old_fs;
+	char *page_data = NULL;
+	loff_t offset;
 
-	dentry = file->f_path.dentry;
 	if (UNIONFS_F(file) == NULL) {
 		err = -ENOENT;
-		goto out_err;
+		goto out;
 	}
 
 	lower_file = unionfs_lower_file(file);
-	inode = dentry->d_inode;
-	lower_inode = unionfs_lower_inode(inode);
+	/* FIXME: is this assertion right here? */
+	BUG_ON(lower_file == NULL);
 
-	lower_page = NULL;
-
-	/* find lower page (returns a locked page) */
-	lower_page = read_cache_page(lower_inode->i_mapping,
-				     page->index,
-				     (filler_t *) lower_inode->i_mapping->
-				     a_ops->readpage, (void *)lower_file);
-
-	if (IS_ERR(lower_page)) {
-		err = PTR_ERR(lower_page);
-		lower_page = NULL;
-		goto out_release;
-	}
+	inode = file->f_path.dentry->d_inode;
 
+	page_data = (char *) kmap(page);
 	/*
-	 * wait for the page data to show up
-	 * (signaled by readpage as unlocking the page)
+	 * Use vfs_read because some lower file systems don't have a
+	 * readpage method, and some file systems (esp. distributed ones)
+	 * don't like their pages to be accessed directly.  Using vfs_read
+	 * may be a little slower, but a lot safer, as the VFS does a lot of
+	 * the necessary magic for us.
 	 */
-	wait_on_page_locked(lower_page);
-	if (!PageUptodate(lower_page)) {
-		/*
-		 * call readpage() again if we returned from wait_on_page
-		 * with a page that's not up-to-date; that can happen when a
-		 * partial page has a few buffers which are ok, but not the
-		 * whole page.
-		 */
-		lock_page(lower_page);
-		err = lower_inode->i_mapping->a_ops->readpage(lower_file,
-							      lower_page);
-		if (err) {
-			lower_page = NULL;
-			goto out_release;
-		}
-
-		wait_on_page_locked(lower_page);
-		if (!PageUptodate(lower_page)) {
-			err = -EIO;
-			goto out_release;
-		}
-	}
-
-	/* map pages, get their addresses */
-	page_data = (char *)kmap(page);
-	lower_page_data = (char *)kmap(lower_page);
+	offset = lower_file->f_pos = (page->index << PAGE_CACHE_SHIFT);
+	old_fs = get_fs();
+	set_fs(KERNEL_DS);
+	err = vfs_read(lower_file, page_data, PAGE_CACHE_SIZE,
+		       &lower_file->f_pos);
+	set_fs(old_fs);
 
-	memcpy(page_data, lower_page_data, PAGE_CACHE_SIZE);
+	kunmap(page);
 
+	if (err < 0)
+		goto out;
 	err = 0;
 
-	kunmap(lower_page);
-	kunmap(page);
-
-out_release:
-	if (lower_page)
-		page_cache_release(lower_page);	/* undo read_cache_page */
+	/* if vfs_read succeeded above, sync up our times */
+	unionfs_copy_attr_times(inode);
 
+out:
 	if (err == 0)
 		SetPageUptodate(page);
 	else
 		ClearPageUptodate(page);
 
-out_err:
 	return err;
 }
 


More information about the unionfs-cvs mailing list