All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: axboe@kernel.dk
Cc: linux-fsdevel@vger.kernel.org
Subject: [PATCH 4/6] writeback: fix writeback_inodes_wb from writeback_inodes_sb
Date: Tue, 8 Jun 2010 18:14:58 +0200	[thread overview]
Message-ID: <20100608161458.GE11735@lst.de> (raw)
In-Reply-To: <20100608161424.GA11735@lst.de>

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>

Index: linux-2.6/fs/fs-writeback.c
===================================================================
--- linux-2.6.orig/fs/fs-writeback.c	2010-06-08 16:36:42.665003940 +0200
+++ linux-2.6/fs/fs-writeback.c	2010-06-08 17:10:12.958255860 +0200
@@ -518,39 +518,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
@@ -559,7 +539,7 @@ static enum sb_pin_state pin_sb_for_writ
 	}
 	sb->s_count--;
 	spin_unlock(&sb_lock);
-	return SB_PIN_FAILED;
+	return false;
 }
 
 /*
@@ -638,24 +618,29 @@ static void writeback_inodes_wb(struct b
 		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 (state == SB_PIN_FAILED) {
-			requeue_io(inode);
-			continue;
+		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;
+			}
+
+			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;
 	}

  parent reply	other threads:[~2010-06-08 16:15 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-06-08 16:14 [PATCH 0/6] writeback fixes - slow unmount and others Christoph Hellwig
2010-06-08 16:14 ` [PATCH 1/6] writeback: fix writeback completion notifications Christoph Hellwig
2010-06-08 19:50   ` Jens Axboe
2010-06-15 17:25   ` Jan Kara
2010-06-15 17:30     ` Christoph Hellwig
2010-06-08 16:14 ` [PATCH 2/6] writeback: queue work on stack in writeback_inodes_sb Christoph Hellwig
2010-06-08 19:51   ` Jens Axboe
2010-06-08 16:14 ` [PATCH 3/6] writeback: enforce s_umount locking " Christoph Hellwig
2010-06-15 17:54   ` Jan Kara
2010-06-15 17:59     ` Christoph Hellwig
2010-06-15 18:04       ` Jan Kara
2010-06-08 16:14 ` Christoph Hellwig [this message]
2010-06-08 19:51   ` [PATCH 4/6] writeback: fix writeback_inodes_wb from writeback_inodes_sb Jens Axboe
2010-06-09 12:25     ` Christoph Hellwig
2010-06-09 12:29       ` Jens Axboe
2010-06-08 16:15 ` [PATCH 5/6] writeback: simplify wakeup_flusher_threads Christoph Hellwig
2010-06-08 19:51   ` Jens Axboe
2010-06-08 16:15 ` [PATCH 6/6] writeback: simplify and split bdi_start_writeback Christoph Hellwig
2010-06-08 19:52   ` Jens Axboe

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=20100608161458.GE11735@lst.de \
    --to=hch@lst.de \
    --cc=axboe@kernel.dk \
    --cc=linux-fsdevel@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.