Re: [PATCH 3/5] mm: Implement IO-less balance_dirty_pages()

[Jan Kara <jack@suse.cz>]

On Wed 09-03-11 19:07:31, Vivek Goyal wrote:
> > +static void balance_dirty_pages(struct address_space *mapping,
> > +				unsigned long write_chunk)
> > +{
> > +	struct backing_dev_info *bdi = mapping->backing_dev_info;
> > +	struct balance_waiter bw;
> > +	struct dirty_limit_state st;
> > +	int dirty_exceeded = check_dirty_limits(bdi, &st);
> > +
> > +	if (dirty_exceeded < DIRTY_MAY_EXCEED_LIMIT ||
> > +	    (dirty_exceeded == DIRTY_MAY_EXCEED_LIMIT &&
> > +	     !bdi_task_limit_exceeded(&st, current))) {
> > +		if (bdi->dirty_exceeded &&
> > +		    dirty_exceeded < DIRTY_MAY_EXCEED_LIMIT)
> > +			bdi->dirty_exceeded = 0;
> >  		/*
> > -		 * Increase the delay for each loop, up to our previous
> > -		 * default of taking a 100ms nap.
> > +		 * In laptop mode, we wait until hitting the higher threshold
> > +		 * before starting background writeout, and then write out all
> > +		 * the way down to the lower threshold.  So slow writers cause
> > +		 * minimal disk activity.
> > +		 *
> > +		 * In normal mode, we start background writeout at the lower
> > +		 * background_thresh, to keep the amount of dirty memory low.
> >  		 */
> > -		pause <<= 1;
> > -		if (pause > HZ / 10)
> > -			pause = HZ / 10;
> > +		if (!laptop_mode && dirty_exceeded == DIRTY_EXCEED_BACKGROUND)
> > +			bdi_start_background_writeback(bdi);
> > +		return;
> >  	}
> >  
> > -	/* Clear dirty_exceeded flag only when no task can exceed the limit */
> > -	if (!min_dirty_exceeded && bdi->dirty_exceeded)
> > -		bdi->dirty_exceeded = 0;
> > +	if (!bdi->dirty_exceeded)
> > +		bdi->dirty_exceeded = 1;
> 
> Will it make sense to move out bdi_task_limit_exceeded() check in a
> separate if condition statement as follows. May be this is little
> easier to read.
> 
> 	if (dirty_exceeded < DIRTY_MAY_EXCEED_LIMIT) {
> 		if (bdi->dirty_exceeded)
> 			bdi->dirty_exceeded = 0;
> 
> 		if (!laptop_mode && dirty_exceeded == DIRTY_EXCEED_BACKGROUND)
> 			bdi_start_background_writeback(bdi);
> 
> 		return;
> 	}
> 
> 	if (dirty_exceeded == DIRTY_MAY_EXCEED_LIMIT &&
> 	    !bdi_task_limit_exceeded(&st, current))
> 		return;   
  But then we have to start background writeback here as well. Which is
actually a bug in the original patch as well! So clearly your way is more
readable :) I'll change it. Thanks.

									Honza
-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR