linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Wu Fengguang <fengguang.wu@intel.com>
To: Kautuk Consul <consul.kautuk@gmail.com>
Cc: Artem Bityutskiy <dedekind1@gmail.com>,
	Mel Gorman <mgorman@suse.de>,
	KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>,
	"linux-mm@kvack.org" <linux-mm@kvack.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
	Jan Kara <jack@suse.cz>, Dave Chinner <david@fromorbit.com>,
	Greg Thelen <gthelen@google.com>
Subject: Re: [PATCH] writeback: Per-block device bdi->dirty_writeback_interval and bdi->dirty_expire_interval.
Date: Sun, 21 Aug 2011 22:11:36 +0800	[thread overview]
Message-ID: <20110821141136.GA29660@localhost> (raw)
In-Reply-To: <CAFPAmTQ2_JdwoLPFWQJze2Zd0QNHwMLWEmktGTQY_jHBAcixKg@mail.gmail.com>

Hi Kautuk,

On Sat, Aug 20, 2011 at 01:20:52AM +0800, Kautuk Consul wrote:
> Hi Wu,
> 
> You're right, the BDI threads should be woken up reliably by the
> balance_dirty_pages() and balance_dirty_pages()
> needs to be called from all code that is responsible for dirtying the pages.
> Sorry, I was not too aware of the balance_dirty_pages() functionality
> and the way it was being called in entirety or I would
> have spotted this.

That's fine. One have to get to know the code bit by bit :)

> Thanks for adding the dirty_background_time into your
> over_bground_thresh() formula.
> 
> Now that you seem to have included the time into the threshold, I can
> relate to your patch better
> as a solution for the problems I earlier mentioned.

Great, thank you.

Thanks,
Fengguang

> On Fri, Aug 19, 2011 at 7:54 PM, Wu Fengguang <fengguang.wu@intel.com> wrote:
> > Hi Kautuk,
> >
> > On Fri, Aug 19, 2011 at 03:00:30PM +0800, Kautuk Consul wrote:
> >> Hi Wu,
> >>
> >> Yes. I think I do understand your approach.
> >>
> >> Your aim is to always retain the per BDI timeout value.
> >>
> >> You want to check for threshholds by mathematically adjusting the
> >> background time too
> >> into your over_bground_thresh() formula so that your understanding
> >> holds true always and also
> >> affects the page dirtying scenario I mentioned.
> >> This definitely helps and refines this scenario in terms of flushing
> >> out of the dirty pages.
> >
> > Thanks.
> >
> >> Doubts:
> >> i)   Your entire implementation seems to be dependent on someone
> >> calling balance_dirty_pages()
> >>      directly or indirectly. This function will call the
> >> bdi_start_background_writeback() which wakes
> >>      up the flusher thread.
> >>      What about those page dirtying code paths which might not call
> >> balance_dirty_pages ?
> >>      Those paths then depend on the BDI thread periodically writing it
> >> to disk and then we are again
> >>      dependent on the writeback interval.
> >>      Can we assume that the kernel will reliably call
> >> balance_dirty_pages() whenever the pages
> >>      are dirtied ? If that was true, then we would not need bdi
> >> periodic writeback threads ever.
> >
> > Yes. The kernel need a way to limit the total number of dirty pages at
> > any given time and to keep them under dirty_ratio/dirty_bytes.
> >
> > balance_dirty_pages() is such a central place to throttle the dirty
> > pages. Whatever code path generating dirty pages are required to call
> > into balance_dirty_pages_ratelimited_nr() which will in turn call
> > balance_dirty_pages().
> >
> > So, the values specified by dirty_ratio/dirty_bytes will be executed
> > effectively by balance_dirty_pages(). In contrast, the values
> > specified by dirty_expire_centisecs is merely a parameter used by
> > wb_writeback() to select the eligible inodes to do writeout. The 30s
> > dirty expire time is never a guarantee that all inodes/pages dirtied
> > before 30s will be timely written to disk. It's better interpreted in
> > the opposite way: when under the dirty_background_ratio threshold and
> > hence background writeout does not kick in, dirty inodes younger than
> > 30s won't be written to disk by the flusher.
> >
> >> ii)  Even after your rigorous checking, the bdi_writeback_thread()
> >> will still do a schedule_timeout()
> >>      with the global value. Will your current solution then handle
> >> Artem's disk removal scenario ?
> >>      Else, you start using your value in the schedule_timeout() call
> >> in the bdi_writeback_thread()
> >>      function, which brings us back to the interval phenomenon I was
> >> talking about.
> >
> > wb_writeback() will keep running as long as over_bground_thresh().
> >
> > The flusher will keep writing as long as there are more works, since
> > there is a
> >
> >                if (!list_empty(&bdi->work_list))
> >                        continue;
> >
> > before the schedule_timeout() call.
> >
> > And the flusher thread will always be woke up timely from
> > balance_dirty_pages().
> >
> > So schedule_timeout() won't block in the way at all.
> >
> >> Does this patch really help the user control exact time when the write
> >> BIO is transferred from the
> >> MM to the Block layer assuming balance_dirty_pages() is not called ?
> >
> > It would be a serious bug if balance_dirty_pages() is somehow not
> > called. But note that balance_dirty_pages() is designed to be called
> > on every N pages to reduce overheads.
> >
> > Thanks,
> > Fengguang
> >
--
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

  reply	other threads:[~2011-08-21 14:11 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CAFPAmTSrh4r71eQqW-+_nS2KFK2S2RQvYBEpa3QnNkZBy8ncbw@mail.gmail.com>
2011-08-18  9:48 ` [PATCH] writeback: Per-block device bdi->dirty_writeback_interval and bdi->dirty_expire_interval Wu Fengguang
2011-08-18  9:51   ` Wu Fengguang
2011-08-18 11:28   ` Kautuk Consul
2011-08-18 12:55     ` Wu Fengguang
2011-08-18 12:14   ` Artem Bityutskiy
2011-08-18 12:35     ` Wu Fengguang
2011-08-18 15:26       ` Kautuk Consul
2011-08-19  2:17         ` Wu Fengguang
2011-08-18 13:13     ` Wu Fengguang
2011-08-18 16:25       ` Kautuk Consul
2011-08-19  2:34         ` Wu Fengguang
2011-08-19  4:38           ` Kautuk Consul
2011-08-19  5:28             ` Wu Fengguang
2011-08-19  6:08               ` Wu Fengguang
2011-08-19  7:00                 ` Kautuk Consul
2011-08-19 14:24                   ` Wu Fengguang
2011-08-19 17:20                     ` Kautuk Consul
2011-08-21 14:11                       ` Wu Fengguang [this message]
2011-08-19 11:55       ` Artem Bityutskiy
2011-08-19 14:27         ` Wu Fengguang

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=20110821141136.GA29660@localhost \
    --to=fengguang.wu@intel.com \
    --cc=consul.kautuk@gmail.com \
    --cc=david@fromorbit.com \
    --cc=dedekind1@gmail.com \
    --cc=gthelen@google.com \
    --cc=jack@suse.cz \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mgorman@suse.de \
    /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).