linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/1] null_blk: allow user to set QUEUE_FLAG_NOWAIT
@ 2023-04-12  8:47 Chaitanya Kulkarni
  2023-04-12  8:47 ` [PATCH 1/1] " Chaitanya Kulkarni
  2023-04-12  8:50 ` [PATCH 0/1] " Chaitanya Kulkarni
  0 siblings, 2 replies; 11+ messages in thread
From: Chaitanya Kulkarni @ 2023-04-12  8:47 UTC (permalink / raw)
  To: linux-block
  Cc: axboe, damien.lemoal, kch, johannes.thumshirn, bvanassche,
	vincent.fu, shinichiro.kawasaki, yukuai3

Hi,

QUEUE_FLAG_NOWAIT is set by default to mq drivers such as null_blk as
a part of QUEUE_FLAG_MQ_DEFAULT that gets assigned in following code
path see blk_mq_init_allocated_queue():-

null_add_dev()
if (dev->queue_mode == NULL_Q_MQ) {
        blk_mq_alloc_disk()
          __blk_mq_alloc_disk()
	    blk_mq_init_queue_data()
              blk_mq_init_allocated_queue()
                q->queue_flags |= QUEUE_FLAG_MQ_DEFAULT;
}

But it not set when null_blk is loaded with NULL_Q_BIO mode in following
code path :-

if (dev->queue_mode == NULL_Q_BIO) {
        nullb->disk = blk_alloc_disk(nullb->dev->home_node);
        	blk_alloc_disk()
        	  blk_alloc_queue()
        	  __alloc_disk_nodw()
}

Add a new module parameter nowait and respective configfs attr
that will set and clear the QUEUE_FLAG_NOWAIT based on a value set by
user in null_add_dev() irrespective of the queue mode, by default keep
it enabled to retain the original behaviour for the NULL_Q_MQ mode.

* Configfs Membacked mode:-

NULL_Q_BIO mode QUEUE_FLAG_NOWAIT disabled:-
---------------------------------------------
configfs-qmode-0-nowait-0-fio-1.log:  read: IOPS=1295k, BW=5058MiB/s
configfs-qmode-0-nowait-0-fio-2.log:  read: IOPS=1362k, BW=5318MiB/s
configfs-qmode-0-nowait-0-fio-3.log:  read: IOPS=1332k, BW=5201MiB/s

NULL_Q_BIO mode QUEUE_FLAG_NOWAIT enabled :-
---------------------------------------------
configfs-qmode-0-nowait-1-fio-1.log:  read: IOPS=2095k, BW=8183MiB/s
configfs-qmode-0-nowait-1-fio-2.log:  read: IOPS=2085k, BW=8145MiB/s
configfs-qmode-0-nowait-1-fio-3.log:  read: IOPS=2036k, BW=7955MiB/s

NULL_Q_MQ mode QUEUE_FLAG_NOWAIT disabled :-
---------------------------------------------
configfs-qmode-2-nowait-0-fio-1.log:  read: IOPS=1288k, BW=5031MiB/s
configfs-qmode-2-nowait-0-fio-2.log:  read: IOPS=1239k, BW=4839MiB/s
configfs-qmode-2-nowait-0-fio-3.log:  read: IOPS=1252k, BW=4889MiB/s

NULL_Q_MQ mode QUEUE_FLAG_NOWAIT enabled :-
---------------------------------------------
configfs-qmode-2-nowait-1-fio-1.log:  read: IOPS=2101k, BW=8208MiB/s
configfs-qmode-2-nowait-1-fio-2.log:  read: IOPS=2091k, BW=8169MiB/s
configfs-qmode-2-nowait-1-fio-3.log:  read: IOPS=2088k, BW=8155MiB/s

* Non Configfs non-membacked mode:-

NULL_Q_BIO mode QUEUE_FLAG_NOWAIT disabled:-
---------------------------------------------
qmode-0-nowait-0-fio-1.log:  read: IOPS=1362k, BW=5321MiB/s
qmode-0-nowait-0-fio-2.log:  read: IOPS=1334k, BW=5210MiB/s
qmode-0-nowait-0-fio-3.log:  read: IOPS=1386k, BW=5416MiB/s

NULL_Q_BIO mode QUEUE_FLAG_NOWAIT enabled :-
---------------------------------------------
qmode-0-nowait-1-fio-1.log:  read: IOPS=5405k, BW=20.6GiB/s
qmode-0-nowait-1-fio-2.log:  read: IOPS=5502k, BW=21.0GiB/s
qmode-0-nowait-1-fio-3.log:  read: IOPS=5518k, BW=21.0GiB/s

NULL_Q_MQ mode QUEUE_FLAG_NOWAIT disabled :-
---------------------------------------------
qmode-2-nowait-0-fio-1.log:  read: IOPS=1356k, BW=5296MiB/s
qmode-2-nowait-0-fio-2.log:  read: IOPS=1318k, BW=5148MiB/s
qmode-2-nowait-0-fio-3.log:  read: IOPS=1252k, BW=4891MiB/s

NULL_Q_MQ mode QUEUE_FLAG_NOWAIT enabled :-
---------------------------------------------
qmode-2-nowait-1-fio-1.log:  read: IOPS=5466k, BW=20.9GiB/s
qmode-2-nowait-1-fio-2.log:  read: IOPS=5446k, BW=20.8GiB/s
qmode-2-nowait-1-fio-3.log:  read: IOPS=5482k, BW=20.9GiB/s

Below is a performance test log for with and without configfs mambeacked
mode.

-ck

Chaitanya Kulkarni (1):
  null_blk: allow user to set QUEUE_FLAG_NOWAIT

 drivers/block/null_blk/main.c     | 22 +++++++++++++++++-----
 drivers/block/null_blk/null_blk.h |  1 +
 2 files changed, 18 insertions(+), 5 deletions(-)

Performance test log for null_blk configfs and non-configfs:-

linux-block (for-next) # sh test-null-blk-no-wait.sh 
+ modprobe -r null_blk
+ ./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/ clean
  CLEAN   drivers/block/Module.symvers
++ nproc
+ make -j 48 M=drivers/block modules
  CC [M]  drivers/block/floppy.o
  CC [M]  drivers/block/brd.o
  CC [M]  drivers/block/loop.o
  CC [M]  drivers/block/nbd.o
  CC [M]  drivers/block/virtio_blk.o
  CC [M]  drivers/block/xen-blkfront.o
  CC [M]  drivers/block/rbd.o
  CC [M]  drivers/block/drbd/drbd_buildtag.o
  CC [M]  drivers/block/xen-blkback/blkback.o
  CC [M]  drivers/block/mtip32xx/mtip32xx.o
  CC [M]  drivers/block/zram/zcomp.o
  CC [M]  drivers/block/drbd/drbd_bitmap.o
  CC [M]  drivers/block/drbd/drbd_proc.o
  CC [M]  drivers/block/xen-blkback/xenbus.o
  CC [M]  drivers/block/zram/zram_drv.o
  CC [M]  drivers/block/drbd/drbd_worker.o
  CC [M]  drivers/block/null_blk/main.o
  CC [M]  drivers/block/drbd/drbd_receiver.o
  CC [M]  drivers/block/null_blk/trace.o
  CC [M]  drivers/block/null_blk/zoned.o
  CC [M]  drivers/block/drbd/drbd_req.o
  CC [M]  drivers/block/drbd/drbd_actlog.o
  CC [M]  drivers/block/drbd/drbd_main.o
  CC [M]  drivers/block/drbd/drbd_strings.o
  CC [M]  drivers/block/drbd/drbd_nl.o
  CC [M]  drivers/block/drbd/drbd_interval.o
  CC [M]  drivers/block/drbd/drbd_state.o
  CC [M]  drivers/block/drbd/drbd_nla.o
  CC [M]  drivers/block/drbd/drbd_debugfs.o
  LD [M]  drivers/block/zram/zram.o
  LD [M]  drivers/block/null_blk/null_blk.o
  LD [M]  drivers/block/xen-blkback/xen-blkback.o
  LD [M]  drivers/block/drbd/drbd.o
  MODPOST drivers/block/Module.symvers
  CC [M]  drivers/block/floppy.mod.o
  CC [M]  drivers/block/brd.mod.o
  CC [M]  drivers/block/loop.mod.o
  CC [M]  drivers/block/nbd.mod.o
  CC [M]  drivers/block/virtio_blk.mod.o
  CC [M]  drivers/block/xen-blkfront.mod.o
  CC [M]  drivers/block/xen-blkback/xen-blkback.mod.o
  CC [M]  drivers/block/drbd/drbd.mod.o
  CC [M]  drivers/block/rbd.mod.o
  CC [M]  drivers/block/mtip32xx/mtip32xx.mod.o
  CC [M]  drivers/block/zram/zram.mod.o
  CC [M]  drivers/block/null_blk/null_blk.mod.o
  LD [M]  drivers/block/xen-blkfront.ko
  LD [M]  drivers/block/brd.ko
  LD [M]  drivers/block/xen-blkback/xen-blkback.ko
  LD [M]  drivers/block/loop.ko
  LD [M]  drivers/block/virtio_blk.ko
  LD [M]  drivers/block/mtip32xx/mtip32xx.ko
  LD [M]  drivers/block/rbd.ko
  LD [M]  drivers/block/zram/zram.ko
  LD [M]  drivers/block/nbd.ko
  LD [M]  drivers/block/drbd/drbd.ko
  LD [M]  drivers/block/floppy.ko
  LD [M]  drivers/block/null_blk/null_blk.ko
