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

Christian Brauner
2024-08-07T14:26:51Z

> +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;

Hm, the audit_inode() on the parent is done independent of whether the
file was actually created or not. But the audit_inode() on the file
itself is only done when it was actually created. Imho, there's no need
to do audit_inode() on the parent when we immediately find that file
already existed. If we accept that then this makes the change a lot
simpler.

The inconsistency would partially remain though. When the file doesn't
exist audit_inode() on the parent is called but by the time we've
grabbed the inode lock someone else might already have created the file
and then again we wouldn't audit_inode() on the file but we would have
on the parent.

I think that's fine. But if that's bothersome the more aggressive thing
to do would be to pull that audit_inode() on the parent further down
after we created the file. Imho, that should be fine?...

See https://gitlab.com/brauner/linux/-/commits/vfs.misc.jeff/?ref_type=heads
for a completely untested draft of what I mean.