linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Curt Wohlgemuth <curtw@google.com>
To: Vivek Goyal <vgoyal@redhat.com>, jack@suse.cz
Cc: linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
	Wu Fengguang <fengguang.wu@intel.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Andrew Morton <akpm@linux-foundation.org>,
	Christoph Hellwig <hch@infradead.org>,
	Dave Chinner <david@fromorbit.com>
Subject: Re: [PATCH 3/5] mm: Implement IO-less balance_dirty_pages()
Date: Wed, 16 Mar 2011 14:26:54 -0700	[thread overview]
Message-ID: <AANLkTikshHSaqfs7_CzL3ofyAV96_NZsOw4dcNbPtnC1@mail.gmail.com> (raw)
In-Reply-To: <20110315152310.GD24984@redhat.com>

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?

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, 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>

  reply	other threads:[~2011-03-16 21:26 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-08 22:31 [PATCH RFC 0/5] IO-less balance_dirty_pages() v2 (simple approach) Jan Kara
2011-03-08 22:31 ` [PATCH 1/5] writeback: account per-bdi accumulated written pages Jan Kara
2011-03-08 22:31 ` [PATCH 2/5] mm: Properly reflect task dirty limits in dirty_exceeded logic Jan Kara
2011-03-09 21:02   ` Vivek Goyal
2011-03-14 20:44     ` Jan Kara
2011-03-15 15:21       ` Vivek Goyal
2011-03-08 22:31 ` [PATCH 3/5] mm: Implement IO-less balance_dirty_pages() Jan Kara
2011-03-10  0:07   ` Vivek Goyal
2011-03-14 20:48     ` Jan Kara
2011-03-15 15:23       ` Vivek Goyal
2011-03-16 21:26         ` Curt Wohlgemuth [this message]
2011-03-16 22:53           ` Curt Wohlgemuth
2011-03-16 16:53   ` Vivek Goyal
2011-03-16 19:10     ` Jan Kara
2011-03-16 19:31       ` Vivek Goyal
2011-03-16 19:58         ` Jan Kara
2011-03-16 20:22           ` Vivek Goyal
2011-03-08 22:31 ` [PATCH 4/5] mm: Remove low limit from sync_writeback_pages() Jan Kara
2011-03-08 22:31 ` [PATCH 5/5] mm: Autotune interval between distribution of page completions Jan Kara
2011-03-17 15:46 ` [PATCH RFC 0/5] IO-less balance_dirty_pages() v2 (simple approach) Curt Wohlgemuth
2011-03-17 15:51   ` Christoph Hellwig
2011-03-17 16:24     ` Curt Wohlgemuth
2011-03-17 16:43       ` Christoph Hellwig
2011-03-17 17:32   ` Jan Kara
2011-03-17 18:55     ` Curt Wohlgemuth
2011-03-17 22:56       ` Vivek Goyal
2011-03-18 14:30 ` Wu Fengguang
2011-03-22 21:43   ` Jan Kara
2011-03-23  4:41     ` Dave Chinner
2011-03-25 12:59       ` Wu Fengguang
2011-03-25 13:44     ` Wu Fengguang
2011-03-25 23:05       ` Jan Kara
2011-03-28  2:44         ` Wu Fengguang
2011-03-28 15:08           ` Jan Kara
2011-03-29  1:44             ` Wu Fengguang
2011-03-29  2:14           ` Dave Chinner
2011-03-29  2:41             ` Wu Fengguang
2011-03-29  5:59               ` Dave Chinner
2011-03-29  7:31                 ` Wu Fengguang
2011-03-29  7:52                   ` Wu Fengguang
  -- strict thread matches above, loose matches on Subject: below --
2011-02-04  1:38 [RFC PATCH 0/5] IO-less balance dirty pages Jan Kara
2011-02-04  1:38 ` [PATCH 3/5] mm: Implement IO-less balance_dirty_pages() Jan Kara
2011-02-04 13:09   ` Peter Zijlstra
2011-02-11 14:56     ` Jan Kara
2011-02-04 13:09   ` Peter Zijlstra
2011-02-11 14:56     ` Jan Kara
2011-02-04 13:09   ` Peter Zijlstra
2011-02-04 13:19     ` Peter Zijlstra
2011-02-11 15:46     ` Jan Kara
2011-02-22 15:40       ` Peter Zijlstra

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=AANLkTikshHSaqfs7_CzL3ofyAV96_NZsOw4dcNbPtnC1@mail.gmail.com \
    --to=curtw@google.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=akpm@linux-foundation.org \
    --cc=david@fromorbit.com \
    --cc=fengguang.wu@intel.com \
    --cc=hch@infradead.org \
    --cc=jack@suse.cz \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=vgoyal@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).