+ HOST=drivers/block/null_blk/
++ uname -r
+ HOST_DEST=/lib/modules/6.3.0-rc5lblk+/kernel/drivers/block/null_blk/
+ cp drivers/block/null_blk//null_blk.ko /lib/modules/6.3.0-rc5lblk+/kernel/drivers/block/null_blk//
+ ls -lrth /lib/modules/6.3.0-rc5lblk+/kernel/drivers/block/null_blk//null_blk.ko
-rw-r--r--. 1 root root 1.2M Apr 12 01:01 /lib/modules/6.3.0-rc5lblk+/kernel/drivers/block/null_blk//null_blk.ko
+ sleep 1
+ dmesg -c
+ test_non_configfs
+ modprobe null_blk nowait=0 queue_mode=0
+ dmesg -c
[ 9509.969343] null_blk: disk nullb0 created
[ 9509.969347] null_blk: module loaded
+ for i in 1 2 3
+ fio fio/randread.fio --filename=/dev/nullb0 --output=qmode-0-nowait-0-fio-1.log
+ for i in 1 2 3 [r(24)][100.0%][r=5415MiB/s][r=1386k IOPS][eta 00m:00s]
+ fio fio/randread.fio --filename=/dev/nullb0 --output=qmode-0-nowait-0-fio-2.log
+ for i in 1 2 3 [r(24)][100.0%][r=5184MiB/s][r=1327k IOPS][eta 00m:00s]
+ fio fio/randread.fio --filename=/dev/nullb0 --output=qmode-0-nowait-0-fio-3.log
+ modprobe -r null_blk)][100.0%][r=5525MiB/s][r=1414k IOPS][eta 00m:00s]
+ modprobe null_blk nowait=1 queue_mode=0
+ dmesg -c
[ 9570.634724] null_blk: disk nullb0 created
[ 9570.634729] null_blk: module loaded
+ for i in 1 2 3
+ fio fio/randread.fio --filename=/dev/nullb0 --output=qmode-0-nowait-1-fio-1.log
+ for i in 1 2 3 [r(24)][100.0%][r=20.8GiB/s][r=5452k IOPS][eta 00m:00s]
+ fio fio/randread.fio --filename=/dev/nullb0 --output=qmode-0-nowait-1-fio-2.log
+ for i in 1 2 3 [r(24)][100.0%][r=21.3GiB/s][r=5577k IOPS][eta 00m:00s]
+ fio fio/randread.fio --filename=/dev/nullb0 --output=qmode-0-nowait-1-fio-3.log
+ modprobe -r null_blk)][100.0%][r=21.1GiB/s][r=5536k IOPS][eta 00m:00s]
+ modprobe null_blk nowait=0 queue_mode=2
+ dmesg -c
[ 9631.320182] null_blk: disk nullb0 created
[ 9631.320186] null_blk: module loaded
+ for i in 1 2 3
+ fio fio/randread.fio --filename=/dev/nullb0 --output=qmode-2-nowait-0-fio-1.log
+ for i in 1 2 3 [r(24)][100.0%][r=6132MiB/s][r=1570k IOPS][eta 00m:00s]
+ fio fio/randread.fio --filename=/dev/nullb0 --output=qmode-2-nowait-0-fio-2.log
+ for i in 1 2 3 [r(24)][100.0%][r=5554MiB/s][r=1422k IOPS][eta 00m:00s]
+ fio fio/randread.fio --filename=/dev/nullb0 --output=qmode-2-nowait-0-fio-3.log
+ modprobe -r null_blk)][100.0%][r=5218MiB/s][r=1336k IOPS][eta 00m:00s]
+ modprobe null_blk nowait=1 queue_mode=2
+ dmesg -c
[ 9691.986330] null_blk: disk nullb0 created
[ 9691.986333] null_blk: module loaded
+ for i in 1 2 3
+ fio fio/randread.fio --filename=/dev/nullb0 --output=qmode-2-nowait-1-fio-1.log
+ for i in 1 2 3 [r(24)][100.0%][r=21.2GiB/s][r=5547k IOPS][eta 00m:00s]
+ fio fio/randread.fio --filename=/dev/nullb0 --output=qmode-2-nowait-1-fio-2.log
+ for i in 1 2 3 [r(24)][100.0%][r=20.3GiB/s][r=5313k IOPS][eta 00m:00s]
+ fio fio/randread.fio --filename=/dev/nullb0 --output=qmode-2-nowait-1-fio-3.log
+ modprobe -r null_blk)][100.0%][r=21.2GiB/s][r=5547k IOPS][eta 00m:00s]
+ test_configfs 0 0
+ qmode=0
+ nowait=0
+ modprobe null_blk nr_devices=0
+ NULLB_DIR=config/nullb/nullb0
+ mkdir config/nullb/nullb0
+ echo 1
+ echo 4096
+ echo 20480
+ echo 0
+ echo 0
+ echo 1
+ '[' 0 -ne 0 ']'
+ sleep .50
+ for i in 1 2 3
+ fio fio/randread.fio --filename=/dev/nullb0 --output=configfs-qmode-0-nowait-0-fio-1.log
+ for i in 1 2 3 [r(24)][100.0%][r=5174MiB/s][r=1325k IOPS][eta 00m:00s]
+ fio fio/randread.fio --filename=/dev/nullb0 --output=configfs-qmode-0-nowait-0-fio-2.log
+ for i in 1 2 3 [r(24)][100.0%][r=5904MiB/s][r=1512k IOPS][eta 00m:00s]
+ fio fio/randread.fio --filename=/dev/nullb0 --output=configfs-qmode-0-nowait-0-fio-3.log
+ rmdir config/nullb/nullb00.0%][r=5493MiB/s][r=1406k IOPS][eta 00m:00s]
+ modprobe -r null_blk
+ test_configfs 0 1
+ qmode=0
+ nowait=1
+ modprobe null_blk nr_devices=0
+ NULLB_DIR=config/nullb/nullb0
+ mkdir config/nullb/nullb0
+ echo 1
+ echo 4096
+ echo 20480
+ echo 0
+ echo 1
+ echo 1
+ '[' 0 -ne 0 ']'
+ sleep .50
+ for i in 1 2 3
+ fio fio/randread.fio --filename=/dev/nullb0 --output=configfs-qmode-0-nowait-1-fio-1.log
+ for i in 1 2 3 [r(24)][100.0%][r=8265MiB/s][r=2116k IOPS][eta 00m:00s]
+ fio fio/randread.fio --filename=/dev/nullb0 --output=configfs-qmode-0-nowait-1-fio-2.log
+ for i in 1 2 3 [r(24)][100.0%][r=8090MiB/s][r=2071k IOPS][eta 00m:00s]
+ fio fio/randread.fio --filename=/dev/nullb0 --output=configfs-qmode-0-nowait-1-fio-3.log
+ rmdir config/nullb/nullb00.0%][r=7971MiB/s][r=2041k IOPS][eta 00m:00s]
+ modprobe -r null_blk
+ test_configfs 2 0
+ qmode=2
+ nowait=0
+ modprobe null_blk nr_devices=0
+ NULLB_DIR=config/nullb/nullb0
+ mkdir config/nullb/nullb0
+ echo 1
+ echo 4096
+ echo 20480
+ echo 2
+ echo 0
+ echo 1
+ '[' 0 -ne 0 ']'
+ sleep .50
+ for i in 1 2 3
+ fio fio/randread.fio --filename=/dev/nullb0 --output=configfs-qmode-2-nowait-0-fio-1.log
+ for i in 1 2 3 [r(24)][100.0%][r=5247MiB/s][r=1343k IOPS][eta 00m:00s]
+ fio fio/randread.fio --filename=/dev/nullb0 --output=configfs-qmode-2-nowait-0-fio-2.log
+ for i in 1 2 3 [r(24)][100.0%][r=5132MiB/s][r=1314k IOPS][eta 00m:00s]
+ fio fio/randread.fio --filename=/dev/nullb0 --output=configfs-qmode-2-nowait-0-fio-3.log
+ rmdir config/nullb/nullb00.0%][r=4960MiB/s][r=1270k IOPS][eta 00m:00s]
+ modprobe -r null_blk
+ test_configfs 2 1
+ qmode=2
+ nowait=1
+ modprobe null_blk nr_devices=0
+ NULLB_DIR=config/nullb/nullb0
+ mkdir config/nullb/nullb0
+ echo 1
+ echo 4096
+ echo 20480
+ echo 2
+ echo 1
+ echo 1
+ '[' 0 -ne 0 ']'
+ sleep .50
+ for i in 1 2 3
+ fio fio/randread.fio --filename=/dev/nullb0 --output=configfs-qmode-2-nowait-1-fio-1.log
+ for i in 1 2 3 [r(24)][100.0%][r=8220MiB/s][r=2104k IOPS][eta 00m:00s]
+ fio fio/randread.fio --filename=/dev/nullb0 --output=configfs-qmode-2-nowait-1-fio-2.log
+ for i in 1 2 3 [r(24)][100.0%][r=8257MiB/s][r=2114k IOPS][eta 00m:00s]
+ fio fio/randread.fio --filename=/dev/nullb0 --output=configfs-qmode-2-nowait-1-fio-3.log
+ rmdir config/nullb/nullb00.0%][r=8163MiB/s][r=2090k IOPS][eta 00m:00s]
+ modprobe -r null_blk
linux-block (for-next) # 
linux-block (for-next) # 
linux-block (for-next) # 
linux-block (for-next) # 
linux-block (for-next) # grep IOPS *fio*log 
configfs-qmode-0-nowait-0-fio-1.log:  read: IOPS=1295k, BW=5058MiB/s (5303MB/s)(98.8GiB/20002msec)
configfs-qmode-0-nowait-0-fio-2.log:  read: IOPS=1362k, BW=5318MiB/s (5577MB/s)(104GiB/20002msec)
configfs-qmode-0-nowait-0-fio-3.log:  read: IOPS=1332k, BW=5201MiB/s (5454MB/s)(102GiB/20001msec)
configfs-qmode-0-nowait-1-fio-1.log:  read: IOPS=2095k, BW=8183MiB/s (8581MB/s)(160GiB/20001msec)
configfs-qmode-0-nowait-1-fio-2.log:  read: IOPS=2085k, BW=8145MiB/s (8541MB/s)(159GiB/20002msec)
configfs-qmode-0-nowait-1-fio-3.log:  read: IOPS=2036k, BW=7955MiB/s (8341MB/s)(155GiB/20001msec)
configfs-qmode-2-nowait-0-fio-1.log:  read: IOPS=1288k, BW=5031MiB/s (5275MB/s)(98.3GiB/20002msec)
configfs-qmode-2-nowait-0-fio-2.log:  read: IOPS=1239k, BW=4839MiB/s (5074MB/s)(94.5GiB/20001msec)
configfs-qmode-2-nowait-0-fio-3.log:  read: IOPS=1252k, BW=4889MiB/s (5127MB/s)(95.5GiB/20001msec)
configfs-qmode-2-nowait-1-fio-1.log:  read: IOPS=2101k, BW=8208MiB/s (8607MB/s)(160GiB/20001msec)
configfs-qmode-2-nowait-1-fio-2.log:  read: IOPS=2091k, BW=8169MiB/s (8565MB/s)(160GiB/20001msec)
configfs-qmode-2-nowait-1-fio-3.log:  read: IOPS=2088k, BW=8155MiB/s (8551MB/s)(159GiB/20001msec)
qmode-0-nowait-0-fio-1.log:  read: IOPS=1362k, BW=5321MiB/s (5579MB/s)(104GiB/20001msec)
qmode-0-nowait-0-fio-2.log:  read: IOPS=1334k, BW=5210MiB/s (5463MB/s)(102GiB/20001msec)
qmode-0-nowait-0-fio-3.log:  read: IOPS=1386k, BW=5416MiB/s (5679MB/s)(106GiB/20001msec)
qmode-0-nowait-1-fio-1.log:  read: IOPS=5405k, BW=20.6GiB/s (22.1GB/s)(412GiB/20002msec)
qmode-0-nowait-1-fio-2.log:  read: IOPS=5502k, BW=21.0GiB/s (22.5GB/s)(420GiB/20001msec)
qmode-0-nowait-1-fio-3.log:  read: IOPS=5518k, BW=21.0GiB/s (22.6GB/s)(421GiB/20002msec)
qmode-2-nowait-0-fio-1.log:  read: IOPS=1356k, BW=5296MiB/s (5554MB/s)(103GiB/20001msec)
qmode-2-nowait-0-fio-2.log:  read: IOPS=1318k, BW=5148MiB/s (5398MB/s)(101GiB/20001msec)
qmode-2-nowait-0-fio-3.log:  read: IOPS=1252k, BW=4891MiB/s (5128MB/s)(95.5GiB/20001msec)
qmode-2-nowait-1-fio-1.log:  read: IOPS=5466k, BW=20.9GiB/s (22.4GB/s)(417GiB/20001msec)
qmode-2-nowait-1-fio-2.log:  read: IOPS=5446k, BW=20.8GiB/s (22.3GB/s)(416GiB/20002msec)
qmode-2-nowait-1-fio-3.log:  read: IOPS=5482k, BW=20.9GiB/s (22.5GB/s)(418GiB/20002msec)

