mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Erez Zadok <ezk@cs.sunysb.edu>
To: akpm@linux-foundation.org
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
	viro@ftp.linux.org.uk, hch@infradead.org,
	Hugh Dickins <hugh@veritas.com>, Erez Zadok <ezk@cs.sunysb.edu>
Subject: [PATCH 10/30] Unionfs: mmap fixes
Date: Fri, 28 Dec 2007 15:42:44 -0500	[thread overview]
Message-ID: <1198874590842-git-send-email-ezk@cs.sunysb.edu> (raw)
In-Reply-To: <11988745841003-git-send-email-ezk@cs.sunysb.edu>

From: Hugh Dickins <hugh@veritas.com>

Remove !mapping_cap_writeback_dirty shortcircuit from unionfs_writepages.

It was introduced to avoid the stray AOP_WRITEPAGE_ACTIVATE coming from
shmem_writepage; but that has since been fixed in shmem_writepage and in
write_cache_pages.  It stayed because it looked like a good optimization,
not to waste time calling down to tmpfs when that would serve no purpose.

But in fact this optimization causes hangs when running LTP with unionfs
over tmpfs.  The problem is that the test comes at the wrong level: unionfs
has already declared in its default_backing_dev_info that it's playing by
cap_writeback_dirty rules.  If it does nothing here in its writepages, its
dirty pages accumulate and choke the system.  What's needed is to carry on
down and let its pages be cleaned while in turn they dirty the lower level.

And this now has an additional benefit for tmpfs, that a sync or pdflush
pushes these pages down to shmem_writepage, letting it match the filepage
coming from unionfs with the swap which may have been allocated earlier,
so it can free the duplication sooner than waiting for further pressure.

Remove unnecessary locking/code from prepare_write.  Handle if no lower
inodes in writepage.

Signed-off-by: Hugh Dickins <hugh@veritas.com>
Signed-off-by: Erez Zadok <ezk@cs.sunysb.edu>
---
 fs/unionfs/mmap.c |   29 +++++++++--------------------
 1 files changed, 9 insertions(+), 20 deletions(-)

diff --git a/fs/unionfs/mmap.c b/fs/unionfs/mmap.c
index 4d05352..aad2137 100644
--- a/fs/unionfs/mmap.c
+++ b/fs/unionfs/mmap.c
@@ -30,6 +30,11 @@ static int unionfs_writepage(struct page *page, struct writeback_control *wbc)
 
 	BUG_ON(!PageUptodate(page));
 	inode = page->mapping->host;
+	/* if no lower inode, nothing to do */
+	if (!inode || !UNIONFS_I(inode) || UNIONFS_I(inode)->lower_inodes) {
+		err = 0;
+		goto out;
+	}
 	lower_inode = unionfs_lower_inode(inode);
 	lower_mapping = lower_inode->i_mapping;
 
