[PATCH v2] btrfs: skip the extent map tree lock for inodes without extent maps

[PATCH v2] btrfs: skip the extent map tree lock for inodes without extent maps

[Breno Leitao]

find_first_inode_to_shrink() takes inode->extent_tree.lock in write mode
on every inode it walks, only to find out whether that inode has any
extent maps. Most inodes have none, so the lock is taken and dropped
again without any work being done.

Check whether the tree is empty before taking the lock. tree->root is
only modified with the tree lock held for write, so the unlocked read is
a harmless race: a false empty just defers the inode to a later scan,
which already happens whenever the write_trylock() below fails, and a
false non-empty falls through to the existing check under the lock.

Across the Meta production fleet the extent map shrinker is ~0.35% of
non-idle kernel CPU. Attributing callees to their caller,
find_first_inode_to_shrink() is ~65% of that, and the write_trylock() it
does is ~30% of the whole shrinker.

Micro benchmark: a 6 GiB btrfs on a loop device, 100000 empty files kept
open, plus 200 1 MiB files created last so they get the highest inode
numbers and every scan has to walk all the empty ones first. Each round
drops the page cache, re-reads the data files to recreate the extent
maps, then triggers the shrinker with "echo 2 > /proc/sys/vm/drop_caches".
15 rounds per run on arm64 (Neoverse V2), 8 CPUs, no lock debugging.
Cost of find_first_inode_to_shrink() from the ftrace function profiler,
in ns per inode walked, median of runs:

                          base   patched    delta
    idle                  46.4      40.1   -13.6%
    4 concurrent readers  47.8      38.4   -19.7%

A separate build with CONFIG_LOCK_STAT, same test, for the extent map
tree rwlock. The shrinker is not the only user of that lock, every
extent map insert and lookup takes it too, which is why the acquisition
count drops by two thirds rather than to nothing:

                              base   patched    delta
    write acquisitions      628016    228000   -63.7%
    hold time total (us)     47512     22717   -52.2%
    acq cacheline bounces     1574      1288   -18.2%

Signed-off-by: Breno Leitao <leitao@debian.org>
---
Changes in v2:
- Better justification for the patch.
- Mark this unlocked read as racy
- use the proper RB_EMPTY_ROOT() primitive
- Link to v1: https://patch.msgid.link/20260821-b4-btrfs-em-shrinker-v1-1-286f3fb15873@debian.org
---
 fs/btrfs/extent_map.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