-- 
2.29.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/1] null_blk: allow user to set QUEUE_FLAG_NOWAIT
  2023-04-12  8:47 [PATCH 0/1] null_blk: allow user to set QUEUE_FLAG_NOWAIT Chaitanya Kulkarni
@ 2023-04-12  8:47 ` Chaitanya Kulkarni
  2023-04-12  8:50   ` Chaitanya Kulkarni
                     ` (2 more replies)
  2023-04-12  8:50 ` [PATCH 0/1] " Chaitanya Kulkarni
  1 sibling, 3 replies; 11+ messages in thread
From: Chaitanya Kulkarni @ 2023-04-12  8:47 UTC (permalink / raw)
  To: linux-block
  Cc: axboe, damien.lemoal, kch, johannes.thumshirn, bvanassche,
	vincent.fu, shinichiro.kawasaki, yukuai3

QUEUE_FLAG_NOWAIT is set by default to mq drivers such null_blk when
it is used with NULL_Q_MQ mode as a part of QUEUE_FLAG_MQ_DEFAULT that
gets assigned in following code path see blk_mq_init_allocated_queue():-

null_add_dev()
if (dev->queue_mode == NULL_Q_MQ) {
        blk_mq_alloc_disk()
          __blk_mq_alloc_disk()
	    blk_mq_init_queue_data()
              blk_mq_init_allocated_queue()
                q->queue_flags |= QUEUE_FLAG_MQ_DEFAULT;
}

But it is not set when null_blk is loaded with NULL_Q_BIO mode in following
code path like other bio drivers do e.g. nvme-multipath :-

if (dev->queue_mode == NULL_Q_BIO) {
        nullb->disk = blk_alloc_disk(nullb->dev->home_node);
        	blk_alloc_disk()
        	  blk_alloc_queue()
        	  __alloc_disk_nodw()
}

Add a new module parameter nowait and respective configfs attr that will
set or clear the QUEUE_FLAG_NOWAIT based on a value set by user in
null_add_dev() irrespective of the queue mode, by default keep it
enabled to retain the original behaviour for the NULL_Q_MQ mode.

Depending on nowait value use GFP_NOWAIT or GFP_NOIO for the alloction
in the null_alloc_page() for alloc_pages() and in null_insert_page()
for radix_tree_preload().   

Observed performance difference with this patch for fio iouring with
configfs and non configfs null_blk with queue modes NULL_Q_BIO and
NULL_Q_MQ:-

* Configfs Membacked mode:-

NULL_Q_BIO mode QUEUE_FLAG_NOWAIT disabled :-
---------------------------------------------
configfs-qmode-0-nowait-0-fio-1.log:  read: IOPS=1295k, BW=5058MiB/s
configfs-qmode-0-nowait-0-fio-2.log:  read: IOPS=1362k, BW=5318MiB/s
configfs-qmode-0-nowait-0-fio-3.log:  read: IOPS=1332k, BW=5201MiB/s

NULL_Q_BIO mode QUEUE_FLAG_NOWAIT enabled :-
---------------------------------------------
configfs-qmode-0-nowait-1-fio-1.log:  read: IOPS=2095k, BW=8183MiB/s
configfs-qmode-0-nowait-1-fio-2.log:  read: IOPS=2085k, BW=8145MiB/s
configfs-qmode-0-nowait-1-fio-3.log:  read: IOPS=2036k, BW=7955MiB/s

NULL_Q_MQ mode QUEUE_FLAG_NOWAIT disabled :-
---------------------------------------------
configfs-qmode-2-nowait-0-fio-1.log:  read: IOPS=1288k, BW=5031MiB/s
configfs-qmode-2-nowait-0-fio-2.log:  read: IOPS=1239k, BW=4839MiB/s
configfs-qmode-2-nowait-0-fio-3.log:  read: IOPS=1252k, BW=4889MiB/s

NULL_Q_MQ mode QUEUE_FLAG_NOWAIT enabled :-
---------------------------------------------
configfs-qmode-2-nowait-1-fio-1.log:  read: IOPS=2101k, BW=8208MiB/s
configfs-qmode-2-nowait-1-fio-2.log:  read: IOPS=2091k, BW=8169MiB/s
configfs-qmode-2-nowait-1-fio-3.log:  read: IOPS=2088k, BW=8155MiB/s

* Non Configfs non-membacked mode:-

NULL_Q_BIO mode QUEUE_FLAG_NOWAIT disabled :-
---------------------------------------------
qmode-0-nowait-0-fio-1.log:  read: IOPS=1362k, BW=5321MiB/s
qmode-0-nowait-0-fio-2.log:  read: IOPS=1334k, BW=5210MiB/s
qmode-0-nowait-0-fio-3.log:  read: IOPS=1386k, BW=5416MiB/s

NULL_Q_BIO mode QUEUE_FLAG_NOWAIT enabled :-
---------------------------------------------
qmode-0-nowait-1-fio-1.log:  read: IOPS=5405k, BW=20.6GiB/s
qmode-0-nowait-1-fio-2.log:  read: IOPS=5502k, BW=21.0GiB/s
qmode-0-nowait-1-fio-3.log:  read: IOPS=5518k, BW=21.0GiB/s

NULL_Q_MQ mode QUEUE_FLAG_NOWAIT disabled :-
---------------------------------------------
qmode-2-nowait-0-fio-1.log:  read: IOPS=1356k, BW=5296MiB/s
qmode-2-nowait-0-fio-2.log:  read: IOPS=1318k, BW=5148MiB/s
qmode-2-nowait-0-fio-3.log:  read: IOPS=1252k, BW=4891MiB/s

NULL_Q_MQ mode QUEUE_FLAG_NOWAIT enabled :-
---------------------------------------------
qmode-2-nowait-1-fio-1.log:  read: IOPS=5466k, BW=20.9GiB/s
qmode-2-nowait-1-fio-2.log:  read: IOPS=5446k, BW=20.8GiB/s
qmode-2-nowait-1-fio-3.log:  read: IOPS=5482k, BW=20.9GiB/s

Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
---

In caes prep patch is needed (ideally it should be) for adding
gfp args to null_alloc_page() please let me know.

-ck

 drivers/block/null_blk/main.c     | 22 +++++++++++++++++-----
 drivers/block/null_blk/null_blk.h |  1 +
 2 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
index bc2c58724df3..67683f2fd6e1 100644
--- a/drivers/block/null_blk/main.c
+++ b/drivers/block/null_blk/main.c
@@ -77,6 +77,10 @@ enum {
 	NULL_IRQ_TIMER		= 2,
 };
 
+static bool g_nowait = true;
+module_param_named(nowait, g_nowait, bool, 0444);
+MODULE_PARM_DESC(virt_boundary, "Set QUEUE_FLAG_NOWAIT irrespective of queue mode. Default: True");
+
 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");
@@ -413,6 +417,7 @@ NULLB_DEVICE_ATTR(irqmode, uint, NULL);
 NULLB_DEVICE_ATTR(hw_queue_depth, uint, NULL);
 NULLB_DEVICE_ATTR(index, uint, NULL);
 NULLB_DEVICE_ATTR(blocking, bool, NULL);
+NULLB_DEVICE_ATTR(nowait, bool, NULL);
 NULLB_DEVICE_ATTR(use_per_node_hctx, bool, NULL);
 NULLB_DEVICE_ATTR(memory_backed, bool, NULL);
 NULLB_DEVICE_ATTR(discard, bool, NULL);
@@ -554,6 +559,7 @@ static struct configfs_attribute *nullb_device_attrs[] = {
 	&nullb_device_attr_hw_queue_depth,
 	&nullb_device_attr_index,
 	&nullb_device_attr_blocking,
+	&nullb_device_attr_nowait,
 	&nullb_device_attr_use_per_node_hctx,
 	&nullb_device_attr_power,
 	&nullb_device_attr_memory_backed,
@@ -628,7 +634,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,
-			"badblocks,blocking,blocksize,cache_size,"
+			"badblocks,blocking,nowait,blocksize,cache_size,"
 			"completion_nsec,discard,home_node,hw_queue_depth,"
 			"irqmode,max_sectors,mbps,memory_backed,no_sched,"
 			"poll_queues,power,queue_mode,shared_tag_bitmap,size,"
@@ -696,6 +702,7 @@ static struct nullb_device *null_alloc_dev(void)
 	dev->irqmode = g_irqmode;
 	dev->hw_queue_depth = g_hw_queue_depth;
 	dev->blocking = g_blocking;
+	dev->nowait = g_nowait;
 	dev->memory_backed = g_memory_backed;
 	dev->discard = g_discard;
 	dev->cache_size = g_cache_size;
@@ -830,7 +837,7 @@ static void null_complete_rq(struct request *rq)
 	end_cmd(blk_mq_rq_to_pdu(rq));
 }
 
