| Message ID | 20221006050514.5564-1-kch@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | null_blk: allow REQ_OP_ZONE_RESET_ALL to configure | expand |
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)) {
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
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, 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
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)) {
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
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
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
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.
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
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)) {