| Message ID | 20210202052544.4108-7-chaitanya.kulkarni@wdc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | blktrace: few clenaup | expand |
Typo in the patch title. On 2021/02/02 14:26, Chaitanya Kulkarni wrote: > null blk driver supports REQ_OP_DISACRD in membacked mode. When testing > special requests having REQ_OP_DISCARD support is helpful to validate > the special request path when mixed with read-write. > > Consider module parameter when setting the queue discard flag when > device is not memory backed. > > This is needed to test the tracepoint related to REQ_OP_DISCARD. > > Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> > --- > # modprobe -r null_blk > # gitlog -7 > 4ac4d49e1028 (HEAD -> for-next) null_blk: add module param queue bounce limit > 468e3617dae8 null_blk: allow disacrd on non-membacked mode > bdc96efe9681 block: get rid of the trace rq insert wrapper > 73bf523a7ce4 blktrace: fix blk_rq_merge documentation > 6016632555da blktrace: fix blk_rq_issue documentation > 58b5d7103a94 blktrace: add blk_fill_rwbs documentation comment > 534f321f57dd block: remove superfluous param in blk_fill_rwbs() > > Test to exercise module parameter vs configfs discard param, set up > nullb0 such a way that it errors out in both cases for module param > discard :- > > for discard in 0 1 > do > modprobe -r null_blk > i=0 > echo "Module param discard ${discard}" > modprobe null_blk nr_devices=0 discard=${discard} > # Create dev 0 no discard > NULLB_DIR=config/nullb/nullb${i} > mkdir config/nullb/nullb${i} > echo 1 > config/nullb/nullb${i}/memory_backed > echo 512 > config/nullb/nullb${i}/blocksize > echo 2048 > config/nullb/nullb${i}/size > echo 1 > config/nullb/nullb${i}/power > echo -n "$nullb${i} membacked : "; > cat /sys/kernel/config/nullb/nullb${i}/memory_backed > echo -n "$nullb${i} discard : "; > cat /sys/kernel/config/nullb/nullb${i}/discard > # Create dev 1 with discard > i=1 > NULLB_DIR=config/nullb/nullb${i} > mkdir config/nullb/nullb${i} > echo 1 > config/nullb/nullb${i}/memory_backed > echo 512 > config/nullb/nullb${i}/blocksize > echo 2048 > config/nullb/nullb${i}/size > echo 1 > config/nullb/nullb${i}/discard > echo 1 > config/nullb/nullb${i}/power > echo -n "$nullb${i} membacked : "; > cat /sys/kernel/config/nullb/nullb${i}/memory_backed > echo -n "$nullb${i} discard : "; > cat /sys/kernel/config/nullb/nullb${i}/discard > > # should fail > blkdiscard -o 0 -l 1024 /dev/nullb0 > # should pass > blkdiscard -o 0 -l 1024 /dev/nullb1 > > echo 0 > config/nullb/nullb0/power > echo 0 > config/nullb/nullb1/power > rmdir config/nullb/nullb* > > modprobe -r null_blk > modprobe null_blk > # should fail > blkdiscard -o 0 -l 1024 /dev/nullb0 > modprobe -r null_blk > modprobe null_blk discard=1 > # should pass > blkdiscard -o 0 -l 1024 /dev/nullb0 > modprobe -r null_blk > echo "--------------------------" > done > > modprobe -r null_blk > modprobe null_blk > blkdiscard -o 0 -l 1024 /dev/nullb0 > modprobe -r null_blk > modprobe null_blk discard=1 > blkdiscard -o 0 -l 1024 /dev/nullb0 > modprobe -r null_blk > > # ./discard_module_param_test.sh > Module param discard 0 > 0 membacked : 1 > 0 discard : 0 > 1 membacked : 1 > 1 discard : 1 > blkdiscard: /dev/nullb0: BLKDISCARD ioctl failed: Operation not supported > blkdiscard: /dev/nullb0: BLKDISCARD ioctl failed: Operation not supported > -------------------------- > Module param discard 1 > 0 membacked : 1 > 0 discard : 0 > 1 membacked : 1 > 1 discard : 1 > blkdiscard: /dev/nullb0: BLKDISCARD ioctl failed: Operation not supported > blkdiscard: /dev/nullb0: BLKDISCARD ioctl failed: Operation not supported > -------------------------- > blkdiscard: /dev/nullb0: BLKDISCARD ioctl failed: Operation not supported > --- > drivers/block/null_blk/main.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c > index d6c821d48090..6e6cbb953a12 100644 > --- a/drivers/block/null_blk/main.c > +++ b/drivers/block/null_blk/main.c > @@ -172,6 +172,10 @@ static bool g_shared_tag_bitmap; > module_param_named(shared_tag_bitmap, g_shared_tag_bitmap, bool, 0444); > MODULE_PARM_DESC(shared_tag_bitmap, "Use shared tag bitmap for all submission queues for blk-mq"); > > +static bool g_discard; > +module_param_named(discard, g_discard, bool, 0444); > +MODULE_PARM_DESC(discard, "Enable queue discard (default: false)"); > + > static int g_irqmode = NULL_IRQ_SOFTIRQ; > > static int null_set_irqmode(const char *str, const struct kernel_param *kp) > @@ -1588,14 +1592,11 @@ static void null_del_dev(struct nullb *nullb) > > static void null_config_discard(struct nullb *nullb) > { > - if (nullb->dev->discard == false) > + if (nullb->dev->memory_backed && nullb->dev->discard == false) What is the point of adding nullb->dev->memory_backed to the test ? If nullb->dev->memory_backed is true, nullb->dev->discard should be forced to true also to retain the behavior we had until now. > return; > > - if (!nullb->dev->memory_backed) { > - nullb->dev->discard = false; > - pr_info("discard option is ignored without memory backing\n"); > + if (!nullb->dev->memory_backed && !g_discard) I do not understand this one either. Why look at g_discard ? > return; > - } > > if (nullb->dev->zoned) { > nullb->dev->discard = false; >
diff --git a/drivers/block/null_blk/main.c b/drivers/block/null_blk/main.c index d6c821d48090..6e6cbb953a12 100644 --- a/drivers/block/null_blk/main.c +++ b/drivers/block/null_blk/main.c @@ -172,6 +172,10 @@ static bool g_shared_tag_bitmap; module_param_named(shared_tag_bitmap, g_shared_tag_bitmap, bool, 0444); MODULE_PARM_DESC(shared_tag_bitmap, "Use shared tag bitmap for all submission queues for blk-mq"); +static bool g_discard; +module_param_named(discard, g_discard, bool, 0444); +MODULE_PARM_DESC(discard, "Enable queue discard (default: false)"); + static int g_irqmode = NULL_IRQ_SOFTIRQ; static int null_set_irqmode(const char *str, const struct kernel_param *kp) @@ -1588,14 +1592,11 @@ static void null_del_dev(struct nullb *nullb) static void null_config_discard(struct nullb *nullb) { - if (nullb->dev->discard == false) + if (nullb->dev->memory_backed && nullb->dev->discard == false) return; - if (!nullb->dev->memory_backed) { - nullb->dev->discard = false; - pr_info("discard option is ignored without memory backing\n"); + if (!nullb->dev->memory_backed && !g_discard) return; - } if (nullb->dev->zoned) { nullb->dev->discard = false;
null blk driver supports REQ_OP_DISACRD in membacked mode. When testing special requests having REQ_OP_DISCARD support is helpful to validate the special request path when mixed with read-write. Consider module parameter when setting the queue discard flag when device is not memory backed. This is needed to test the tracepoint related to REQ_OP_DISCARD. Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@wdc.com> --- # modprobe -r null_blk # gitlog -7 4ac4d49e1028 (HEAD -> for-next) null_blk: add module param queue bounce limit 468e3617dae8 null_blk: allow disacrd on non-membacked mode bdc96efe9681 block: get rid of the trace rq insert wrapper 73bf523a7ce4 blktrace: fix blk_rq_merge documentation 6016632555da blktrace: fix blk_rq_issue documentation 58b5d7103a94 blktrace: add blk_fill_rwbs documentation comment 534f321f57dd block: remove superfluous param in blk_fill_rwbs() Test to exercise module parameter vs configfs discard param, set up nullb0 such a way that it errors out in both cases for module param discard :- for discard in 0 1 do modprobe -r null_blk i=0 echo "Module param discard ${discard}" modprobe null_blk nr_devices=0 discard=${discard} # Create dev 0 no discard NULLB_DIR=config/nullb/nullb${i} mkdir config/nullb/nullb${i} echo 1 > config/nullb/nullb${i}/memory_backed echo 512 > config/nullb/nullb${i}/blocksize echo 2048 > config/nullb/nullb${i}/size echo 1 > config/nullb/nullb${i}/power echo -n "$nullb${i} membacked : "; cat /sys/kernel/config/nullb/nullb${i}/memory_backed echo -n "$nullb${i} discard : "; cat /sys/kernel/config/nullb/nullb${i}/discard # Create dev 1 with discard i=1 NULLB_DIR=config/nullb/nullb${i} mkdir config/nullb/nullb${i} echo 1 > config/nullb/nullb${i}/memory_backed echo 512 > config/nullb/nullb${i}/blocksize echo 2048 > config/nullb/nullb${i}/size echo 1 > config/nullb/nullb${i}/discard echo 1 > config/nullb/nullb${i}/power echo -n "$nullb${i} membacked : "; cat /sys/kernel/config/nullb/nullb${i}/memory_backed echo -n "$nullb${i} discard : "; cat /sys/kernel/config/nullb/nullb${i}/discard # should fail blkdiscard -o 0 -l 1024 /dev/nullb0 # should pass blkdiscard -o 0 -l 1024 /dev/nullb1 echo 0 > config/nullb/nullb0/power echo 0 > config/nullb/nullb1/power rmdir config/nullb/nullb* modprobe -r null_blk modprobe null_blk # should fail blkdiscard -o 0 -l 1024 /dev/nullb0 modprobe -r null_blk modprobe null_blk discard=1 # should pass blkdiscard -o 0 -l 1024 /dev/nullb0 modprobe -r null_blk echo "--------------------------" done modprobe -r null_blk modprobe null_blk blkdiscard -o 0 -l 1024 /dev/nullb0 modprobe -r null_blk modprobe null_blk discard=1 blkdiscard -o 0 -l 1024 /dev/nullb0 modprobe -r null_blk # ./discard_module_param_test.sh Module param discard 0 0 membacked : 1 0 discard : 0 1 membacked : 1 1 discard : 1 blkdiscard: /dev/nullb0: BLKDISCARD ioctl failed: Operation not supported blkdiscard: /dev/nullb0: BLKDISCARD ioctl failed: Operation not supported -------------------------- Module param discard 1 0 membacked : 1 0 discard : 0 1 membacked : 1 1 discard : 1 blkdiscard: /dev/nullb0: BLKDISCARD ioctl failed: Operation not supported blkdiscard: /dev/nullb0: BLKDISCARD ioctl failed: Operation not supported -------------------------- blkdiscard: /dev/nullb0: BLKDISCARD ioctl failed: Operation not supported --- drivers/block/null_blk/main.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)