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

[Curt Wohlgemuth <curtw@google.com>]

On Wed, Mar 16, 2011 at 2:26 PM, Curt Wohlgemuth <curtw@google.com> wrote:
> Hi Jan:
>
> On Tue, Mar 15, 2011 at 8:23 AM, Vivek Goyal <vgoyal@redhat.com> wrote:
>> On Mon, Mar 14, 2011 at 09:48:21PM +0100, Jan Kara wrote:
>>> 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.
>>
>> I was thinking about that starting of bdi writeback here. But I was
>> assuming that if we are here then we most likely have visited above
>> loop of < DIRTY_MAY_EXCEED_LIMIT and started background writeback.
>
> Maybe I'm missing something, but at the point in balance_dirty_pages()
> where we kick the flusher thread , before we put the current task to
> sleep, how do you know that background writeback is taking place?  Are
> you simply assuming that in previous calls to balance_dirty_pages(),
> that background writeback has been started, and is still taking place
> at the time we need to do throttling?

Never mind, I see that I'm not completely familiar with the writeback
changes for 2.6.38.  I see where we'll kick of BG writeback in
wb_check_background_flush() once we kick the flusher thread.

Curt

>
> Thanks,
> Curt
>
>>
>> Thanks
>> Vivek
>>
>> --
>> To unsubscribe, send a message with 'unsubscribe linux-mm' in
>> the body to majordomo@kvack.org.  For more info on Linux MM,
>> see: http://www.linux-mm.org/ .
>> Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
>> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html