41d2d848e5c0 ("md: improve io stats accounting") was added during the 5.9 cycle and therefore is present in the 5.10 branch. This patch was then reverted in mainline during the 5.14 cycle (ad3fc798800f) due to report of double faults [1].
However the revert was not picked up for the 5.10 branch. I believe it should be queued up.
Unfortunately, 41d2d848e5c0 in 5.10 cannot be reverted cleanly because of the later changes in 00fe60eae94. The mainline 5.14 revert commit also does not apply cleanly on 5.10 because 99dfc43ecbf6 is not in 5.10. Manually merging the revert is trivial though (I could provide the patch I've been testing if that's helpful).
Guillaume.
[1]. https://lore.kernel.org/linux-raid/3bf04253-3fad-434a-63a7-20214e38cf26@gmai...
On Tue, Jan 11, 2022 at 07:40:13PM +0100, Guillaume Morin wrote:
41d2d848e5c0 ("md: improve io stats accounting") was added during the 5.9 cycle and therefore is present in the 5.10 branch. This patch was then reverted in mainline during the 5.14 cycle (ad3fc798800f) due to report of double faults [1].
However the revert was not picked up for the 5.10 branch. I believe it should be queued up.
Unfortunately, 41d2d848e5c0 in 5.10 cannot be reverted cleanly because of the later changes in 00fe60eae94. The mainline 5.14 revert commit also does not apply cleanly on 5.10 because 99dfc43ecbf6 is not in 5.10. Manually merging the revert is trivial though (I could provide the patch I've been testing if that's helpful).
Please provide a working revert and we will be glad to queue it up.
thanks,
greg k-h
commit ad3fc798800fb7ca04c1dfc439dba946818048d8 upstream.
The commit 41d2d848e5c0 ("md: improve io stats accounting") could cause double fault problem per the report [1], and also it is not correct to change ->bi_end_io if md don't own it, so let's revert it.
And io stats accounting will be replemented in later commits.
[1]. https://lore.kernel.org/linux-raid/3bf04253-3fad-434a-63a7-20214e38cf26@gmai...
Fixes: 41d2d848e5c0 ("md: improve io stats accounting") Signed-off-by: Guoqing Jiang jiangguoqing@kylinos.cn Signed-off-by: Song Liu song@kernel.org [GM: backport to 5.10-stable] Signed-off-by: Guillaume Morin guillaume@morinfr.org --- drivers/md/md.c | 57 +++++++++++-------------------------------------- drivers/md/md.h | 1 - 2 files changed, 12 insertions(+), 46 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c index 2069b16b50ec..cc3876500c4b 100644 --- a/drivers/md/md.c +++ b/drivers/md/md.c @@ -459,34 +459,12 @@ void md_handle_request(struct mddev *mddev, struct bio *bio) } EXPORT_SYMBOL(md_handle_request);
-struct md_io { - struct mddev *mddev; - bio_end_io_t *orig_bi_end_io; - void *orig_bi_private; - unsigned long start_time; - struct hd_struct *part; -}; - -static void md_end_io(struct bio *bio) -{ - struct md_io *md_io = bio->bi_private; - struct mddev *mddev = md_io->mddev; - - part_end_io_acct(md_io->part, bio, md_io->start_time); - - bio->bi_end_io = md_io->orig_bi_end_io; - bio->bi_private = md_io->orig_bi_private; - - mempool_free(md_io, &mddev->md_io_pool); - - if (bio->bi_end_io) - bio->bi_end_io(bio); -} - static blk_qc_t md_submit_bio(struct bio *bio) { const int rw = bio_data_dir(bio); + const int sgrp = op_stat_group(bio_op(bio)); struct mddev *mddev = bio->bi_disk->private_data; + unsigned int sectors;
if (mddev == NULL || mddev->pers == NULL) { bio_io_error(bio); @@ -507,26 +485,21 @@ static blk_qc_t md_submit_bio(struct bio *bio) return BLK_QC_T_NONE; }
- if (bio->bi_end_io != md_end_io) { - struct md_io *md_io; - - md_io = mempool_alloc(&mddev->md_io_pool, GFP_NOIO); - md_io->mddev = mddev; - md_io->orig_bi_end_io = bio->bi_end_io; - md_io->orig_bi_private = bio->bi_private; - - bio->bi_end_io = md_end_io; - bio->bi_private = md_io; - - md_io->start_time = part_start_io_acct(mddev->gendisk, - &md_io->part, bio); - } - + /* + * save the sectors now since our bio can + * go away inside make_request + */ + sectors = bio_sectors(bio); /* bio could be mergeable after passing to underlayer */ bio->bi_opf &= ~REQ_NOMERGE;
md_handle_request(mddev, bio);
+ part_stat_lock(); + part_stat_inc(&mddev->gendisk->part0, ios[sgrp]); + part_stat_add(&mddev->gendisk->part0, sectors[sgrp], sectors); + part_stat_unlock(); + return BLK_QC_T_NONE; }
@@ -5636,7 +5609,6 @@ static void md_free(struct kobject *ko)
bioset_exit(&mddev->bio_set); bioset_exit(&mddev->sync_set); - mempool_exit(&mddev->md_io_pool); kfree(mddev); }
@@ -5732,11 +5704,6 @@ static int md_alloc(dev_t dev, char *name) */ mddev->hold_active = UNTIL_STOP;
- error = mempool_init_kmalloc_pool(&mddev->md_io_pool, BIO_POOL_SIZE, - sizeof(struct md_io)); - if (error) - goto abort; - error = -ENOMEM; mddev->queue = blk_alloc_queue(NUMA_NO_NODE); if (!mddev->queue) diff --git a/drivers/md/md.h b/drivers/md/md.h index 2175a5ac4f7c..c94811cf2600 100644 --- a/drivers/md/md.h +++ b/drivers/md/md.h @@ -487,7 +487,6 @@ struct mddev { struct bio_set sync_set; /* for sync operations like * metadata and bitmap writes */ - mempool_t md_io_pool;
/* Generic flush handling. * The last to finish preflush schedules a worker to submit
On Wed, Jan 12, 2022 at 06:26:44PM +0100, Guillaume Morin wrote:
commit ad3fc798800fb7ca04c1dfc439dba946818048d8 upstream.
The commit 41d2d848e5c0 ("md: improve io stats accounting") could cause double fault problem per the report [1], and also it is not correct to change ->bi_end_io if md don't own it, so let's revert it.
And io stats accounting will be replemented in later commits.
[1]. https://lore.kernel.org/linux-raid/3bf04253-3fad-434a-63a7-20214e38cf26@gmai...
Fixes: 41d2d848e5c0 ("md: improve io stats accounting") Signed-off-by: Guoqing Jiang jiangguoqing@kylinos.cn Signed-off-by: Song Liu song@kernel.org [GM: backport to 5.10-stable] Signed-off-by: Guillaume Morin guillaume@morinfr.org
drivers/md/md.c | 57 +++++++++++-------------------------------------- drivers/md/md.h | 1 - 2 files changed, 12 insertions(+), 46 deletions(-)
Now queued up, thanks.
greg k-h
linux-stable-mirror@lists.linaro.org
-
Greg KH -
Guillaume Morin