Skip to contentSkip to conversation

writeback: requeue_io_wait() on locked buffers

PATCH New Fengguang Wu 1 participant 11 messages Dec 28, 2007 1 message not in the archive

Subsystems (3)
lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

#1 lore raw

PATCH 1/11 writeback: introduce s_more_io_wait for inodes to be re-synced after a while

Introduce super_block.s_more_io_wait and writeback_control.more_io_wait. They are for inodes that for some reason cannot be synced immediately.

These inodes will be moved to s_more_io_wait and retried after a while(waiting up to 0.1s). The normal lots-of-dirty-pages inodes will be moved to more_io and be synced after other inodes in s_io have been serviced(no sleep).

The new data flow is now simple and fair:
- to fill s_io:

		s_more_io +
		s_dirty(expired) +
		s_more_io_wait
				---> s_io
- to drain s_io:
		s_io -+--> clean inodes in inode_in_use/inode_unused
		      |
		      +--> s_more_io
		      |
		      +--> s_more_io_wait

- s_dirty is now a strict FIFO queue
- inode.dirtied_when now really means the first dirty time
- once exipired, the dirty inode will stay in s_*io* queues until made clean
- the dirty inodes in s_*io* queues will be revisted in order: no starvation

Cc Michael Rubin <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 fs/fs-writeback.c         |   22 +++++++++++---
 fs/super.c                |    1 
 include/linux/fs.h        |    1 
 include/linux/writeback.h |    1 
 mm/page-writeback.c       |   56 ++++++++++++++++++++----------------
 5 files changed, 53 insertions(+), 28 deletions(-)
