Re: [REGRESSION] fsnotify: concurrent inotify watch add/remove slows after 94bd01253c3d

[Jan Kara]

Hello!

On Tue 21-07-26 00:00:32, Chengfeng Lin wrote:
> I found a repeatable slowdown in a concurrent inotify workload. An exact
> bare-metal comparison points to:
> 
>   94bd01253c3d ("fsnotify: Track inode connectors for a superblock")
> 
> #regzbot introduced: 94bd01253c3d
> #regzbot title: fsnotify concurrent inotify watch add/remove slowdown
> 
> This is a focused synthetic microbenchmark, not an application benchmark. It
> uses tmpfs, 96 separate inotify instances, and 96 files with different inodes.
> Eight threads, each pinned to a different P-core, concurrently add one watch
> for each instance/inode pair and then remove it.

Thanks for the detailed and throughout analysis! TL;DR for the ones too
lazy to read the whole report: Heavily parallel addition of inode marks to
distinct inodes contends on the superblock's inode_conn_list (and the
corresponding lock) which can reduce scalability of adding inode marks.

Just as a verification: Do I get it right that you observe no regression for
a case where single process is adding inode marks?

If the single-threaded case indeed isn't measurably affected, then I'm not
seriously concerned. Adding inode watch is comparably rare, I would
consider workloads which intensively add inode marks as kind of
pathological and so a scalability regression is acceptable tradeoff in my
opinion (at least for the time being). That being said I have experimental
patches sitting in my local tree which replace the list with rhashtable and
avoid pinning inodes by inode marks. These patches should improve the
scalability as well as a side effect. But I have to find time to revive
them and push them to completion...

								Honza 

> The matched control uses the same instances and pathnames, but all pathnames
> are hard links to one inode. A keeper watch keeps that inode's connector alive
> during timing. The control therefore measures watch add/remove without
> connector creation and destruction.
> 
> I used a fresh boot for each point:
> 
>   6c790212c588 parent A -> 94bd01253c3d child -> 6c790212c588 parent B
> 
> Each point had 2 warm-up rounds and 25 measured rounds. The kernels used the
> same normalized config, compiler, Kbuild metadata, module-signing key,
> preemption mode, and equal-length release strings.
> 
> The machine was an Intel Core i7-12700KF system with 32 GiB RAM. The workers
> used CPUs 0,2,4,6,8,10,12,14. The CPU governor and EPP were set to performance,
> Turbo was disabled, and the runtime setting was preempt=none.
> 
> The primary medians were (lower is better):
> 
>   metric                         parent A      child   parent B
>   P8 distinct, aggregate worker ns/watch
>                                   4182.614   4920.865   4274.895
>   P8 distinct/shared ratio       0.248777   0.292725   0.240657
> 
> P8 means eight workers. The first metric sums the time spent by all workers
> adding and removing watches, then divides by 96; it is not wall-clock time.
> The ratio compares each distinct-inode round with its matched shared-inode
> control round.
> 
> Relative to parent A and parent B, the child was slower by 17.65% and 15.11%
> for the distinct-inode metric, and by 17.67% and 21.64% for the ratio. Parent
> drift was 2.18% and 3.32%, respectively. All correctness and affinity checks
> passed. Updating an existing watch mask and pathname lookup both changed by
> less than 0.3%; the shared-inode control was slightly faster in the child.
> 
> A second parent-child-parent sequence checked worker-count scaling. At P6,
> the distinct-inode metric increased by 9.03% and 9.56%, and the ratio by
> 10.75% and 10.85%. At P8, the distinct-inode increases were 19.58% and
> 19.11%, and the ratio increases were 17.62% and 16.80%. Exact P1 and P4 tests
> stayed below the pre-registered 5% threshold. A 16-worker SMT run changed in
> the same direction, but missed the variability gate and is not part of the
> main result.
> 
> I also tested two diagnostic variants at the child commit in this order:
> 
>   full -> lock-only -> full -> nolist -> full
> 
> Full is the unmodified child. The reductions below are averages relative to
> the full builds immediately before and after each probe:
> 
>   build       list updates   list_lock   P8 distinct reduction   ratio reduction
>   full        kept           kept        baseline                baseline
>   lock-only   removed        kept        14.27%                  12.70%
>   nolist      removed        removed     21.43%                  20.46%
> 
> The lock-only result shows a cost from the list updates or the extra time they
> hold the lock. Removing the lock recovered about 7 percentage points more.
> This suggests that lock acquisition and handoff also contribute. These builds
> are diagnostic probes, not proposed fixes.
> 
> Before sending this report, I checked v7.1.4, v7.2-rc4, Linus' master at
> 1590cf032971, next-20260717, and the linux-fs fsnotify, for_next, and for_linus
> branches. Connector creation and detach still use the list_lock and
> list_add()/list_del() operations introduced by 94bd. I found no equivalent fix
> or matching report in the official linux-fsdevel and regressions archives
> through July 20. The exact timing claim above remains limited to the direct
> parent/child comparison.
> 
> As a topology check, I traced eight unmodified Ubuntu inotifywait processes,
> each watching a different subtree of a Linux 7.1.3 source tree on one ext4
> filesystem. They created 4,177 directory watches. For each process, the counts
> of fsnotify_add_mark_locked(), inode-connector allocations, and
> fsnotify_detach_connector_from_object() matched its watch count. Watch-add
> events from all eight processes appeared in the same 1 ms interval. I started
> them together on purpose, so this trace shows that normal software can create
> the tested topology; it does not measure application performance or how often
> this concurrency occurs in normal use.
> 
> I understand that a05fc7edd988 ("fsnotify: Use connector list for destroying
> inode marks") uses this list to avoid scanning all inodes during unmount. I am
> therefore not asking to revert 94bd.
> 
> Is the extra watch add/remove cost an expected trade-off? If not, would it be
> useful to develop an optimization that preserves the connector list and the
> current unmount behavior?
> 
> Evidence bundle:
> 
>   https://github.com/lcf0399/linux-regression-evidence/tree/245fec71aa05eb2dbd68f11e4c98729f22bbc541/fsnotify-concurrent-inotify-watch-setup
> 
> Standalone reproducer:
> 
>   https://github.com/lcf0399/linux-regression-evidence/tree/245fec71aa05eb2dbd68f11e4c98729f22bbc541/fsnotify-concurrent-inotify-watch-setup/reproducer
> 
> Thanks,
> Chengfeng
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR