Re: [PATCH 3/3] writeback: avoid extra sync work at enqueue time

[Jan Kara]

On Thu 05-05-11 22:10:39, Wu Fengguang wrote:
> On Thu, May 05, 2011 at 10:01:34PM +0800, Jan Kara wrote:
> > On Thu 05-05-11 20:27:32, Wu Fengguang wrote:
> > > On Thu, May 05, 2011 at 05:24:27AM +0800, Jan Kara wrote:
> > > > On Mon 02-05-11 11:17:53, Wu Fengguang wrote:
> > > > > This removes writeback_control.wb_start and does more straightforward
> > > > > sync livelock prevention by setting .older_than_this to prevent extra
> > > > > inodes from being enqueued in the first place.
> > > > > 
> > > > > --- linux-next.orig/fs/fs-writeback.c	2011-05-02 11:17:24.000000000 +0800
> > > > > +++ linux-next/fs/fs-writeback.c	2011-05-02 11:17:27.000000000 +0800
> > > > > @@ -683,10 +672,12 @@ static long wb_writeback(struct bdi_writ
> > > > >  	 *                   (quickly) tag currently dirty pages
> > > > >  	 *                   (maybe slowly) sync all tagged pages
> > > > >  	 */
> > > > > -	if (wbc.sync_mode == WB_SYNC_ALL || wbc.tagged_sync)
> > > > > +	if (wbc.sync_mode == WB_SYNC_ALL || wbc.tagged_sync) {
> > > > >  		write_chunk = LONG_MAX;
> > > > > +		oldest_jif = jiffies;
> > > > > +		wbc.older_than_this = &oldest_jif;
> > > > > +	}
> > > >   What are the implications of not doing dirty-time livelock avoidance for
> > > > other types of writeback? Is that a mistake? I'd prefer to have in
> > > > wb_writeback():
> > > > if (wbc.for_kupdate)
> > > > 	oldest_jif = jiffies - msecs_to_jiffies(dirty_expire_interval * 10);
> > > > else
> > > > 	oldest_jif = jiffies;
> > > > wbc.older_than_this = &oldest_jif;
> > > > 
> > > > And when you have this, you can make wbc.older_than_this just a plain
> > > > number and remove all those checks for wbc.older_than_this == NULL.
> > > 
> > > Good point. Here is the fixed patch. Will you send the patch to change
> > > the type when the current patches are settled down?
> >   OK, I will do that.
> 
> Thank you.
> 
> > > @@ -686,7 +674,9 @@ static long wb_writeback(struct bdi_writ
> > >  	if (wbc.sync_mode == WB_SYNC_ALL || wbc.tagged_sync)
> > >  		write_chunk = LONG_MAX;
> > >  
> > > -	wbc.wb_start = jiffies; /* livelock avoidance */
> > > +	oldest_jif = jiffies;
> > > +	wbc.older_than_this = &oldest_jif;
> > > +
> >   I might be already confused with all the code moving around but won't
> > this overwrite the value set for the for_kupdate case?
> 
> It's the opposite -- it will be overwritten inside the loop by
> for_kupdate, which may run for long time and hence need to update
> oldest_jif from time to time.
  Oh, right. I guess I'll setup that git tree (at least for myself if for
nothing else) so that I can see what is actually the current code state.
Because I think I've already lost the track of all the patches...

I've looked at mmotm on the web but it seems to have older versions of the
patches and also some of the patches which I think Andrew took (like this
one) are not there. Can you please make latest versions of all the patches
available somewhere so that I don't have dig them from the mailing list
archives?

								Honza
-- 
Jan Kara <jack@xxxxxxx>
SUSE Labs, CR
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/