Re: [PATCH v2 04/14] fuse: use enum types for header copying

[Jeff Layton <jlayton@kernel.org>]

On Thu, 2026-04-02 at 09:28 -0700, Joanne Koong wrote:
> Use enum types to identify which part of the header needs to be copied.
> This improves the interface and will simplify both kernel-space and
> user-space header addresses copying when buffer rings are added.
> 
> Signed-off-by: Joanne Koong <joannelkoong@gmail.com>
> Reviewed-by: Bernd Schubert <bschubert@ddn.com>
> ---
>  fs/fuse/dev_uring.c | 66 ++++++++++++++++++++++++++++++++++++---------
>  1 file changed, 53 insertions(+), 13 deletions(-)
> 
> diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
> index 86f9bb94b45a..cca795dd72e1 100644
> --- a/fs/fuse/dev_uring.c
> +++ b/fs/fuse/dev_uring.c
> @@ -31,6 +31,15 @@ struct fuse_uring_pdu {
>  
>  static const struct fuse_iqueue_ops fuse_io_uring_ops;
>  
> +enum fuse_uring_header_type {
> +	/* struct fuse_in_header / struct fuse_out_header */
> +	FUSE_URING_HEADER_IN_OUT,
> +	/* per op code header */
> +	FUSE_URING_HEADER_OP,
> +	/* struct fuse_uring_ent_in_out header */
> +	FUSE_URING_HEADER_RING_ENT,
> +};
> +
>  static void uring_cmd_set_ring_ent(struct io_uring_cmd *cmd,
>  				   struct fuse_ring_ent *ring_ent)
>  {
> @@ -575,10 +584,33 @@ static int fuse_uring_out_header_has_err(struct fuse_out_header *oh,
>  	return err;
>  }
>  
> -static __always_inline int copy_header_to_ring(void __user *ring,
> -					       const void *header,
> -					       size_t header_size)
> +static int ring_header_type_offset(enum fuse_uring_header_type type)
>  {
> +	switch (type) {
> +	case FUSE_URING_HEADER_IN_OUT:
> +		return 0;
> +	case FUSE_URING_HEADER_OP:
> +		return offsetof(struct fuse_uring_req_header, op_in);
> +	case FUSE_URING_HEADER_RING_ENT:
> +		return offsetof(struct fuse_uring_req_header, ring_ent_in_out);
> +	default:
> +		WARN_ONCE(1, "Invalid header type: %d\n", type);
> +		return -EINVAL;
> +	}
> +}
> +
> +static int copy_header_to_ring(struct fuse_ring_ent *ent,
> +			       enum fuse_uring_header_type type,
> +			       const void *header, size_t header_size)
> +{
> +	int offset = ring_header_type_offset(type);
> +	void __user *ring;
> +
> +	if (offset < 0)
> +		return offset;
> +
> +	ring = (void __user *)ent->headers + offset;
> +
>  	if (copy_to_user(ring, header, header_size)) {
>  		pr_info_ratelimited("Copying header to ring failed.\n");
>  		return -EFAULT;
> @@ -587,10 +619,18 @@ static __always_inline int copy_header_to_ring(void __user *ring,
>  	return 0;
>  }
>  
> -static __always_inline int copy_header_from_ring(void *header,
> -						 const void __user *ring,
> -						 size_t header_size)

nit: this patch drops the __always_inline's (which is probably a good
idea), but there is no mention of why you did it in the changelog.

> +static int copy_header_from_ring(struct fuse_ring_ent *ent,
> +				 enum fuse_uring_header_type type, void *header,
> +				 size_t header_size)
>  {
> +	int offset = ring_header_type_offset(type);
> +	const void __user *ring;
> +
> +	if (offset < 0)
> +		return offset;
> +
> +	ring = (void __user *)ent->headers + offset;
> +
>  	if (copy_from_user(header, ring, header_size)) {
>  		pr_info_ratelimited("Copying header from ring failed.\n");
>  		return -EFAULT;
> @@ -609,8 +649,8 @@ static int fuse_uring_copy_from_ring(struct fuse_ring *ring,
>  	int err;
>  	struct fuse_uring_ent_in_out ring_in_out;
>  
> -	err = copy_header_from_ring(&ring_in_out, &ent->headers->ring_ent_in_out,
> -				    sizeof(ring_in_out));
> +	err = copy_header_from_ring(ent, FUSE_URING_HEADER_RING_ENT,
> +				    &ring_in_out, sizeof(ring_in_out));
>  	if (err)
>  		return err;
>  
> @@ -661,7 +701,7 @@ static int fuse_uring_args_to_ring(struct fuse_ring *ring, struct fuse_req *req,
>  		 * Some op code have that as zero size.
>  		 */
>  		if (args->in_args[0].size > 0) {
> -			err = copy_header_to_ring(&ent->headers->op_in,
> +			err = copy_header_to_ring(ent, FUSE_URING_HEADER_OP,
>  						  in_args->value,
>  						  in_args->size);
>  			if (err)
> @@ -681,8 +721,8 @@ static int fuse_uring_args_to_ring(struct fuse_ring *ring, struct fuse_req *req,
>  	}
>  
>  	ent_in_out.payload_sz = cs.ring.copied_sz;
> -	return copy_header_to_ring(&ent->headers->ring_ent_in_out, &ent_in_out,
> -				   sizeof(ent_in_out));
> +	return copy_header_to_ring(ent, FUSE_URING_HEADER_RING_ENT,
> +				   &ent_in_out, sizeof(ent_in_out));
>  }
>  
>  static int fuse_uring_copy_to_ring(struct fuse_ring_ent *ent,
> @@ -711,7 +751,7 @@ static int fuse_uring_copy_to_ring(struct fuse_ring_ent *ent,
>  	}
>  
>  	/* copy fuse_in_header */
> -	return copy_header_to_ring(&ent->headers->in_out, &req->in.h,
> +	return copy_header_to_ring(ent, FUSE_URING_HEADER_IN_OUT, &req->in.h,
>  				   sizeof(req->in.h));
>  }
>  
> @@ -806,7 +846,7 @@ static void fuse_uring_commit(struct fuse_ring_ent *ent, struct fuse_req *req,
>  	struct fuse_conn *fc = ring->fc;
>  	ssize_t err = 0;
>  
> -	err = copy_header_from_ring(&req->out.h, &ent->headers->in_out,
> +	err = copy_header_from_ring(ent, FUSE_URING_HEADER_IN_OUT, &req->out.h,
>  				    sizeof(req->out.h));
>  	if (err) {
>  		req->out.h.error = err;

Reviewed-by: Jeff Layton <jlayton@kernel.org>