On Sat, 13 Sep 2025, Al Viro wrote: > On Sat, Sep 13, 2025 at 01:36:49PM +1000, NeilBrown wrote: > > > > Umm... Unless I'm misunderstanding you, that *would* change the > > > calling conventions - dentry could bloody well be positive, couldn't it? > > > And that changes quite a bit - without O_CREAT in flags you could get > > > parent locked only shared and pass a positive hashed dentry attached > > > to a directory inode to ->atomic_open(). The thing is, in that case it > > > can be moved by d_splice_alias()... > > > > Once we get per-dentry locking for dirops this will cease to be a > > problem. The dentry would be locked exclusively whether we create or > > not and the lock would prevent the d_splice_alias() rename. > > Umm... Interesting, but... what would happen in such case, really? > > You have one thread with allegedly directory dentry it had skipped > verification for. It tries to combine open with revalidation, and > sends such request. In the meanwhile, another thread has found the > same thing in a different place - possibly because of fs corruption, > possibly because the damn thing got moved on server, right after it has > sent "OK, I've opened it" reply to the first thread. What would you do? > Have the second thread spin/fail with some error/something else? If there is a race with d_splice_alias() wanting the move the directory while open_atomic has it locked, -ESTALE will be returned and the lookup will be retried with LOOKUP_REVAL which should allow the filesystem to do any needed synchronisation. If the directory has simply been moved on the server, then the revalidate in atomic_open will notice this and invalidate the old dentry and use lookup to create a new one. Importantly it will have a stable d_name so that it can do that. This "invalidate and create a new one with the same name" will require careful locking but should be achievable. NeilBrown
