Re: [PATCH] null_blk: allow REQ_OP_ZONE_RESET_ALL to configure

[Chaitanya Kulkarni <chaitanyak@nvidia.com>]

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