Re: [PATCH v2 22/50] convert efivarfs

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

On Tue, Oct 28, 2025 at 08:53:45AM -0400, James Bottomley wrote:

> That dput looks misplaced in a creation routine and this is a common
> pattern in pseudo filesystems that either pre-populate the dentry state
> or create effectively unused dentries on other changes.  I know not
> every pseudo filesystem does this, but it did make me wonder if it
> should have it's own API, say d_create_persistent()?

That dput() is paired with efivarfs_alloc_dentry(); the real problem
here is different - efivarfs_create_dentry() relies upon the external
serialization.  Have it race with lookup (let alone unlink()) and
there's a lot of headache.

Most of the callers should be safe, but... I'm not sure that unfreeze
case can't run into trouble.

It might need to be fixed; I don't want to mix that with this series,
so I went for the minimal transformation here.  I suspect that we
ought to use simple_start_creating()/simple_done_creating() instead
of those efivarfs_alloc_dentry()/dput(), but I'll need to look at
the locking environments in all call chains ;-/

FWIW, having a special path for "we are in foofs_fill_super(), fuck
the locking - nobody's going to access it anyway" is not a great
idea, simply because the helpers tend to get reused on codepaths
where we can't cut corners that way.

It *may* be useful to grow a set of primitives for something like "we are
forming a detached tree, will splice it into the final position once we
are entirely done", and configfs might shed a lot of kludges if massaged
in that direction, but I'd rather see what configfs massage converges
to before settling on an API for that.