blk-lib: cleanup bdev_get_queue()

PATCH Applied Chaitanya Kulkarni 3 participants 4 messages Feb 15, 2022 Block layer

lore

Share & tools

lore

Chaitanya Kulkarni

author

2022-02-15 11:53 UTC #0 lore raw

v2 cover blk-lib: cleanup bdev_get_queue()

Hi,

Based on the comment in the include/linux/blkdev.h:bdev_get_queue() the return value of the function will never be NULL. This patch series removes those checks present in the blk-lib.c, also removes the not needed local variable pointer to request_queue from the write_zeroes helpers.

Below is the test log for discard (__blkdev_issue_disacrd()) and write-zeroes (__blkdev_issue_write_zeroes/__blkdev_issue_zero_pages()).

-ck

changes from V1:-

1. squash all patches into single patch. (Jens)

Chaitanya Kulkarni (1):
blk-lib: don't check bdev_get_queue() NULL check

 block/blk-lib.c | 14 --------------
 1 file changed, 14 deletions(-)

1. Execute discard (__blkdev_issue_disacrd()) and write-zeroes (__blkdev_issue_write_zeroes, __blkdev_issue_zero_pages())

+ git diff block drivers/block/null_blk

block/blk-lib.c +6 -0

index fc6ea52e7482..84e5f31436d7 100644
@@ -79,6 +79,8 @@ int __blkdev_issue_discard(struct block_device *bdev, sector_t sector,
 
                WARN_ON_ONCE((req_sects << 9) > UINT_MAX);
 
+               pr_info("%s %d sector %llu nr_sects %llu\n",
+                               __func__, __LINE__, sector, nr_sects);
                bio = blk_next_bio(bio, bdev, 0, op, gfp_mask);
                bio->bi_iter.bi_sector = sector;
                bio->bi_iter.bi_size = req_sects << 9;

@@ -237,6 +239,8 @@ static int __blkdev_issue_write_zeroes(struct block_device *bdev,
                return -EOPNOTSUPP;
 
        while (nr_sects) {
+               pr_info("%s %d sector %llu nr_sects %llu\n",
+                               __func__, __LINE__, sector, nr_sects);
                bio = blk_next_bio(bio, bdev, 0, REQ_OP_WRITE_ZEROES, gfp_mask);
                bio->bi_iter.bi_sector = sector;
                if (flags & BLKDEV_ZERO_NOUNMAP)

@@ -282,6 +286,8 @@ static int __blkdev_issue_zero_pages(struct block_device *bdev,
                return -EPERM;
 
        while (nr_sects != 0) {
+               pr_info("%s %d sector %llu nr_sects %llu\n",
+                               __func__, __LINE__, sector, nr_sects);
                bio = blk_next_bio(bio, bdev, __blkdev_sectors_to_bio_pages(nr_sects),
                                   REQ_OP_WRITE, gfp_mask);
                bio->bi_iter.bi_sector = sector;

drivers/block/null_blk/main.c +5 ~~-15~~

index 13004beb48ca..584ac0519c3e 100644
@@ -84,6 +84,9 @@ 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");
 
+static bool g_write_zeroes = false;
+module_param_named(write_zeroes, g_write_zeroes, bool, 0444);
+
 static int g_no_sched;
 module_param_named(no_sched, g_no_sched, int, 0444);
 MODULE_PARM_DESC(no_sched, "No io scheduler");

@@ -1755,25 +1758,12 @@ static void null_del_dev(struct nullb *nullb)
 
 static void null_config_discard(struct nullb *nullb)
 {
-       if (nullb->dev->discard == false)
-               return;
-
-       if (!nullb->dev->memory_backed) {
-               nullb->dev->discard = false;
-               pr_info("discard option is ignored without memory backing\n");
-               return;
-       }
-
-       if (nullb->dev->zoned) {
-               nullb->dev->discard = false;
-               pr_info("discard option is ignored in zoned mode\n");
-               return;
-       }
-
        nullb->q->limits.discard_granularity = nullb->dev->blocksize;
        nullb->q->limits.discard_alignment = nullb->dev->blocksize;
        blk_queue_max_discard_sectors(nullb->q, UINT_MAX >> 9);
        blk_queue_flag_set(QUEUE_FLAG_DISCARD, nullb->q);
+       if (g_write_zeroes)
+               blk_queue_max_write_zeroes_sectors(nullb->q, UINT_MAX >> 9);
 }
 
 static const struct block_device_operations null_bio_ops = {

+ modprobe -r null_blk
+ modprobe null_blk
+ blkdiscard -o 0 -l 40960 /dev/nullb0
+ dmesg -c

[  261.219011] null_blk: module loaded

*[ 261.232872] __blkdev_issue_discard 82 sector 0 nr_sects 80*
+ blkdiscard -z -o 0 -l 40960 /dev/nullb0
+ dmesg -c
*[ 261.245818] __blkdev_issue_zero_pages 289 sector 0 nr_sects 80*
+ modprobe -r null_blk
+ modprobe null_blk write_zeroes=1
+ blkdiscard -z -o 0 -l 40960 /dev/nullb0
+ dmesg -c

[  261.328251] null_blk: module loaded

*[ 261.337313] __blkdev_issue_write_zeroes 242 sector 0 nr_sects 80*
+ modprobe -r null_blk
+ set +x

Signature · 1 line

2.29.0

Chaitanya Kulkarni

author

2022-02-15 11:53 UTC #1 lore raw

PATCH v2 1/1 blk-lib: don't check bdev_get_queue() NULL check

Based on the comment present in the bdev_get_queue() bdev->bd_queue can never be NULL. Remove the NULL check for the local variable q that is set from bdev_get_queue() for discard, write_same, and write_zeroes.

Signed-off-by Chaitanya Kulkarni <[email protected]>

---
 block/blk-lib.c | 14 --------------
 1 file changed, 14 deletions(-)

block/blk-lib.c +0 ~~-14~~

index 1b8ced45e4e5..fc6ea52e7482 100644
@@ -19,9 +19,6 @@ int __blkdev_issue_discard(struct block_device *bdev, sector_t sector,
 	unsigned int op;
 	sector_t bs_mask, part_offset = 0;
 
-	if (!q)
-		return -ENXIO;
-
 	if (bdev_read_only(bdev))
 		return -EPERM;

@@ -156,9 +153,6 @@ static int __blkdev_issue_write_same(struct block_device *bdev, sector_t sector,
 	struct bio *bio = *biop;
 	sector_t bs_mask;
 
-	if (!q)
-		return -ENXIO;
-
 	if (bdev_read_only(bdev))
 		return -EPERM;

@@ -232,10 +226,6 @@ static int __blkdev_issue_write_zeroes(struct block_device *bdev,
 {
 	struct bio *bio = *biop;
 	unsigned int max_write_zeroes_sectors;
-	struct request_queue *q = bdev_get_queue(bdev);
-
-	if (!q)
-		return -ENXIO;
 
 	if (bdev_read_only(bdev))
 		return -EPERM;

@@ -284,14 +274,10 @@ static int __blkdev_issue_zero_pages(struct block_device *bdev,
 		sector_t sector, sector_t nr_sects, gfp_t gfp_mask,
 		struct bio **biop)
 {
-	struct request_queue *q = bdev_get_queue(bdev);
 	struct bio *bio = *biop;
 	int bi_size = 0;
 	unsigned int sz;
 
-	if (!q)
-		return -ENXIO;
-
 	if (bdev_read_only(bdev))
 		return -EPERM;

Signature · 1 line

2.29.0

Ming Lei

Reviewed-by on v2 1/1

2022-02-15 12:02 UTC #2 lore raw

Re: [PATCH V2 1/1] blk-lib: don't check bdev_get_queue() NULL check

Quoted 6 lines · Chaitanya Kulkarni

Based on the comment present in the bdev_get_queue() bdev->bd_queue can never be NULL. Remove the NULL check for the local variable q that is set from bdev_get_queue() for discard, write_same, and write_zeroes.
Signed-off-by Chaitanya Kulkarni <[email protected]>

Looks fine,

Reviewed-by Ming Lei <[email protected]>

Signature · 1 line

Ming

Jens Axboe

maintainer Applied

2022-02-15 14:53 UTC #3 lore raw

In reply to Chaitanya Kulkarni

Quoted 10 lines · Chaitanya Kulkarni

Based on the comment in the include/linux/blkdev.h:bdev_get_queue() the return value of the function will never be NULL. This patch series removes those checks present in the blk-lib.c, also removes the not needed local variable pointer to request_queue from the write_zeroes helpers.
Below is the test log for discard (__blkdev_issue_disacrd()) and write-zeroes (__blkdev_issue_write_zeroes/__blkdev_issue_zero_pages()).
[...]

Applied, thanks!

[0/1] blk-lib: cleanup bdev_get_queue()
(no commit info)
[1/1] blk-lib: don't check bdev_get_queue() NULL check
(no commit info)

Best regards,

Signature · 1 line

Jens Axboe