fs/fs-writeback.c +18 -4
@@ -170,10 +170,18 @@ static void redirty_tail(struct inode *i
 static void requeue_io(struct inode *inode)
 {
 	list_move(&inode->i_list, &inode->i_sb->s_more_io);
 }
 
+/*
+ * The inode should be retried after _sleeping_ for a while.
+ */
+static void requeue_io_wait(struct inode *inode)
+{
+	list_move(&inode->i_list, &inode->i_sb->s_more_io_wait);
+}
+
 static void inode_sync_complete(struct inode *inode)
 {
 	/*
 	 * Prevent speculative execution through spin_unlock(&inode_lock);
 	 */
@@ -204,17 +212,19 @@ static void move_expired_inodes(struct l
 static void queue_io(struct super_block *sb,
 				unsigned long *older_than_this)
 {
 	list_splice_init(&sb->s_more_io, sb->s_io.prev);
 	move_expired_inodes(&sb->s_dirty, &sb->s_io, older_than_this);
+	list_splice_init(&sb->s_more_io_wait, sb->s_io.prev);
 }
 
 int sb_has_dirty_inodes(struct super_block *sb)
 {
-	return !list_empty(&sb->s_dirty) ||
-	       !list_empty(&sb->s_io) ||
-	       !list_empty(&sb->s_more_io);
+	return !list_empty(&sb->s_dirty)   ||
+	       !list_empty(&sb->s_io)      ||
+	       !list_empty(&sb->s_more_io) ||
+	       !list_empty(&sb->s_more_io_wait);
 }
 EXPORT_SYMBOL(sb_has_dirty_inodes);
 
 /*
  * Write a single inode's dirty pages and inode data out to disk.
@@ -470,15 +480,19 @@ int generic_sync_sb_inodes(struct super_
 		}
 		spin_unlock(&inode_lock);
 		iput(inode);
 		cond_resched();
 		spin_lock(&inode_lock);
-		if (wbc->nr_to_write <= 0)
+		if (wbc->nr_to_write <= 0) {
+			wbc->more_io = 1;
 			break;
+		}
 	}
 	if (!list_empty(&sb->s_more_io))
 		wbc->more_io = 1;
+	if (!list_empty(&sb->s_more_io_wait))
+		wbc->more_io_wait = 1;
 	spin_unlock(&inode_lock);
 	return ret;		/* Leave any unwritten inodes on s_io */
 }
 EXPORT_SYMBOL(generic_sync_sb_inodes);
 
mm/page-writeback.c +32 -24

107 lines not on this page · raw message

fs/super.c +1 -0
@@ -62,10 +62,11 @@ static struct super_block *alloc_super(s
 			goto out;
 		}
 		INIT_LIST_HEAD(&s->s_dirty);
 		INIT_LIST_HEAD(&s->s_io);
 		INIT_LIST_HEAD(&s->s_more_io);
+		INIT_LIST_HEAD(&s->s_more_io_wait);
 		INIT_LIST_HEAD(&s->s_files);
 		INIT_LIST_HEAD(&s->s_instances);
 		INIT_HLIST_HEAD(&s->s_anon);
 		INIT_LIST_HEAD(&s->s_inodes);
 		init_rwsem(&s->s_umount);
include/linux/fs.h +1 -0
@@ -1009,10 +1009,11 @@ struct super_block {
 
 	struct list_head	s_inodes;	/* all inodes */
 	struct list_head	s_dirty;	/* dirty inodes */
 	struct list_head	s_io;		/* parked for writeback */
 	struct list_head	s_more_io;	/* parked for more writeback */
+	struct list_head	s_more_io_wait;	/* parked for sleep-then-retry */
 	struct hlist_head	s_anon;		/* anonymous dentries for (nfs) exporting */
 	struct list_head	s_files;
 
 	struct block_device	*s_bdev;
 	struct mtd_info		*s_mtd;
include/linux/writeback.h +1 -0
@@ -61,10 +61,11 @@ struct writeback_control {
 	unsigned for_kupdate:1;		/* A kupdate writeback */
 	unsigned for_reclaim:1;		/* Invoked from the page allocator */
 	unsigned for_writepages:1;	/* This is a writepages() call */
 	unsigned range_cyclic:1;	/* range_start is cyclic */
 	unsigned more_io:1;		/* more io to be dispatched */
+	unsigned more_io_wait:1;	/* more io to be dispatched after a while */
 };
 
 /*
  * fs/fs-writeback.c
  */	
#2 lore raw

PATCH 2/11 writeback: requeue_io_wait() on blocked kupdate

In reply to a message not in the archive

Convert to requeue_io_wait() for case:

kupdate cannot write all pages because of uncontrollable bad conditions.

Cc Michael Rubin <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 fs/fs-writeback.c |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
fs/fs-writeback.c +6 -1
@@ -292,11 +292,16 @@ __sync_single_inode(struct inode *inode,
 				 * For the kupdate function we move the inode
 				 * to s_more_io so it will get more writeout as
 				 * soon as the queue becomes uncongested.
 				 */
 				inode->i_state |= I_DIRTY_PAGES;
-				requeue_io(inode);
+				if (wbc->nr_to_write > 0)
+					/* somehow blocked: wait and retry */
+					requeue_io_wait(inode);
+				else
+					/* slice used up: queue for next turn */
+					requeue_io(inode);
 			} else {
 				/*
 				 * Otherwise fully redirty the inode so that
 				 * other inodes on this superblock will get some
 				 * writeout.  Otherwise heavy writing to one
#3 lore raw

PATCH 3/11 writeback: replace redirty_tail() on more-pages-to-sync

In reply to a message not in the archive

Replace redirty_tail() with requeue_io()/requeue_io_wait() when there are more pages to sync. There won't be starvation.

Cc Michael Rubin <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 fs/fs-writeback.c |   43 ++++++++-----------------------------------
 1 file changed, 8 insertions(+), 35 deletions(-)
fs/fs-writeback.c +8 -35
@@ -273,46 +273,19 @@ __sync_single_inode(struct inode *inode,
 	if (!(inode->i_state & I_FREEING)) {
 		if (!(inode->i_state & I_DIRTY) &&
 		    mapping_tagged(mapping, PAGECACHE_TAG_DIRTY)) {
 			/*
 			 * We didn't write back all the pages.  nfs_writepages()
-			 * sometimes bales out without doing anything. Redirty
-			 * the inode; Move it from s_io onto s_more_io/s_dirty.
+			 * sometimes bales out without doing anything.
 			 */
-			/*
-			 * akpm: if the caller was the kupdate function we put
-			 * this inode at the head of s_dirty so it gets first
-			 * consideration.  Otherwise, move it to the tail, for
-			 * the reasons described there.  I'm not really sure
-			 * how much sense this makes.  Presumably I had a good
-			 * reasons for doing it this way, and I'd rather not
-			 * muck with it at present.
-			 */
-			if (wbc->for_kupdate) {
-				/*
-				 * For the kupdate function we move the inode
-				 * to s_more_io so it will get more writeout as
-				 * soon as the queue becomes uncongested.
-				 */
-				inode->i_state |= I_DIRTY_PAGES;
-				if (wbc->nr_to_write > 0)
-					/* somehow blocked: wait and retry */
-					requeue_io_wait(inode);
-				else
-					/* slice used up: queue for next turn */
-					requeue_io(inode);
-			} else {
-				/*
-				 * Otherwise fully redirty the inode so that
-				 * other inodes on this superblock will get some
-				 * writeout.  Otherwise heavy writing to one
-				 * file would indefinitely suspend writeout of
-				 * all the other files.
-				 */
-				inode->i_state |= I_DIRTY_PAGES;
-				redirty_tail(inode);
-			}
+			inode->i_state |= I_DIRTY_PAGES;
+			if (wbc->nr_to_write > 0)
+				/* somehow blocked: wait and retry */
+				requeue_io_wait(inode);
+			else
+				/* slice used up: queue for next turn */
+				requeue_io(inode);
 		} else if (inode->i_state & I_DIRTY) {
 			/*
 			 * Someone redirtied the inode while were writing back
 			 * the pages.
 			 */
#4 lore raw

PATCH 4/11 writeback: requeue_io_wait() on locked inode

In reply to a message not in the archive

Convert to requeue_io_wait() for case:

inode is locked

Cc Michael Rubin <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 fs/fs-writeback.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
fs/fs-writeback.c +2 -5
@@ -322,16 +322,13 @@ __writeback_single_inode(struct inode *i
 		WARN_ON(inode->i_state & I_WILL_FREE);
 
 	if ((wbc->sync_mode != WB_SYNC_ALL) && (inode->i_state & I_SYNC)) {
 		/*
 		 * We're skipping this inode because it's locked, and we're not
-		 * doing writeback-for-data-integrity.  Move it to s_more_io so
-		 * that writeback can proceed with the other inodes on s_io.
-		 * We'll have another go at writing back this inode when we
-		 * completed a full scan of s_io.
+		 * doing writeback-for-data-integrity. Recheck it after a while.
 		 */
-		requeue_io(inode);
+		requeue_io_wait(inode);
 		return 0;
 	}
 
 	/*
 	 * It's a data-integrity sync.  We must wait.
#5 lore raw

PATCH 5/11 writeback: requeue_io_wait() on congested blockdev

In reply to a message not in the archive

Convert to requeue_io_wait() for case:

congested blockdev

Cc Michael Rubin <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 fs/fs-writeback.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/fs-writeback.c +1 -1
@@ -413,11 +413,11 @@ int generic_sync_sb_inodes(struct super_
 
 		if (wbc->nonblocking && bdi_write_congested(bdi)) {
 			wbc->encountered_congestion = 1;
 			if (!sb_is_blkdev_sb(sb))
 				break;		/* Skip a congested fs */
-			requeue_io(inode);
+			requeue_io_wait(inode);
 			continue;		/* Skip a congested blockdev */
 		}
 
 		if (wbc->bdi && bdi != wbc->bdi) {
 			if (!sb_is_blkdev_sb(sb))
#6 lore raw

PATCH 6/11 writeback: requeue_io_wait() on locked buffers

In reply to a message not in the archive

Convert to requeue_io_wait() for case:

pages skipped due to locked buffers

Cc Michael Rubin <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 fs/fs-writeback.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/fs-writeback.c +1 -1
@@ -449,11 +449,11 @@ int generic_sync_sb_inodes(struct super_
 		if (wbc->pages_skipped != pages_skipped) {
 			/*
 			 * writeback is not making progress due to locked
 			 * buffers.  Skip this inode for now.
 			 */
-			redirty_tail(inode);
+			requeue_io_wait(inode);
 		}
 		spin_unlock(&inode_lock);
 		iput(inode);
 		cond_resched();
 		spin_lock(&inode_lock);
#7 lore raw

PATCH 7/11 writeback: requeue_io() on redirtied inode

In reply to a message not in the archive

Redirtied inodes could be seen in really fast writes. They should really be synced as soon as possible.

redirty_tail() could delay the inode for up to 30s. Kill the delay by using requeue_io() instead.

Cc Michael Rubin <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 fs/fs-writeback.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/fs-writeback.c +1 -1
@@ -287,11 +287,11 @@ __sync_single_inode(struct inode *inode,
 		} else if (inode->i_state & I_DIRTY) {
 			/*
 			 * Someone redirtied the inode while were writing back
 			 * the pages.
 			 */
-			redirty_tail(inode);
+			requeue_io(inode);
 		} else if (atomic_read(&inode->i_count)) {
 			/*
 			 * The inode is clean, inuse
 			 */
 			list_move(&inode->i_list, &inode_in_use);
#8 lore raw

PATCH 8/11 writeback: introduce queue_dirty()

In reply to a message not in the archive

Introduce queue_dirty() to enqueue a newly dirtied inode. It helps remove duplicate code.

Cc Michael Rubin <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 fs/fs-writeback.c |   21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)
fs/fs-writeback.c +13 -8
@@ -23,10 +23,19 @@
 #include <linux/blkdev.h>
 #include <linux/backing-dev.h>
 #include <linux/buffer_head.h>
 #include "internal.h"
 
+/*
+ * Enqueue a newly dirtied inode.
+ */
+static void queue_dirty(struct inode *inode)
+{
+	inode->dirtied_when = jiffies;
+	list_move(&inode->i_list, &inode->i_sb->s_dirty);
+}
+
 /**
  *	__mark_inode_dirty -	internal function
  *	@inode: inode to mark
  *	@flags: what kind of dirty (i.e. I_DIRTY_SYNC)
  *	Mark an inode as dirty. Callers should use mark_inode_dirty or
@@ -120,14 +129,12 @@ void __mark_inode_dirty(struct inode *in
 
 		/*
 		 * If the inode was already on s_dirty/s_io/s_more_io, don't
 		 * reposition it (that would break s_dirty time-ordering).
 		 */
-		if (!was_dirty) {
-			inode->dirtied_when = jiffies;
-			list_move(&inode->i_list, &sb->s_dirty);
-		}
+		if (!was_dirty)
+			queue_dirty(inode);
 	}
 out:
 	spin_unlock(&inode_lock);
 }
 
@@ -438,14 +445,12 @@ int generic_sync_sb_inodes(struct super_
 		__iget(inode);
 		pages_skipped = wbc->pages_skipped;
 		err = __writeback_single_inode(inode, wbc);
 		if (!ret)
 			ret = err;
-		if (wbc->sync_mode == WB_SYNC_HOLD) {
-			inode->dirtied_when = jiffies;
-			list_move(&inode->i_list, &sb->s_dirty);
-		}
+		if (wbc->sync_mode == WB_SYNC_HOLD)
+			queue_dirty(inode);
 		if (current_is_pdflush())
 			writeback_release(bdi);
 		if (wbc->pages_skipped != pages_skipped) {
 			/*
 			 * writeback is not making progress due to locked
#9 lore raw

PATCH 9/11 writeback: replace redirty_tail() on memory-backed bdi

In reply to a message not in the archive

Replace redirty_tail() with queue_dirty() on memory backed bdi.

Cc Michael Rubin <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 fs/fs-writeback.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
fs/fs-writeback.c +1 -1
@@ -400,11 +400,11 @@ int generic_sync_sb_inodes(struct super_
 		struct backing_dev_info *bdi = mapping->backing_dev_info;
 		long pages_skipped;
 		int err;
 
 		if (!bdi_cap_writeback_dirty(bdi)) {
-			redirty_tail(inode);
+			queue_dirty(inode);
 			if (sb_is_blkdev_sb(sb)) {
 				/*
 				 * Dirty memory-backed blockdev: the ramdisk
 				 * driver does this.  Skip just this inode
 				 */
#10 lore raw

PATCH 10/11 writeback: remove redirty_tail()

In reply to a message not in the archive

Remove redirty_tail(). It's no longer used.

Cc Michael Rubin <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 fs/fs-writeback.c |   24 ------------------------
 1 file changed, 24 deletions(-)
fs/fs-writeback.c +0 -24
@@ -146,34 +146,10 @@ static int write_inode(struct inode *ino
 		return inode->i_sb->s_op->write_inode(inode, sync);
 	return 0;
 }
 
 /*
- * Redirty an inode: set its when-it-was dirtied timestamp and move it to the
- * furthest end of its superblock's dirty-inode list.
- *
- * Before stamping the inode's ->dirtied_when, we check to see whether it is
- * already the most-recently-dirtied inode on the s_dirty list.  If that is
- * the case then the inode must have been redirtied while it was being written
- * out and we don't reset its dirtied_when.
- */
-static void redirty_tail(struct inode *inode)
-{
-	struct super_block *sb = inode->i_sb;
-
-	if (!list_empty(&sb->s_dirty)) {
-		struct inode *tail_inode;
-
-		tail_inode = list_entry(sb->s_dirty.next, struct inode, i_list);
-		if (!time_after_eq(inode->dirtied_when,
-				tail_inode->dirtied_when))
-			inode->dirtied_when = jiffies;
-	}
-	list_move(&inode->i_list, &sb->s_dirty);
-}
-
-/*
  * requeue inode for re-scanning after sb->s_io list is exhausted.
  */
 static void requeue_io(struct inode *inode)
 {
 	list_move(&inode->i_list, &inode->i_sb->s_more_io);
#11 lore raw

PATCH 11/11 writeback: __sync_single_inode() code cleanup

In reply to a message not in the archive

Make the if-else straight in __sync_single_inode(). No behavior change.

Cc Michael Rubin <[email protected]>
Cc Peter Zijlstra <[email protected]>
---
 fs/fs-writeback.c |   15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)
fs/fs-writeback.c +7 -8
@@ -248,37 +248,36 @@ __sync_single_inode(struct inode *inode,
 	if (wait) {
 		int err = filemap_fdatawait(mapping);
 		if (ret == 0)
 			ret = err;
 	}
 
 	spin_lock(&inode_lock);
 	inode->i_state &= ~I_SYNC;
 	if (!(inode->i_state & I_FREEING)) {
-		if (!(inode->i_state & I_DIRTY) &&
-		    mapping_tagged(mapping, PAGECACHE_TAG_DIRTY)) {
+		if (inode->i_state & I_DIRTY) {
+			/*
+			 * Someone redirtied the inode while were writing back
+			 * the pages.
+			 */
+			requeue_io(inode);
+		} else if (mapping_tagged(mapping, PAGECACHE_TAG_DIRTY)) {
 			/*
 			 * We didn't write back all the pages.  nfs_writepages()
 			 * sometimes bales out without doing anything.
 			 */
 			inode->i_state |= I_DIRTY_PAGES;
 			if (wbc->nr_to_write > 0)
 				/* somehow blocked: wait and retry */
 				requeue_io_wait(inode);
 			else
 				/* slice used up: queue for next turn */
 				requeue_io(inode);
-		} else if (inode->i_state & I_DIRTY) {
-			/*
-			 * Someone redirtied the inode while were writing back
-			 * the pages.
-			 */
-			requeue_io(inode);
 		} else if (atomic_read(&inode->i_count)) {
 			/*
 			 * The inode is clean, inuse
 			 */
 			list_move(&inode->i_list, &inode_in_use);
 		} else {
 			/*
 			 * The inode is clean, unused
 			 */