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

[Guoqing Jiang <jgq516@gmail.com>]

On 5/18/21 9:35 PM, Christoph Hellwig wrote:
>> +	struct bio orig_bio_clone;
> Maybe call this bio_clone?  or just bio?

Ok, will rename it.

>> +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.

>> +	/*
>> +	 * 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.

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