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@xxxxxxxxxx> 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@xxxxxxxxxx>

Reviewed-by: Filipe Manana <fdmanana@xxxxxxxx>

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@xxxxxxxxxx
> ---
>  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@xxxxxxxxxx>
>
>