[md] Missing revert in 5.10

[md] Missing revert in 5.10

[Guillaume Morin]

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@gmail.com/T/#t

-- 
Guillaume Morin <guillaume@morinfr.org>

Re: [md] Missing revert in 5.10

[Greg KH]

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

[PATCH backport for 5.10]: md: revert io stats accounting

[Guillaume Morin]

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@gmail.com/T/#t

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
-- 
2.23.0

-- 
Guillaume Morin <guillaume@morinfr.org>

Re: [PATCH backport for 5.10]: md: revert io stats accounting

[Greg KH]

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@gmail.com/T/#t
> 
> 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