* Re: [PATCH] null_blk: allow REQ_OP_ZONE_RESET_ALL to configure
2022-10-06 5:05 ` [PATCH] null_blk: allow REQ_OP_ZONE_RESET_ALL to configure Chaitanya Kulkarni
@ 2022-10-06 5:15 ` Damien Le Moal
2022-10-07 5:38 ` Chaitanya Kulkarni
2022-10-19 6:44 ` Damien Le Moal
2022-10-27 20:07 ` Vincent Fu
2 siblings, 1 reply; 11+ messages in thread
From: Damien Le Moal @ 2022-10-06 5:15 UTC (permalink / raw)
To: Chaitanya Kulkarni, linux-block
On 10/6/22 14:05, Chaitanya Kulkarni wrote:
> For a Zoned Block Device zone reset all is emulated if underlaying
> device doesn't support REQ_OP_ZONE_RESET_ALL operation. In null_blk
> Zoned mode there is no way to test zone reset all emulation present in
> the block layer since we enable it by default :-
>
> blkdev_zone_mgmt()
> blkdev_zone_reset_all_emulation() <---
> blkdev_zone_reset_all()
>
> Add a module parameter zone_reset_all to enable or disable
> REQ_OP_ZONE_RESET_ALL, enable it by default to retain the existing
> behaviour.
>
> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
> ---
> # git log -1
> commit 8ca0bd53a9c9e2f58c4fc9e38e3f5f82d26d3851 (HEAD -> for-next)
> Author: Chaitanya Kulkarni <kch@nvidia.com>
> Date: Wed Oct 5 21:57:00 2022 -0700
>
> null_blk: allow REQ_OP_ZONE_RESET_ALL to configure
>
> For a Zoned Block Device zone reset all is emulated if underlaying
> device doesn't support REQ_OP_ZONE_RESET_ALL operation. In null_blk
> Zoned mode there is no way to test zone reset all emulation present in
> the block layer since we enable it by default :-
>
> blkdev_zone_mgmt()
> blkdev_zone_reset_all_emulation() <---
> blkdev_zone_reset_all()
>
> Add a module parameter zone_reset_all to enable or disable
> REQ_OP_ZONE_RESET_ALL, enable it by default to retain the existing
> behaviour.
>
> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
> # ./compile_nullb.sh
> + umount /mnt/nullb0
> umount: /mnt/nullb0: not mounted.
> + rmdir 'config/nullb/nullb*'
> rmdir: failed to remove 'config/nullb/nullb*': No such file or directory
> + dmesg -c
> + modprobe -r null_blk
> + lsmod
> + grep null_blk
> ++ nproc
> + make -j 48 M=drivers/block modules
> + HOST=drivers/block/null_blk/
> ++ uname -r
> + HOST_DEST=/lib/modules/6.0.0-rc7blk+/kernel/drivers/block/null_blk/
> + cp drivers/block/null_blk//null_blk.ko /lib/modules/6.0.0-rc7blk+/kernel/drivers/block/null_blk//
> + ls -lrth /lib/modules/6.0.0-rc7blk+/kernel/drivers/block/null_blk//null_blk.ko
> -rw-r--r--. 1 root root 1.2M Oct 5 21:57 /lib/modules/6.0.0-rc7blk+/kernel/drivers/block/null_blk//null_blk.ko
> + sleep 1
> + dmesg -c
> # modprobe null_blk gb=1 zoned=1 zone_size=128 <---
> # lsblk
> NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
> sda 8:0 0 50G 0 disk
> ├─sda1 8:1 0 1G 0 part /boot
> └─sda2 8:2 0 49G 0 part /home
> sdb 8:16 0 100G 0 disk /mnt/data
> sr0 11:0 1 1024M 0 rom
> nullb0 250:0 0 1G 0 disk
> zram0 251:0 0 8G 0 disk [SWAP]
> vda 252:0 0 512M 0 disk
> nvme0n1 259:0 0 1G 0 disk
> # blkzone reset /dev/nullb0
> # dmesg -c
> [ 397.079221] null_blk: disk nullb0 created
> [ 397.079226] null_blk: module loaded
> [ 406.626500] blkdev_zone_reset_all 237 <---
> # modprobe -r null_blk
> #
> #
> #
> # modprobe null_blk gb=1 zoned=1 zone_size=128 zone_reset_all=0 <---
> # lsblk
> NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
> sda 8:0 0 50G 0 disk
> ├─sda1 8:1 0 1G 0 part /boot
> └─sda2 8:2 0 49G 0 part /home
> sdb 8:16 0 100G 0 disk /mnt/data
> sr0 11:0 1 1024M 0 rom
> nullb0 250:0 0 1G 0 disk
> zram0 251:0 0 8G 0 disk [SWAP]
> vda 252:0 0 512M 0 disk
> nvme0n1 259:0 0 1G 0 disk
> # blkzone reset /dev/nullb0
> # dmesg -c
> [ 425.456187] null_blk: disk nullb0 created
> [ 425.456192] null_blk: module loaded
> [ 438.419529] blkdev_zone_reset_all_emulated 197 <---
> # modprobe -r null_blk
> #
> ---
> drivers/block/null_blk/main.c | 5 +++++
> drivers/block/null_blk/null_blk.h | 1 +
> drivers/block/null_blk/zoned.c | 3 ++-
> 3 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
> index 8b7f42024f14..a0572e6c28ce 100644
> --- a/drivers/block/null_blk/main.c
> +++ b/drivers/block/null_blk/main.c
> @@ -260,6 +260,10 @@ static unsigned int g_zone_max_active;
> module_param_named(zone_max_active, g_zone_max_active, uint, 0444);
> MODULE_PARM_DESC(zone_max_active, "Maximum number of active zones when block device is zoned. Default: 0 (no limit)");
>
> +static bool g_zone_reset_all = true;
> +module_param_named(zone_reset_all, g_zone_reset_all, bool, 0444);
Nit: Why read-only ? You can make it writable without any issue, no ?
> +MODULE_PARM_DESC(zone_reset_all, "Allow REQ_OP_ZONE_RESET_ALL. Default: true");
> +
> static struct nullb_device *null_alloc_dev(void);
> static void null_free_dev(struct nullb_device *dev);
> static void null_del_dev(struct nullb *nullb);
> @@ -715,6 +719,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->zone_reset_all = g_zone_reset_all;
> dev->virt_boundary = g_virt_boundary;
> dev->no_sched = g_no_sched;
> dev->shared_tag_bitmap = g_shared_tag_bitmap;
> diff --git a/drivers/block/null_blk/null_blk.h b/drivers/block/null_blk/null_blk.h
> index e692c2a7369e..e7efe8de4ebf 100644
> --- a/drivers/block/null_blk/null_blk.h
> +++ b/drivers/block/null_blk/null_blk.h
> @@ -115,6 +115,7 @@ struct nullb_device {
> bool discard; /* if support discard */
> bool write_zeroes; /* if support write_zeroes */
> bool zoned; /* if device is zoned */
> + bool zone_reset_all; /* if support REQ_OP_ZONE_RESET_ALL */
> bool virt_boundary; /* virtual boundary on/off for the device */
> bool no_sched; /* no IO scheduler for the device */
> bool shared_tag_bitmap; /* use hostwide shared tags */
> diff --git a/drivers/block/null_blk/zoned.c b/drivers/block/null_blk/zoned.c
> index 55a69e48ef8b..7310d1c3f9ec 100644
> --- a/drivers/block/null_blk/zoned.c
> +++ b/drivers/block/null_blk/zoned.c
> @@ -160,7 +160,8 @@ int null_register_zoned_dev(struct nullb *nullb)
> struct request_queue *q = nullb->q;
>
> disk_set_zoned(nullb->disk, BLK_ZONED_HM);
> - blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
> + if (dev->zone_reset_all)
> + blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
> blk_queue_required_elevator_features(q, ELEVATOR_F_ZBD_SEQ_WRITE);
>
> if (queue_is_mq(q)) {
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] null_blk: allow REQ_OP_ZONE_RESET_ALL to configure
2022-10-06 5:15 ` Damien Le Moal
@ 2022-10-07 5:38 ` Chaitanya Kulkarni
2022-10-07 6:08 ` Damien Le Moal
0 siblings, 1 reply; 11+ messages in thread
From: Chaitanya Kulkarni @ 2022-10-07 5:38 UTC (permalink / raw)
To: Damien Le Moal, Chaitanya Kulkarni, linux-block@vger.kernel.org
On 10/5/22 22:15, Damien Le Moal wrote:
> On 10/6/22 14:05, Chaitanya Kulkarni wrote:
>> For a Zoned Block Device zone reset all is emulated if underlaying
>> device doesn't support REQ_OP_ZONE_RESET_ALL operation. In null_blk
>> Zoned mode there is no way to test zone reset all emulation present in
>> the block layer since we enable it by default :-
>>
>> blkdev_zone_mgmt()
>> blkdev_zone_reset_all_emulation() <---
>> blkdev_zone_reset_all()
>>
>> Add a module parameter zone_reset_all to enable or disable
>> REQ_OP_ZONE_RESET_ALL, enable it by default to retain the existing
>> behaviour.
>>
>> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
>> ---
>> # git log -1
>> commit 8ca0bd53a9c9e2f58c4fc9e38e3f5f82d26d3851 (HEAD -> for-next)
>> Author: Chaitanya Kulkarni <kch@nvidia.com>
>> Date: Wed Oct 5 21:57:00 2022 -0700
>>
>> null_blk: allow REQ_OP_ZONE_RESET_ALL to configure
>>
>> For a Zoned Block Device zone reset all is emulated if underlaying
>> device doesn't support REQ_OP_ZONE_RESET_ALL operation. In null_blk
>> Zoned mode there is no way to test zone reset all emulation present in
>> the block layer since we enable it by default :-
>>
>> blkdev_zone_mgmt()
>> blkdev_zone_reset_all_emulation() <---
>> blkdev_zone_reset_all()
>>
>> Add a module parameter zone_reset_all to enable or disable
>> REQ_OP_ZONE_RESET_ALL, enable it by default to retain the existing
>> behaviour.
>>
>> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
>> # ./compile_nullb.sh
>> + umount /mnt/nullb0
>> umount: /mnt/nullb0: not mounted.
>> + rmdir 'config/nullb/nullb*'
>> rmdir: failed to remove 'config/nullb/nullb*': No such file or directory
>> + dmesg -c
>> + modprobe -r null_blk
>> + lsmod
>> + grep null_blk
>> ++ nproc
>> + make -j 48 M=drivers/block modules
>> + HOST=drivers/block/null_blk/
>> ++ uname -r
>> + HOST_DEST=/lib/modules/6.0.0-rc7blk+/kernel/drivers/block/null_blk/
>> + cp drivers/block/null_blk//null_blk.ko /lib/modules/6.0.0-rc7blk+/kernel/drivers/block/null_blk//
>> + ls -lrth /lib/modules/6.0.0-rc7blk+/kernel/drivers/block/null_blk//null_blk.ko
>> -rw-r--r--. 1 root root 1.2M Oct 5 21:57 /lib/modules/6.0.0-rc7blk+/kernel/drivers/block/null_blk//null_blk.ko
>> + sleep 1
>> + dmesg -c
>> # modprobe null_blk gb=1 zoned=1 zone_size=128 <---
>> # lsblk
>> NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
>> sda 8:0 0 50G 0 disk
>> ├─sda1 8:1 0 1G 0 part /boot
>> └─sda2 8:2 0 49G 0 part /home
>> sdb 8:16 0 100G 0 disk /mnt/data
>> sr0 11:0 1 1024M 0 rom
>> nullb0 250:0 0 1G 0 disk
>> zram0 251:0 0 8G 0 disk [SWAP]
>> vda 252:0 0 512M 0 disk
>> nvme0n1 259:0 0 1G 0 disk
>> # blkzone reset /dev/nullb0
>> # dmesg -c
>> [ 397.079221] null_blk: disk nullb0 created
>> [ 397.079226] null_blk: module loaded
>> [ 406.626500] blkdev_zone_reset_all 237 <---
>> # modprobe -r null_blk
>> #
>> #
>> #
>> # modprobe null_blk gb=1 zoned=1 zone_size=128 zone_reset_all=0 <---
>> # lsblk
>> NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
>> sda 8:0 0 50G 0 disk
>> ├─sda1 8:1 0 1G 0 part /boot
>> └─sda2 8:2 0 49G 0 part /home
>> sdb 8:16 0 100G 0 disk /mnt/data
>> sr0 11:0 1 1024M 0 rom
>> nullb0 250:0 0 1G 0 disk
>> zram0 251:0 0 8G 0 disk [SWAP]
>> vda 252:0 0 512M 0 disk
>> nvme0n1 259:0 0 1G 0 disk
>> # blkzone reset /dev/nullb0
>> # dmesg -c
>> [ 425.456187] null_blk: disk nullb0 created
>> [ 425.456192] null_blk: module loaded
>> [ 438.419529] blkdev_zone_reset_all_emulated 197 <---
>> # modprobe -r null_blk
>> #
>> ---
>> drivers/block/null_blk/main.c | 5 +++++
>> drivers/block/null_blk/null_blk.h | 1 +
>> drivers/block/null_blk/zoned.c | 3 ++-
>> 3 files changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
>> index 8b7f42024f14..a0572e6c28ce 100644
>> --- a/drivers/block/null_blk/main.c
>> +++ b/drivers/block/null_blk/main.c
>> @@ -260,6 +260,10 @@ static unsigned int g_zone_max_active;
>> module_param_named(zone_max_active, g_zone_max_active, uint, 0444);
>> MODULE_PARM_DESC(zone_max_active, "Maximum number of active zones when block device is zoned. Default: 0 (no limit)");
>>
>> +static bool g_zone_reset_all = true;
>> +module_param_named(zone_reset_all, g_zone_reset_all, bool, 0444);
>
> Nit: Why read-only ? You can make it writable without any issue, no ?
>
Sorry I didn't understand this comment. In case this comment is about
new zone_reset_all modpram being readonly and not allowed to set via
command line? The test log shows I was able to set it to 0 and trigger
call to blkdev_zone_reset_all_emulated() and default behavior ends up
calling blkdev_zone_reset_all().
-ck
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] null_blk: allow REQ_OP_ZONE_RESET_ALL to configure
2022-10-07 5:38 ` Chaitanya Kulkarni
@ 2022-10-07 6:08 ` Damien Le Moal
2022-10-19 6:17 ` Chaitanya Kulkarni
0 siblings, 1 reply; 11+ messages in thread
From: Damien Le Moal @ 2022-10-07 6:08 UTC (permalink / raw)
To: Chaitanya Kulkarni, linux-block@vger.kernel.org
On 10/7/22 14:38, Chaitanya Kulkarni wrote:
> On 10/5/22 22:15, Damien Le Moal wrote:
>> On 10/6/22 14:05, Chaitanya Kulkarni wrote:
>>> For a Zoned Block Device zone reset all is emulated if underlaying
>>> device doesn't support REQ_OP_ZONE_RESET_ALL operation. In null_blk
>>> Zoned mode there is no way to test zone reset all emulation present in
>>> the block layer since we enable it by default :-
>>>
>>> blkdev_zone_mgmt()
>>> blkdev_zone_reset_all_emulation() <---
>>> blkdev_zone_reset_all()
>>>
>>> Add a module parameter zone_reset_all to enable or disable
>>> REQ_OP_ZONE_RESET_ALL, enable it by default to retain the existing
>>> behaviour.
>>>
>>> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
>>> ---
>>> # git log -1
>>> commit 8ca0bd53a9c9e2f58c4fc9e38e3f5f82d26d3851 (HEAD -> for-next)
>>> Author: Chaitanya Kulkarni <kch@nvidia.com>
>>> Date: Wed Oct 5 21:57:00 2022 -0700
>>>
>>> null_blk: allow REQ_OP_ZONE_RESET_ALL to configure
>>>
>>> For a Zoned Block Device zone reset all is emulated if underlaying
>>> device doesn't support REQ_OP_ZONE_RESET_ALL operation. In null_blk
>>> Zoned mode there is no way to test zone reset all emulation present in
>>> the block layer since we enable it by default :-
>>>
>>> blkdev_zone_mgmt()
>>> blkdev_zone_reset_all_emulation() <---
>>> blkdev_zone_reset_all()
>>>
>>> Add a module parameter zone_reset_all to enable or disable
>>> REQ_OP_ZONE_RESET_ALL, enable it by default to retain the existing
>>> behaviour.
>>>
>>> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
>>> # ./compile_nullb.sh
>>> + umount /mnt/nullb0
>>> umount: /mnt/nullb0: not mounted.
>>> + rmdir 'config/nullb/nullb*'
>>> rmdir: failed to remove 'config/nullb/nullb*': No such file or directory
>>> + dmesg -c
>>> + modprobe -r null_blk
>>> + lsmod
>>> + grep null_blk
>>> ++ nproc
>>> + make -j 48 M=drivers/block modules
>>> + HOST=drivers/block/null_blk/
>>> ++ uname -r
>>> + HOST_DEST=/lib/modules/6.0.0-rc7blk+/kernel/drivers/block/null_blk/
>>> + cp drivers/block/null_blk//null_blk.ko /lib/modules/6.0.0-rc7blk+/kernel/drivers/block/null_blk//
>>> + ls -lrth /lib/modules/6.0.0-rc7blk+/kernel/drivers/block/null_blk//null_blk.ko
>>> -rw-r--r--. 1 root root 1.2M Oct 5 21:57 /lib/modules/6.0.0-rc7blk+/kernel/drivers/block/null_blk//null_blk.ko
>>> + sleep 1
>>> + dmesg -c
>>> # modprobe null_blk gb=1 zoned=1 zone_size=128 <---
>>> # lsblk
>>> NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
>>> sda 8:0 0 50G 0 disk
>>> ├─sda1 8:1 0 1G 0 part /boot
>>> └─sda2 8:2 0 49G 0 part /home
>>> sdb 8:16 0 100G 0 disk /mnt/data
>>> sr0 11:0 1 1024M 0 rom
>>> nullb0 250:0 0 1G 0 disk
>>> zram0 251:0 0 8G 0 disk [SWAP]
>>> vda 252:0 0 512M 0 disk
>>> nvme0n1 259:0 0 1G 0 disk
>>> # blkzone reset /dev/nullb0
>>> # dmesg -c
>>> [ 397.079221] null_blk: disk nullb0 created
>>> [ 397.079226] null_blk: module loaded
>>> [ 406.626500] blkdev_zone_reset_all 237 <---
>>> # modprobe -r null_blk
>>> #
>>> #
>>> #
>>> # modprobe null_blk gb=1 zoned=1 zone_size=128 zone_reset_all=0 <---
>>> # lsblk
>>> NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
>>> sda 8:0 0 50G 0 disk
>>> ├─sda1 8:1 0 1G 0 part /boot
>>> └─sda2 8:2 0 49G 0 part /home
>>> sdb 8:16 0 100G 0 disk /mnt/data
>>> sr0 11:0 1 1024M 0 rom
>>> nullb0 250:0 0 1G 0 disk
>>> zram0 251:0 0 8G 0 disk [SWAP]
>>> vda 252:0 0 512M 0 disk
>>> nvme0n1 259:0 0 1G 0 disk
>>> # blkzone reset /dev/nullb0
>>> # dmesg -c
>>> [ 425.456187] null_blk: disk nullb0 created
>>> [ 425.456192] null_blk: module loaded
>>> [ 438.419529] blkdev_zone_reset_all_emulated 197 <---
>>> # modprobe -r null_blk
>>> #
>>> ---
>>> drivers/block/null_blk/main.c | 5 +++++
>>> drivers/block/null_blk/null_blk.h | 1 +
>>> drivers/block/null_blk/zoned.c | 3 ++-
>>> 3 files changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
>>> index 8b7f42024f14..a0572e6c28ce 100644
>>> --- a/drivers/block/null_blk/main.c
>>> +++ b/drivers/block/null_blk/main.c
>>> @@ -260,6 +260,10 @@ static unsigned int g_zone_max_active;
>>> module_param_named(zone_max_active, g_zone_max_active, uint, 0444);
>>> MODULE_PARM_DESC(zone_max_active, "Maximum number of active zones when block device is zoned. Default: 0 (no limit)");
>>>
>>> +static bool g_zone_reset_all = true;
>>> +module_param_named(zone_reset_all, g_zone_reset_all, bool, 0444);
>>
>> Nit: Why read-only ? You can make it writable without any issue, no ?
>>
>
> Sorry I didn't understand this comment. In case this comment is about
> new zone_reset_all modpram being readonly and not allowed to set via
> command line? The test log shows I was able to set it to 0 and trigger
> call to blkdev_zone_reset_all_emulated() and default behavior ends up
> calling blkdev_zone_reset_all().
>
> -ck
>
If you change the param mode to 644, one can do:
echo 0 >/sys/module/null_blk/parameters/zone_reset_all
or
echo 1 >/sys/module/null_blk/parameters/zone_reset_all
But given that all null_blk parameters are read-only, not a big deal.
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] null_blk: allow REQ_OP_ZONE_RESET_ALL to configure
2022-10-07 6:08 ` Damien Le Moal
@ 2022-10-19 6:17 ` Chaitanya Kulkarni
0 siblings, 0 replies; 11+ messages in thread
From: Chaitanya Kulkarni @ 2022-10-19 6:17 UTC (permalink / raw)
To: Damien Le Moal; +Cc: linux-block@vger.kernel.org
Damien,
MODULE_PARM_DESC(zone_max_active, "Maximum number of active zones when
block device is zoned. Default: 0 (no limit)");
>>>>
>>>> +static bool g_zone_reset_all = true;
>>>> +module_param_named(zone_reset_all, g_zone_reset_all, bool, 0444);
>>>
>>> Nit: Why read-only ? You can make it writable without any issue, no ?
>>>
>>
>> Sorry I didn't understand this comment. In case this comment is about
>> new zone_reset_all modpram being readonly and not allowed to set via
>> command line? The test log shows I was able to set it to 0 and trigger
>> call to blkdev_zone_reset_all_emulated() and default behavior ends up
>> calling blkdev_zone_reset_all().
>>
>> -ck
>>
>
> If you change the param mode to 644, one can do:
>
> echo 0 >/sys/module/null_blk/parameters/zone_reset_all
> or
> echo 1 >/sys/module/null_blk/parameters/zone_reset_all
>
> But given that all null_blk parameters are read-only, not a big deal.
>
if you are okay with this can I get your reviewed-by ?
-ck
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] null_blk: allow REQ_OP_ZONE_RESET_ALL to configure
2022-10-06 5:05 ` [PATCH] null_blk: allow REQ_OP_ZONE_RESET_ALL to configure Chaitanya Kulkarni
2022-10-06 5:15 ` Damien Le Moal
@ 2022-10-19 6:44 ` Damien Le Moal
2022-10-27 20:07 ` Vincent Fu
2 siblings, 0 replies; 11+ messages in thread
From: Damien Le Moal @ 2022-10-19 6:44 UTC (permalink / raw)
To: Chaitanya Kulkarni, linux-block
On 10/6/22 14:05, Chaitanya Kulkarni wrote:
> For a Zoned Block Device zone reset all is emulated if underlaying
> device doesn't support REQ_OP_ZONE_RESET_ALL operation. In null_blk
> Zoned mode there is no way to test zone reset all emulation present in
> the block layer since we enable it by default :-
>
> blkdev_zone_mgmt()
> blkdev_zone_reset_all_emulation() <---
> blkdev_zone_reset_all()
>
> Add a module parameter zone_reset_all to enable or disable
> REQ_OP_ZONE_RESET_ALL, enable it by default to retain the existing
> behaviour.
>
> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
> ---
> # git log -1
> commit 8ca0bd53a9c9e2f58c4fc9e38e3f5f82d26d3851 (HEAD -> for-next)
> Author: Chaitanya Kulkarni <kch@nvidia.com>
> Date: Wed Oct 5 21:57:00 2022 -0700
>
> null_blk: allow REQ_OP_ZONE_RESET_ALL to configure
>
> For a Zoned Block Device zone reset all is emulated if underlaying
> device doesn't support REQ_OP_ZONE_RESET_ALL operation. In null_blk
> Zoned mode there is no way to test zone reset all emulation present in
> the block layer since we enable it by default :-
>
> blkdev_zone_mgmt()
> blkdev_zone_reset_all_emulation() <---
> blkdev_zone_reset_all()
>
> Add a module parameter zone_reset_all to enable or disable
> REQ_OP_ZONE_RESET_ALL, enable it by default to retain the existing
> behaviour.
>
> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
> # ./compile_nullb.sh
> + umount /mnt/nullb0
> umount: /mnt/nullb0: not mounted.
> + rmdir 'config/nullb/nullb*'
> rmdir: failed to remove 'config/nullb/nullb*': No such file or directory
> + dmesg -c
> + modprobe -r null_blk
> + lsmod
> + grep null_blk
> ++ nproc
> + make -j 48 M=drivers/block modules
> + HOST=drivers/block/null_blk/
> ++ uname -r
> + HOST_DEST=/lib/modules/6.0.0-rc7blk+/kernel/drivers/block/null_blk/
> + cp drivers/block/null_blk//null_blk.ko /lib/modules/6.0.0-rc7blk+/kernel/drivers/block/null_blk//
> + ls -lrth /lib/modules/6.0.0-rc7blk+/kernel/drivers/block/null_blk//null_blk.ko
> -rw-r--r--. 1 root root 1.2M Oct 5 21:57 /lib/modules/6.0.0-rc7blk+/kernel/drivers/block/null_blk//null_blk.ko
> + sleep 1
> + dmesg -c
> # modprobe null_blk gb=1 zoned=1 zone_size=128 <---
> # lsblk
> NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
> sda 8:0 0 50G 0 disk
> ├─sda1 8:1 0 1G 0 part /boot
> └─sda2 8:2 0 49G 0 part /home
> sdb 8:16 0 100G 0 disk /mnt/data
> sr0 11:0 1 1024M 0 rom
> nullb0 250:0 0 1G 0 disk
> zram0 251:0 0 8G 0 disk [SWAP]
> vda 252:0 0 512M 0 disk
> nvme0n1 259:0 0 1G 0 disk
> # blkzone reset /dev/nullb0
> # dmesg -c
> [ 397.079221] null_blk: disk nullb0 created
> [ 397.079226] null_blk: module loaded
> [ 406.626500] blkdev_zone_reset_all 237 <---
> # modprobe -r null_blk
> #
> #
> #
> # modprobe null_blk gb=1 zoned=1 zone_size=128 zone_reset_all=0 <---
> # lsblk
> NAME MAJ:MIN RM SIZE RO TYPE MOUNTPOINT
> sda 8:0 0 50G 0 disk
> ├─sda1 8:1 0 1G 0 part /boot
> └─sda2 8:2 0 49G 0 part /home
> sdb 8:16 0 100G 0 disk /mnt/data
> sr0 11:0 1 1024M 0 rom
> nullb0 250:0 0 1G 0 disk
> zram0 251:0 0 8G 0 disk [SWAP]
> vda 252:0 0 512M 0 disk
> nvme0n1 259:0 0 1G 0 disk
> # blkzone reset /dev/nullb0
> # dmesg -c
> [ 425.456187] null_blk: disk nullb0 created
> [ 425.456192] null_blk: module loaded
> [ 438.419529] blkdev_zone_reset_all_emulated 197 <---
> # modprobe -r null_blk
> #
> ---
> drivers/block/null_blk/main.c | 5 +++++
> drivers/block/null_blk/null_blk.h | 1 +
> drivers/block/null_blk/zoned.c | 3 ++-
> 3 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
> index 8b7f42024f14..a0572e6c28ce 100644
> --- a/drivers/block/null_blk/main.c
> +++ b/drivers/block/null_blk/main.c
> @@ -260,6 +260,10 @@ static unsigned int g_zone_max_active;
> module_param_named(zone_max_active, g_zone_max_active, uint, 0444);
> MODULE_PARM_DESC(zone_max_active, "Maximum number of active zones when block device is zoned. Default: 0 (no limit)");
>
> +static bool g_zone_reset_all = true;
> +module_param_named(zone_reset_all, g_zone_reset_all, bool, 0444);
> +MODULE_PARM_DESC(zone_reset_all, "Allow REQ_OP_ZONE_RESET_ALL. Default: true");
> +
> static struct nullb_device *null_alloc_dev(void);
> static void null_free_dev(struct nullb_device *dev);
> static void null_del_dev(struct nullb *nullb);
> @@ -715,6 +719,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->zone_reset_all = g_zone_reset_all;
> dev->virt_boundary = g_virt_boundary;
> dev->no_sched = g_no_sched;
> dev->shared_tag_bitmap = g_shared_tag_bitmap;
> diff --git a/drivers/block/null_blk/null_blk.h b/drivers/block/null_blk/null_blk.h
> index e692c2a7369e..e7efe8de4ebf 100644
> --- a/drivers/block/null_blk/null_blk.h
> +++ b/drivers/block/null_blk/null_blk.h
> @@ -115,6 +115,7 @@ struct nullb_device {
> bool discard; /* if support discard */
> bool write_zeroes; /* if support write_zeroes */
> bool zoned; /* if device is zoned */
> + bool zone_reset_all; /* if support REQ_OP_ZONE_RESET_ALL */
> bool virt_boundary; /* virtual boundary on/off for the device */
> bool no_sched; /* no IO scheduler for the device */
> bool shared_tag_bitmap; /* use hostwide shared tags */
> diff --git a/drivers/block/null_blk/zoned.c b/drivers/block/null_blk/zoned.c
> index 55a69e48ef8b..7310d1c3f9ec 100644
> --- a/drivers/block/null_blk/zoned.c
> +++ b/drivers/block/null_blk/zoned.c
> @@ -160,7 +160,8 @@ int null_register_zoned_dev(struct nullb *nullb)
> struct request_queue *q = nullb->q;
>
> disk_set_zoned(nullb->disk, BLK_ZONED_HM);
> - blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
> + if (dev->zone_reset_all)
> + blk_queue_flag_set(QUEUE_FLAG_ZONE_RESETALL, q);
> blk_queue_required_elevator_features(q, ELEVATOR_F_ZBD_SEQ_WRITE);
>
> if (queue_is_mq(q)) {
--
Damien Le Moal
Western Digital Research
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH] null_blk: allow REQ_OP_ZONE_RESET_ALL to configure
2022-10-06 5:05 ` [PATCH] null_blk: allow REQ_OP_ZONE_RESET_ALL to configure Chaitanya Kulkarni
2022-10-06 5:15 ` Damien Le Moal
2022-10-19 6:44 ` Damien Le Moal
@ 2022-10-27 20:07 ` Vincent Fu
2022-11-02 0:52 ` Chaitanya Kulkarni
2 siblings, 1 reply; 11+ messages in thread
From: Vincent Fu @ 2022-10-27 20:07 UTC (permalink / raw)
To: kch@nvidia.com; +Cc: linux-block@vger.kernel.org, Vincent Fu
On Wed, Oct 05, 2022 at 10:05:13PM -0700, Chaitanya Kulkarni wrote:
>For a Zoned Block Device zone reset all is emulated if underlaying
>device doesn't support REQ_OP_ZONE_RESET_ALL operation. In null_blk
>Zoned mode there is no way to test zone reset all emulation present in
>the block layer since we enable it by default :-
>
>blkdev_zone_mgmt()
> blkdev_zone_reset_all_emulation() <---
> blkdev_zone_reset_all()
>
>Add a module parameter zone_reset_all to enable or disable
>REQ_OP_ZONE_RESET_ALL, enable it by default to retain the existing
>behaviour.
>
>Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
For the sake of completeness would it be reasonable to also provide a
means to set this option via configfs?
Vincent
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] null_blk: allow REQ_OP_ZONE_RESET_ALL to configure
2022-10-27 20:07 ` Vincent Fu
@ 2022-11-02 0:52 ` Chaitanya Kulkarni
2022-11-04 18:52 ` Vincent Fu
0 siblings, 1 reply; 11+ messages in thread
From: Chaitanya Kulkarni @ 2022-11-02 0:52 UTC (permalink / raw)
To: Vincent Fu, Chaitanya Kulkarni; +Cc: linux-block@vger.kernel.org
On 10/27/22 13:07, Vincent Fu wrote:
> On Wed, Oct 05, 2022 at 10:05:13PM -0700, Chaitanya Kulkarni wrote:
>> For a Zoned Block Device zone reset all is emulated if underlaying
>> device doesn't support REQ_OP_ZONE_RESET_ALL operation. In null_blk
>> Zoned mode there is no way to test zone reset all emulation present in
>> the block layer since we enable it by default :-
>>
>> blkdev_zone_mgmt()
>> blkdev_zone_reset_all_emulation() <---
>> blkdev_zone_reset_all()
>>
>> Add a module parameter zone_reset_all to enable or disable
>> REQ_OP_ZONE_RESET_ALL, enable it by default to retain the existing
>> behaviour.
>>
>> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
>
> For the sake of completeness would it be reasonable to also provide a
> means to set this option via configfs?
>
> Vincent
I deliberately avoided that as I don't see any need for it as of now.
but if it is a hard requirement I Can certainly send V2 with configfs
param.
-ck
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] null_blk: allow REQ_OP_ZONE_RESET_ALL to configure
2022-11-02 0:52 ` Chaitanya Kulkarni
@ 2022-11-04 18:52 ` Vincent Fu
2022-11-05 15:24 ` Jens Axboe
0 siblings, 1 reply; 11+ messages in thread
From: Vincent Fu @ 2022-11-04 18:52 UTC (permalink / raw)
To: kch@nvidia.com, Chaitanya Kulkarni
Cc: linux-block@vger.kernel.org, Vincent Fu
On Wed, Nov 02, 2022 at 12:52:06AM +0000, Chaitanya Kulkarni wrote:
>On 10/27/22 13:07, Vincent Fu wrote:
>> On Wed, Oct 05, 2022 at 10:05:13PM -0700, Chaitanya Kulkarni wrote:
>>> For a Zoned Block Device zone reset all is emulated if underlaying
>>> device doesn't support REQ_OP_ZONE_RESET_ALL operation. In null_blk
>>> Zoned mode there is no way to test zone reset all emulation present in
>>> the block layer since we enable it by default :-
>>>
>>> blkdev_zone_mgmt()
>>> blkdev_zone_reset_all_emulation() <---
>>> blkdev_zone_reset_all()
>>>
>>> Add a module parameter zone_reset_all to enable or disable
>>> REQ_OP_ZONE_RESET_ALL, enable it by default to retain the existing
>>> behaviour.
>>>
>>> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
>>
>> For the sake of completeness would it be reasonable to also provide a
>> means to set this option via configfs?
>>
>> Vincent
>
>I deliberately avoided that as I don't see any need for it as of now.
>but if it is a hard requirement I Can certainly send V2 with configfs
>param.
>
>-ck
I think the default should be to have features available as both module
options and via configfs unless there are good reasons to make an
exception.
https://lore.kernel.org/linux-block/f0dadb60-c02d-d569-3004-81eafeebb95f@kernel.dk/
https://lore.kernel.org/linux-block/ca206223-112f-2d60-34a3-bb7e6295de7a@kernel.dk/
Vincent
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] null_blk: allow REQ_OP_ZONE_RESET_ALL to configure
2022-11-04 18:52 ` Vincent Fu
@ 2022-11-05 15:24 ` Jens Axboe
2022-11-05 16:35 ` Chaitanya Kulkarni
0 siblings, 1 reply; 11+ messages in thread
From: Jens Axboe @ 2022-11-05 15:24 UTC (permalink / raw)
To: Vincent Fu, kch@nvidia.com, Chaitanya Kulkarni
Cc: linux-block@vger.kernel.org
On 11/4/22 12:52 PM, Vincent Fu wrote:
> On Wed, Nov 02, 2022 at 12:52:06AM +0000, Chaitanya Kulkarni wrote:
>> On 10/27/22 13:07, Vincent Fu wrote:
>>> On Wed, Oct 05, 2022 at 10:05:13PM -0700, Chaitanya Kulkarni wrote:
>>>> For a Zoned Block Device zone reset all is emulated if underlaying
>>>> device doesn't support REQ_OP_ZONE_RESET_ALL operation. In null_blk
>>>> Zoned mode there is no way to test zone reset all emulation present in
>>>> the block layer since we enable it by default :-
>>>>
>>>> blkdev_zone_mgmt()
>>>> blkdev_zone_reset_all_emulation() <---
>>>> blkdev_zone_reset_all()
>>>>
>>>> Add a module parameter zone_reset_all to enable or disable
>>>> REQ_OP_ZONE_RESET_ALL, enable it by default to retain the existing
>>>> behaviour.
>>>>
>>>> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
>>>
>>> For the sake of completeness would it be reasonable to also provide a
>>> means to set this option via configfs?
>>>
>>> Vincent
>>
>> I deliberately avoided that as I don't see any need for it as of now.
>> but if it is a hard requirement I Can certainly send V2 with configfs
>> param.
>>
>> -ck
>
> I think the default should be to have features available as both module
> options and via configfs unless there are good reasons to make an
> exception.
>
> https://lore.kernel.org/linux-block/f0dadb60-c02d-d569-3004-81eafeebb95f@kernel.dk/
> https://lore.kernel.org/linux-block/ca206223-112f-2d60-34a3-bb7e6295de7a@kernel.dk/
Yes agree, work was done to bring those closer, would be silly to add
something new and NOT have both available.
--
Jens Axboe
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] null_blk: allow REQ_OP_ZONE_RESET_ALL to configure
2022-11-05 15:24 ` Jens Axboe
@ 2022-11-05 16:35 ` Chaitanya Kulkarni
0 siblings, 0 replies; 11+ messages in thread
From: Chaitanya Kulkarni @ 2022-11-05 16:35 UTC (permalink / raw)
To: Jens Axboe, Vincent Fu, Chaitanya Kulkarni; +Cc: linux-block@vger.kernel.org
On 11/5/22 08:24, Jens Axboe wrote:
> On 11/4/22 12:52 PM, Vincent Fu wrote:
>> On Wed, Nov 02, 2022 at 12:52:06AM +0000, Chaitanya Kulkarni wrote:
>>> On 10/27/22 13:07, Vincent Fu wrote:
>>>> On Wed, Oct 05, 2022 at 10:05:13PM -0700, Chaitanya Kulkarni wrote:
>>>>> For a Zoned Block Device zone reset all is emulated if underlaying
>>>>> device doesn't support REQ_OP_ZONE_RESET_ALL operation. In null_blk
>>>>> Zoned mode there is no way to test zone reset all emulation present in
>>>>> the block layer since we enable it by default :-
>>>>>
>>>>> blkdev_zone_mgmt()
>>>>> blkdev_zone_reset_all_emulation() <---
>>>>> blkdev_zone_reset_all()
>>>>>
>>>>> Add a module parameter zone_reset_all to enable or disable
>>>>> REQ_OP_ZONE_RESET_ALL, enable it by default to retain the existing
>>>>> behaviour.
>>>>>
>>>>> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
>>>>
>>>> For the sake of completeness would it be reasonable to also provide a
>>>> means to set this option via configfs?
>>>>
>>>> Vincent
>>>
>>> I deliberately avoided that as I don't see any need for it as of now.
>>> but if it is a hard requirement I Can certainly send V2 with configfs
>>> param.
>>>
>>> -ck
>>
>> I think the default should be to have features available as both module
>> options and via configfs unless there are good reasons to make an
>> exception.
>>
>> https://lore.kernel.org/linux-block/f0dadb60-c02d-d569-3004-81eafeebb95f@kernel.dk/
>> https://lore.kernel.org/linux-block/ca206223-112f-2d60-34a3-bb7e6295de7a@kernel.dk/
>
> Yes agree, work was done to bring those closer, would be silly to add
> something new and NOT have both available.
>
I was trying to avoid polluting configfs space with tons of params guess
which are only required for blktetsts, will send out V2 with added
parameter.
-ck
^ permalink raw reply [flat|nested] 11+ messages in thread