Patchwork [08/16] writeback: fix writeback_inodes_wb from writeback_inodes_sb

login
register
mail settings
Submitter Stefan Bader
Date Aug. 10, 2010, 9:28 a.m.
Message ID <1281432544-20831-9-git-send-email-stefan.bader@canonical.com>
Download mbox | patch
Permalink /patch/61357/
State Accepted
Delegated to: Stefan Bader
Headers show

Comments

Stefan Bader - Aug. 10, 2010, 9:28 a.m.
From: Christoph Hellwig <hch@lst.de>

BugLink: http://bugs.launchpad.net/bugs/585092

commit d19de7edf59cdd586777b009e0e8fbe5412dd35f upstream

When we call writeback_inodes_wb from writeback_inodes_sb we always have
s_umount held, which currently makes the whole operation a no-op.

But if we are called to write out inodes for a specific superblock we always
have s_umount held, so replace the incorrect logic checking for WB_SYNC_ALL
which only worked by coincidence with the proper check for an explicit
superblock argument.

Signed-off-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Jens Axboe <jaxboe@fusionio.com>
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
---
 fs/fs-writeback.c |   61 ++++++++++++++++++++---------------------------------
 1 files changed, 23 insertions(+), 38 deletions(-)

Patch

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 7f91e6f..71f53f9 100644
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -506,39 +506,19 @@  select_queue:
 	return ret;
 }
 
-static void unpin_sb_for_writeback(struct super_block *sb)
-{
-	up_read(&sb->s_umount);
-	put_super(sb);
-}
-
-enum sb_pin_state {
-	SB_PINNED,
-	SB_NOT_PINNED,
-	SB_PIN_FAILED
-};
-
 /*
- * For WB_SYNC_NONE writeback, the caller does not have the sb pinned
+ * For background writeback the caller does not have the sb pinned
  * before calling writeback. So make sure that we do pin it, so it doesn't
  * go away while we are writing inodes from it.
  */
-static enum sb_pin_state pin_sb_for_writeback(struct writeback_control *wbc,
-					      struct super_block *sb)
+static bool pin_sb_for_writeback(struct super_block *sb)
 {
-	/*
-	 * Caller must already hold the ref for this
-	 */
-	if (wbc->sync_mode == WB_SYNC_ALL) {
-		WARN_ON(!rwsem_is_locked(&sb->s_umount));
-		return SB_NOT_PINNED;
-	}
 	spin_lock(&sb_lock);
 	sb->s_count++;
 	if (down_read_trylock(&sb->s_umount)) {
 		if (sb->s_root) {
 			spin_unlock(&sb_lock);
-			return SB_PINNED;
+			return true;
 		}
 		/*
 		 * umounted, drop rwsem again and fall through to failure
@@ -547,7 +527,7 @@  static enum sb_pin_state pin_sb_for_writeback(struct writeback_control *wbc,
 	}
 	sb->s_count--;
 	spin_unlock(&sb_lock);
-	return SB_PIN_FAILED;
+	return false;
 }
 
 /*
@@ -626,24 +606,29 @@  static void writeback_inodes_wb(struct bdi_writeback *wb,
 		struct inode *inode = list_entry(wb->b_io.prev,
 						 struct inode, i_list);
 		struct super_block *sb = inode->i_sb;
-		enum sb_pin_state state;
 
-		if (wbc->sb && sb != wbc->sb) {
-			/* super block given and doesn't
-			   match, skip this inode */
-			redirty_tail(inode);
-			continue;
-		}
-		state = pin_sb_for_writeback(wbc, sb);
+		if (wbc->sb) {
+			/*
+			 * We are requested to write out inodes for a specific
+			 * superblock.  This means we already have s_umount
+			 * taken by the caller which also waits for us to
+			 * complete the writeout.
+			 */
+			if (sb != wbc->sb) {
+				redirty_tail(inode);
+				continue;
+			}
 
-		if (state == SB_PIN_FAILED) {
-			requeue_io(inode);
-			continue;
+			WARN_ON(!rwsem_is_locked(&sb->s_umount));
+
+			ret = writeback_sb_inodes(sb, wb, wbc);
+		} else {
+			if (!pin_sb_for_writeback(sb))
+				continue;
+			ret = writeback_sb_inodes(sb, wb, wbc);
+			drop_super(sb);
 		}
-		ret = writeback_sb_inodes(sb, wb, wbc);
 
-		if (state == SB_PINNED)
-			unpin_sb_for_writeback(sb);
 		if (ret)
 			break;
 	}