Daeho,
On 6/8/20 8:05 PM, Daeho Jeong wrote:
> Hi guys,
>
> When I looked into blkdev_issue_zeroout(), I found something that I
> cannot understand there. In my understanding, all the submitted bio
> will stay in the plug list during plugging. But, I found
> submit_bio_wait() calling during the plugging. I guess the below
> submit_bio_wait() might wait forever in the lower kernel version like
> 4.14, because of plugging. If I am wrong, plz, correct me.
>
> >>> blk_start_plug(&plug); <<<
> if (try_write_zeroes) {
> ret = __blkdev_issue_write_zeroes(bdev, sector, nr_sects,
> gfp_mask, &bio, flags);
> } else if (!(flags & BLKDEV_ZERO_NOFALLBACK)) {
> ret = __blkdev_issue_zero_pages(bdev, sector, nr_sects,
> gfp_mask, &bio);
> } else {
> /* No zeroing offload support */
> ret = -EOPNOTSUPP;
> }
> if (ret == 0 && bio) {
> ret = submit_bio_wait(bio);
> bio_put(bio);
> }
> >>> blk_finish_plug(&plug); <<<
>
If your analysis is correct then this is true for
blkdev_issue_write_same() and blkdev_issue_discard() also ?
If I understand plugging correctly then when a process is going to wait
on the I/O to finish (here submit_bio_wait(), the device is unplugged
and request dispatching to the device driver is started. Does that mean
we should finish plug before we wait ?
Just for a discussion following untested patch which unplugs before
submit_bio_wait() in blk-lib.c and maintains the original behaviour :-
From 253ae720f721b2789d8fcde3861aeac6766b4836 Mon Sep 17 00:00:00 2001
From: Chaitanya Kulkarni <chaitanya.kulkarni@xxxxxxx>
Date: Wed, 10 Jun 2020 18:23:19 -0700
Subject: [PATCH] block: finish plug before submit_bio_wait()
Signed-off-by: Chaitanya Kulkarni <chaitanya.kulkarni@xxxxxxx>
---
block/blk-lib.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/block/blk-lib.c b/block/blk-lib.c
index 5f2c429d4378..992de09b258d 100644
--- a/block/blk-lib.c
+++ b/block/blk-lib.c
@@ -104,13 +104,13 @@ int blkdev_issue_discard(struct block_device
*bdev, sector_t sector,
blk_start_plug(&plug);
ret = __blkdev_issue_discard(bdev, sector, nr_sects, gfp_mask, flags,
&bio);
+ blk_finish_plug(&plug);
if (!ret && bio) {
ret = submit_bio_wait(bio);
if (ret == -EOPNOTSUPP)
ret = 0;
bio_put(bio);
}
- blk_finish_plug(&plug);
return ret;
}
@@ -200,11 +200,11 @@ int blkdev_issue_write_same(struct block_device
*bdev, sector_t sector,
blk_start_plug(&plug);
ret = __blkdev_issue_write_same(bdev, sector, nr_sects, gfp_mask, page,
&bio);
+ blk_finish_plug(&plug);
if (ret == 0 && bio) {
ret = submit_bio_wait(bio);
bio_put(bio);
}
- blk_finish_plug(&plug);
return ret;
}
EXPORT_SYMBOL(blkdev_issue_write_same);
@@ -381,11 +381,11 @@ int blkdev_issue_zeroout(struct block_device
*bdev, sector_t sector,
/* No zeroing offload support */
ret = -EOPNOTSUPP;
}
+ blk_finish_plug(&plug);
if (ret == 0 && bio) {
ret = submit_bio_wait(bio);
bio_put(bio);
}
- blk_finish_plug(&plug);
if (ret && try_write_zeroes) {
if (!(flags & BLKDEV_ZERO_NOFALLBACK)) {
try_write_zeroes = false;
--
2.27.0
