On 2021/04/12 18:55, Max Gurtovoy wrote:
> This will enable changing the virtual boundary of null blk devices. For
> now, null blk devices didn't have any restriction on the scatter/gather
> elements received from the block layer. Add a module parameter and a
> configfs option that will control the virtual boundary. This will
> enable testing the efficiency of the block layer bounce buffer in case
> a suitable application will send discontiguous IO to the given device.
>
> Initial testing with patched FIO showed the following results (64 jobs,
> 128 iodepth, 1 nullb device):
> IO size READ (virt=false) READ (virt=true) Write (virt=false) Write (virt=true)
> ---------- ------------------- ----------------- ------------------- -------------------
> 1k 10.7M 8482k 10.8M 8471k
> 2k 10.4M 8266k 10.4M 8271k
> 4k 10.4M 8274k 10.3M 8226k
> 8k 10.2M 8131k 9800k 7933k
> 16k 9567k 7764k 8081k 6828k
> 32k 8865k 7309k 5570k 5153k
> 64k 7695k 6586k 2682k 2617k
> 128k 5346k 5489k 1320k 1296k
>
> Signed-off-by: Max Gurtovoy <mgurtovoy@xxxxxxxxxx>
> ---
>
> changes from v1:
> - added configfs option (Chaitanya and Damien)
> - added virt_boundary to feature list
>
> ---
> drivers/block/null_blk/main.c | 12 +++++++++++-
> drivers/block/null_blk/null_blk.h | 1 +
> 2 files changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
> index d6c821d48090..c35872cc5f37 100644
> --- a/drivers/block/null_blk/main.c
> +++ b/drivers/block/null_blk/main.c
> @@ -84,6 +84,10 @@ enum {
> NULL_Q_MQ = 2,
> };
>
> +static bool g_virt_boundary = false;
> +module_param_named(virt_boundary, g_virt_boundary, bool, 0444);
> +MODULE_PARM_DESC(virt_boundary, "Require a virtual boundary for the device. Default: False");
> +
> static int g_no_sched;
> module_param_named(no_sched, g_no_sched, int, 0444);
> MODULE_PARM_DESC(no_sched, "No io scheduler");
> @@ -366,6 +370,7 @@ NULLB_DEVICE_ATTR(zone_capacity, ulong, NULL);
> NULLB_DEVICE_ATTR(zone_nr_conv, uint, NULL);
> NULLB_DEVICE_ATTR(zone_max_open, uint, NULL);
> NULLB_DEVICE_ATTR(zone_max_active, uint, NULL);
> +NULLB_DEVICE_ATTR(virt_boundary, bool, NULL);
>
> static ssize_t nullb_device_power_show(struct config_item *item, char *page)
> {
> @@ -486,6 +491,7 @@ static struct configfs_attribute *nullb_device_attrs[] = {
> &nullb_device_attr_zone_nr_conv,
> &nullb_device_attr_zone_max_open,
> &nullb_device_attr_zone_max_active,
> + &nullb_device_attr_virt_boundary,
> NULL,
> };
>
> @@ -539,7 +545,7 @@ nullb_group_drop_item(struct config_group *group, struct config_item *item)
> static ssize_t memb_group_features_show(struct config_item *item, char *page)
> {
> return snprintf(page, PAGE_SIZE,
> - "memory_backed,discard,bandwidth,cache,badblocks,zoned,zone_size,zone_capacity,zone_nr_conv,zone_max_open,zone_max_active,blocksize,max_sectors\n");
> + "memory_backed,discard,bandwidth,cache,badblocks,zoned,zone_size,zone_capacity,zone_nr_conv,zone_max_open,zone_max_active,blocksize,max_sectors,virt_boundary\n");
> }
>
> CONFIGFS_ATTR_RO(memb_group_, features);
> @@ -605,6 +611,7 @@ static struct nullb_device *null_alloc_dev(void)
> dev->zone_nr_conv = g_zone_nr_conv;
> dev->zone_max_open = g_zone_max_open;
> dev->zone_max_active = g_zone_max_active;
> + dev->virt_boundary = g_virt_boundary;
> return dev;
> }
>
> @@ -1880,6 +1887,9 @@ static int null_add_dev(struct nullb_device *dev)
> BLK_DEF_MAX_SECTORS);
> blk_queue_max_hw_sectors(nullb->q, dev->max_sectors);
>
> + if (dev->virt_boundary)
> + blk_queue_virt_boundary(nullb->q, PAGE_SIZE - 1);
> +
> null_config_discard(nullb);
>
> sprintf(nullb->disk_name, "nullb%d", nullb->index);
> diff --git a/drivers/block/null_blk/null_blk.h b/drivers/block/null_blk/null_blk.h
> index 83504f3cc9d6..5ad5087ebe39 100644
> --- a/drivers/block/null_blk/null_blk.h
> +++ b/drivers/block/null_blk/null_blk.h
> @@ -96,6 +96,7 @@ struct nullb_device {
> bool memory_backed; /* if data is stored in memory */
> bool discard; /* if support discard */
> bool zoned; /* if device is zoned */
> + bool virt_boundary; /* virtual boundary on/off for the device */
> };
>
> struct nullb {
>
Looks good to me.
Reviewed-by: Damien Le Moal <damien.lemoal@xxxxxxx>
--
Damien Le Moal
Western Digital Research
