Re: [PATCH v2 7/8] vfs: open inside ->tmpfile()

[Al Viro <viro@xxxxxxxxxxxxxxxxxx>]

On Mon, Sep 19, 2022 at 04:10:30PM +0200, Miklos Szeredi wrote:
> This is in preparation for adding tmpfile support to fuse, which requires
> that the tmpfile creation and opening are done as a single operation.
> 
> Replace the 'struct dentry *' argument of i_op->tmpfile with
> 'struct file *'.
> 
> Call finish_open_simple() as the last thing in ->tmpfile() instances (may be
> omitted in the error case).
> 
> Change d_tmpfile() argument to 'struct file *' as well to make callers more
> readable.

It really needs to add to Documentation/filesystems/porting.

> --- a/fs/hugetlbfs/inode.c
> +++ b/fs/hugetlbfs/inode.c
> @@ -889,7 +889,7 @@ static int do_hugetlbfs_mknod(struct inode *dir,
>  			struct dentry *dentry,
>  			umode_t mode,
>  			dev_t dev,
> -			bool tmpfile)
> +			struct file *tmpfile)
>  {
>  	struct inode *inode;
>  	int error = -ENOSPC;
> @@ -898,7 +898,7 @@ static int do_hugetlbfs_mknod(struct inode *dir,
>  	if (inode) {
>  		dir->i_ctime = dir->i_mtime = current_time(dir);
>  		if (tmpfile) {
> -			d_tmpfile(dentry, inode);
> +			d_tmpfile(tmpfile, inode);
>  		} else {
>  			d_instantiate(dentry, inode);
>  			dget(dentry);/* Extra count - pin the dentry in core */
> @@ -911,7 +911,7 @@ static int do_hugetlbfs_mknod(struct inode *dir,
>  static int hugetlbfs_mknod(struct user_namespace *mnt_userns, struct inode *dir,
>  			   struct dentry *dentry, umode_t mode, dev_t dev)
>  {
> -	return do_hugetlbfs_mknod(dir, dentry, mode, dev, false);
> +	return do_hugetlbfs_mknod(dir, dentry, mode, dev, NULL);
>  }
>  
>  static int hugetlbfs_mkdir(struct user_namespace *mnt_userns, struct inode *dir,
> @@ -932,10 +932,12 @@ static int hugetlbfs_create(struct user_namespace *mnt_userns,
>  }
>  
>  static int hugetlbfs_tmpfile(struct user_namespace *mnt_userns,
> -			     struct inode *dir, struct dentry *dentry,
> +			     struct inode *dir, struct file *file,
>  			     umode_t mode)
>  {
> -	return do_hugetlbfs_mknod(dir, dentry, mode | S_IFREG, 0, true);
> +	int err = do_hugetlbfs_mknod(dir, file->f_path.dentry, mode | S_IFREG, 0, file);
> +
> +	return finish_open_simple(file, err);
>  }
>  
>  static int hugetlbfs_symlink(struct user_namespace *mnt_userns,

I would prefer to separate tmpfile from mknod in that one.  And to hell
with the last argument in do_hugetlbfs_mknod().  Something like (completely
untested) patch below as prereq:

diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index f7a5b5124d8a..0b458beb318c 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -885,33 +885,18 @@ static struct inode *hugetlbfs_get_inode(struct super_block *sb,
 /*
  * File creation. Allocate an inode, and we're done..
  */
-static int do_hugetlbfs_mknod(struct inode *dir,
-			struct dentry *dentry,
-			umode_t mode,
-			dev_t dev,
-			bool tmpfile)
+static int hugetlbfs_mknod(struct user_namespace *mnt_userns, struct inode *dir,
+			   struct dentry *dentry, umode_t mode, dev_t dev)
 {
 	struct inode *inode;
-	int error = -ENOSPC;
 
 	inode = hugetlbfs_get_inode(dir->i_sb, dir, mode, dev);
-	if (inode) {
-		dir->i_ctime = dir->i_mtime = current_time(dir);
-		if (tmpfile) {
-			d_tmpfile(dentry, inode);
-		} else {
-			d_instantiate(dentry, inode);
-			dget(dentry);/* Extra count - pin the dentry in core */
-		}
-		error = 0;
-	}
-	return error;
-}
-
-static int hugetlbfs_mknod(struct user_namespace *mnt_userns, struct inode *dir,
-			   struct dentry *dentry, umode_t mode, dev_t dev)
-{
-	return do_hugetlbfs_mknod(dir, dentry, mode, dev, false);
+	if (!inode)
+		return -ENOSPC;
+	dir->i_ctime = dir->i_mtime = current_time(dir);
+	d_instantiate(dentry, inode);
+	dget(dentry);/* Extra count - pin the dentry in core */
+	return 0;
 }
 
 static int hugetlbfs_mkdir(struct user_namespace *mnt_userns, struct inode *dir,
@@ -935,7 +920,14 @@ static int hugetlbfs_tmpfile(struct user_namespace *mnt_userns,
 			     struct inode *dir, struct dentry *dentry,
 			     umode_t mode)
 {
-	return do_hugetlbfs_mknod(dir, dentry, mode | S_IFREG, 0, true);
+	struct inode *inode;
+
+	inode = hugetlbfs_get_inode(dir->i_sb, dir, mode | S_IFREG, 0);
+	if (!inode)
+		return -ENOSPC;
+	dir->i_ctime = dir->i_mtime = current_time(dir);
+	d_tmpfile(dentry, inode);
+	return 0;
 }
 
 static int hugetlbfs_symlink(struct user_namespace *mnt_userns,