writeback-do-not-sync-data-dirtied-after-sync-start-fix-2.txt

On Wed 09-10-13 14:21:25, Andrew Morton wrote:
> On Wed, 9 Oct 2013 17:03:25 +0200 Jan Kara <jack@suse.cz> wrote:
>
> > From: Jan Kara <jack@suse.cz>
> > Date: Wed, 9 Oct 2013 15:41:50 +0200
> > Subject: [PATCH] writeback: Use older_than_this_is_set instead of magic
> >  older_than_this == 0
> >
> > Currently we use 0 as a special value of work->older_than_this to
> > indicate that wb_writeback() should set work->older_that_this to current
> > time. This works but it is a bit magic. So use a special flag in
> > work_struct for that.
>
> OK.
>
> > -	if (!work->older_than_this)
> > +	if (!work->older_than_this_is_set)
> > 		work->older_than_this = jiffies;
>
> It would be logical although presumably unneeded to set
> older_than_this_is_set here?
  Yes. Updated.

> > Also fixup writeback from workqueue rescuer to include all inodes.
>
> There's nothing in the patch which matches this sentence?
  The sentence is about the hunk below. writeback_inodes_wb() is special in
that it directly calls queue_io() (everything else goes through
wb_writeback()) and my previous patch thus resulted in using 0 as an
older_than_this value => likely we wouldn't queue any inodes for writeback.

I've added WARN_ON_ONCE into move_expired_inodes() to increase a chance of
catching such mistakes in future (although in this particular case it
wouldn't really help because writeback_inodes_wb() gets hardly ever
called).

Currently we use 0 as a special value of work->older_than_this to
indicate that wb_writeback() should set work->older_that_this to current
time. This works but it is a bit magic. So use a special flag in
work_struct for that.

Also fixup writeback from workqueue rescuer (writeback_inodes_wb()) to
include all inodes. Currently it would use 0 as an older_than_this value
thus queue_io() would likely not queue any inodes for writeback.

Signed-off-by: Jan Kara <jack@suse.cz>
Cc: Fengguang Wu <fengguang.wu@intel.com>
Cc: Dave Chinner <dchinner@redhat.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

diff --git a/fs/fs-writeback.c b/fs/fs-writeback.c
index 65e66caec76f177f31dbe0c1bec4590a72b4c94c..ba332d2c596adb9827afc31c5393a3a7dc8f6a16 100644 (file)
--- a/fs/fs-writeback.c
+++ b/fs/fs-writeback.c
@@ -251,6 +251,7 @@ static int move_expired_inodes(struct list_head *delaying_queue,
        int do_sb_sort = 0;
        int moved = 0;
 
+       WARN_ON_ONCE(!work->older_than_this_is_set);
        while (!list_empty(delaying_queue)) {
                inode = wb_inode(delaying_queue->prev);
                if (inode_dirtied_after(inode, work->older_than_this))
@@ -800,8 +801,10 @@ static long wb_writeback(struct bdi_writeback *wb,
        struct inode *inode;
        long progress;
 
-       if (!work->older_than_this_is_set)
+       if (!work->older_than_this_is_set) {
                work->older_than_this = jiffies;
+               work->older_than_this_is_set = 1;
+       }
 
        spin_lock(&wb->list_lock);
        for (;;) {