Re: [PATCH 2/5] md: the latest try for improve io stats accounting

On 5/18/21 9:35 PM, Christoph Hellwig wrote:

quoted

+	struct bio orig_bio_clone;

Maybe call this bio_clone?  or just bio?

Ok, will rename it.

quoted

+static void md_end_io(struct bio *bio)
+{
+	struct md_io *md_io = bio->bi_private;
+	struct bio *orig_bio = md_io->orig_bio;
+
+	orig_bio->bi_status = bio->bi_status;
+
+	bio_end_io_acct_remapped(orig_bio, md_io->start_time, orig_bio->bi_bdev);

Overly long line.  And trivially fixed by just using bio_end_io_acct.

Thanks for reminder.

quoted

+	/*
+	 * We don't clone bio for multipath, raid1 and raid10 since we can reuse
+	 * their clone infrastructure.
+	 */
+	if (blk_queue_io_stat(bio->bi_bdev->bd_disk->queue) &&
+	    (bio->bi_pool != &mddev->md_io_bs) &&
+	    (mddev->level != 1) && (mddev->level != 10) &&
+	    (mddev->level != LEVEL_MULTIPATH)) {

This should use a flag in struct md_personality instead.

Yes.

quoted

-	if (bioset_integrity_create(&mddev->bio_set, BIO_POOL_SIZE)) {
+	if (bioset_integrity_create(&mddev->bio_set, BIO_POOL_SIZE) ||
+	    bioset_integrity_create(&mddev->md_io_bs, BIO_POOL_SIZE)) {

If the md_io_bs creation fails bio_set needs to be cleaned up.

Good catch, and seems we need to call bioset_exit(&mddev->bio_set) as well.

Thanks,
Guoqing