Re: [PATCH v3 06/25] io_uring/kbuf: add buffer ring pinning/unpinning

[Gabriel Krisman Bertazi <krisman@suse.de>]

Joanne Koong <joannelkoong@gmail.com> writes:

> +int io_kbuf_ring_pin(struct io_kiocb *req, unsigned buf_group,
> +		     unsigned issue_flags, struct io_buffer_list **bl)
> +{
> +	struct io_buffer_list *buffer_list;
> +	struct io_ring_ctx *ctx = req->ctx;
> +	int ret = -EINVAL;
> +
> +	io_ring_submit_lock(ctx, issue_flags);
> +
> +	buffer_list = io_buffer_get_list(ctx, buf_group);
> +	if (likely(buffer_list) && likely(buffer_list->flags & IOBL_BUF_RING)) {

FWIW, the likely construct is unnecessary here. At least, it should
encompass the entire expression:

    if (likely(buffer_list && buffer_list->flags & IOBL_BUF_RING))

But you can just drop it.

> +		if (unlikely(buffer_list->flags & IOBL_PINNED)) {
> +			ret = -EALREADY;
> +		} else {
> +			buffer_list->flags |= IOBL_PINNED;
> +			ret = 0;
> +			*bl = buffer_list;
> +		}
> +	}
> +
> +	io_ring_submit_unlock(ctx, issue_flags);
> +	return ret;
> +}
> +
> +int io_kbuf_ring_unpin(struct io_kiocb *req, unsigned buf_group,
> +		       unsigned issue_flags)
> +{
> +	struct io_ring_ctx *ctx = req->ctx;
> +	struct io_buffer_list *bl;
> +	int ret = -EINVAL;
> +
> +	io_ring_submit_lock(ctx, issue_flags);
> +
> +	bl = io_buffer_get_list(ctx, buf_group);
> +	if (likely(bl) && likely(bl->flags & IOBL_BUF_RING) &&
> +	    likely(bl->flags & IOBL_PINNED)) {

likewise.

> +		bl->flags &= ~IOBL_PINNED;
> +		ret = 0;
> +	}
> +
> +	io_ring_submit_unlock(ctx, issue_flags);
> +	return ret;
> +}
> +
>  /* cap it at a reasonable 256, will be one page even for 4K */
>  #define PEEK_MAX_IMPORT		256
>  
> @@ -744,6 +788,8 @@ int io_unregister_buf_ring(struct io_ring_ctx *ctx, void __user *arg)
>  		return -ENOENT;
>  	if (!(bl->flags & IOBL_BUF_RING))
>  		return -EINVAL;
> +	if (bl->flags & IOBL_PINNED)
> +		return -EBUSY;
>  
>  	scoped_guard(mutex, &ctx->mmap_lock)
>  		xa_erase(&ctx->io_bl_xa, bl->bgid);
> diff --git a/io_uring/kbuf.h b/io_uring/kbuf.h
> index 11d165888b8e..c4368f35cf11 100644
> --- a/io_uring/kbuf.h
> +++ b/io_uring/kbuf.h
> @@ -12,6 +12,11 @@ enum {
>  	IOBL_INC		= 2,
>  	/* buffers are kernel managed */
>  	IOBL_KERNEL_MANAGED	= 4,
> +	/*
> +	 * buffer ring is pinned and cannot be unregistered by userspace until
> +	 * it has been unpinned
> +	 */
> +	IOBL_PINNED		= 8,
>  };
>  
>  struct io_buffer_list {
> @@ -136,4 +141,9 @@ static inline unsigned int io_put_kbufs(struct io_kiocb *req, int len,
>  		return 0;
>  	return __io_put_kbufs(req, bl, len, nbufs);
>  }
> +
> +int io_kbuf_ring_pin(struct io_kiocb *req, unsigned buf_group,
> +		     unsigned issue_flags, struct io_buffer_list **bl);
> +int io_kbuf_ring_unpin(struct io_kiocb *req, unsigned buf_group,
> +		       unsigned issue_flags);
>  #endif
> diff --git a/io_uring/uring_cmd.c b/io_uring/uring_cmd.c
> index 197474911f04..8ac79ead4158 100644
> --- a/io_uring/uring_cmd.c
> +++ b/io_uring/uring_cmd.c
> @@ -398,3 +398,21 @@ bool io_uring_mshot_cmd_post_cqe(struct io_uring_cmd *ioucmd,
>  	return true;
>  }
>  EXPORT_SYMBOL_GPL(io_uring_mshot_cmd_post_cqe);
> +
> +int io_uring_cmd_buf_ring_pin(struct io_uring_cmd *ioucmd, unsigned buf_group,
> +			      unsigned issue_flags, struct io_buffer_list **bl)
> +{
> +	struct io_kiocb *req = cmd_to_io_kiocb(ioucmd);
> +
> +	return io_kbuf_ring_pin(req, buf_group, issue_flags, bl);
> +}
> +EXPORT_SYMBOL_GPL(io_uring_cmd_buf_ring_pin);
> +
> +int io_uring_cmd_buf_ring_unpin(struct io_uring_cmd *ioucmd, unsigned buf_group,
> +				unsigned issue_flags)
> +{
> +	struct io_kiocb *req = cmd_to_io_kiocb(ioucmd);
> +
> +	return io_kbuf_ring_unpin(req, buf_group, issue_flags);
> +}
> +EXPORT_SYMBOL_GPL(io_uring_cmd_buf_ring_unpin);

-- 
Gabriel Krisman Bertazi