Re: [PATCHv7 4/6] ublk: zc register/unregister bvec

[Jens Axboe <axboe@kernel.dk>]

On 2/26/25 11:20 AM, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> Provide new operations for the user to request mapping an active request
> to an io uring instance's buf_table. The user has to provide the index
> it wants to install the buffer.
> 
> A reference count is taken on the request to ensure it can't be
> completed while it is active in a ring's buf_table.

Looks pretty sane to me, just a few minor nits below where only one of
them actually is required to change.

> +static int ublk_unregister_io_buf(struct io_uring_cmd *cmd,
> +				  const struct ublksrv_io_cmd *ub_cmd,
> +				  unsigned int issue_flags)
> +{
> +	int index = (int)ub_cmd->addr;
> +
> +	io_buffer_unregister_bvec(cmd, index, issue_flags);
> +	return 0;
> +}
> +

Minor nit here too, I'd drop 'index' and just cast it in the argument.

> -static inline struct request *__ublk_check_and_get_req(struct ublk_device *ub,
> -		struct ublk_queue *ubq, int tag, size_t offset)
> -{
> -	struct request *req;
> -
> -	if (!ublk_need_req_ref(ubq))
> -		return NULL;
> -
> -	req = blk_mq_tag_to_rq(ub->tag_set.tags[ubq->q_id], tag);
> -	if (!req)
> -		return NULL;
> -
> -	if (!ublk_get_req_ref(ubq, req))
> -		return NULL;
> -
> -	if (unlikely(!blk_mq_request_started(req) || req->tag != tag))
> -		goto fail_put;
> -
> -	if (!ublk_rq_has_data(req))
> -		goto fail_put;
> -
> -	if (offset > blk_rq_bytes(req))
> -		goto fail_put;
> -
> -	return req;
> -fail_put:
> -	ublk_put_req_ref(ubq, req);
> -	return NULL;
> -}
> -

This could be a prep patch to cut down on unrelated changes, but not
really important.

> @@ -2459,7 +2507,7 @@ static int ublk_ctrl_add_dev(struct io_uring_cmd *cmd)
>  		 * buffer by pwrite() to ublk char device, which can't be
>  		 * used for unprivileged device
>  		 */
> -		if (info.flags & UBLK_F_USER_COPY)
> +		if (info.flags & UBLK_F_USER_COPY | UBLK_F_SUPPORT_ZERO_COPY)
>  			return -EINVAL;
>  	}

Missing parens here around mask.

-- 
Jens Axboe