Re: [PATCH] btrfs: skip the extent map tree lock for inodes without extent maps
Breno Leitao <[email protected]>
| Newsgroups | org.kernel.vger.linux-btrfs,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
Hello Qu, On Fri, Aug 21, 2026 at 08:43:00PM +0930, Qu Wenruo wrote: > 在 2026/8/21 20:08, Breno Leitao 写道: > > The shrinker (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 have none, from what I understand, so, avoid the lock by testing the > > tree with a plain read before taking the lock. tree->root is only > > modified with the tree lock held for write, so the unlocked read is > > a benign race: a false negative just defers the inode to a later scan. > > > > On my tests, find_first_inode_to_shrink() was a bit faster, so, if this > > patch is correct, I think it is worth having to reduce lock contention. > > How much faster? Filipe has asked the same question, and I used that thread to reply, let's use that thread for performance discussions. > We are using write_trylock() already, meaning if it's not locked we should > get the lock immediately, otherwise we skip the inode. You are right that the shrinker itself never waits. But there are two costs left that the trylock does not avoid: 1) Even uncontended, the atomic is not free. write_trylock() is a cmpxchg that has to pull the extent_map_tree cache line in exclusive. 2) The trylock only protects the shrinker from waiting. When it succeeds we do hold the lock, briefly, on an inode we are about to skip anyway, and anyone arriving in that window block While my microbenchmark results in Filipe's answer might be skewed toward this case, the fleet profiler shows find_first_inode_to_shrink() is far from negligible in production.