Re: [PATCH v2] fs: try an opportunistic lookup for O_CREAT opens too

[Al Viro]

On Tue, Aug 06, 2024 at 03:51:35PM -0400, Jeff Layton wrote:

> > +static struct dentry *lookup_fast_for_open(struct nameidata *nd, int open_flag)
> > +{
> > +	struct dentry *dentry;
> > +
> > +	if (open_flag & O_CREAT) {
> > +		/* Don't bother on an O_EXCL create */
> > +		if (open_flag & O_EXCL)
> > +			return NULL;
> > +
> > +		/*
> > +		 * FIXME: If auditing is enabled, then we'll have to unlazy to
> > +		 * use the dentry. For now, don't do this, since it shifts
> > +		 * contention from parent's i_rwsem to its d_lockref spinlock.
> > +		 * Reconsider this once dentry refcounting handles heavy
> > +		 * contention better.
> > +		 */
> > +		if ((nd->flags & LOOKUP_RCU) && !audit_dummy_context())
> > +			return NULL;
> > +	}
> > +
> > +	if (trailing_slashes(nd))
> > +		nd->flags |= LOOKUP_FOLLOW | LOOKUP_DIRECTORY;
> > +
> > +	dentry = lookup_fast(nd);
> 
> Self-NAK on this patch. We have to test for IS_ERR on the returned
> dentry here. I'll send a v3 along after I've retested it.

That's not the only problem; your "is it negative" test is inherently
racy in RCU mode.  IOW, what is positive at the time you get here can
bloody well go negative immediately afterwards.  Hit that with
O_CREAT and you've got a bogus ENOENT...