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

From: Al Viro <[email-masked]>
Date: Wed, 29 Oct 2025 17:55:01 +0000

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 <[email-masked]>
> > ---
> >   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.