Hi,
due to a recent report [1] we need to revert the radix tree to xarray
conversion patches. There's a problem with sleeping under spinlock, when
xa_insert could allocate memory under pressure. We use GFP_NOFS so this
is a real problem that we unfortunately did not discover during review.
I'm sorry to do such change at rc6 time but the revert is IMO the safer
option, there are patches to use mutex instead of the spin locks but
that would need more testing. The revert branch has been tested on a
few setups, all seem ok. The conversion to xarray will be revisited in
the future.
[1] https://lore.kernel.org/linux-btrfs/[email protected]/
Note about the xarray API:
The possible sleeping is documented next to xa_insert, however there's
no runtime check for that, like is eg. in radix_tree_preload. The
context does not need to be atomic so it's not as simple as
might_sleep_if(gfpflags_allow_blocking(gfp));
or
WARN_ON_ONCE(gfpflags_allow_blocking(gfp));
Some kind of development time debugging/assertion aid would be nice.
----------------------------------------------------------------
The following changes since commit b3a3b0255797e1d395253366ba24a4cc6c8bdf9c:
btrfs: zoned: drop optimization of zone finish (2022-07-08 19:18:00 +0200)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-5.19-rc7-tag
for you to fetch changes up to 088aea3b97e0ae5a2a86f5d142ad10fec8a1b80f:
Revert "btrfs: turn delayed_nodes_tree into an XArray" (2022-07-15 19:15:19 +0200)
----------------------------------------------------------------
David Sterba (4):
Revert "btrfs: turn fs_roots_radix in btrfs_fs_info into an XArray"
Revert "btrfs: turn fs_info member buffer_radix into XArray"
Revert "btrfs: turn name_cache radix tree into XArray in send_ctx"
Revert "btrfs: turn delayed_nodes_tree into an XArray"
fs/btrfs/ctree.h | 18 ++---
fs/btrfs/delayed-inode.c | 84 ++++++++++----------
fs/btrfs/disk-io.c | 179 ++++++++++++++++++++++++-------------------
fs/btrfs/extent-tree.c | 2 +-
fs/btrfs/extent_io.c | 122 +++++++++++++++++------------
fs/btrfs/inode.c | 15 ++--
fs/btrfs/send.c | 40 +++++-----
fs/btrfs/tests/btrfs-tests.c | 24 +++++-
fs/btrfs/transaction.c | 112 +++++++++++++++------------
9 files changed, 340 insertions(+), 256 deletions(-)
On Sat, Jul 16, 2022 at 04:06:20PM +0200, David Sterba wrote:
> Note about the xarray API:
>
> The possible sleeping is documented next to xa_insert, however there's
> no runtime check for that, like is eg. in radix_tree_preload. The
> context does not need to be atomic so it's not as simple as
>
> might_sleep_if(gfpflags_allow_blocking(gfp));
>
> or
>
> WARN_ON_ONCE(gfpflags_allow_blocking(gfp));
>
> Some kind of development time debugging/assertion aid would be nice.
Are you saying that
https://git.infradead.org/users/willy/xarray.git/commitdiff/c195d497ca1ff673c2e6935152a0a5b6be2efdc9
is wrong? It's been in linux-next for the last week since you drew it
to my attention that this would be useful.
The pull request you sent on Sat, 16 Jul 2022 16:06:20 +0200:
> git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git for-5.19-rc7-tag
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/972a278fe60c361eb8f37619f562f092e8786d7c
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
On Sat, Jul 16, 2022 at 04:34:12PM +0100, Matthew Wilcox wrote:
> On Sat, Jul 16, 2022 at 04:06:20PM +0200, David Sterba wrote:
> > Note about the xarray API:
> >
> > The possible sleeping is documented next to xa_insert, however there's
> > no runtime check for that, like is eg. in radix_tree_preload. The
> > context does not need to be atomic so it's not as simple as
> >
> > might_sleep_if(gfpflags_allow_blocking(gfp));
> >
> > or
> >
> > WARN_ON_ONCE(gfpflags_allow_blocking(gfp));
> >
> > Some kind of development time debugging/assertion aid would be nice.
>
> Are you saying that
> https://git.infradead.org/users/willy/xarray.git/commitdiff/c195d497ca1ff673c2e6935152a0a5b6be2efdc9
>
> is wrong? It's been in linux-next for the last week since you drew it
> to my attention that this would be useful.
I have misinterpreted what might_sleep_if does in case it's under mutex,
it won't warn so the linked commit (adding might_alloc) should be
enough, thanks.
On Thu, Jul 21, 2022 at 04:45:08PM +0200, David Sterba wrote:
> On Sat, Jul 16, 2022 at 04:34:12PM +0100, Matthew Wilcox wrote:
> > On Sat, Jul 16, 2022 at 04:06:20PM +0200, David Sterba wrote:
> > > Note about the xarray API:
> > >
> > > The possible sleeping is documented next to xa_insert, however there's
> > > no runtime check for that, like is eg. in radix_tree_preload. The
> > > context does not need to be atomic so it's not as simple as
> > >
> > > might_sleep_if(gfpflags_allow_blocking(gfp));
> > >
> > > or
> > >
> > > WARN_ON_ONCE(gfpflags_allow_blocking(gfp));
> > >
> > > Some kind of development time debugging/assertion aid would be nice.
> >
> > Are you saying that
> > https://git.infradead.org/users/willy/xarray.git/commitdiff/c195d497ca1ff673c2e6935152a0a5b6be2efdc9
> >
> > is wrong? It's been in linux-next for the last week since you drew it
> > to my attention that this would be useful.
>
> I have misinterpreted what might_sleep_if does in case it's under mutex,
> it won't warn so the linked commit (adding might_alloc) should be
> enough, thanks.
I did a quick test, with spin locks it's detected right away, with
mutexes no warnings as expected.