From: Liu Bo <bo.li.liu@oracle.com>
To: dsterba@suse.cz, linux-btrfs@vger.kernel.org
Subject: Re: [PATCH 3/3] Btrfs: remove nr_async_submits and async_submit_draining
Date: Wed, 11 Oct 2017 10:49:14 -0600 [thread overview]
Message-ID: <20171011164914.GA16979@dhcp-10-211-47-181.usdhcp.oraclecorp.com> (raw)
In-Reply-To: <20171011172032.GY3521@twin.jikos.cz>
On Wed, Oct 11, 2017 at 07:20:32PM +0200, David Sterba wrote:
> On Wed, Sep 27, 2017 at 02:31:17PM +0200, David Sterba wrote:
> > On Thu, Sep 07, 2017 at 11:22:22AM -0600, Liu Bo wrote:
> > > Now that we have the combo of flushing twice, which can make sure IO
> > > have started since the second flush will wait for page lock which
> > > won't be unlocked unless setting page writeback and queuing ordered
> > > extents, we don't need %async_submit_draining, %async_delalloc_pages
> > > and %nr_async_submits to tell whether IO have actually started.
> > >
> > > Moreover, all the flushers in use are followed by functions that wait
> > > for ordered extents to complete, so %nr_async_submits, which tracks
> > > whether bio's async submit has made progress, doesn't really make
> > > sense.
> > >
> > > However, %async_delalloc_pages is still required by shrink_delalloc()
> > > as that function doesn't flush twice in the normal case (just issues a
> > > writeback with WB_REASON_FS_FREE_SPACE).
> > >
> > > Signed-off-by: Liu Bo <bo.li.liu@oracle.com>
> >
> > Patches like this are almost impossible to review just from the code.
> > This has runtime implications that we'd need to observe in real on
> > various workloads.
> >
> > So, the patches "look good", but I did not try to forsee all the
> > scenarios where threads interact through the counters. I think it should
> > be safe to add them to for-next and give enough time to test them.
> >
> > The performance has varied wildly in the last cycle(s) so it's hard to
> > get a baseline, other than another major release. I do expect changes in
> > performance characteristics but still hope it'll be the same on average.
>
> Testing appears normal, we get more weirdnes just by enabling the
> writeback throttling, but without that I haven't observed anything
> unusual. Patches go to the misc-next part, meaning I don't expect
> changes other than fixups, as separate patches. Thanks.
Thank you Dave, I'm interesting in that weirdness part, will look into
it.
Thanks,
-liubo
next prev parent reply other threads:[~2017-10-11 17:50 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-07 17:22 [PATCH 0/3] kill async counters Liu Bo
2017-09-07 17:22 ` [PATCH 1/3] Btrfs: remove nr_async_bios Liu Bo
2017-09-27 11:30 ` David Sterba
2017-09-30 1:28 ` Liu Bo
2017-09-07 17:22 ` [PATCH 2/3] Btrfs: do not make defrag wait on async_delalloc_pages Liu Bo
2017-09-07 17:22 ` [PATCH 3/3] Btrfs: remove nr_async_submits and async_submit_draining Liu Bo
2017-09-27 12:31 ` David Sterba
2017-10-11 17:20 ` David Sterba
2017-10-11 16:49 ` Liu Bo [this message]
2017-10-11 18:04 ` David Sterba
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=20171011164914.GA16979@dhcp-10-211-47-181.usdhcp.oraclecorp.com \
--to=bo.li.liu@oracle.com \
--cc=dsterba@suse.cz \
--cc=linux-btrfs@vger.kernel.org \
/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).