On Sat, Jul 24, 2010 at 02:15:21AM +0800, Jan Kara wrote:
> On Thu 22-07-10 13:09:32, Wu Fengguang wrote:
> > A background flush work may run for ever. So it's reasonable for it to
> > mimic the kupdate behavior of syncing old/expired inodes first.
> >
> > The policy is
> > - enqueue all newly expired inodes at each queue_io() time
> > - retry with halfed expire interval until get some inodes to sync
> Hmm, this logic looks a bit arbitrary to me. What I actually don't like
> very much about this that when there aren't inodes older than say 2
> seconds, you'll end up queueing just inodes between 2s and 1s. So I'd
> rather just queue inodes older than the limit and if there are none, just
> queue all other dirty inodes.
You are proposing
- expire_interval >>= 1;
+ expire_interval = 0;
IMO this does not really simplify code or concept. If we can get the
"smoother" behavior in original patch without extra cost, why not?
Thanks,
Fengguang
> > CC: Jan Kara <jack@xxxxxxx>
> > Signed-off-by: Wu Fengguang <fengguang.wu@xxxxxxxxx>
> > ---
> > fs/fs-writeback.c | 20 ++++++++++++++------
> > 1 file changed, 14 insertions(+), 6 deletions(-)
> >
> > --- linux-next.orig/fs/fs-writeback.c 2010-07-22 12:56:42.000000000 +0800
> > +++ linux-next/fs/fs-writeback.c 2010-07-22 13:07:51.000000000 +0800
> > @@ -217,14 +217,14 @@ static void move_expired_inodes(struct l
> > struct writeback_control *wbc)
> > {
> > unsigned long expire_interval = 0;
> > - unsigned long older_than_this;
> > + unsigned long older_than_this = 0; /* reset to kill gcc warning */
> > LIST_HEAD(tmp);
> > struct list_head *pos, *node;
> > struct super_block *sb = NULL;
> > struct inode *inode;
> > int do_sb_sort = 0;
> >
> > - if (wbc->for_kupdate) {
> > + if (wbc->for_kupdate || wbc->for_background) {
> > expire_interval = msecs_to_jiffies(dirty_expire_interval * 10);
> > older_than_this = jiffies - expire_interval;
> > }
> > @@ -232,8 +232,15 @@ static void move_expired_inodes(struct l
> > while (!list_empty(delaying_queue)) {
> > inode = list_entry(delaying_queue->prev, struct inode, i_list);
> > if (expire_interval &&
> > - inode_dirtied_after(inode, older_than_this))
> > - break;
> > + inode_dirtied_after(inode, older_than_this)) {
> > + if (wbc->for_background &&
> > + list_empty(dispatch_queue) && list_empty(&tmp)) {
> > + expire_interval >>= 1;
> > + older_than_this = jiffies - expire_interval;
> > + continue;
> > + } else
> > + break;
> > + }
> > if (sb && sb != inode->i_sb)
> > do_sb_sort = 1;
> > sb = inode->i_sb;
> > @@ -521,7 +528,8 @@ void writeback_inodes_wb(struct bdi_writ
> >
> > wbc->wb_start = jiffies; /* livelock avoidance */
> > spin_lock(&inode_lock);
> > - if (!wbc->for_kupdate || list_empty(&wb->b_io))
> > +
> > + if (!(wbc->for_kupdate || wbc->for_background) || list_empty(&wb->b_io))
> > queue_io(wb, wbc);
> >
> > while (!list_empty(&wb->b_io)) {
> > @@ -550,7 +558,7 @@ static void __writeback_inodes_sb(struct
> >
> > wbc->wb_start = jiffies; /* livelock avoidance */
> > spin_lock(&inode_lock);
> > - if (!wbc->for_kupdate || list_empty(&wb->b_io))
> > + if (!(wbc->for_kupdate || wbc->for_background) || list_empty(&wb->b_io))
> > queue_io(wb, wbc);
> > writeback_sb_inodes(sb, wb, wbc, true);
> > spin_unlock(&inode_lock);
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> Jan Kara <jack@xxxxxxx>
> SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
