On Mon 26-07-10 19:51:53, Wu Fengguang wrote:
> 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?
I agree there's no substantial code simplification. But I see a
substantial "behavior" simplification (just two sweeps instead of 10 or
so). But I don't really insist on the two sweeps, it's just that I don't
see a justification for the exponencial back off here... I mean what's the
point if the interval we queue gets really small? Why not just use
expire_interval/2 as a step if you want a smoother behavior?
Honza
> > > 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
--
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxxx For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>