Re: [PATCH v3] kernfs: allocate inodes and dentry outside kernfs_rwsem

[Jori Koolstra]

Hi Shakeel,

> Op 19-10-2026 19:34 EDT schreef Shakeel Butt <shakeel.butt@xxxxxxxxx>:
> 
>  
> Hi Jori and thanks a lot for taking a look.
> 

You're welcome! Thanks for looking into this.

> > 
> > I understand this is intended to have some kind of ownership semantics,
> > but I must say for such a simple function, I find the contract quite
> > complex to check. Do we really need an out parameter here?
> 
> Not really. I just used it simply call iput(new) without a check. I can change
> it and make it more like inode_insert5() and let caller do check before calling
> iput().
> 

I'd prefer no out parameter then. I actually need the same function, more or less.
My problem is that I want inode allocation before calling scops->mkdir, so that
if that goes -ENOMEM, then we don't create the directory. However, since I can
expect that there's not going to be an inode present, the ilookup is not needed.
So I guess I need a kernfs_get_inode_locked_fast() or something.

But also, shouldn't we just replicate the structure of iget5_locked() here? Except
that we pass in an allocated inode. We invoke a lot more code if we rely on iput()
instead of destroy_inode(). Then maybe we need to use alloc_inode() instead of
new_inode() as well.

> > 
> [...]
> > > @@ -294,7 +298,7 @@ static int kernfs_fill_super(struct super_block *sb, struct kernfs_fs_context *k
> > >  {
> > >  	struct kernfs_super_info *info = kernfs_info(sb);
> > >  	struct kernfs_root *kf_root = kfc->root;
> > > -	struct inode *inode;
> > > +	struct inode *inode, *new;
> > >  	struct dentry *root;
> > >  
> > >  	info->sb = sb;
> > > @@ -314,10 +318,12 @@ static int kernfs_fill_super(struct super_block *sb, struct kernfs_fs_context *k
> > >  	sb->s_shrink->seeks = 0;
> > >  
> > >  	/* get root inode, initialize and unlock it */
> > > +	new = new_inode(sb);
> > >  	down_read(&kf_root->kernfs_rwsem);
> > > -	inode = kernfs_get_inode(sb, info->root->kn);
> > > +	inode = kernfs_get_inode_locked(sb, info->root->kn, &new);
> > 
> > What are you serializing against here? I've been looking at this as well
> > recently, and I thought we could just get rid of the locking here. But
> > on second look, I am not so sure what is correct.
> > 
> > Is there anything that serializes kernfs_get_tree() with root node
> > removal?
> 
> That is a good question and honestly I didn't know the answer. After digging a
> bit, I think 07/12 patch [1] of your series is right.
> 
> Superblock is still private here and it is added to the supers list after
> kernfs_fill_super. Root removal is not serialized by kernfs and kernfs users
> keep the root alive across the mount.
> 

I am not sure that last statement is true for all kernfs users. I will have to
verify.

> I can drop the kernfs_fill_super change from my patch and I think your patch
> will cover it better.
> 

OK.

> [1] https://lore.kernel.org/20261016143806.2946578-8-jkoolstra@xxxxxxxxx

Best,
Jori.