Re: [PATCH 3/4] fuse: remove redundant calls to fuse_copy_finish() in fuse_notify()

[Luis Henriques <luis@igalia.com>]

On Tue, Sep 02 2025, Miklos Szeredi wrote:

> Remove tail calls of fuse_copy_finish(), since it's now done from
> fuse_dev_do_write().
>
> No functional change.
>
> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
> ---
>  fs/fuse/dev.c | 79 +++++++++++++++------------------------------------
>  1 file changed, 23 insertions(+), 56 deletions(-)
>
> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
> index 85d05a5e40e9..1258acee9704 100644
> --- a/fs/fuse/dev.c
> +++ b/fs/fuse/dev.c
> @@ -1622,35 +1622,31 @@ static int fuse_notify_poll(struct fuse_conn *fc, unsigned int size,
>  			    struct fuse_copy_state *cs)
>  {
>  	struct fuse_notify_poll_wakeup_out outarg;
> -	int err = -EINVAL;
> +	int err;
>  
>  	if (size != sizeof(outarg))
> -		goto err;
> +		return -EINVAL;
>  
>  	err = fuse_copy_one(cs, &outarg, sizeof(outarg));
>  	if (err)
> -		goto err;
> +		return err;
>  
>  	fuse_copy_finish(cs);
>  	return fuse_notify_poll_wakeup(fc, &outarg);
> -
> -err:
> -	fuse_copy_finish(cs);
> -	return err;
>  }
>  
>  static int fuse_notify_inval_inode(struct fuse_conn *fc, unsigned int size,
>  				   struct fuse_copy_state *cs)
>  {
>  	struct fuse_notify_inval_inode_out outarg;
> -	int err = -EINVAL;
> +	int err;
>  
>  	if (size != sizeof(outarg))
> -		goto err;
> +		return -EINVAL;
>  
>  	err = fuse_copy_one(cs, &outarg, sizeof(outarg));
>  	if (err)
> -		goto err;
> +		return err;
>  	fuse_copy_finish(cs);
 
I wonder if these extra fuse_copy_finish() calls should also be removed.
It doesn't seem to be a problem to call it twice, but maybe it's not
needed, or am I missing something?  This happens in a few places.

Other than that, and FWIW, the series look good to me.

Cheers,
-- 
Luís

> 
>  	down_read(&fc->killsb);
> @@ -1658,10 +1654,6 @@ static int fuse_notify_inval_inode(struct fuse_conn *fc, unsigned int size,
>  				       outarg.off, outarg.len);
>  	up_read(&fc->killsb);
>  	return err;
> -
> -err:
> -	fuse_copy_finish(cs);
> -	return err;
>  }
>  
>  static int fuse_notify_inval_entry(struct fuse_conn *fc, unsigned int size,
> @@ -1669,29 +1661,26 @@ static int fuse_notify_inval_entry(struct fuse_conn *fc, unsigned int size,
>  {
>  	struct fuse_notify_inval_entry_out outarg;
>  	int err;
> -	char *buf = NULL;
> +	char *buf;
>  	struct qstr name;
>  
> -	err = -EINVAL;
>  	if (size < sizeof(outarg))
> -		goto err;
> +		return -EINVAL;
>  
>  	err = fuse_copy_one(cs, &outarg, sizeof(outarg));
>  	if (err)
> -		goto err;
> +		return err;
>  
> -	err = -ENAMETOOLONG;
>  	if (outarg.namelen > fc->name_max)
> -		goto err;
> +		return -ENAMETOOLONG;
>  
>  	err = -EINVAL;
>  	if (size != sizeof(outarg) + outarg.namelen + 1)
> -		goto err;
> +		return -EINVAL;
>  
> -	err = -ENOMEM;
>  	buf = kzalloc(outarg.namelen + 1, GFP_KERNEL);
>  	if (!buf)
> -		goto err;
> +		return -ENOMEM;
>  
>  	name.name = buf;
>  	name.len = outarg.namelen;
> @@ -1704,12 +1693,8 @@ static int fuse_notify_inval_entry(struct fuse_conn *fc, unsigned int size,
>  	down_read(&fc->killsb);
>  	err = fuse_reverse_inval_entry(fc, outarg.parent, 0, &name, outarg.flags);
>  	up_read(&fc->killsb);
> -	kfree(buf);
> -	return err;
> -
>  err:
>  	kfree(buf);
> -	fuse_copy_finish(cs);
>  	return err;
>  }
>  
> @@ -1718,29 +1703,25 @@ static int fuse_notify_delete(struct fuse_conn *fc, unsigned int size,
>  {
>  	struct fuse_notify_delete_out outarg;
>  	int err;
> -	char *buf = NULL;
> +	char *buf;
>  	struct qstr name;
>  
> -	err = -EINVAL;
>  	if (size < sizeof(outarg))
> -		goto err;
> +		return -EINVAL;
>  
>  	err = fuse_copy_one(cs, &outarg, sizeof(outarg));
>  	if (err)
> -		goto err;
> +		return err;
>  
> -	err = -ENAMETOOLONG;
>  	if (outarg.namelen > fc->name_max)
> -		goto err;
> +		return -ENAMETOOLONG;
>  
> -	err = -EINVAL;
>  	if (size != sizeof(outarg) + outarg.namelen + 1)
> -		goto err;
> +		return -EINVAL;
>  
> -	err = -ENOMEM;
>  	buf = kzalloc(outarg.namelen + 1, GFP_KERNEL);
>  	if (!buf)
> -		goto err;
> +		return -ENOMEM;
>  
>  	name.name = buf;
>  	name.len = outarg.namelen;
> @@ -1753,12 +1734,8 @@ static int fuse_notify_delete(struct fuse_conn *fc, unsigned int size,
>  	down_read(&fc->killsb);
>  	err = fuse_reverse_inval_entry(fc, outarg.parent, outarg.child, &name, 0);
>  	up_read(&fc->killsb);
> -	kfree(buf);
> -	return err;
> -
>  err:
>  	kfree(buf);
> -	fuse_copy_finish(cs);
>  	return err;
>  }
>  
> @@ -1776,17 +1753,15 @@ static int fuse_notify_store(struct fuse_conn *fc, unsigned int size,
>  	loff_t file_size;
>  	loff_t end;
>  
> -	err = -EINVAL;
>  	if (size < sizeof(outarg))
> -		goto out_finish;
> +		return -EINVAL;
>  
>  	err = fuse_copy_one(cs, &outarg, sizeof(outarg));
>  	if (err)
> -		goto out_finish;
> +		return err;
>  
> -	err = -EINVAL;
>  	if (size - sizeof(outarg) != outarg.size)
> -		goto out_finish;
> +		return -EINVAL;
>  
>  	nodeid = outarg.nodeid;
>  
> @@ -1846,8 +1821,6 @@ static int fuse_notify_store(struct fuse_conn *fc, unsigned int size,
>  	iput(inode);
>  out_up_killsb:
>  	up_read(&fc->killsb);
> -out_finish:
> -	fuse_copy_finish(cs);
>  	return err;
>  }
>  
> @@ -1962,13 +1935,12 @@ static int fuse_notify_retrieve(struct fuse_conn *fc, unsigned int size,
>  	u64 nodeid;
>  	int err;
>  
> -	err = -EINVAL;
>  	if (size != sizeof(outarg))
> -		goto copy_finish;
> +		return -EINVAL;
>  
>  	err = fuse_copy_one(cs, &outarg, sizeof(outarg));
>  	if (err)
> -		goto copy_finish;
> +		return err;
>  
>  	fuse_copy_finish(cs);
>  
> @@ -1984,10 +1956,6 @@ static int fuse_notify_retrieve(struct fuse_conn *fc, unsigned int size,
>  	up_read(&fc->killsb);
>  
>  	return err;
> -
> -copy_finish:
> -	fuse_copy_finish(cs);
> -	return err;
>  }
>  
>  /*
> @@ -2098,7 +2066,6 @@ static int fuse_notify(struct fuse_conn *fc, enum fuse_notify_code code,
>  		return fuse_notify_inc_epoch(fc);
>  
>  	default:
> -		fuse_copy_finish(cs);
>  		return -EINVAL;
>  	}
>  }
> -- 
> 2.49.0
>