Re: [PATCH v3 08/38] vfs: make vfs_mknod break delegations on parent directory

On Fri, 2025-09-26 at 17:32 +0200, Jan Kara wrote:
> On Wed 24-09-25 14:05:54, Jeff Layton wrote:
> > In order to add directory delegation support, we need to break
> > delegations on the parent whenever there is going to be a change in the
> > directory.
> > 
> > Rename vfs_mknod as __vfs_mknod, make it static, and add a new
> > delegated_inode parameter.  Make do_mknodat call __vfs_mknod and wait
> > synchronously for delegation breaks to complete. Add a new exported
> > vfs_mknod wrapper that calls __vfs_mknod with a NULL delegated_inode
> > pointer.
> > 
> > Signed-off-by: Jeff Layton <[email protected]>
> 
> Looks good. Feel free to add:
> 
> Reviewed-by: Jan Kara <[email protected]>
> 

Thanks.

FYI, I've revised this and the rmdir patches to get rid of the wrapper,
and just have the callers pass in NULL directly. I think that's more
along the lines of what Christian preferred.


> 
> > ---
> >  fs/namei.c | 57 +++++++++++++++++++++++++++++++++++----------------------
> >  1 file changed, 35 insertions(+), 22 deletions(-)
> > 
> > diff --git a/fs/namei.c b/fs/namei.c
> > index d4b8330a3eb97e205dc2e71766fed1e45503323b..7bcd898c84138061030f1f8b91273261cdf2a9b4 100644
> > --- a/fs/namei.c
> > +++ b/fs/namei.c
> > @@ -4215,24 +4215,9 @@ inline struct dentry *user_path_create(int dfd, const char __user *pathname,
> >  }
> >  EXPORT_SYMBOL(user_path_create);
> >  
> > -/**
> > - * vfs_mknod - create device node or file
> > - * @idmap:	idmap of the mount the inode was found from
> > - * @dir:	inode of the parent directory
> > - * @dentry:	dentry of the child device node
> > - * @mode:	mode of the child device node
> > - * @dev:	device number of device to create
> > - *
> > - * Create a device node or file.
> > - *
> > - * If the inode has been found through an idmapped mount the idmap of
> > - * the vfsmount must be passed through @idmap. This function will then take
> > - * care to map the inode according to @idmap before checking permissions.
> > - * On non-idmapped mounts or if permission checking is to be performed on the
> > - * raw inode simply pass @nop_mnt_idmap.
> > - */
> > -int vfs_mknod(struct mnt_idmap *idmap, struct inode *dir,
> > -	      struct dentry *dentry, umode_t mode, dev_t dev)
> > +static int __vfs_mknod(struct mnt_idmap *idmap, struct inode *dir,
> > +		       struct dentry *dentry, umode_t mode, dev_t dev,
> > +		       struct inode **delegated_inode)
> >  {
> >  	bool is_whiteout = S_ISCHR(mode) && dev == WHITEOUT_DEV;
> >  	int error = may_create(idmap, dir, dentry);
> > @@ -4256,11 +4241,37 @@ int vfs_mknod(struct mnt_idmap *idmap, struct inode *dir,
> >  	if (error)
> >  		return error;
> >  
> > +	error = try_break_deleg(dir, delegated_inode);
> > +	if (error)
> > +		return error;
> > +
> >  	error = dir->i_op->mknod(idmap, dir, dentry, mode, dev);
> >  	if (!error)
> >  		fsnotify_create(dir, dentry);
> >  	return error;
> >  }
> > +
> > +/**
> > + * vfs_mknod - create device node or file
> > + * @idmap:	idmap of the mount the inode was found from
> > + * @dir:	inode of the parent directory
> > + * @dentry:	dentry of the child device node
> > + * @mode:	mode of the child device node
> > + * @dev:	device number of device to create
> > + *
> > + * Create a device node or file.
> > + *
> > + * If the inode has been found through an idmapped mount the idmap of
> > + * the vfsmount must be passed through @idmap. This function will then take
> > + * care to map the inode according to @idmap before checking permissions.
> > + * On non-idmapped mounts or if permission checking is to be performed on the
> > + * raw inode simply pass @nop_mnt_idmap.
> > + */
> > +int vfs_mknod(struct mnt_idmap *idmap, struct inode *dir,
> > +	      struct dentry *dentry, umode_t mode, dev_t dev)
> > +{
> > +	return __vfs_mknod(idmap, dir, dentry, mode, dev, NULL);
> > +}
> >  EXPORT_SYMBOL(vfs_mknod);
> >  
> >  static int may_mknod(umode_t mode)
> > @@ -4314,12 +4325,14 @@ static int do_mknodat(int dfd, struct filename *name, umode_t mode,
> >  				security_path_post_mknod(idmap, dentry);
> >  			break;
> >  		case S_IFCHR: case S_IFBLK:
> > -			error = vfs_mknod(idmap, path.dentry->d_inode,
> > -					  dentry, mode, new_decode_dev(dev));
> > +			error = __vfs_mknod(idmap, path.dentry->d_inode,
> > +					    dentry, mode, new_decode_dev(dev),
> > +					    &delegated_inode);
> >  			break;
> >  		case S_IFIFO: case S_IFSOCK:
> > -			error = vfs_mknod(idmap, path.dentry->d_inode,
> > -					  dentry, mode, 0);
> > +			error = __vfs_mknod(idmap, path.dentry->d_inode,
> > +					    dentry, mode, 0,
> > +					    &delegated_inode);
> >  			break;
> >  	}
> >  out2:
> > 
> > -- 
> > 2.51.0
> > 

-- 
Jeff Layton <[email protected]>