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

[Filipe Manana]

On Fri, Aug 21, 2026 at 5:07 PM Breno Leitao <leitao@xxxxxxxxxx> wrote:
>
> Hello Filipe,
>
> On Fri, Aug 21, 2026 at 12:17:26PM +0100, Filipe Manana wrote:
> > On Fri, Aug 21, 2026 at 11:39 AM Breno Leitao <leitao@xxxxxxxxxx> wrote:
> > > +               if (!READ_ONCE(tree->root.rb_node))
> >
> > Please use the proper rbtree api to check if a tree is empty:
> >
> > RB_EMPTY_ROOT(&tree->root)
> >
> > That's a lot more elegant and hides the use of READ_ONCE().
> >
> > But really when there are harmless races, I like to see an explicit
> > data_race() annotation and a comment about why the race is harmless.
> >
> > Nevertheless, first, I would like to know how much the performance
> > improvement was...
>
> Sorry for not posting it earlier.
>
> How I got into this: I've found while profiling arm64 production hosts
> that had high system utilization and : _raw_write_trylock and xa_find
> dominated, and the callchain pointed at find_first_inode_to_shrink()
> locking every inode it walks.
>
> Looking at the whole the Meta production fleet, the extent map shrinker
> is ~0.35% of non-idle kernel CPU.  Within it, and attributing callees to
> their caller, find_first_inode_to_shrink() is ~65% and the
> write_trylock() it does is ~30% of the whole shrinker. Two thirds of
> what the shrinker costs is looking for inodes that have anything to
> shrink, and half of that is a lock we do not need to take.
>
>
> micro benchmarks
> ================
>
> I've hacked up a 6 GiB btrfs on a loop device, 100000 empty files
> kept open, plus 200 1 MiB files created last, so they have 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, arm64 (Neoverse V2),
> 8 CPUs, no lock debugging, cost of find_first_inode_to_shrink() from the
>  ftrace function profiler:
>
>   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.  Note 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%
>
> Does it help?

Yes, and all that should be in the changelog.

Thanks.
>
> Thanks for the quick reply,
> --breno
>