On 2/26/25 11:20 AM, Keith Busch wrote:
> From: Keith Busch <kbusch@xxxxxxxxxx>
>
> 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
