Re: [PATCH v3 10/19] VFS/ovl: add d_alloc_noblock_return()

NeilBrown
2026-04-28T11:44:09Z

On Tue, 28 Apr 2026, Al Viro wrote:
> On Tue, Apr 28, 2026 at 10:34:50AM +1000, NeilBrown wrote:
>
> > > This contract is a bit subtle.
> > > We have plenty of contracts where the caller must dput() in case of success
> > > or in case of error, but must dput in case of a specific error that
> > > sounds fragile.
> > >
> > > How about:
> > > * If the existing dentry is d_in_lookup(), d_alloc_noblock()
> > > * returns with error %-EWOULDBLOCK and the blocking dentry is passed
> > > * in @dentryp. Regardless of the returned error, if @dentryp is set by this
> > > * function, the returned dentry must be dput() by the caller.
> >
> > That is sensible, though I've used slightly different words.
>
> I would add "dentry reference stored in *dentryp may be in any state -
> the only thing promised is that the reference is counting one;
> do *NOT* expect it to be in-lookup or in the same directory or
> hashed at any point or anything whatsoever, really. Users beware."
>
> In case it's not obvious from the above, I don't think it's a safe API to have -
> there's very little you can do to that dentry other than dput() it and it does
> change things in a fairly subtle way: right now in-lookup dentry is *not*
> visible to any thread other than caller of d_alloc_parallel() that has created
> it. In particular, d_in_lookup(dentry) can only change due to actions of the
> same thread. With this primitive added this is no longer obviously true.
>

Maybe we could require the caller to hold an exclusive lock on the parent
inode (which the only proposed use case does). Then we can promise that
the dentry will remain in-lookup as the owner will not proceed with the
lookup until it can get a shared lock.

* The caller MUST hold an exclusive lock on the parent inode and can
* be certain that if a dentry is provided in @dentryp, then it is
* in-lookup and will remain in-lookup while the lock is held.

I don't have the patches to prove it yet but I expect this to be a
short-lived interface. Once ovl has full control of the locking context
it should be able to allow lookup to always proceed while readdir is
active, so readdir will be able to use a regular d_alloc_parallel()
call.

Thanks,
NeilBrown