Re: [PATCH v5 08/13] io_uring: generalize files_update functionlity to rsrc_update

[Pavel Begunkov <asml.silence@gmail.com>]

On 12/01/2021 21:33, Bijan Mottahedeh wrote:
> Generalize files_update functionality to rsrc_update in order to
> leverage it for buffers updates.
> 
> Signed-off-by: Bijan Mottahedeh <bijan.mottahedeh@oracle.com>
> ---
>  fs/io_uring.c                 | 19 ++++++++++++++-----
>  include/uapi/linux/io_uring.h |  6 +++++-
>  2 files changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 6ebfe1f..f9f458c 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -5954,7 +5954,7 @@ static int io_async_cancel(struct io_kiocb *req)
>  }
>  
>  static int io_rsrc_update_prep(struct io_kiocb *req,
> -				const struct io_uring_sqe *sqe)
> +			       const struct io_uring_sqe *sqe)
>  {
>  	if (unlikely(req->ctx->flags & IORING_SETUP_SQPOLL))
>  		return -EINVAL;
> @@ -5971,8 +5971,11 @@ static int io_rsrc_update_prep(struct io_kiocb *req,
>  	return 0;
>  }
>  
> -static int io_files_update(struct io_kiocb *req, bool force_nonblock,
> -			   struct io_comp_state *cs)
> +static int io_rsrc_update(struct io_kiocb *req, bool force_nonblock,
> +			  struct io_comp_state *cs,
> +			  int (*update)(struct io_ring_ctx *ctx,
> +					struct io_uring_rsrc_update *up,
> +					unsigned int nr_args))

I don't like excessive use of higher order functions in C. How about
replacing it with a switch/ifs? e.g.

... /* prepare up */
mutex_lock()
if (req->opcode == FILES_UPDATE)
	update_file(); // the one that was in callback @update arg
else (req->opcode == ...)
	...
mutex_unlock();

>  {
>  	struct io_ring_ctx *ctx = req->ctx;
>  	struct io_uring_rsrc_update up;
> @@ -5982,10 +5985,10 @@ static int io_files_update(struct io_kiocb *req, bool force_nonblock,
>  		return -EAGAIN;
>  
>  	up.offset = req->rsrc_update.offset;
> -	up.fds = req->rsrc_update.arg;
> +	up.rsrc = req->rsrc_update.arg;
>  
>  	mutex_lock(&ctx->uring_lock);
> -	ret = __io_sqe_files_update(ctx, &up, req->rsrc_update.nr_args);
> +	ret = (*update)(ctx, &up, req->rsrc_update.nr_args);
>  	mutex_unlock(&ctx->uring_lock);
>  
>  	if (ret < 0)
> @@ -5994,6 +5997,12 @@ static int io_files_update(struct io_kiocb *req, bool force_nonblock,
>  	return 0;
>  }
>  
> +static int io_files_update(struct io_kiocb *req, bool force_nonblock,
> +			   struct io_comp_state *cs)
> +{
> +	return io_rsrc_update(req, force_nonblock, cs, __io_sqe_files_update);
> +}
> +
>  static int io_req_prep(struct io_kiocb *req, const struct io_uring_sqe *sqe)
>  {
>  	switch (req->opcode) {
> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
> index 77de7c0..f51190b 100644
> --- a/include/uapi/linux/io_uring.h
> +++ b/include/uapi/linux/io_uring.h
> @@ -288,7 +288,11 @@ enum {
>  struct io_uring_rsrc_update {
>  	__u32 offset;
>  	__u32 resv;
> -	__aligned_u64 /* __s32 * */ fds;
> +	union {
> +		__aligned_u64 /* __s32 * */ fds;
> +		__aligned_u64 /* __s32 * */ iovs;
> +		__aligned_u64 /* __s32 * */ rsrc;
> +	};
>  };
>  
>  #define io_uring_files_update	io_uring_rsrc_update
> 

-- 
Pavel Begunkov