Re: [PATCH for-next v2 21/25] io_uring: add IORING_SETUP_SINGLE_ISSUER

[Hao Xu <hao.xu@linux.dev>]

On 6/14/22 22:37, Pavel Begunkov wrote:
> Add a new IORING_SETUP_SINGLE_ISSUER flag and the userspace visible part
> of it, i.e. put limitations of submitters. Also, don't allow it together
> with IOPOLL as we're not going to put it to good use.
> 
> Signed-off-by: Pavel Begunkov <asml.silence@gmail.com>
> ---
>   include/uapi/linux/io_uring.h |  5 ++++-
>   io_uring/io_uring.c           |  7 +++++--
>   io_uring/io_uring_types.h     |  1 +
>   io_uring/tctx.c               | 27 ++++++++++++++++++++++++---
>   io_uring/tctx.h               |  4 ++--
>   5 files changed, 36 insertions(+), 8 deletions(-)
> 
> diff --git a/include/uapi/linux/io_uring.h b/include/uapi/linux/io_uring.h
> index a41ddb8c5e1f..a3a691340d3e 100644
> --- a/include/uapi/linux/io_uring.h
> +++ b/include/uapi/linux/io_uring.h
> @@ -138,9 +138,12 @@ enum {
>    * IORING_SQ_TASKRUN in the sq ring flags. Not valid with COOP_TASKRUN.
>    */
>   #define IORING_SETUP_TASKRUN_FLAG	(1U << 9)
> -
>   #define IORING_SETUP_SQE128		(1U << 10) /* SQEs are 128 byte */
>   #define IORING_SETUP_CQE32		(1U << 11) /* CQEs are 32 byte */
> +/*
> + * Only one task is allowed to submit requests
> + */
> +#define IORING_SETUP_SINGLE_ISSUER	(1U << 12)
>   
>   enum io_uring_op {
>   	IORING_OP_NOP,
> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
> index 15d209f334eb..4b90439808e3 100644
> --- a/io_uring/io_uring.c
> +++ b/io_uring/io_uring.c
> @@ -3020,6 +3020,8 @@ static __cold void io_ring_ctx_free(struct io_ring_ctx *ctx)
>   	io_destroy_buffers(ctx);
>   	if (ctx->sq_creds)
>   		put_cred(ctx->sq_creds);
> +	if (ctx->submitter_task)
> +		put_task_struct(ctx->submitter_task);
>   
>   	/* there are no registered resources left, nobody uses it */
>   	if (ctx->rsrc_node)
> @@ -3752,7 +3754,7 @@ static int io_uring_install_fd(struct io_ring_ctx *ctx, struct file *file)
>   	if (fd < 0)
>   		return fd;
>   
> -	ret = io_uring_add_tctx_node(ctx);
> +	ret = __io_uring_add_tctx_node(ctx, false);
>   	if (ret) {
>   		put_unused_fd(fd);
>   		return ret;
> @@ -3972,7 +3974,8 @@ static long io_uring_setup(u32 entries, struct io_uring_params __user *params)
>   			IORING_SETUP_CLAMP | IORING_SETUP_ATTACH_WQ |
>   			IORING_SETUP_R_DISABLED | IORING_SETUP_SUBMIT_ALL |
>   			IORING_SETUP_COOP_TASKRUN | IORING_SETUP_TASKRUN_FLAG |
> -			IORING_SETUP_SQE128 | IORING_SETUP_CQE32))
> +			IORING_SETUP_SQE128 | IORING_SETUP_CQE32 |
> +			IORING_SETUP_SINGLE_ISSUER))
>   		return -EINVAL;
>   
>   	return io_uring_create(entries, &p, params);
> diff --git a/io_uring/io_uring_types.h b/io_uring/io_uring_types.h
> index aba0f8cd6f49..f6d0ad25f377 100644
> --- a/io_uring/io_uring_types.h
> +++ b/io_uring/io_uring_types.h
> @@ -241,6 +241,7 @@ struct io_ring_ctx {
>   	/* Keep this last, we don't need it for the fast path */
>   
>   	struct io_restriction		restrictions;
> +	struct task_struct		*submitter_task;
>   
>   	/* slow path rsrc auxilary data, used by update/register */
>   	struct io_rsrc_node		*rsrc_backup_node;
> diff --git a/io_uring/tctx.c b/io_uring/tctx.c
> index 6adf659687f8..012be261dc50 100644
> --- a/io_uring/tctx.c
> +++ b/io_uring/tctx.c
> @@ -81,12 +81,32 @@ __cold int io_uring_alloc_task_context(struct task_struct *task,
>   	return 0;
>   }
>   
> -int __io_uring_add_tctx_node(struct io_ring_ctx *ctx)
> +static int io_register_submitter(struct io_ring_ctx *ctx)
> +{
> +	int ret = 0;
> +
> +	mutex_lock(&ctx->uring_lock);
> +	if (!ctx->submitter_task)
> +		ctx->submitter_task = get_task_struct(current);
> +	else if (ctx->submitter_task != current)
> +		ret = -EEXIST;
> +	mutex_unlock(&ctx->uring_lock);
> +
> +	return ret;
> +}

Seems we don't need this uring_lock:
When we create a ring, we setup ctx->submitter_task before uring fd is
installed so at that time nobody else can enter this code.
when we enter this code later in io_uring_enter, we just read it.