On Mon, Dec 09, 2024 at 07:33:00AM +0100, Mateusz Guzik wrote: > On Mon, Dec 09, 2024 at 03:52:51AM +0000, Al Viro wrote: > > There's a bunch of places where we are accessing dentry names without > > sufficient protection and where locking environment is not predictable > > enough to fix the things that way; take_dentry_name_snapshot() is > > one variant of solution. It does, however, have a problem - copying > > is cheap, but bouncing ->d_lock may be nasty on seriously shared dentries. > > > > How about the following (completely untested)? > > > > Use ->d_seq instead of grabbing ->d_lock; in case of shortname dentries > > that avoids any stores to shared data objects and in case of long names > > we are down to (unavoidable) atomic_inc on the external_name refcount. > > > > Makes the thing safer as well - the areas where ->d_seq is held odd are > > all nested inside the areas where ->d_lock is held, and the latter are > > much more numerous. > > Is there a problem retaining the lock acquire if things fail? > > As in maybe loop 2-3 times, but eventually take the lock to guarantee forward > progress. > > I don't think there is a *real* workload where this would be a problem, > but with core counts seen today one may be able to purposefuly introduce > stalls when running this. By renaming the poor sucker back and forth in a tight loop? Would be hard to trigger on anything short of ramfs... Hell knows - if anything, I was thinking about a variant that would *not* loop at all, but take seq as an argument and return whether it had been successful. That could be adapted to build such thing - with "pass ->d_seq sampled value (always even) *or* call it with the name stabilized in some other way (e.g. ->d_lock, rename_lock or ->s_vfs_rename_mutex held) and pass 1 as argument to suppress checks" for calling conventions. The thing is, when its done to a chain of ancestors of some dentry, with rename_lock retries around the entire thing, running into ->d_seq change pretty much guarantees that you'll need to retry the whole thing anyway.
