Re: ->atomic_open() fun (was Re: [RFC] a possible way of reducing the PITA of ->d_name audits)

[Al Viro <viro@zeniv.linux.org.uk>]

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?

Alternatively, move succeeds first, the lookup in the new place arrives,
then revalidate+open.  The first thread gets a nodeid mismatch, and...?

How would that combined revalidate+open work for fuse, anyway?  The former
is basically a lookup - you send nodeid of parent + name, get nodeid +
attributes of child.  The latter goes strictly by nodeid of child and
gets a 64bit number that apparently tells one opened file from another
(not to be confused with fhandle).  Combined request of some sort?

I think we need Bernd to bring a fresh braindump on the plans in that
area.  And we'd damn better *NOT* make it another "this filesystem is
special, the locking works for it because $REASONS (half missing from
the discussion) and nobody else returns that special ->d_revalidate()
return value (at the moment)" - every time something of that sort had
been kludged in we ended up paying for that later.