| Message ID | 20250325015746.3195035-1-yukuai1@huaweicloud.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | md/raid10: fix missing discard IO accounting | expand |
| Context | Check | Description |
|---|---|---|
| mdraidci/vmtest-md-6_14-PR | success | PR summary |
| mdraidci/vmtest-md-6_14-VM_Test-0 | success | Logs for per-patch-testing |
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 > >
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 >> >> >
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 > >
> 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
在 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; >
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;