Re: [External] : [PATCH v2 07/50] convert simple_{link,unlink,rmdir,rename,fill_super}() to new primitives

Al Viro
2025-10-29T17:55:06Z

On Wed, Oct 29, 2025 at 09:02:33AM -0500, Mark Tinguely wrote:
> On 10/27/25 7:45 PM, Al Viro wrote:
> > Note that simple_unlink() et.al. are used by many filesystems; for now
> > they can not assume that persistency mark will have been set back
> > when the object got created. Once all conversions are done we'll
> > have them complain if called for something that had not been marked
> > persistent.
> >
> > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> > ---
> > fs/libfs.c | 10 +++++-----
> > 1 file changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/fs/libfs.c b/fs/libfs.c
> > index a033f35493d0..80f288a771e3 100644
> > --- a/fs/libfs.c
> > +++ b/fs/libfs.c
>
> ...
>
> > EXPORT_SYMBOL(simple_unlink);
> > @@ -1078,7 +1077,8 @@ int simple_fill_super(struct super_block *s, unsigned long magic,
> > simple_inode_init_ts(inode);
> > inode->i_fop = files->ops;
> > inode->i_ino = i;
> > - d_add(dentry, inode);
> > + d_make_persistent(dentry, inode);
> > + dput(dentry);
> > }
> > return 0;
> > }
>
> Putting on the dunce hat for the rest of us:
>
> I think I understand the dput() for d_add() changes, but it is non-obvious.
> Thinking of future maintenance, you may want to make a comment.

As in
dput(dentry); // paired with d_alloc_name()
or
dput(dentry); // that would've been simple_done_creating(),
// if we bothered with directory lock here
or...?

The thing is, d_alloc_name()/dput() instead of simple_start_creating()/
simple_done_creating() is a bit of a shortcut, possible since we
* know that in this case nobody else could access that fs
(we are in the middle of setting it up)
* know that directory we are populating started empty (we'd just
created it) and nobody else had a chance to mess with it (see above)
* trust the caller to have all names in files[] array valid and
unique

And for simple_fill_super() that's pretty straightforward, but in other
cases... Rationale for taking that shortcut needs to be good.