Re: [PATCH v3 5/6] null_blk: Support configuring the maximum DMA segment size

[Ming Lei <ming.lei@redhat.com>]

On Sun, Mar 29, 2026 at 06:00:31PM +0530, Nilay Shroff wrote:
> On 3/28/26 2:43 AM, Bart Van Assche wrote:
> > Add support for configuring the maximum DMA segment size. The maximum DMA
> > segment size may be set to a value smaller than the virtual memory page
> > size. Reject invalid max_segment_size values.
> > 
> > Since rq_for_each_segment() may yield bvecs larger than the maximum DMA
> > segment size, add code in the rq_for_each_segment() loop that restricts
> > the bvec length to the maximum DMA segment size.
> > 
> > Cc: Christoph Hellwig<hch@lst.de>
> > Cc: Ming Lei<ming.lei@redhat.com>
> > Cc: Damien Le Moal<damien.lemoal@opensource.wdc.com>
> > Cc: Chaitanya Kulkarni<kch@nvidia.com>
> > Signed-off-by: Bart Van Assche<bvanassche@acm.org>
> > ---
> >   drivers/block/null_blk/main.c     | 43 +++++++++++++++++++++++++++++++
> >   drivers/block/null_blk/null_blk.h |  1 +
> >   2 files changed, 44 insertions(+)
> > 
> > diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
> > index f8c0fd57e041..d5fbbc5d63ed 100644
> > --- a/drivers/block/null_blk/main.c
> > +++ b/drivers/block/null_blk/main.c
> > @@ -169,6 +169,32 @@ static int g_max_sectors;
> >   module_param_named(max_sectors, g_max_sectors, int, 0444);
> >   MODULE_PARM_DESC(max_sectors, "Maximum size of a command (in 512B sectors)");
> > +static unsigned int g_max_segment_size = BLK_MAX_SEGMENT_SIZE;
> > +
> > +static int nullb_set_max_segment_size(const char *val,
> > +				      const struct kernel_param *kp)
> > +{
> > +	int res;
> > +
> > +	res = kstrtouint(val, 0, &g_max_segment_size);
> > +	if (res < 0)
> > +		return res;
> > +
> > +	if (g_max_segment_size < BLK_MIN_SEGMENT_SIZE)
> > +		return -EINVAL;
> > +
> > +	return 0;
> > +}
> > +
> > +static const struct kernel_param_ops max_segment_size_ops = {
> > +	.set = nullb_set_max_segment_size,
> > +	.get = param_get_uint,
> > +};
> > +
> > +module_param_cb(max_segment_size, &max_segment_size_ops, &g_max_segment_size,
> > +		0444);
> > +MODULE_PARM_DESC(max_segment_size, "Maximum size of a DMA segment in bytes");
> > +
> >   static unsigned int nr_devices = 1;
> >   module_param(nr_devices, uint, 0444);
> >   MODULE_PARM_DESC(nr_devices, "Number of devices to register");
> > @@ -442,6 +468,14 @@ static int nullb_apply_poll_queues(struct nullb_device *dev,
> >   	return ret;
> >   }
> > +static int nullb_apply_max_segment_size(struct nullb_device *dev,
> > +					unsigned int max_segment_size)
> > +{
> > +	if (max_segment_size < BLK_MIN_SEGMENT_SIZE)
> > +		return -EINVAL;
> > +	return 0;
> > +}
> > +
> >   NULLB_DEVICE_ATTR(size, ulong, NULL);
> >   NULLB_DEVICE_ATTR(completion_nsec, ulong, NULL);
> >   NULLB_DEVICE_ATTR(submit_queues, uint, nullb_apply_submit_queues);
> > @@ -450,6 +484,7 @@ NULLB_DEVICE_ATTR(home_node, uint, NULL);
> >   NULLB_DEVICE_ATTR(queue_mode, uint, NULL);
> >   NULLB_DEVICE_ATTR(blocksize, uint, NULL);
> >   NULLB_DEVICE_ATTR(max_sectors, uint, NULL);
> > +NULLB_DEVICE_ATTR(max_segment_size, uint, nullb_apply_max_segment_size);
> >   NULLB_DEVICE_ATTR(irqmode, uint, NULL);
> >   NULLB_DEVICE_ATTR(hw_queue_depth, uint, NULL);
> >   NULLB_DEVICE_ATTR(index, uint, NULL);
> > @@ -608,6 +643,7 @@ static struct configfs_attribute *nullb_device_attrs[] = {
> >   	&nullb_device_attr_index,
> >   	&nullb_device_attr_irqmode,
> >   	&nullb_device_attr_max_sectors,
> > +	&nullb_device_attr_max_segment_size,
> >   	&nullb_device_attr_mbps,
> >   	&nullb_device_attr_memory_backed,
> >   	&nullb_device_attr_no_sched,
> > @@ -805,6 +841,7 @@ static struct nullb_device *null_alloc_dev(void)
> >   	dev->queue_mode = g_queue_mode;
> >   	dev->blocksize = g_bs;
> >   	dev->max_sectors = g_max_sectors;
> > +	dev->max_segment_size = g_max_segment_size;
> >   	dev->irqmode = g_irqmode;
> >   	dev->hw_queue_depth = g_hw_queue_depth;
> >   	dev->blocking = g_blocking;
> > @@ -1248,6 +1285,9 @@ static blk_status_t null_transfer(struct nullb *nullb, struct page *page,
> >   	unsigned int valid_len = len;
> >   	void *p;
> > +	WARN_ONCE(len > dev->max_segment_size, "%u > %u\n", len,
> > +		  dev->max_segment_size);
> > +
> >   	p = kmap_local_page(page) + off;
> >   	if (!is_write) {
> >   		if (dev->zoned) {
> > @@ -1295,6 +1335,8 @@ static blk_status_t null_handle_data_transfer(struct nullb_cmd *cmd,
> >   	spin_lock_irq(&nullb->lock);
> >   	rq_for_each_segment(bvec, rq, iter) {
> >   		len = bvec.bv_len;
> > +		len = min(bvec.bv_len, nullb->dev->max_segment_size);
> > +		bvec.bv_len = len;
> >   		if (transferred_bytes + len > max_bytes)
> >   			len = max_bytes - transferred_bytes;
> >   		err = null_transfer(nullb, bvec.bv_page, len, bvec.bv_offset,
> 
> 
> IMO, since max_segment_size is now configurable, should we consider using
> blk_rq_map_sg() instead of rq_for_each_segment()?

blk_rq_map_sg() requires to allocate sgl, actually it can be done in
request sg iterator way:

	blk_rq_map_iter_init();
	while (blk_map_iter_next(rq, &iter, &vec)) {
		consume each segment;
	}

Just the two helpers needs to be exported.


thanks,
Ming