-static struct nullb_page *null_alloc_page(void)
+static struct nullb_page *null_alloc_page(gfp_t gfp)
 {
 	struct nullb_page *t_page;
 
@@ -838,7 +845,7 @@ static struct nullb_page *null_alloc_page(void)
 	if (!t_page)
 		return NULL;
 
-	t_page->page = alloc_pages(GFP_NOIO, 0);
+	t_page->page = alloc_pages(gfp, 0);
 	if (!t_page->page) {
 		kfree(t_page);
 		return NULL;
@@ -983,11 +990,11 @@ static struct nullb_page *null_insert_page(struct nullb *nullb,
 
 	spin_unlock_irq(&nullb->lock);
 
-	t_page = null_alloc_page();
+	t_page = null_alloc_page(nullb->dev->nowait ? GFP_NOWAIT : GFP_NOIO);
 	if (!t_page)
 		goto out_lock;
 
-	if (radix_tree_preload(GFP_NOIO))
+	if (radix_tree_preload(nullb->dev->nowait ? GFP_NOWAIT : GFP_NOIO))
 		goto out_freepage;
 
 	spin_lock_irq(&nullb->lock);
@@ -2093,6 +2100,11 @@ static int null_add_dev(struct nullb_device *dev)
 	blk_queue_flag_set(QUEUE_FLAG_NONROT, nullb->q);
 	blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM, nullb->q);
 
+	if (dev->nowait)
+		blk_queue_flag_set(QUEUE_FLAG_NOWAIT, nullb->q);
+	else
+		blk_queue_flag_clear(QUEUE_FLAG_NOWAIT, nullb->q);
+
 	mutex_lock(&lock);
 	rv = ida_simple_get(&nullb_indexes, 0, 0, GFP_KERNEL);
 	if (rv < 0) {
diff --git a/drivers/block/null_blk/null_blk.h b/drivers/block/null_blk/null_blk.h
index eb5972c50be8..1d7af446d728 100644
--- a/drivers/block/null_blk/null_blk.h
+++ b/drivers/block/null_blk/null_blk.h
@@ -107,6 +107,7 @@ struct nullb_device {
 	unsigned int index; /* index of the disk, only valid with a disk */
 	unsigned int mbps; /* Bandwidth throttle cap (in MB/s) */
 	bool blocking; /* blocking blk-mq device */
+	bool nowait; /* to set QUEUE_FLAG_NOWAIT on device queue */
 	bool use_per_node_hctx; /* use per-node allocation for hardware context */
 	bool power; /* power on/off the device */
 	bool memory_backed; /* if data is stored in memory */
-- 
2.29.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/1] null_blk: allow user to set QUEUE_FLAG_NOWAIT
  2023-04-12  8:47 [PATCH 0/1] null_blk: allow user to set QUEUE_FLAG_NOWAIT Chaitanya Kulkarni
  2023-04-12  8:47 ` [PATCH 1/1] " Chaitanya Kulkarni
@ 2023-04-12  8:50 ` Chaitanya Kulkarni
  2023-04-12  9:34   ` Damien Le Moal
  1 sibling, 1 reply; 11+ messages in thread
From: Chaitanya Kulkarni @ 2023-04-12  8:50 UTC (permalink / raw)
  To: Chaitanya Kulkarni, linux-block@vger.kernel.org
  Cc: axboe@kernel.dk, damien.lemoal@opensource.wdc.com,
	johannes.thumshirn@wdc.com, bvanassche@acm.org,
	vincent.fu@samsung.com, shinichiro.kawasaki@wdc.com,
	yukuai3@huawei.com, dlemoal@kenrel.org

+ dlemoal@kenrel.org.


On 4/12/23 01:47, Chaitanya Kulkarni wrote:
> Hi,
>
> QUEUE_FLAG_NOWAIT is set by default to mq drivers such as null_blk as
> a part of QUEUE_FLAG_MQ_DEFAULT that gets assigned in following code
> path see blk_mq_init_allocated_queue():-
>
> null_add_dev()
> if (dev->queue_mode == NULL_Q_MQ) {
>          blk_mq_alloc_disk()
>            __blk_mq_alloc_disk()
> 	    blk_mq_init_queue_data()
>                blk_mq_init_allocated_queue()
>                  q->queue_flags |= QUEUE_FLAG_MQ_DEFAULT;
> }
>
> But it not set when null_blk is loaded with NULL_Q_BIO mode in following
> code path :-
>
> if (dev->queue_mode == NULL_Q_BIO) {
>          nullb->disk = blk_alloc_disk(nullb->dev->home_node);
>          	blk_alloc_disk()
>          	  blk_alloc_queue()
>          	  __alloc_disk_nodw()
> }
>
> Add a new module parameter nowait and respective configfs attr
> that will set and clear the QUEUE_FLAG_NOWAIT based on a value set by
> user in null_add_dev() irrespective of the queue mode, by default keep
> it enabled to retain the original behaviour for the NULL_Q_MQ mode.
>
> * Configfs Membacked mode:-
>
> NULL_Q_BIO mode QUEUE_FLAG_NOWAIT disabled:-
> ---------------------------------------------
> configfs-qmode-0-nowait-0-fio-1.log:  read: IOPS=1295k, BW=5058MiB/s
> configfs-qmode-0-nowait-0-fio-2.log:  read: IOPS=1362k, BW=5318MiB/s
> configfs-qmode-0-nowait-0-fio-3.log:  read: IOPS=1332k, BW=5201MiB/s
>
> NULL_Q_BIO mode QUEUE_FLAG_NOWAIT enabled :-
> ---------------------------------------------
> configfs-qmode-0-nowait-1-fio-1.log:  read: IOPS=2095k, BW=8183MiB/s
> configfs-qmode-0-nowait-1-fio-2.log:  read: IOPS=2085k, BW=8145MiB/s
> configfs-qmode-0-nowait-1-fio-3.log:  read: IOPS=2036k, BW=7955MiB/s
>
> NULL_Q_MQ mode QUEUE_FLAG_NOWAIT disabled :-
> ---------------------------------------------
> configfs-qmode-2-nowait-0-fio-1.log:  read: IOPS=1288k, BW=5031MiB/s
> configfs-qmode-2-nowait-0-fio-2.log:  read: IOPS=1239k, BW=4839MiB/s
> configfs-qmode-2-nowait-0-fio-3.log:  read: IOPS=1252k, BW=4889MiB/s
>
> NULL_Q_MQ mode QUEUE_FLAG_NOWAIT enabled :-
> ---------------------------------------------
> configfs-qmode-2-nowait-1-fio-1.log:  read: IOPS=2101k, BW=8208MiB/s
> configfs-qmode-2-nowait-1-fio-2.log:  read: IOPS=2091k, BW=8169MiB/s
> configfs-qmode-2-nowait-1-fio-3.log:  read: IOPS=2088k, BW=8155MiB/s
>
> * Non Configfs non-membacked mode:-
>
> NULL_Q_BIO mode QUEUE_FLAG_NOWAIT disabled:-
> ---------------------------------------------
> qmode-0-nowait-0-fio-1.log:  read: IOPS=1362k, BW=5321MiB/s
> qmode-0-nowait-0-fio-2.log:  read: IOPS=1334k, BW=5210MiB/s
> qmode-0-nowait-0-fio-3.log:  read: IOPS=1386k, BW=5416MiB/s
>
> NULL_Q_BIO mode QUEUE_FLAG_NOWAIT enabled :-
> ---------------------------------------------
> qmode-0-nowait-1-fio-1.log:  read: IOPS=5405k, BW=20.6GiB/s
> qmode-0-nowait-1-fio-2.log:  read: IOPS=5502k, BW=21.0GiB/s
> qmode-0-nowait-1-fio-3.log:  read: IOPS=5518k, BW=21.0GiB/s
>
> NULL_Q_MQ mode QUEUE_FLAG_NOWAIT disabled :-
> ---------------------------------------------
> qmode-2-nowait-0-fio-1.log:  read: IOPS=1356k, BW=5296MiB/s
> qmode-2-nowait-0-fio-2.log:  read: IOPS=1318k, BW=5148MiB/s
> qmode-2-nowait-0-fio-3.log:  read: IOPS=1252k, BW=4891MiB/s
>
> NULL_Q_MQ mode QUEUE_FLAG_NOWAIT enabled :-
> ---------------------------------------------
> qmode-2-nowait-1-fio-1.log:  read: IOPS=5466k, BW=20.9GiB/s
> qmode-2-nowait-1-fio-2.log:  read: IOPS=5446k, BW=20.8GiB/s
> qmode-2-nowait-1-fio-3.log:  read: IOPS=5482k, BW=20.9GiB/s
>
> Below is a performance test log for with and without configfs mambeacked
> mode.
>
> -ck
>
> Chaitanya Kulkarni (1):
>    null_blk: allow user to set QUEUE_FLAG_NOWAIT
>
>   drivers/block/null_blk/main.c     | 22 +++++++++++++++++-----
>   drivers/block/null_blk/null_blk.h |  1 +
>   2 files changed, 18 insertions(+), 5 deletions(-)
>
> Performance test log for null_blk configfs and non-configfs:-
>
> linux-block (for-next) # sh test-null-blk-no-wait.sh
> + modprobe -r null_blk
> + ./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/ clean
>    CLEAN   drivers/block/Module.symvers
> ++ nproc
> + make -j 48 M=drivers/block modules
>    CC [M]  drivers/block/floppy.o
>    CC [M]  drivers/block/brd.o
>    CC [M]  drivers/block/loop.o
>    CC [M]  drivers/block/nbd.o
>    CC [M]  drivers/block/virtio_blk.o
>    CC [M]  drivers/block/xen-blkfront.o
>    CC [M]  drivers/block/rbd.o
>    CC [M]  drivers/block/drbd/drbd_buildtag.o
>    CC [M]  drivers/block/xen-blkback/blkback.o
>    CC [M]  drivers/block/mtip32xx/mtip32xx.o
>    CC [M]  drivers/block/zram/zcomp.o
>    CC [M]  drivers/block/drbd/drbd_bitmap.o
>    CC [M]  drivers/block/drbd/drbd_proc.o
>    CC [M]  drivers/block/xen-blkback/xenbus.o
>    CC [M]  drivers/block/zram/zram_drv.o
>    CC [M]  drivers/block/drbd/drbd_worker.o
>    CC [M]  drivers/block/null_blk/main.o
>    CC [M]  drivers/block/drbd/drbd_receiver.o
>    CC [M]  drivers/block/null_blk/trace.o
>    CC [M]  drivers/block/null_blk/zoned.o
>    CC [M]  drivers/block/drbd/drbd_req.o
>    CC [M]  drivers/block/drbd/drbd_actlog.o
>    CC [M]  drivers/block/drbd/drbd_main.o
>    CC [M]  drivers/block/drbd/drbd_strings.o
>    CC [M]  drivers/block/drbd/drbd_nl.o
>    CC [M]  drivers/block/drbd/drbd_interval.o
>    CC [M]  drivers/block/drbd/drbd_state.o
>    CC [M]  drivers/block/drbd/drbd_nla.o
>    CC [M]  drivers/block/drbd/drbd_debugfs.o
>    LD [M]  drivers/block/zram/zram.o
>    LD [M]  drivers/block/null_blk/null_blk.o
>    LD [M]  drivers/block/xen-blkback/xen-blkback.o
>    LD [M]  drivers/block/drbd/drbd.o
>    MODPOST drivers/block/Module.symvers
>    CC [M]  drivers/block/floppy.mod.o
>    CC [M]  drivers/block/brd.mod.o
>    CC [M]  drivers/block/loop.mod.o
>    CC [M]  drivers/block/nbd.mod.o
>    CC [M]  drivers/block/virtio_blk.mod.o
>    CC [M]  drivers/block/xen-blkfront.mod.o
>    CC [M]  drivers/block/xen-blkback/xen-blkback.mod.o
>    CC [M]  drivers/block/drbd/drbd.mod.o
>    CC [M]  drivers/block/rbd.mod.o
>    CC [M]  drivers/block/mtip32xx/mtip32xx.mod.o
>    CC [M]  drivers/block/zram/zram.mod.o
>    CC [M]  drivers/block/null_blk/null_blk.mod.o
>    LD [M]  drivers/block/xen-blkfront.ko
>    LD [M]  drivers/block/brd.ko
>    LD [M]  drivers/block/xen-blkback/xen-blkback.ko
>    LD [M]  drivers/block/loop.ko
>    LD [M]  drivers/block/virtio_blk.ko
>    LD [M]  drivers/block/mtip32xx/mtip32xx.ko
>    LD [M]  drivers/block/rbd.ko
>    LD [M]  drivers/block/zram/zram.ko
>    LD [M]  drivers/block/nbd.ko
>    LD [M]  drivers/block/drbd/drbd.ko
>    LD [M]  drivers/block/floppy.ko
>    LD [M]  drivers/block/null_blk/null_blk.ko
> + HOST=drivers/block/null_blk/
> ++ uname -r
> + HOST_DEST=/lib/modules/6.3.0-rc5lblk+/kernel/drivers/block/null_blk/
> + cp drivers/block/null_blk//null_blk.ko /lib/modules/6.3.0-rc5lblk+/kernel/drivers/block/null_blk//
> + ls -lrth /lib/modules/6.3.0-rc5lblk+/kernel/drivers/block/null_blk//null_blk.ko
> -rw-r--r--. 1 root root 1.2M Apr 12 01:01 /lib/modules/6.3.0-rc5lblk+/kernel/drivers/block/null_blk//null_blk.ko
> + sleep 1
> + dmesg -c
> + test_non_configfs
> + modprobe null_blk nowait=0 queue_mode=0
> + dmesg -c
> [ 9509.969343] null_blk: disk nullb0 created
> [ 9509.969347] null_blk: module loaded
> + for i in 1 2 3
> + fio fio/randread.fio --filename=/dev/nullb0 --output=qmode-0-nowait-0-fio-1.log
> + for i in 1 2 3 [r(24)][100.0%][r=5415MiB/s][r=1386k IOPS][eta 00m:00s]
> + fio fio/randread.fio --filename=/dev/nullb0 --output=qmode-0-nowait-0-fio-2.log
> + for i in 1 2 3 [r(24)][100.0%][r=5184MiB/s][r=1327k IOPS][eta 00m:00s]
> + fio fio/randread.fio --filename=/dev/nullb0 --output=qmode-0-nowait-0-fio-3.log
> + modprobe -r null_blk)][100.0%][r=5525MiB/s][r=1414k IOPS][eta 00m:00s]
> + modprobe null_blk nowait=1 queue_mode=0
> + dmesg -c
> [ 9570.634724] null_blk: disk nullb0 created
> [ 9570.634729] null_blk: module loaded
> + for i in 1 2 3
> + fio fio/randread.fio --filename=/dev/nullb0 --output=qmode-0-nowait-1-fio-1.log
> + for i in 1 2 3 [r(24)][100.0%][r=20.8GiB/s][r=5452k IOPS][eta 00m:00s]
> + fio fio/randread.fio --filename=/dev/nullb0 --output=qmode-0-nowait-1-fio-2.log
> + for i in 1 2 3 [r(24)][100.0%][r=21.3GiB/s][r=5577k IOPS][eta 00m:00s]
> + fio fio/randread.fio --filename=/dev/nullb0 --output=qmode-0-nowait-1-fio-3.log
> + modprobe -r null_blk)][100.0%][r=21.1GiB/s][r=5536k IOPS][eta 00m:00s]
> + modprobe null_blk nowait=0 queue_mode=2
> + dmesg -c
> [ 9631.320182] null_blk: disk nullb0 created
> [ 9631.320186] null_blk: module loaded
> + for i in 1 2 3
> + fio fio/randread.fio --filename=/dev/nullb0 --output=qmode-2-nowait-0-fio-1.log
> + for i in 1 2 3 [r(24)][100.0%][r=6132MiB/s][r=1570k IOPS][eta 00m:00s]
> + fio fio/randread.fio --filename=/dev/nullb0 --output=qmode-2-nowait-0-fio-2.log
> + for i in 1 2 3 [r(24)][100.0%][r=5554MiB/s][r=1422k IOPS][eta 00m:00s]
> + fio fio/randread.fio --filename=/dev/nullb0 --output=qmode-2-nowait-0-fio-3.log
> + modprobe -r null_blk)][100.0%][r=5218MiB/s][r=1336k IOPS][eta 00m:00s]
> + modprobe null_blk nowait=1 queue_mode=2
> + dmesg -c
> [ 9691.986330] null_blk: disk nullb0 created
> [ 9691.986333] null_blk: module loaded
> + for i in 1 2 3
> + fio fio/randread.fio --filename=/dev/nullb0 --output=qmode-2-nowait-1-fio-1.log
> + for i in 1 2 3 [r(24)][100.0%][r=21.2GiB/s][r=5547k IOPS][eta 00m:00s]
> + fio fio/randread.fio --filename=/dev/nullb0 --output=qmode-2-nowait-1-fio-2.log
> + for i in 1 2 3 [r(24)][100.0%][r=20.3GiB/s][r=5313k IOPS][eta 00m:00s]
> + fio fio/randread.fio --filename=/dev/nullb0 --output=qmode-2-nowait-1-fio-3.log
> + modprobe -r null_blk)][100.0%][r=21.2GiB/s][r=5547k IOPS][eta 00m:00s]
> + test_configfs 0 0
> + qmode=0
> + nowait=0
> + modprobe null_blk nr_devices=0
> + NULLB_DIR=config/nullb/nullb0
> + mkdir config/nullb/nullb0
> + echo 1
> + echo 4096
> + echo 20480
> + echo 0
> + echo 0
> + echo 1
> + '[' 0 -ne 0 ']'
> + sleep .50
> + for i in 1 2 3
> + fio fio/randread.fio --filename=/dev/nullb0 --output=configfs-qmode-0-nowait-0-fio-1.log
> + for i in 1 2 3 [r(24)][100.0%][r=5174MiB/s][r=1325k IOPS][eta 00m:00s]
> + fio fio/randread.fio --filename=/dev/nullb0 --output=configfs-qmode-0-nowait-0-fio-2.log
> + for i in 1 2 3 [r(24)][100.0%][r=5904MiB/s][r=1512k IOPS][eta 00m:00s]
> + fio fio/randread.fio --filename=/dev/nullb0 --output=configfs-qmode-0-nowait-0-fio-3.log
> + rmdir config/nullb/nullb00.0%][r=5493MiB/s][r=1406k IOPS][eta 00m:00s]
> + modprobe -r null_blk
> + test_configfs 0 1
> + qmode=0
> + nowait=1
> + modprobe null_blk nr_devices=0
> + NULLB_DIR=config/nullb/nullb0
> + mkdir config/nullb/nullb0
> + echo 1
> + echo 4096
> + echo 20480
> + echo 0
> + echo 1
> + echo 1
> + '[' 0 -ne 0 ']'
> + sleep .50
> + for i in 1 2 3
> + fio fio/randread.fio --filename=/dev/nullb0 --output=configfs-qmode-0-nowait-1-fio-1.log
> + for i in 1 2 3 [r(24)][100.0%][r=8265MiB/s][r=2116k IOPS][eta 00m:00s]
> + fio fio/randread.fio --filename=/dev/nullb0 --output=configfs-qmode-0-nowait-1-fio-2.log
> + for i in 1 2 3 [r(24)][100.0%][r=8090MiB/s][r=2071k IOPS][eta 00m:00s]
> + fio fio/randread.fio --filename=/dev/nullb0 --output=configfs-qmode-0-nowait-1-fio-3.log
> + rmdir config/nullb/nullb00.0%][r=7971MiB/s][r=2041k IOPS][eta 00m:00s]
> + modprobe -r null_blk
> + test_configfs 2 0
> + qmode=2
> + nowait=0
> + modprobe null_blk nr_devices=0
> + NULLB_DIR=config/nullb/nullb0
> + mkdir config/nullb/nullb0
> + echo 1
> + echo 4096
> + echo 20480
> + echo 2
> + echo 0
> + echo 1
> + '[' 0 -ne 0 ']'
> + sleep .50
> + for i in 1 2 3
> + fio fio/randread.fio --filename=/dev/nullb0 --output=configfs-qmode-2-nowait-0-fio-1.log
> + for i in 1 2 3 [r(24)][100.0%][r=5247MiB/s][r=1343k IOPS][eta 00m:00s]
> + fio fio/randread.fio --filename=/dev/nullb0 --output=configfs-qmode-2-nowait-0-fio-2.log
> + for i in 1 2 3 [r(24)][100.0%][r=5132MiB/s][r=1314k IOPS][eta 00m:00s]
> + fio fio/randread.fio --filename=/dev/nullb0 --output=configfs-qmode-2-nowait-0-fio-3.log
> + rmdir config/nullb/nullb00.0%][r=4960MiB/s][r=1270k IOPS][eta 00m:00s]
> + modprobe -r null_blk
> + test_configfs 2 1
> + qmode=2
> + nowait=1
> + modprobe null_blk nr_devices=0
> + NULLB_DIR=config/nullb/nullb0
> + mkdir config/nullb/nullb0
> + echo 1
> + echo 4096
> + echo 20480
> + echo 2
> + echo 1
> + echo 1
> + '[' 0 -ne 0 ']'
> + sleep .50
> + for i in 1 2 3
> + fio fio/randread.fio --filename=/dev/nullb0 --output=configfs-qmode-2-nowait-1-fio-1.log
> + for i in 1 2 3 [r(24)][100.0%][r=8220MiB/s][r=2104k IOPS][eta 00m:00s]
> + fio fio/randread.fio --filename=/dev/nullb0 --output=configfs-qmode-2-nowait-1-fio-2.log
> + for i in 1 2 3 [r(24)][100.0%][r=8257MiB/s][r=2114k IOPS][eta 00m:00s]
> + fio fio/randread.fio --filename=/dev/nullb0 --output=configfs-qmode-2-nowait-1-fio-3.log
> + rmdir config/nullb/nullb00.0%][r=8163MiB/s][r=2090k IOPS][eta 00m:00s]
> + modprobe -r null_blk
> linux-block (for-next) #
> linux-block (for-next) #
> linux-block (for-next) #
> linux-block (for-next) #
> linux-block (for-next) # grep IOPS *fio*log
> configfs-qmode-0-nowait-0-fio-1.log:  read: IOPS=1295k, BW=5058MiB/s (5303MB/s)(98.8GiB/20002msec)
> configfs-qmode-0-nowait-0-fio-2.log:  read: IOPS=1362k, BW=5318MiB/s (5577MB/s)(104GiB/20002msec)
> configfs-qmode-0-nowait-0-fio-3.log:  read: IOPS=1332k, BW=5201MiB/s (5454MB/s)(102GiB/20001msec)
> configfs-qmode-0-nowait-1-fio-1.log:  read: IOPS=2095k, BW=8183MiB/s (8581MB/s)(160GiB/20001msec)
> configfs-qmode-0-nowait-1-fio-2.log:  read: IOPS=2085k, BW=8145MiB/s (8541MB/s)(159GiB/20002msec)
> configfs-qmode-0-nowait-1-fio-3.log:  read: IOPS=2036k, BW=7955MiB/s (8341MB/s)(155GiB/20001msec)
> configfs-qmode-2-nowait-0-fio-1.log:  read: IOPS=1288k, BW=5031MiB/s (5275MB/s)(98.3GiB/20002msec)
> configfs-qmode-2-nowait-0-fio-2.log:  read: IOPS=1239k, BW=4839MiB/s (5074MB/s)(94.5GiB/20001msec)
> configfs-qmode-2-nowait-0-fio-3.log:  read: IOPS=1252k, BW=4889MiB/s (5127MB/s)(95.5GiB/20001msec)
> configfs-qmode-2-nowait-1-fio-1.log:  read: IOPS=2101k, BW=8208MiB/s (8607MB/s)(160GiB/20001msec)
> configfs-qmode-2-nowait-1-fio-2.log:  read: IOPS=2091k, BW=8169MiB/s (8565MB/s)(160GiB/20001msec)
> configfs-qmode-2-nowait-1-fio-3.log:  read: IOPS=2088k, BW=8155MiB/s (8551MB/s)(159GiB/20001msec)
> qmode-0-nowait-0-fio-1.log:  read: IOPS=1362k, BW=5321MiB/s (5579MB/s)(104GiB/20001msec)
> qmode-0-nowait-0-fio-2.log:  read: IOPS=1334k, BW=5210MiB/s (5463MB/s)(102GiB/20001msec)
> qmode-0-nowait-0-fio-3.log:  read: IOPS=1386k, BW=5416MiB/s (5679MB/s)(106GiB/20001msec)
> qmode-0-nowait-1-fio-1.log:  read: IOPS=5405k, BW=20.6GiB/s (22.1GB/s)(412GiB/20002msec)
> qmode-0-nowait-1-fio-2.log:  read: IOPS=5502k, BW=21.0GiB/s (22.5GB/s)(420GiB/20001msec)
> qmode-0-nowait-1-fio-3.log:  read: IOPS=5518k, BW=21.0GiB/s (22.6GB/s)(421GiB/20002msec)
> qmode-2-nowait-0-fio-1.log:  read: IOPS=1356k, BW=5296MiB/s (5554MB/s)(103GiB/20001msec)
> qmode-2-nowait-0-fio-2.log:  read: IOPS=1318k, BW=5148MiB/s (5398MB/s)(101GiB/20001msec)
> qmode-2-nowait-0-fio-3.log:  read: IOPS=1252k, BW=4891MiB/s (5128MB/s)(95.5GiB/20001msec)
> qmode-2-nowait-1-fio-1.log:  read: IOPS=5466k, BW=20.9GiB/s (22.4GB/s)(417GiB/20001msec)
> qmode-2-nowait-1-fio-2.log:  read: IOPS=5446k, BW=20.8GiB/s (22.3GB/s)(416GiB/20002msec)
> qmode-2-nowait-1-fio-3.log:  read: IOPS=5482k, BW=20.9GiB/s (22.5GB/s)(418GiB/20002msec)
>


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] null_blk: allow user to set QUEUE_FLAG_NOWAIT
  2023-04-12  8:47 ` [PATCH 1/1] " Chaitanya Kulkarni
@ 2023-04-12  8:50   ` Chaitanya Kulkarni
  2023-04-12  9:41   ` Damien Le Moal
  2023-04-18 11:41   ` Pankaj Raghav
  2 siblings, 0 replies; 11+ messages in thread
From: Chaitanya Kulkarni @ 2023-04-12  8:50 UTC (permalink / raw)
  To: Chaitanya Kulkarni, linux-block@vger.kernel.org
  Cc: axboe@kernel.dk, damien.lemoal@opensource.wdc.com,
	johannes.thumshirn@wdc.com, bvanassche@acm.org,
	vincent.fu@samsung.com, shinichiro.kawasaki@wdc.com,
	yukuai3@huawei.com, dlemoal@kenrel.org

+ dlemoal@kenrel.org.

On 4/12/23 01:47, Chaitanya Kulkarni wrote:
> QUEUE_FLAG_NOWAIT is set by default to mq drivers such null_blk when
> it is used with NULL_Q_MQ mode as a part of QUEUE_FLAG_MQ_DEFAULT that
> gets assigned in following code path see blk_mq_init_allocated_queue():-
>
> null_add_dev()
> if (dev->queue_mode == NULL_Q_MQ) {
>          blk_mq_alloc_disk()
>            __blk_mq_alloc_disk()
> 	    blk_mq_init_queue_data()
>                blk_mq_init_allocated_queue()
>                  q->queue_flags |= QUEUE_FLAG_MQ_DEFAULT;
> }
>
> But it is not set when null_blk is loaded with NULL_Q_BIO mode in following
> code path like other bio drivers do e.g. nvme-multipath :-
>
> if (dev->queue_mode == NULL_Q_BIO) {
>          nullb->disk = blk_alloc_disk(nullb->dev->home_node);
>          	blk_alloc_disk()
>          	  blk_alloc_queue()
>          	  __alloc_disk_nodw()
> }
>
> Add a new module parameter nowait and respective configfs attr that will
> set or clear the QUEUE_FLAG_NOWAIT based on a value set by user in
> null_add_dev() irrespective of the queue mode, by default keep it
> enabled to retain the original behaviour for the NULL_Q_MQ mode.
>
> Depending on nowait value use GFP_NOWAIT or GFP_NOIO for the alloction
> in the null_alloc_page() for alloc_pages() and in null_insert_page()
> for radix_tree_preload().
>
> Observed performance difference with this patch for fio iouring with
> configfs and non configfs null_blk with queue modes NULL_Q_BIO and
> NULL_Q_MQ:-
>
> * Configfs Membacked mode:-
>
> NULL_Q_BIO mode QUEUE_FLAG_NOWAIT disabled :-
> ---------------------------------------------
> configfs-qmode-0-nowait-0-fio-1.log:  read: IOPS=1295k, BW=5058MiB/s
> configfs-qmode-0-nowait-0-fio-2.log:  read: IOPS=1362k, BW=5318MiB/s
> configfs-qmode-0-nowait-0-fio-3.log:  read: IOPS=1332k, BW=5201MiB/s
>
> NULL_Q_BIO mode QUEUE_FLAG_NOWAIT enabled :-
> ---------------------------------------------
> configfs-qmode-0-nowait-1-fio-1.log:  read: IOPS=2095k, BW=8183MiB/s
> configfs-qmode-0-nowait-1-fio-2.log:  read: IOPS=2085k, BW=8145MiB/s
> configfs-qmode-0-nowait-1-fio-3.log:  read: IOPS=2036k, BW=7955MiB/s
>
> NULL_Q_MQ mode QUEUE_FLAG_NOWAIT disabled :-
> ---------------------------------------------
> configfs-qmode-2-nowait-0-fio-1.log:  read: IOPS=1288k, BW=5031MiB/s
> configfs-qmode-2-nowait-0-fio-2.log:  read: IOPS=1239k, BW=4839MiB/s
> configfs-qmode-2-nowait-0-fio-3.log:  read: IOPS=1252k, BW=4889MiB/s
>
> NULL_Q_MQ mode QUEUE_FLAG_NOWAIT enabled :-
> ---------------------------------------------
> configfs-qmode-2-nowait-1-fio-1.log:  read: IOPS=2101k, BW=8208MiB/s
> configfs-qmode-2-nowait-1-fio-2.log:  read: IOPS=2091k, BW=8169MiB/s
> configfs-qmode-2-nowait-1-fio-3.log:  read: IOPS=2088k, BW=8155MiB/s
>
> * Non Configfs non-membacked mode:-
>
> NULL_Q_BIO mode QUEUE_FLAG_NOWAIT disabled :-
> ---------------------------------------------
> qmode-0-nowait-0-fio-1.log:  read: IOPS=1362k, BW=5321MiB/s
> qmode-0-nowait-0-fio-2.log:  read: IOPS=1334k, BW=5210MiB/s
> qmode-0-nowait-0-fio-3.log:  read: IOPS=1386k, BW=5416MiB/s
>
> NULL_Q_BIO mode QUEUE_FLAG_NOWAIT enabled :-
> ---------------------------------------------
> qmode-0-nowait-1-fio-1.log:  read: IOPS=5405k, BW=20.6GiB/s
> qmode-0-nowait-1-fio-2.log:  read: IOPS=5502k, BW=21.0GiB/s
> qmode-0-nowait-1-fio-3.log:  read: IOPS=5518k, BW=21.0GiB/s
>
> NULL_Q_MQ mode QUEUE_FLAG_NOWAIT disabled :-
> ---------------------------------------------
> qmode-2-nowait-0-fio-1.log:  read: IOPS=1356k, BW=5296MiB/s
> qmode-2-nowait-0-fio-2.log:  read: IOPS=1318k, BW=5148MiB/s
> qmode-2-nowait-0-fio-3.log:  read: IOPS=1252k, BW=4891MiB/s
>
> NULL_Q_MQ mode QUEUE_FLAG_NOWAIT enabled :-
> ---------------------------------------------
> qmode-2-nowait-1-fio-1.log:  read: IOPS=5466k, BW=20.9GiB/s
> qmode-2-nowait-1-fio-2.log:  read: IOPS=5446k, BW=20.8GiB/s
> qmode-2-nowait-1-fio-3.log:  read: IOPS=5482k, BW=20.9GiB/s
>
> Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
> ---
>
> In caes prep patch is needed (ideally it should be) for adding
> gfp args to null_alloc_page() please let me know.
>
> -ck
>
>   drivers/block/null_blk/main.c     | 22 +++++++++++++++++-----
>   drivers/block/null_blk/null_blk.h |  1 +
>   2 files changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c
> index bc2c58724df3..67683f2fd6e1 100644
> --- a/drivers/block/null_blk/main.c
> +++ b/drivers/block/null_blk/main.c
> @@ -77,6 +77,10 @@ enum {
>   	NULL_IRQ_TIMER		= 2,
>   };
>   
> +static bool g_nowait = true;
> +module_param_named(nowait, g_nowait, bool, 0444);
> +MODULE_PARM_DESC(virt_boundary, "Set QUEUE_FLAG_NOWAIT irrespective of queue mode. Default: True");
> +
>   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");
> @@ -413,6 +417,7 @@ NULLB_DEVICE_ATTR(irqmode, uint, NULL);
>   NULLB_DEVICE_ATTR(hw_queue_depth, uint, NULL);
>   NULLB_DEVICE_ATTR(index, uint, NULL);
>   NULLB_DEVICE_ATTR(blocking, bool, NULL);
> +NULLB_DEVICE_ATTR(nowait, bool, NULL);
>   NULLB_DEVICE_ATTR(use_per_node_hctx, bool, NULL);
>   NULLB_DEVICE_ATTR(memory_backed, bool, NULL);
>   NULLB_DEVICE_ATTR(discard, bool, NULL);
> @@ -554,6 +559,7 @@ static struct configfs_attribute *nullb_device_attrs[] = {
>   	&nullb_device_attr_hw_queue_depth,
>   	&nullb_device_attr_index,
>   	&nullb_device_attr_blocking,
> +	&nullb_device_attr_nowait,
>   	&nullb_device_attr_use_per_node_hctx,
>   	&nullb_device_attr_power,
>   	&nullb_device_attr_memory_backed,
> @@ -628,7 +634,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,
> -			"badblocks,blocking,blocksize,cache_size,"
> +			"badblocks,blocking,nowait,blocksize,cache_size,"
>   			"completion_nsec,discard,home_node,hw_queue_depth,"
>   			"irqmode,max_sectors,mbps,memory_backed,no_sched,"
>   			"poll_queues,power,queue_mode,shared_tag_bitmap,size,"
> @@ -696,6 +702,7 @@ static struct nullb_device *null_alloc_dev(void)
>   	dev->irqmode = g_irqmode;
>   	dev->hw_queue_depth = g_hw_queue_depth;
>   	dev->blocking = g_blocking;
> +	dev->nowait = g_nowait;
>   	dev->memory_backed = g_memory_backed;
>   	dev->discard = g_discard;
>   	dev->cache_size = g_cache_size;
> @@ -830,7 +837,7 @@ static void null_complete_rq(struct request *rq)
>   	end_cmd(blk_mq_rq_to_pdu(rq));
>   }
>   
> -static struct nullb_page *null_alloc_page(void)
> +static struct nullb_page *null_alloc_page(gfp_t gfp)
>   {
>   	struct nullb_page *t_page;
>   
> @@ -838,7 +845,7 @@ static struct nullb_page *null_alloc_page(void)
>   	if (!t_page)
>   		return NULL;
>   
> -	t_page->page = alloc_pages(GFP_NOIO, 0);
> +	t_page->page = alloc_pages(gfp, 0);
>   	if (!t_page->page) {
>   		kfree(t_page);
>   		return NULL;
> @@ -983,11 +990,11 @@ static struct nullb_page *null_insert_page(struct nullb *nullb,
>   
>   	spin_unlock_irq(&nullb->lock);
>   
> -	t_page = null_alloc_page();
> +	t_page = null_alloc_page(nullb->dev->nowait ? GFP_NOWAIT : GFP_NOIO);
>   	if (!t_page)
>   		goto out_lock;
>   
> -	if (radix_tree_preload(GFP_NOIO))
> +	if (radix_tree_preload(nullb->dev->nowait ? GFP_NOWAIT : GFP_NOIO))
>   		goto out_freepage;
>   
>   	spin_lock_irq(&nullb->lock);
> @@ -2093,6 +2100,11 @@ static int null_add_dev(struct nullb_device *dev)
>   	blk_queue_flag_set(QUEUE_FLAG_NONROT, nullb->q);
>   	blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM, nullb->q);
>   
> +	if (dev->nowait)
> +		blk_queue_flag_set(QUEUE_FLAG_NOWAIT, nullb->q);
> +	else
> +		blk_queue_flag_clear(QUEUE_FLAG_NOWAIT, nullb->q);
> +
>   	mutex_lock(&lock);
>   	rv = ida_simple_get(&nullb_indexes, 0, 0, GFP_KERNEL);
>   	if (rv < 0) {
> diff --git a/drivers/block/null_blk/null_blk.h b/drivers/block/null_blk/null_blk.h
> index eb5972c50be8..1d7af446d728 100644
> --- a/drivers/block/null_blk/null_blk.h
> +++ b/drivers/block/null_blk/null_blk.h
> @@ -107,6 +107,7 @@ struct nullb_device {
>   	unsigned int index; /* index of the disk, only valid with a disk */
>   	unsigned int mbps; /* Bandwidth throttle cap (in MB/s) */
>   	bool blocking; /* blocking blk-mq device */
> +	bool nowait; /* to set QUEUE_FLAG_NOWAIT on device queue */
>   	bool use_per_node_hctx; /* use per-node allocation for hardware context */
>   	bool power; /* power on/off the device */
>   	bool memory_backed; /* if data is stored in memory */


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 0/1] null_blk: allow user to set QUEUE_FLAG_NOWAIT
  2023-04-12  8:50 ` [PATCH 0/1] " Chaitanya Kulkarni
@ 2023-04-12  9:34   ` Damien Le Moal
  0 siblings, 0 replies; 11+ messages in thread
From: Damien Le Moal @ 2023-04-12  9:34 UTC (permalink / raw)
  To: Chaitanya Kulkarni, linux-block@vger.kernel.org
  Cc: axboe@kernel.dk, johannes.thumshirn@wdc.com, bvanassche@acm.org,
	vincent.fu@samsung.com, shinichiro.kawasaki@wdc.com,
	yukuai3@huawei.com

On 4/12/23 17:50, Chaitanya Kulkarni wrote:
> + dlemoal@kenrel.org.

Missplelled email address


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] null_blk: allow user to set QUEUE_FLAG_NOWAIT
  2023-04-12  8:47 ` [PATCH 1/1] " Chaitanya Kulkarni
  2023-04-12  8:50   ` Chaitanya Kulkarni
@ 2023-04-12  9:41   ` Damien Le Moal
  2023-04-18  7:00     ` Chaitanya Kulkarni
  2023-04-18 11:41   ` Pankaj Raghav
  2 siblings, 1 reply; 11+ messages in thread
From: Damien Le Moal @ 2023-04-12  9:41 UTC (permalink / raw)
  To: Chaitanya Kulkarni, linux-block
  Cc: axboe, bvanassche, vincent.fu, shinichiro.kawasaki, yukuai3

On 4/12/23 17:47, Chaitanya Kulkarni wrote:
> QUEUE_FLAG_NOWAIT is set by default to mq drivers such null_blk when
> it is used with NULL_Q_MQ mode as a part of QUEUE_FLAG_MQ_DEFAULT that
> gets assigned in following code path see blk_mq_init_allocated_queue():-

Can you fix the grammar/punctuation in this sentence ? Looks like some words are
missing, making it hard to read.

> 
> null_add_dev()
> if (dev->queue_mode == NULL_Q_MQ) {
>         blk_mq_alloc_disk()
>           __blk_mq_alloc_disk()
> 	    blk_mq_init_queue_data()
>               blk_mq_init_allocated_queue()
>                 q->queue_flags |= QUEUE_FLAG_MQ_DEFAULT;
> }
> 
> But it is not set when null_blk is loaded with NULL_Q_BIO mode in following
> code path like other bio drivers do e.g. nvme-multipath :-
> 
> if (dev->queue_mode == NULL_Q_BIO) {
>         nullb->disk = blk_alloc_disk(nullb->dev->home_node);
>         	blk_alloc_disk()
>         	  blk_alloc_queue()
>         	  __alloc_disk_nodw()
> }
> 
> Add a new module parameter nowait and respective configfs attr that will
> set or clear the QUEUE_FLAG_NOWAIT based on a value set by user in
> null_add_dev() irrespective of the queue mode, by default keep it
> enabled to retain the original behaviour for the NULL_Q_MQ mode.

Nope. You are changing the behavior. See below.

> 
> Depending on nowait value use GFP_NOWAIT or GFP_NOIO for the alloction
> in the null_alloc_page() for alloc_pages() and in null_insert_page()
> for radix_tree_preload().   
> 
> Observed performance difference with this patch for fio iouring with
> configfs and non configfs null_blk with queue modes NULL_Q_BIO and
> NULL_Q_MQ:-

[...]

> @@ -983,11 +990,11 @@ static struct nullb_page *null_insert_page(struct nullb *nullb,
>  
>  	spin_unlock_irq(&nullb->lock);
>  
> -	t_page = null_alloc_page();
> +	t_page = null_alloc_page(nullb->dev->nowait ? GFP_NOWAIT : GFP_NOIO);

This can potentially result in failed allocations, so IO errors, which otherwise
would not happen without this change. Not nice. Memory backing also sets
BLK_MQ_F_BLOCKING, which I am not sure is compatible with QUEUE_FLAG_NOWAIT...
Would need to check that again.



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] null_blk: allow user to set QUEUE_FLAG_NOWAIT
  2023-04-12  9:41   ` Damien Le Moal
@ 2023-04-18  7:00     ` Chaitanya Kulkarni
  0 siblings, 0 replies; 11+ messages in thread
From: Chaitanya Kulkarni @ 2023-04-18  7:00 UTC (permalink / raw)
  To: Damien Le Moal, Chaitanya Kulkarni, linux-block@vger.kernel.org
  Cc: axboe@kernel.dk, bvanassche@acm.org, vincent.fu@samsung.com,
	shinichiro.kawasaki@wdc.com, yukuai3@huawei.com

On 4/12/23 02:41, Damien Le Moal wrote:
> On 4/12/23 17:47, Chaitanya Kulkarni wrote:
>> QUEUE_FLAG_NOWAIT is set by default to mq drivers such null_blk when
>> it is used with NULL_Q_MQ mode as a part of QUEUE_FLAG_MQ_DEFAULT that
>> gets assigned in following code path see blk_mq_init_allocated_queue():-
> Can you fix the grammar/punctuation in this sentence ? Looks like some words are
> missing, making it hard to read.

done.

>> null_add_dev()
>> if (dev->queue_mode == NULL_Q_MQ) {
>>          blk_mq_alloc_disk()
>>            __blk_mq_alloc_disk()
>> 	    blk_mq_init_queue_data()
>>                blk_mq_init_allocated_queue()
>>                  q->queue_flags |= QUEUE_FLAG_MQ_DEFAULT;
>> }
>>
>> But it is not set when null_blk is loaded with NULL_Q_BIO mode in following
>> code path like other bio drivers do e.g. nvme-multipath :-
>>
>> if (dev->queue_mode == NULL_Q_BIO) {
>>          nullb->disk = blk_alloc_disk(nullb->dev->home_node);
>>          	blk_alloc_disk()
>>          	  blk_alloc_queue()
>>          	  __alloc_disk_nodw()
>> }
>>
>> Add a new module parameter nowait and respective configfs attr that will
>> set or clear the QUEUE_FLAG_NOWAIT based on a value set by user in
>> null_add_dev() irrespective of the queue mode, by default keep it
>> enabled to retain the original behaviour for the NULL_Q_MQ mode.
> Nope. You are changing the behavior. See below.
>

true, changed it so that this flag is only set for the NULL_Q_BIO ...

>> Depending on nowait value use GFP_NOWAIT or GFP_NOIO for the alloction
>> in the null_alloc_page() for alloc_pages() and in null_insert_page()
>> for radix_tree_preload().
>>
>> Observed performance difference with this patch for fio iouring with
>> configfs and non configfs null_blk with queue modes NULL_Q_BIO and
>> NULL_Q_MQ:-
> [...]
>
>> @@ -983,11 +990,11 @@ static struct nullb_page *null_insert_page(struct nullb *nullb,
>>   
>>   	spin_unlock_irq(&nullb->lock);
>>   
>> -	t_page = null_alloc_page();
>> +	t_page = null_alloc_page(nullb->dev->nowait ? GFP_NOWAIT : GFP_NOIO);
> This can potentially result in failed allocations, so IO errors, which otherwise
> would not happen without this change. Not nice. Memory backing also sets
> BLK_MQ_F_BLOCKING, which I am not sure is compatible with QUEUE_FLAG_NOWAIT...
> Would need to check that again.
>
>

yes we should avoid that, here is explanation :-

BLK_MQ_F_BLOCKING is set in null_init_tag_set().
null_init_tag_set() has two callers:-

1. null_init() :-   checks if queue_mode=NULL_Q_MQ and shared_tags=1
                     before setting BLK_MQ_F_BLOCKING for shared tagset.
2. null_add_dev():- checks if queue_mode=NULL_Q_MQ before
                     setting BLK_MQ_F_BLOCKING for non-shared tagset.

With nowait only allowed for qmode=NULL_Q_BIO (see [1]) it will error
out with qmode=NULL_Q_MQ and will never set BLK_MQ_F_BLOCKING,
I'll add check in the null_validate_conf() just to make sure in V3.

Thanks for review comments.

-ck

[1] V2:- https://marc.info/?l=linux-block&r=1&b=202304&w=2



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] null_blk: allow user to set QUEUE_FLAG_NOWAIT
  2023-04-12  8:47 ` [PATCH 1/1] " Chaitanya Kulkarni
  2023-04-12  8:50   ` Chaitanya Kulkarni
  2023-04-12  9:41   ` Damien Le Moal
@ 2023-04-18 11:41   ` Pankaj Raghav
  2023-04-18 19:40     ` Chaitanya Kulkarni
  2023-04-18 19:46     ` Chaitanya Kulkarni
  2 siblings, 2 replies; 11+ messages in thread
From: Pankaj Raghav @ 2023-04-18 11:41 UTC (permalink / raw)
  To: Chaitanya Kulkarni
  Cc: linux-block, axboe, damien.lemoal, johannes.thumshirn, bvanassche,
	vincent.fu, shinichiro.kawasaki, yukuai3, p.raghav

[-- Attachment #1: Type: text/plain, Size: 2642 bytes --]

> +static bool g_nowait = true;
> +module_param_named(nowait, g_nowait, bool, 0444);
> +MODULE_PARM_DESC(virt_boundary, "Set QUEUE_FLAG_NOWAIT irrespective of queue mode. Default: True");
Copy paste error. MODULE_PARM_DESC(nowait,...
> +
>  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");
> @@ -983,11 +990,11 @@ static struct nullb_page *null_insert_page(struct nullb *nullb,
>  
>  	spin_unlock_irq(&nullb->lock);
>  
> -	t_page = null_alloc_page();
> +	t_page = null_alloc_page(nullb->dev->nowait ? GFP_NOWAIT : GFP_NOIO);
>  	if (!t_page)
>  		goto out_lock;
>  
> -	if (radix_tree_preload(GFP_NOIO))
> +	if (radix_tree_preload(nullb->dev->nowait ? GFP_NOWAIT : GFP_NOIO))

This is not correct. You need to use radix_tree_maybe_preload because
GFP_NOWAIT should not block and this WARN_ON_ONCE flag will trigger in
radix_tree_preload:

/* Warn on non-sensical use... */
WARN_ON_ONCE(!gfpflags_allow_blocking(gfp_mask));

I also verified this locally with your patch and while doing a simple fio 
write with setting memory_backed=1.

WARNING: CPU: 2 PID: 515 at lib/radix-tree.c:366 radix_tree_preload+0x12/0x20
...
RIP: 0010:radix_tree_preload+0x12/0x20
<snip>
Call Trace:
 <TASK>
 null_insert_page+0x186/0x4e0 [null_blk]
 null_transfer+0x588/0x990 [null_blk]
<snip>

>  		goto out_freepage;
>  
>  	spin_lock_irq(&nullb->lock);
> @@ -2093,6 +2100,11 @@ static int null_add_dev(struct nullb_device *dev)
>  	blk_queue_flag_set(QUEUE_FLAG_NONROT, nullb->q);
>  	blk_queue_flag_clear(QUEUE_FLAG_ADD_RANDOM, nullb->q);
>  
> +	if (dev->nowait)
> +		blk_queue_flag_set(QUEUE_FLAG_NOWAIT, nullb->q);
> +	else
> +		blk_queue_flag_clear(QUEUE_FLAG_NOWAIT, nullb->q);
> +
>  	mutex_lock(&lock);
>  	rv = ida_simple_get(&nullb_indexes, 0, 0, GFP_KERNEL);
>  	if (rv < 0) {
> diff --git a/drivers/block/null_blk/null_blk.h b/drivers/block/null_blk/null_blk.h
> index eb5972c50be8..1d7af446d728 100644
> --- a/drivers/block/null_blk/null_blk.h
> +++ b/drivers/block/null_blk/null_blk.h
> @@ -107,6 +107,7 @@ struct nullb_device {
>  	unsigned int index; /* index of the disk, only valid with a disk */
>  	unsigned int mbps; /* Bandwidth throttle cap (in MB/s) */
>  	bool blocking; /* blocking blk-mq device */
> +	bool nowait; /* to set QUEUE_FLAG_NOWAIT on device queue */
>  	bool use_per_node_hctx; /* use per-node allocation for hardware context */
>  	bool power; /* power on/off the device */
>  	bool memory_backed; /* if data is stored in memory */
> -- 
> 2.29.0
> 

-- 
Pankaj Raghav

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] null_blk: allow user to set QUEUE_FLAG_NOWAIT
  2023-04-18 11:41   ` Pankaj Raghav
@ 2023-04-18 19:40     ` Chaitanya Kulkarni
  2023-04-18 19:46     ` Chaitanya Kulkarni
  1 sibling, 0 replies; 11+ messages in thread
From: Chaitanya Kulkarni @ 2023-04-18 19:40 UTC (permalink / raw)
  To: Pankaj Raghav, Chaitanya Kulkarni
  Cc: linux-block@vger.kernel.org, axboe@kernel.dk,
	damien.lemoal@opensource.wdc.com, johannes.thumshirn@wdc.com,
	bvanassche@acm.org, vincent.fu@samsung.com,
	shinichiro.kawasaki@wdc.com, yukuai3@huawei.com

On 4/18/23 04:41, Pankaj Raghav wrote:
>> +static bool g_nowait = true;
>> +module_param_named(nowait, g_nowait, bool, 0444);
>> +MODULE_PARM_DESC(virt_boundary, "Set QUEUE_FLAG_NOWAIT irrespective of queue mode. Default: True");
> Copy paste error. MODULE_PARM_DESC(nowait,...

fixed..

>> +
>>   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");
>> @@ -983,11 +990,11 @@ static struct nullb_page *null_insert_page(struct nullb *nullb,
>>   
>>   	spin_unlock_irq(&nullb->lock);
>>   
>> -	t_page = null_alloc_page();
>> +	t_page = null_alloc_page(nullb->dev->nowait ? GFP_NOWAIT : GFP_NOIO);
>>   	if (!t_page)
>>   		goto out_lock;
>>   
>> -	if (radix_tree_preload(GFP_NOIO))
>> +	if (radix_tree_preload(nullb->dev->nowait ? GFP_NOWAIT : GFP_NOIO))
> This is not correct. You need to use radix_tree_maybe_preload because
> GFP_NOWAIT should not block and this WARN_ON_ONCE flag will trigger in
> radix_tree_preload:
>
> /* Warn on non-sensical use... */
> WARN_ON_ONCE(!gfpflags_allow_blocking(gfp_mask));
>
> I also verified this locally with your patch and while doing a simple fio
> write with setting memory_backed=1.
>
> WARNING: CPU: 2 PID: 515 at lib/radix-tree.c:366 radix_tree_preload+0x12/0x20
> ...
> RIP: 0010:radix_tree_preload+0x12/0x20
> <snip>
> Call Trace:
>   <TASK>
>   null_insert_page+0x186/0x4e0 [null_blk]
>   null_transfer+0x588/0x990 [null_blk]
> <snip>
>

thanks for pointing this out, will sendout V3 with above fix...

-ck



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] null_blk: allow user to set QUEUE_FLAG_NOWAIT
  2023-04-18 11:41   ` Pankaj Raghav
  2023-04-18 19:40     ` Chaitanya Kulkarni
@ 2023-04-18 19:46     ` Chaitanya Kulkarni
  2023-04-19  7:42       ` Pankaj Raghav
  1 sibling, 1 reply; 11+ messages in thread
From: Chaitanya Kulkarni @ 2023-04-18 19:46 UTC (permalink / raw)
  To: Pankaj Raghav, Chaitanya Kulkarni
  Cc: linux-block@vger.kernel.org, axboe@kernel.dk,
	damien.lemoal@opensource.wdc.com, johannes.thumshirn@wdc.com,
	bvanassche@acm.org, vincent.fu@samsung.com,
	shinichiro.kawasaki@wdc.com, yukuai3@huawei.com

On 4/18/23 04:41, Pankaj Raghav wrote:
>
> This is not correct. You need to use radix_tree_maybe_preload because
> GFP_NOWAIT should not block and this WARN_ON_ONCE flag will trigger in
> radix_tree_preload:
>
> /* Warn on non-sensical use... */
> WARN_ON_ONCE(!gfpflags_allow_blocking(gfp_mask));
>
> I also verified this locally with your patch and while doing a simple fio
> write with setting memory_backed=1.
>
>

plz share the exact fio job you have used to generate this error,
I need add that to the patch testlog.

-ck



^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/1] null_blk: allow user to set QUEUE_FLAG_NOWAIT
  2023-04-18 19:46     ` Chaitanya Kulkarni
@ 2023-04-19  7:42       ` Pankaj Raghav
  0 siblings, 0 replies; 11+ messages in thread
From: Pankaj Raghav @ 2023-04-19  7:42 UTC (permalink / raw)
  To: Chaitanya Kulkarni
  Cc: linux-block@vger.kernel.org, axboe@kernel.dk,
	damien.lemoal@opensource.wdc.com, johannes.thumshirn@wdc.com,
	bvanassche@acm.org, vincent.fu@samsung.com,
	shinichiro.kawasaki@wdc.com, yukuai3@huawei.com

On 2023-04-18 21:46, Chaitanya Kulkarni wrote:
> On 4/18/23 04:41, Pankaj Raghav wrote:
>>
>> This is not correct. You need to use radix_tree_maybe_preload because
>> GFP_NOWAIT should not block and this WARN_ON_ONCE flag will trigger in
>> radix_tree_preload:
>>
>> /* Warn on non-sensical use... */
>> WARN_ON_ONCE(!gfpflags_allow_blocking(gfp_mask));
>>
>> I also verified this locally with your patch and while doing a simple fio
>> write with setting memory_backed=1.
>>
>>
> 
> plz share the exact fio job you have used to generate this error,
> I need add that to the patch testlog.
> 
This should do it:

$ modprobe null_blk nowait=1 queue_mode=0 memory_backed=1
$ fio -iodepth=1 -rw=write -ioengine=io_uring -size=2M -name=io_uring_1  -filename=/dev/nullb0


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2023-04-19  7:43 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-04-12  8:47 [PATCH 0/1] null_blk: allow user to set QUEUE_FLAG_NOWAIT Chaitanya Kulkarni
2023-04-12  8:47 ` [PATCH 1/1] " Chaitanya Kulkarni
2023-04-12  8:50   ` Chaitanya Kulkarni
2023-04-12  9:41   ` Damien Le Moal
2023-04-18  7:00     ` Chaitanya Kulkarni
2023-04-18 11:41   ` Pankaj Raghav
2023-04-18 19:40     ` Chaitanya Kulkarni
2023-04-18 19:46     ` Chaitanya Kulkarni
2023-04-19  7:42       ` Pankaj Raghav
2023-04-12  8:50 ` [PATCH 0/1] " Chaitanya Kulkarni
2023-04-12  9:34   ` Damien Le Moal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).