On Mon, Mar 24, 2025 at 6:49 AM Ming Lei <ming.lei@xxxxxxxxxx> wrote:
>
> IO split is usually bad in io_uring world, since -EAGAIN is caused and
> IO handling may have to fallback to io-wq, this way does hurt performance.
>
> ublk starts to support zero copy recently, for avoiding unnecessary IO
> split, ublk driver's segment limit should be aligned with backend
> device's segment limit.
>
> Another reason is that io_buffer_register_bvec() needs to allocate bvecs,
> which number is aligned with ublk request segment number, so that big
> memory allocation can be avoided by setting reasonable max_segments limit.
>
> So add segment parameter for providing ublk server chance to align
> segment limit with backend, and keep it reasonable from implementation
> viewpoint.
>
> Signed-off-by: Ming Lei <ming.lei@xxxxxxxxxx>
> ---
> drivers/block/ublk_drv.c | 15 ++++++++++++++-
> include/uapi/linux/ublk_cmd.h | 9 +++++++++
> 2 files changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c
> index acb6aed7be75..53a463681a41 100644
> --- a/drivers/block/ublk_drv.c
> +++ b/drivers/block/ublk_drv.c
> @@ -74,7 +74,7 @@
> #define UBLK_PARAM_TYPE_ALL \
> (UBLK_PARAM_TYPE_BASIC | UBLK_PARAM_TYPE_DISCARD | \
> UBLK_PARAM_TYPE_DEVT | UBLK_PARAM_TYPE_ZONED | \
> - UBLK_PARAM_TYPE_DMA_ALIGN)
> + UBLK_PARAM_TYPE_DMA_ALIGN | UBLK_PARAM_TYPE_SEGMENT)
>
> struct ublk_rq_data {
> struct kref ref;
> @@ -580,6 +580,13 @@ static int ublk_validate_params(const struct ublk_device *ub)
> return -EINVAL;
> }
>
> + if (ub->params.types & UBLK_PARAM_TYPE_SEGMENT) {
> + const struct ublk_param_segment *p = &ub->params.seg;
> +
> + if (!is_power_of_2(p->seg_boundary_mask + 1))
> + return -EINVAL;
Looking at blk_validate_limits(), it seems like there are some
additional requirements? Looks like seg_boundary_mask has to be at
least PAGE_SIZE - 1 and max_segment_size has to be at least PAGE_SIZE
if virt_boundary_mask is set?
Aside from that, this looks good to me.
Best,
Caleb
> + }
> +
> return 0;
> }
>
> @@ -2350,6 +2357,12 @@ static int ublk_ctrl_start_dev(struct ublk_device *ub, struct io_uring_cmd *cmd)
> if (ub->params.types & UBLK_PARAM_TYPE_DMA_ALIGN)
> lim.dma_alignment = ub->params.dma.alignment;
>
> + if (ub->params.types & UBLK_PARAM_TYPE_SEGMENT) {
> + lim.seg_boundary_mask = ub->params.seg.seg_boundary_mask;
> + lim.max_segment_size = ub->params.seg.max_segment_size;
> + lim.max_segments = ub->params.seg.max_segments;
> + }
> +
> if (wait_for_completion_interruptible(&ub->completion) != 0)
> return -EINTR;
>
> diff --git a/include/uapi/linux/ublk_cmd.h b/include/uapi/linux/ublk_cmd.h
> index 7255b36b5cf6..83c2b94251f0 100644
> --- a/include/uapi/linux/ublk_cmd.h
> +++ b/include/uapi/linux/ublk_cmd.h
> @@ -410,6 +410,13 @@ struct ublk_param_dma_align {
> __u8 pad[4];
> };
>
> +struct ublk_param_segment {
> + __u64 seg_boundary_mask;
> + __u32 max_segment_size;
> + __u16 max_segments;
> + __u8 pad[2];
> +};
> +
> struct ublk_params {
> /*
> * Total length of parameters, userspace has to set 'len' for both
> @@ -423,6 +430,7 @@ struct ublk_params {
> #define UBLK_PARAM_TYPE_DEVT (1 << 2)
> #define UBLK_PARAM_TYPE_ZONED (1 << 3)
> #define UBLK_PARAM_TYPE_DMA_ALIGN (1 << 4)
> +#define UBLK_PARAM_TYPE_SEGMENT (1 << 5)
> __u32 types; /* types of parameter included */
>
> struct ublk_param_basic basic;
> @@ -430,6 +438,7 @@ struct ublk_params {
> struct ublk_param_devt devt;
> struct ublk_param_zoned zoned;
> struct ublk_param_dma_align dma;
> + struct ublk_param_segment seg;
> };
>
> #endif
> --
> 2.47.0
>
