From: Nikolay Borisov <nborisov@suse.com>
To: Josef Bacik <josef@toxicpanda.com>,
linux-btrfs@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH 0/5][v2] Deal with a few ENOSPC corner cases
Date: Tue, 17 Mar 2020 17:46:58 +0200 [thread overview]
Message-ID: <233da6a0-233b-ceb8-c6b4-4e3326d37f7b@suse.com> (raw)
In-Reply-To: <20200313195809.141753-1-josef@toxicpanda.com>
On 13.03.20 г. 21:58 ч., Josef Bacik wrote:
> v1->v2:
> - Dropped "btrfs: only take normal tickets into account in
> may_commit_transaction" because "btrfs: only check priority tickets for
> priority flushing" should actually fix the problem, and Nikolay pointed out
> that evict uses the priority list but is allowed to commit, so we need to take
> into account priority tickets sometimes.
> - Added "btrfs: allow us to use up to 90% of the global rsv for" so that the
> global rsv change was separate from the serialization patch.
> - Fixed up some changelogs.
> - Dropped an extra trace_printk that made it into v2.
>
> ----------------------- Original email --------------------------------------
>
> Nikolay has been digging into a failure of generic/320 on ppc64. This has
> shaken out a variety of issues, and he's done a good job at running all of the
> weird corners down and then testing my ideas to get them all fixed. This is the
> series that has survived the longest, so we're declaring victory.
>
> First there is the global reserve stealing logic. The way unlink works is it
> attempts to start a transaction with a normal reservation amount, and if this
> fails with ENOSPC we fall back to stealing from the global reserve. This is
> problematic because of all the same reasons we had with previous iterations of
> the ENOSPC handling, thundering herd. We get a bunch of failures all at once,
> everybody tries to allocate from the global reserve, some win and some lose, we
> get an ENSOPC.
>
> To fix this we need to integrate this logic into the normal ENOSPC
> infrastructure. The idea is simple, we add a new flushing state that indicates
> we are allowed to steal from the global reserve. We still go through all of the
> normal flushing work, and at the moment we begin to fail all the tickets we try
> to satisfy any tickets that are allowed to steal by stealing from the global
> reserve. If this works we start the flushing system over again just like we
> would with a normal ticket satisfaction. This serializes our global reserve
> stealing, so we don't have the thundering herd problem
>
> This isn't the only problem however. Nikolay also noticed that we would
> sometimes have huge amounts of space in the trans block rsv and we would ENOSPC
> out. This is because the may_commit_transaction() logic didn't take into
> account the space that would be reclaimed by all of the outstanding trans
> handles being required to stop in order to commit the transaction.
>
> Another corner here was that priority tickets could race in and make
> may_commit_transaction() think that it had no work left to do, and thus not
> commit the transaction.
>
> Those fixes all address the failures that Nikolay was seeing. The last two
> patches are just cleanups around how we handle priority tickets. We shouldn't
> even be serializing priority tickets behind normal tickets, only behind other
> priority tickets. And finally there would be a small window where priority
> tickets would fail out if there were multiple priority tickets and one of them
> failed. This is addressed by the previous patch.
>
> Nikolay has put these through many iterations of generic/320, and so far it
> hasn't failed. Thanks,
>
> Josef
>
I tested this on PPC64LE and didn't observe any regressions (apart form
the one fixed by [PATCH] btrfs: force chunk allocation if our global rsv
is larger than metadata), so:
Tested-by: Nikolay Borisov <nborisov@suse.com>
next prev parent reply other threads:[~2020-03-17 15:47 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-13 19:58 [PATCH 0/5][v2] Deal with a few ENOSPC corner cases Josef Bacik
2020-03-13 19:58 ` [PATCH 1/5] btrfs: Improve global reserve stealing logic Josef Bacik
2020-03-17 12:46 ` Nikolay Borisov
2020-03-13 19:58 ` [PATCH 2/5] btrfs: allow us to use up to 90% of the global rsv for unlink Josef Bacik
2020-03-17 12:46 ` Nikolay Borisov
2020-03-13 19:58 ` [PATCH 3/5] btrfs: Account for trans_block_rsv in may_commit_transaction Josef Bacik
2020-03-13 19:58 ` [PATCH 4/5] btrfs: only check priority tickets for priority flushing Josef Bacik
2020-03-17 12:55 ` Nikolay Borisov
2020-03-13 19:58 ` [PATCH 5/5] btrfs: run btrfs_try_granting_tickets if a priority ticket fails Josef Bacik
2020-03-17 12:59 ` Nikolay Borisov
2020-03-17 15:46 ` Nikolay Borisov [this message]
2020-03-25 15:50 ` [PATCH 0/5][v2] Deal with a few ENOSPC corner cases David Sterba
2020-03-25 15:52 ` Nikolay Borisov
2020-03-25 18:33 ` David Sterba
2020-04-03 15:46 ` 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=233da6a0-233b-ceb8-c6b4-4e3326d37f7b@suse.com \
--to=nborisov@suse.com \
--cc=josef@toxicpanda.com \
--cc=kernel-team@fb.com \
--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).