@@ -130,9 +135,6 @@ static int unionfs_writepages(struct address_space *mapping,
 	if (!lower_inode)
 		goto out;
 
-	if (!mapping_cap_writeback_dirty(lower_inode->i_mapping))
-		goto out;
-
 	err = generic_writepages(mapping, wbc);
 	if (!err)
 		unionfs_copy_attr_times(inode);
@@ -222,26 +224,13 @@ out:
 static int unionfs_prepare_write(struct file *file, struct page *page,
 				 unsigned from, unsigned to)
 {
-	int err;
-
-	unionfs_read_lock(file->f_path.dentry->d_sb);
 	/*
-	 * This is the only place where we unconditionally copy the lower
-	 * attribute times before calling unionfs_file_revalidate.  The
-	 * reason is that our ->write calls do_sync_write which in turn will
-	 * call our ->prepare_write and then ->commit_write.  Before our
-	 * ->write is called, the lower mtimes are in sync, but by the time
-	 * the VFS calls our ->commit_write, the lower mtimes have changed.
-	 * Therefore, the only reasonable time for us to sync up from the
-	 * changed lower mtimes, and avoid an invariant violation warning,
-	 * is here, in ->prepare_write.
+	 * Just copy lower inode attributes and return success.  Not much
+	 * else to do here.  No need to lock either (lockdep won't like it).
+	 * Let commit_write do all the hard work instead.
 	 */
 	unionfs_copy_attr_times(file->f_path.dentry->d_inode);
-	err = unionfs_file_revalidate(file, true);
-	unionfs_check_file(file);
-	unionfs_read_unlock(file->f_path.dentry->d_sb);
-
-	return err;
+	return 0;
 }
 
 static int unionfs_commit_write(struct file *file, struct page *page,
-- 
1.5.2.2


  parent reply	other threads:[~2007-12-28 20:48 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-12-28 20:42 [GIT PULL -mm] 00/30 Unionfs+fsstack updates/fixes/cleanups Erez Zadok
2007-12-28 20:42 ` [PATCH 01/30] VFS/fs_stack: drop cast on inode passed to i_size_read Erez Zadok
2007-12-28 20:42 ` [PATCH 02/30] VFS/fs_stack: use locking around i_size_write in 32-bit systems Erez Zadok
2007-12-28 20:42 ` [PATCH 03/30] Unionfs: " Erez Zadok
2007-12-28 20:42 ` [PATCH 04/30] Unionfs: clarify usage.txt read/write behavior Erez Zadok
2007-12-28 20:42 ` [PATCH 05/30] Unionfs: interpose cleanup and fix for spliced dentries Erez Zadok
2007-12-28 20:42 ` [PATCH 06/30] Unionfs: initialize inode times for reused inodes Erez Zadok
2007-12-28 20:42 ` [PATCH 07/30] Unionfs: create new special files only in first branch Erez Zadok
2007-12-28 20:42 ` [PATCH 08/30] Unionfs: create new symlinks " Erez Zadok
2007-12-28 20:42 ` [PATCH 09/30] Unionfs: release special files on copyup Erez Zadok
2007-12-28 20:42 ` Erez Zadok [this message]
2007-12-28 20:42 ` [PATCH 11/30] Unionfs: restructure unionfs_setattr and fix truncation order Erez Zadok
2007-12-28 20:42 ` [PATCH 12/30] Unionfs: remove custom read/write methods Erez Zadok
2007-12-28 20:42 ` [PATCH 13/30] Unionfs: prevent deadlock in cache coherency Erez Zadok
2007-12-28 20:42 ` [PATCH 14/30] Unionfs: remove unnecessary conditional inode lock Erez Zadok
2007-12-28 20:42 ` [PATCH 15/30] Unionfs: remove unnecessary lock when deleting whiteouts Erez Zadok
2007-12-28 20:42 ` [PATCH 16/30] Unionfs: remove unnecessary lock in read_inode Erez Zadok
2007-12-28 20:42 ` [PATCH 17/30] Unionfs: remove unnecessary locking in follow-link Erez Zadok
2007-12-28 20:42 ` [PATCH 18/30] Unionfs: remove unnecessary parent lock in create Erez Zadok
2007-12-28 20:42 ` [PATCH 19/30] Unionfs: prevent false lockdep warnings in stacking Erez Zadok
2007-12-28 20:42 ` [PATCH 20/30] Unionfs: implement lockdep classes Erez Zadok
2007-12-28 20:42 ` [PATCH 21/30] Unionfs: minor code rearrangement in rename Erez Zadok
2007-12-28 20:42 ` [PATCH 22/30] Unionfs: handle on lower inodes in lookup Erez Zadok
2007-12-28 20:42 ` [PATCH 23/30] Unionfs: set our superblock a/m/ctime granularity Erez Zadok
2007-12-28 20:42 ` [PATCH 24/30] Unionfs: update inode times after a successful open Erez Zadok
2007-12-28 20:42 ` [PATCH 25/30] Unionfs: minor cleanup in check_empty Erez Zadok
2007-12-28 20:43 ` [PATCH 26/30] Unionfs: initialize namelist variable in rename Erez Zadok
2007-12-28 20:43 ` [PATCH 27/30] Unionfs: cleanup lower inodes after successful unlink Erez Zadok
2007-12-28 20:43 ` [PATCH 28/30] Unionfs: don't check dentry on error Erez Zadok
2007-12-28 20:43 ` [PATCH 29/30] Unionfs: implement d_iput method Erez Zadok
2007-12-28 20:43 ` [PATCH 30/30] Unionfs: don't check parent dentries Erez Zadok

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1198874590842-git-send-email-ezk@cs.sunysb.edu \
    --to=ezk@cs.sunysb.edu \
    --cc=akpm@linux-foundation.org \
    --cc=hch@infradead.org \
    --cc=hugh@veritas.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@ftp.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®