Writeback fix for concurrent large and small file writes

Quoted 1 line

From [email protected] Wed Nov 28 11:10:06 2007

Message-Id: <[email protected]>
Date: Wed, 28 Nov 2007 11:01:21 -0800
From: [email protected]
To: [email protected]
Subject: [patch 1/1] Writeback fix for concurrent large and small file writes.

From: Michael Rubin <[email protected]>

Fixing a bug where writing to large files while concurrently writing to smaller ones creates a situation where writeback cannot keep up with the

traffic and memory baloons until the we hit the threshold watermark. This can result in surprising latency spikes when syncing. This latency can take minutes on large memory systems. Upon request I can provide a test to reproduce this situation. The flush tree fixes this issue and fixes several other minor issues with fairness also.

1) Adding a data structure to guarantee fairness when writing inodes to disk. The flush_tree is based on an rbtree. The only difference is how duplicate keys are chained off the same rb_node.

2) Added a FS flag to mark file systems that are not disk backed so we don't have to flush them. Not sure I marked all of them. But just marking these improves writeback performance.

3) Added an inode flag to allow inodes to be marked so that they are never written back to disk. See get_pipe_inode.

Under autotest this patch has passed: fsx, bonnie, and iozone. I am currently writing more writeback focused tests (which so far have been passed) to add into autotest.

Signed-off-by Michael Rubin <[email protected]>

---

Quoted 11 lines · Michael Rubin

Quoted 1 line

From [email protected] Wed Nov 28 11:10:06 2007

Message-Id: <[email protected]>
Date: Wed, 28 Nov 2007 11:01:21 -0800
From: [email protected]
To: [email protected]
Subject: [patch 1/1] Writeback fix for concurrent large and small file writes.

From: Michael Rubin <[email protected]>

Fixing a bug where writing to large files while concurrently writing to smaller ones creates a situation where writeback cannot keep up with the

Could you demonstrate the situation? Or if I guess it right, could it be fixed by the following patch? (not a nack: If so, your patch could also be considered as a general purpose improvement, instead of a bug fix.)

fs/fs-writeback.c +1 -1

index 0fca820..62e62e2 100644
@@ -301,7 +301,7 @@ __sync_single_inode(struct inode *inode, struct writeback_control *wbc)
                         * 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

Thank you,
Fengguang

Quoted 23 lines

traffic and memory baloons until the we hit the threshold watermark. This can result in surprising latency spikes when syncing. This latency can take minutes on large memory systems. Upon request I can provide a test to reproduce this situation. The flush tree fixes this issue and fixes several other minor issues with fairness also.

1) Adding a data structure to guarantee fairness when writing inodes to disk. The flush_tree is based on an rbtree. The only difference is how duplicate keys are chained off the same rb_node.

2) Added a FS flag to mark file systems that are not disk backed so we don't have to flush them. Not sure I marked all of them. But just marking these improves writeback performance.

3) Added an inode flag to allow inodes to be marked so that they are never written back to disk. See get_pipe_inode.

Under autotest this patch has passed: fsx, bonnie, and iozone. I am currently writing more writeback focused tests (which so far have been passed) to add into autotest.

Signed-off-by Michael Rubin <[email protected]>

---

Could you demonstrate the situation? Or if I guess it right, could it be fixed by the following patch? (not a nack: If so, your patch could also be considered as a general purpose improvement, instead of a bug fix.)

fs/fs-writeback.c +0 -0

index 0fca820..62e62e2 100644

Due to my faux pas of top posting (see
http://www.zip.com.au/~akpm/linux/patches/stuff/top-posting.txt) I am resending this email.

Quoted 11 lines · Fengguang Wu

Could you demonstrate the situation? Or if I guess it right, could it be fixed by the following patch? (not a nack: If so, your patch could also be considered as a general purpose improvement, instead of a bug fix.)

fs/fs-writeback.c +0 -0

index 0fca820..62e62e2 100644

fs/fs-writeback.c __sync_single_inode(struct inode *inode, struct writeback_control *wbc) · line 307

Quoted 8 lines

fs/fs-writeback.c +1 -1

@@ -301,7 +301,7 @@ __sync_single_inode(struct inode *inode, struct writeback_control *wbc)
                         * 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

By testing the situation I can confirm that the one line patch above fixes the problem.

I will continue testing some other cases to see if it cause any other issues but I don't expect it to.

I will post this change for 2.6.24 and list Feng as author. If that's ok with Feng.

As for the original patch I will resubmit it for 2.6.25 as a general purpose improvement.

Quoted 3 lines · Fengguang Wu

Quoted 2 lines

Could you demonstrate the situation? Or if I guess it right, could it be fixed by the following patch?

One major concern could be whether a continuous writer dirting pages at the 'right' pace will generate a steady flow of write I/Os which are _tiny_hence_inefficient_.

So it's not a problem in *theory* :-)

Quoted 2 lines

I will post this change for 2.6.24 and list Feng as author. If that's ok with Feng.

+ * The flush tree organizes the dirtied_when keys with the rb_tree. Any
+ * inodes with a duplicate dirtied_when value are link listed together. This + * link list is sorted by the inode's i_flushed_when. When both the
+ * dirited_when and the i_flushed_when are indentical the order in the
+ * linked list determines the order we flush the inodes.

+ * Here is where we interate to find the next inode to process. The
+ * strategy is to first look for any other inodes with the same dirtied_when + * value. If we have already processed that node then we need to find + * the next highest dirtied_when value in the tree.

Two typos in comments.

Cheers,
FJP

Quoted 5 lines · Michael Rubin

+ * The flush tree organizes the dirtied_when keys with the rb_tree. Any
+ * inodes with a duplicate dirtied_when value are link listed together. This + * link list is sorted by the inode's i_flushed_when. When both the
+ * dirited_when and the i_flushed_when are indentical the order in the
+ * linked list determines the order we flush the inodes.

s/dirited_when/dirtied_when/

Quoted 4 lines

+ * Here is where we interate to find the next inode to process. The
+ * strategy is to first look for any other inodes with the same dirtied_when + * value. If we have already processed that node then we need to find + * the next highest dirtied_when value in the tree.

s/interate/iterate/