[PATCH] md/raid10: fix missing discard IO accounting

Yu Kuai posted 1 patch 8 months, 3 weeks ago
drivers/md/raid10.c | 1 +
1 file changed, 1 insertion(+)
[PATCH] md/raid10: fix missing discard IO accounting
Posted by Yu Kuai 8 months, 3 weeks ago
From: Yu Kuai <yukuai3@huawei.com>

md_account_bio() is not called from raid10_handle_discard(), now that we
handle bitmap inside md_account_bio(), also fix missing
bitmap_startwrite for discard.

Test whole disk discard for 20G raid10:

Before:
Device   d/s     dMB/s   drqm/s  %drqm d_await dareq-sz
md0    48.00     16.00     0.00   0.00    5.42   341.33

After:
Device   d/s     dMB/s   drqm/s  %drqm d_await dareq-sz
md0    68.00  20462.00     0.00   0.00    2.65 308133.65

Fixes: 528bc2cf2fcc ("md/raid10: enable io accounting")
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 drivers/md/raid10.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 9d8516acf2fd..6ef65b4d1093 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -1735,6 +1735,7 @@ static int raid10_handle_discard(struct mddev *mddev, struct bio *bio)
 	 * The discard bio returns only first r10bio finishes
 	 */
 	if (first_copy) {
+		md_account_bio(mddev, &bio);
 		r10_bio->master_bio = bio;
 		set_bit(R10BIO_Discard, &r10_bio->state);
 		first_copy = false;
-- 
2.39.2
Re: [PATCH] md/raid10: fix missing discard IO accounting
Posted by Yu Kuai 8 months, 3 weeks ago
在 2025/03/25 9:57, Yu Kuai 写道:
> From: Yu Kuai <yukuai3@huawei.com>
> 
> md_account_bio() is not called from raid10_handle_discard(), now that we
> handle bitmap inside md_account_bio(), also fix missing
> bitmap_startwrite for discard.
> 
> Test whole disk discard for 20G raid10:
> 
> Before:
> Device   d/s     dMB/s   drqm/s  %drqm d_await dareq-sz
> md0    48.00     16.00     0.00   0.00    5.42   341.33
> 
> After:
> Device   d/s     dMB/s   drqm/s  %drqm d_await dareq-sz
> md0    68.00  20462.00     0.00   0.00    2.65 308133.65
> 
> Fixes: 528bc2cf2fcc ("md/raid10: enable io accounting")
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
>   drivers/md/raid10.c | 1 +
>   1 file changed, 1 insertion(+)
> 
Applied to md-6.15
Thanks

> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 9d8516acf2fd..6ef65b4d1093 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -1735,6 +1735,7 @@ static int raid10_handle_discard(struct mddev *mddev, struct bio *bio)
>   	 * The discard bio returns only first r10bio finishes
>   	 */
>   	if (first_copy) {
> +		md_account_bio(mddev, &bio);
>   		r10_bio->master_bio = bio;
>   		set_bit(R10BIO_Discard, &r10_bio->state);
>   		first_copy = false;
> 

Re: [PATCH] md/raid10: fix missing discard IO accounting
Posted by Coly Li 8 months, 3 weeks ago
On Tue, Mar 25, 2025 at 09:57:46AM +0800, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@huawei.com>
> 
> md_account_bio() is not called from raid10_handle_discard(), now that we
> handle bitmap inside md_account_bio(), also fix missing
> bitmap_startwrite for discard.
> 
> Test whole disk discard for 20G raid10:
> 
> Before:
> Device   d/s     dMB/s   drqm/s  %drqm d_await dareq-sz
> md0    48.00     16.00     0.00   0.00    5.42   341.33
> 
> After:
> Device   d/s     dMB/s   drqm/s  %drqm d_await dareq-sz
> md0    68.00  20462.00     0.00   0.00    2.65 308133.65
> 
> Fixes: 528bc2cf2fcc ("md/raid10: enable io accounting")
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>


Acked-by: Coly Li <colyli@kernel.org>

Thanks.

> ---
>  drivers/md/raid10.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 9d8516acf2fd..6ef65b4d1093 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -1735,6 +1735,7 @@ static int raid10_handle_discard(struct mddev *mddev, struct bio *bio)
>  	 * The discard bio returns only first r10bio finishes
>  	 */
>  	if (first_copy) {
> +		md_account_bio(mddev, &bio);
>  		r10_bio->master_bio = bio;
>  		set_bit(R10BIO_Discard, &r10_bio->state);
>  		first_copy = false;
> -- 
> 2.39.2
> 
> 

-- 
Coly Li
Re: [PATCH] md/raid10: fix missing discard IO accounting
Posted by Coly Li 8 months, 3 weeks ago
On Tue, Mar 25, 2025 at 09:57:46AM +0800, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@huawei.com>
> 
> md_account_bio() is not called from raid10_handle_discard(), now that we
> handle bitmap inside md_account_bio(), also fix missing
> bitmap_startwrite for discard.
> 
> Test whole disk discard for 20G raid10:
> 
> Before:
> Device   d/s     dMB/s   drqm/s  %drqm d_await dareq-sz
> md0    48.00     16.00     0.00   0.00    5.42   341.33
> 
> After:
> Device   d/s     dMB/s   drqm/s  %drqm d_await dareq-sz
> md0    68.00  20462.00     0.00   0.00    2.65 308133.65
> 
> Fixes: 528bc2cf2fcc ("md/raid10: enable io accounting")
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>

Should we treat discard request as real I/O?

Normally IMHO discard request should not be counted as real data transfer,
correct me if I am wrong.

Thanks.


> ---
>  drivers/md/raid10.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 9d8516acf2fd..6ef65b4d1093 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -1735,6 +1735,7 @@ static int raid10_handle_discard(struct mddev *mddev, struct bio *bio)
>  	 * The discard bio returns only first r10bio finishes
>  	 */
>  	if (first_copy) {
> +		md_account_bio(mddev, &bio);
>  		r10_bio->master_bio = bio;
>  		set_bit(R10BIO_Discard, &r10_bio->state);
>  		first_copy = false;
> -- 
> 2.39.2
> 
> 

-- 
Coly Li
Re: [PATCH] md/raid10: fix missing discard IO accounting
Posted by Yu Kuai 8 months, 3 weeks ago
Hi,

在 2025/03/25 15:04, Coly Li 写道:
> On Tue, Mar 25, 2025 at 09:57:46AM +0800, Yu Kuai wrote:
>> From: Yu Kuai <yukuai3@huawei.com>
>>
>> md_account_bio() is not called from raid10_handle_discard(), now that we
>> handle bitmap inside md_account_bio(), also fix missing
>> bitmap_startwrite for discard.
>>
>> Test whole disk discard for 20G raid10:
>>
>> Before:
>> Device   d/s     dMB/s   drqm/s  %drqm d_await dareq-sz
>> md0    48.00     16.00     0.00   0.00    5.42   341.33
>>
>> After:
>> Device   d/s     dMB/s   drqm/s  %drqm d_await dareq-sz
>> md0    68.00  20462.00     0.00   0.00    2.65 308133.65
>>
>> Fixes: 528bc2cf2fcc ("md/raid10: enable io accounting")
>> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> 
> Should we treat discard request as real I/O?
> 
> Normally IMHO discard request should not be counted as real data transfer,
> correct me if I am wrong.

Normally it's not, that's why discard IOs are accounted separately in
the block layer.

Also notice that discard should be treated as write, because after
discard, reading will get zero data.

Thanks,
Kuai

> 
> Thanks.
> 
> 
>> ---
>>   drivers/md/raid10.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
>> index 9d8516acf2fd..6ef65b4d1093 100644
>> --- a/drivers/md/raid10.c
>> +++ b/drivers/md/raid10.c
>> @@ -1735,6 +1735,7 @@ static int raid10_handle_discard(struct mddev *mddev, struct bio *bio)
>>   	 * The discard bio returns only first r10bio finishes
>>   	 */
>>   	if (first_copy) {
>> +		md_account_bio(mddev, &bio);
>>   		r10_bio->master_bio = bio;
>>   		set_bit(R10BIO_Discard, &r10_bio->state);
>>   		first_copy = false;
>> -- 
>> 2.39.2
>>
>>
> 

Re: [PATCH] md/raid10: fix missing discard IO accounting
Posted by Coly Li 8 months, 3 weeks ago

> 2025年3月25日 15:27,Yu Kuai <yukuai1@huaweicloud.com> 写道:
> 
> Hi,
> 
> 在 2025/03/25 15:04, Coly Li 写道:
>> On Tue, Mar 25, 2025 at 09:57:46AM +0800, Yu Kuai wrote:
>>> From: Yu Kuai <yukuai3@huawei.com>
>>> 
>>> md_account_bio() is not called from raid10_handle_discard(), now that we
>>> handle bitmap inside md_account_bio(), also fix missing
>>> bitmap_startwrite for discard.
>>> 
>>> Test whole disk discard for 20G raid10:
>>> 
>>> Before:
>>> Device   d/s     dMB/s   drqm/s  %drqm d_await dareq-sz
>>> md0    48.00     16.00     0.00   0.00    5.42   341.33
>>> 
>>> After:
>>> Device   d/s     dMB/s   drqm/s  %drqm d_await dareq-sz
>>> md0    68.00  20462.00     0.00   0.00    2.65 308133.65
>>> 
>>> Fixes: 528bc2cf2fcc ("md/raid10: enable io accounting")
>>> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
>> Should we treat discard request as real I/O?
>> Normally IMHO discard request should not be counted as real data transfer,
>> correct me if I am wrong.
> 
> Normally it's not, that's why discard IOs are accounted separately in
> the block layer.
> 
> Also notice that discard should be treated as write, because after
> discard, reading will get zero data.

I see, it is for the separated discard counting.  Sure, I reply my Acked-by on the original patch.

Thank you for the explanation.

Coly Li