index 6ad7b39ae358b..86d9c6f5ff4bd 100644
--- a/fs/btrfs/extent_map.c
+++ b/fs/btrfs/extent_map.c
@@ -1219,6 +1219,14 @@ static struct btrfs_inode *find_first_inode_to_shrink(struct btrfs_root *root,
 
 		tree = &inode->extent_tree;
 
+		/*
+		 * Most inodes have no extent maps, so check without the lock.
+		 * The race is harmless: a false empty just defers the inode to
+		 * a later scan, and a false non-empty is caught under the lock.
+		 */
+		if (data_race(RB_EMPTY_ROOT(&tree->root)))
+			goto next;
+
 		/*
 		 * We want to be fast so if the lock is busy we don't want to
 		 * spend time waiting for it (some task is about to do IO for

---
base-commit: 6a746cd265aed59107ebdaa9ce039bb832922969
change-id: 20260820-b4-btrfs-em-shrinker-7382d7f0dd05

Best regards,
--  
Breno Leitao <leitao@debian.org>

Re: [PATCH v2] btrfs: skip the extent map tree lock for inodes without extent maps

[Filipe Manana]

On Mon, Aug 24, 2026 at 12:59 PM Breno Leitao <leitao@debian.org> wrote:
>
> find_first_inode_to_shrink() takes inode->extent_tree.lock in write mode
> on every inode it walks, only to find out whether that inode has any
> extent maps. Most inodes have none, so the lock is taken and dropped
> again without any work being done.
>
> Check whether the tree is empty before taking the lock. tree->root is
> only modified with the tree lock held for write, so the unlocked read is
> a harmless race: a false empty just defers the inode to a later scan,
> which already happens whenever the write_trylock() below fails, and a
> false non-empty falls through to the existing check under the lock.
>
> Across the Meta production fleet the extent map shrinker is ~0.35% of
> non-idle kernel CPU. Attributing callees to their caller,
> find_first_inode_to_shrink() is ~65% of that, and the write_trylock() it
> does is ~30% of the whole shrinker.
>
> Micro benchmark: a 6 GiB btrfs on a loop device, 100000 empty files kept
> open, plus 200 1 MiB files created last so they get the highest inode
> numbers and every scan has to walk all the empty ones first. Each round
> drops the page cache, re-reads the data files to recreate the extent
> maps, then triggers the shrinker with "echo 2 > /proc/sys/vm/drop_caches".
> 15 rounds per run on arm64 (Neoverse V2), 8 CPUs, no lock debugging.
> Cost of find_first_inode_to_shrink() from the ftrace function profiler,
> in ns per inode walked, median of runs:
>
>                           base   patched    delta
>     idle                  46.4      40.1   -13.6%
>     4 concurrent readers  47.8      38.4   -19.7%
>
> A separate build with CONFIG_LOCK_STAT, same test, for the extent map
> tree rwlock. The shrinker is not the only user of that lock, every
> extent map insert and lookup takes it too, which is why the acquisition
> count drops by two thirds rather than to nothing:
>
>                               base   patched    delta
>     write acquisitions      628016    228000   -63.7%
>     hold time total (us)     47512     22717   -52.2%
>     acq cacheline bounces     1574      1288   -18.2%
>
> Signed-off-by: Breno Leitao <leitao@debian.org>

Reviewed-by: Filipe Manana <fdmanana@suse.com>

The current subject:

"btrfs: skip the extent map tree lock for inodes without extent maps"

It is a bit too generic, giving no clue that it concerns the shrinker.
If you agree, I'll change it to:

"btrfs: skip extent tree lock in the shrinker for inodes without extent maps"

Thanks.

> ---
> Changes in v2:
> - Better justification for the patch.
> - Mark this unlocked read as racy
> - use the proper RB_EMPTY_ROOT() primitive
> - Link to v1: https://patch.msgid.link/20260821-b4-btrfs-em-shrinker-v1-1-286f3fb15873@debian.org
> ---
>  fs/btrfs/extent_map.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/fs/btrfs/extent_map.c b/fs/btrfs/extent_map.c
> index 6ad7b39ae358b..86d9c6f5ff4bd 100644
> --- a/fs/btrfs/extent_map.c
> +++ b/fs/btrfs/extent_map.c
> @@ -1219,6 +1219,14 @@ static struct btrfs_inode *find_first_inode_to_shrink(struct btrfs_root *root,
>
>                 tree = &inode->extent_tree;
>
> +               /*
> +                * Most inodes have no extent maps, so check without the lock.
> +                * The race is harmless: a false empty just defers the inode to
> +                * a later scan, and a false non-empty is caught under the lock.
> +                */
> +               if (data_race(RB_EMPTY_ROOT(&tree->root)))
> +                       goto next;
> +
>                 /*
>                  * We want to be fast so if the lock is busy we don't want to
>                  * spend time waiting for it (some task is about to do IO for
>
> ---
> base-commit: 6a746cd265aed59107ebdaa9ce039bb832922969
> change-id: 20260820-b4-btrfs-em-shrinker-7382d7f0dd05
>
> Best regards,
> --
> Breno Leitao <leitao@debian.org>
>
>

Re: [PATCH v2] btrfs: skip the extent map tree lock for inodes without extent maps

[Breno Leitao]

Hello Filipe,

On Mon, Aug 24, 2026 at 09:14:10PM +0100, Filipe Manana wrote:
> Reviewed-by: Filipe Manana <fdmanana@suse.com>

Thanks for the review and feedback.

> The current subject:
> 
> "btrfs: skip the extent map tree lock for inodes without extent maps"
> 
> It is a bit too generic, giving no clue that it concerns the shrinker.
> If you agree, I'll change it to:
> 
> "btrfs: skip extent tree lock in the shrinker for inodes without extent maps"

Please do, your suggestion makes more sense than the current one.

Just to be clear, I won't be respinning this patch: you'll update the
subject while merging it.

Thanks,
--breno

Re: [PATCH v2] btrfs: skip the extent map tree lock for inodes without extent maps

[Filipe Manana]

On Tue, Aug 25, 2026 at 9:10 AM Breno Leitao <leitao@debian.org> wrote:
>
> Hello Filipe,
>
> On Mon, Aug 24, 2026 at 09:14:10PM +0100, Filipe Manana wrote:
> > Reviewed-by: Filipe Manana <fdmanana@suse.com>
>
> Thanks for the review and feedback.
>
> > The current subject:
> >
> > "btrfs: skip the extent map tree lock for inodes without extent maps"
> >
> > It is a bit too generic, giving no clue that it concerns the shrinker.
> > If you agree, I'll change it to:
> >
> > "btrfs: skip extent tree lock in the shrinker for inodes without extent maps"
>
> Please do, your suggestion makes more sense than the current one.
>
> Just to be clear, I won't be respinning this patch: you'll update the
> subject while merging it.

Correct.

>
> Thanks,
> --breno