On 2021/02/02 14:26, Chaitanya Kulkarni wrote:
> This patch adds a new module parameter to set the queue bounce limit.
> Various queue limits parameters are usually present in the sysfs.
> This is needed for testing purpose only so instead of poluting the
> sysfs space just update the null_blk drivers.
>
> This is needed especially for blktrace bounce related tracepoint
> testing.
>
> Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@xxxxxxx>
> ---
> # modprobe null_blk bounce_pfn=0x1000
> # for bs in 512 1024 2048 4096 8192
>> do
>> dd if=/dev/zero of=/dev/nullb0 bs=${bs} count=1 oflag=direct
>> done
> # ~/blktrace/blktrace -a write -d /dev/nullb0 -o -| ~/blktrace/blkparse -i -
> #
> # # With null_blk bounce
> # modprobe null_blk bounce_pfn=0x1000
> #
> 252,0 34 1 0.000000000 4390 Q WS 0 + 1 [dd]
> 252,0 34 2 0.000010139 4390 B WS 0 + 1 [dd]
> 252,0 34 3 0.000018996 4390 G WS 0 + 1 [dd]
> 252,0 34 4 0.000024576 4390 I WS 0 + 1 [dd]
> 252,0 34 5 0.000065152 829 D WS 0 + 1 [kworker/34:1H]
> 252,0 34 6 0.000092303 182 C WS 0 + 1 [0]
> 252,0 34 7 0.000098164 182 C WS 0 + 1 [0]
> 252,0 27 1 0.004573597 4391 Q WS 0 + 2 [dd]
> 252,0 27 2 0.004580891 4391 B WS 0 + 2 [dd]
> 252,0 27 3 0.004589687 4391 G WS 0 + 2 [dd]
> 252,0 27 4 0.004594356 4391 I WS 0 + 2 [dd]
> 252,0 27 5 0.004619473 1049 D WS 0 + 2 [kworker/27:1H]
> 252,0 27 6 0.004634411 147 C WS 0 + 2 [0]
> 252,0 27 7 0.004638609 147 C WS 0 + 2 [0]
> 252,0 29 1 0.014589610 4394 Q WS 0 + 8 [dd]
> 252,0 29 2 0.014595792 4394 B WS 0 + 8 [dd]
> 252,0 29 3 0.014599028 4394 G WS 0 + 8 [dd]
> 252,0 29 4 0.014601713 4394 I WS 0 + 8 [dd]
> 252,0 29 5 0.014621330 807 D WS 0 + 8 [kworker/29:1H]
> 252,0 29 6 0.014633813 157 C WS 0 + 8 [0]
> 252,0 29 7 0.014637660 157 C WS 0 + 8 [0]
> 252,0 30 1 0.009147174 4392 Q WS 0 + 4 [dd]
> 252,0 30 2 0.009154377 4392 B WS 0 + 4 [dd]
> 252,0 30 3 0.009163064 4392 G WS 0 + 4 [dd]
> 252,0 30 4 0.009167161 4392 I WS 0 + 4 [dd]
> 252,0 30 5 0.009191437 1084 D WS 0 + 4 [kworker/30:1H]
> 252,0 30 6 0.009206084 162 C WS 0 + 4 [0]
> 252,0 30 7 0.009210863 162 C WS 0 + 4 [0]
> 252,0 30 8 0.018510473 4395 Q WS 0 + 16 [dd]
> 252,0 30 9 0.018517767 4395 B WS 0 + 16 [dd]
> 252,0 30 10 0.018521213 4395 G WS 0 + 16 [dd]
> 252,0 30 11 0.018524229 4395 I WS 0 + 16 [dd]
> 252,0 30 12 0.018544026 1084 D WS 0 + 16 [kworker/30:1H]
> 252,0 30 13 0.018555127 162 C WS 0 + 16 [0]
> 252,0 30 14 0.018558333 162 C WS 0 + 16 [0]
>
> # # Without null_blk bounce
> 252,0 38 1 0.000000000 4744 Q WS 0 + 1 [dd]
> 252,0 38 2 0.000014768 4744 G WS 0 + 1 [dd]
> 252,0 38 3 0.000021771 4744 I WS 0 + 1 [dd]
> 252,0 38 4 0.000062557 914 D WS 0 + 1 [kworker/38:1H]
> 252,0 38 5 0.000089768 202 C WS 0 + 1 [0]
> 252,0 32 1 0.018453947 4749 Q WS 0 + 16 [dd]
> 252,0 32 2 0.018466080 4749 G WS 0 + 16 [dd]
> 252,0 32 3 0.018470619 4749 I WS 0 + 16 [dd]
> 252,0 32 4 0.018495195 514 D WS 0 + 16 [kworker/32:1H]
> 252,0 32 5 0.018509942 172 C WS 0 + 16 [0]
> 252,0 55 1 0.005000087 4745 Q WS 0 + 2 [dd]
> 252,0 55 2 0.005005637 4745 G WS 0 + 2 [dd]
> 252,0 55 3 0.005008453 4745 I WS 0 + 2 [dd]
> 252,0 55 4 0.005029352 1082 D WS 0 + 2 [kworker/55:1H]
> 252,0 55 5 0.005041915 287 C WS 0 + 2 [0]
> 252,0 31 1 0.014158231 4748 Q WS 0 + 8 [dd]
> 252,0 31 2 0.014164173 4748 G WS 0 + 8 [dd]
> 252,0 31 3 0.014167308 4748 I WS 0 + 8 [dd]
> 252,0 31 4 0.014188759 1151 D WS 0 + 8 [kworker/31:1H]
> 252,0 31 5 0.014203015 167 C WS 0 + 8 [0]
> 252,0 58 1 0.010016655 4747 D WS 0 + 4 [systemd-udevd]
> 252,0 60 1 0.009982571 4746 Q WS 0 + 4 [dd]
> 252,0 60 2 0.009993942 4746 G WS 0 + 4 [dd]
> 252,0 60 3 0.009998661 4746 I WS 0 + 4 [dd]
> 252,0 60 4 0.010136269 0 C WS 0 + 4 [0]
>
> ---
> drivers/block/null_blk/main.c | 20 +++++++++++++++++++-
> drivers/block/null_blk/null_blk.h | 1 +
> 2 files changed, 20 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
> index 6e6cbb953a12..8ddf2ba961f7 100644
> --- a/drivers/block/null_blk/main.c
> +++ b/drivers/block/null_blk/main.c
> @@ -156,6 +156,10 @@ 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_bounce_pfn;
> +module_param_named(bounce_pfn, g_bounce_pfn, int, 0444);
> +MODULE_PARM_DESC(bounce_pfn, "Queue Bounce limit (default: 0)");
> +
> static unsigned int nr_devices = 1;
> module_param(nr_devices, uint, 0444);
> MODULE_PARM_DESC(nr_devices, "Number of devices to register");
> @@ -354,6 +358,7 @@ NULLB_DEVICE_ATTR(submit_queues, uint, nullb_apply_submit_queues);
> NULLB_DEVICE_ATTR(home_node, uint, NULL);
> NULLB_DEVICE_ATTR(queue_mode, uint, NULL);
> NULLB_DEVICE_ATTR(blocksize, uint, NULL);
> +NULLB_DEVICE_ATTR(bounce_pfn, uint, NULL);
> NULLB_DEVICE_ATTR(max_sectors, uint, NULL);
> NULLB_DEVICE_ATTR(irqmode, uint, NULL);
> NULLB_DEVICE_ATTR(hw_queue_depth, uint, NULL);
> @@ -472,6 +477,7 @@ static struct configfs_attribute *nullb_device_attrs[] = {
> &nullb_device_attr_home_node,
> &nullb_device_attr_queue_mode,
> &nullb_device_attr_blocksize,
> + &nullb_device_attr_bounce_pfn,
> &nullb_device_attr_max_sectors,
> &nullb_device_attr_irqmode,
> &nullb_device_attr_hw_queue_depth,
> @@ -543,7 +549,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,bounce_pfn,bandwidth,cache,badblocks,zoned,zone_size,zone_capacity,zone_nr_conv\n");
> }
>
> CONFIGFS_ATTR_RO(memb_group_, features);
> @@ -1610,6 +1616,17 @@ static void null_config_discard(struct nullb *nullb)
> blk_queue_flag_set(QUEUE_FLAG_DISCARD, nullb->q);
> }
>
> +static void null_config_bounce_pfn(struct nullb *nullb)
> +{
> + if (nullb->dev->memory_backed && nullb->dev->bounce_pfn == false)
nullb->dev->bounce_pfn is not a bool... So maybe !nullb->dev->bounce_pfn ?
> + return;
> +
> + if (!nullb->dev->memory_backed && !g_bounce_pfn)
Why ?
> + return;
> +
> + blk_queue_bounce_limit(nullb->q, nullb->dev->bounce_pfn);
Shouldn't this function simply be:
static void null_config_bounce_pfn(struct nullb *nullb)
{
if (nullb->dev->memory_backed && nullb->dev->bounce_pfn)
blk_queue_bounce_limit(nullb->q, nullb->dev->bounce_pfn);
}
?
> +}
> +
> static const struct block_device_operations null_bio_ops = {
> .owner = THIS_MODULE,
> .submit_bio = null_submit_bio,
> @@ -1882,6 +1899,7 @@ static int null_add_dev(struct nullb_device *dev)
> blk_queue_max_hw_sectors(nullb->q, dev->max_sectors);
>
> null_config_discard(nullb);
> + null_config_bounce_pfn(nullb);
Where do you assign bounce_pfn to nullb->dev->bounce_pfn ? Something is missing
here...
>
> 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..cd55f99118bf 100644
> --- a/drivers/block/null_blk/null_blk.h
> +++ b/drivers/block/null_blk/null_blk.h
> @@ -86,6 +86,7 @@ struct nullb_device {
> unsigned int queue_mode; /* block interface */
> unsigned int blocksize; /* block size */
> unsigned int max_sectors; /* Max sectors per command */
> + unsigned int bounce_pfn; /* bounce page frame number */
> unsigned int irqmode; /* IRQ completion handler */
> unsigned int hw_queue_depth; /* queue depth */
> unsigned int index; /* index of the disk, only valid with a disk */
>
--
Damien Le Moal
Western Digital Research
