2015-04-03 01:03:30

by Gu Zheng

[permalink] [raw]
Subject: [PATCH] md: fix md io stats accounting broken

Simon reported the md io stats accounting issue:
"
I'm seeing "iostat -x -k 1" print this after a RAID1 rebuild on 4.0-rc5.
It's not abnormal other than it's 3-disk, with one being SSD (sdc) and
the other two being write-mostly:

Device: rrqm/s wrqm/s r/s w/s rkB/s wkB/s avgrq-sz avgqu-sz await r_await w_await svctm %util
sda 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00
sdb 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00
sdc 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00
md0 0.00 0.00 0.00 0.00 0.00 0.00 0.00 345.00 0.00 0.00 0.00 0.00 100.00
md2 0.00 0.00 0.00 0.00 0.00 0.00 0.00 58779.00 0.00 0.00 0.00 0.00 100.00
md1 0.00 0.00 0.00 0.00 0.00 0.00 0.00 12.00 0.00 0.00 0.00 0.00 100.00
"
The cause is commit "18c0b223cf9901727ef3b02da6711ac930b4e5d4" uses the
generic_start_io_acct to account the disk stats rather than the open code,
but it also introduced the increase to .in_flight[rw] which is needless to
md. So we re-use the open code here to fix it.

Reported-by: Simon Kirby <[email protected]>
Cc: <[email protected]> 3.19
Signed-off-by: Gu Zheng <[email protected]>
---
drivers/md/md.c | 6 +++++-
1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 717daad..e617878 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -249,6 +249,7 @@ static void md_make_request(struct request_queue *q, struct bio *bio)
const int rw = bio_data_dir(bio);
struct mddev *mddev = q->queuedata;
unsigned int sectors;
+ int cpu;

if (mddev == NULL || mddev->pers == NULL
|| !mddev->ready) {
@@ -284,7 +285,10 @@ static void md_make_request(struct request_queue *q, struct bio *bio)
sectors = bio_sectors(bio);
mddev->pers->make_request(mddev, bio);

- generic_start_io_acct(rw, sectors, &mddev->gendisk->part0);
+ cpu = part_stat_lock();
+ part_stat_inc(cpu, &mddev->gendisk->part0, ios[rw]);
+ part_stat_add(cpu, &mddev->gendisk->part0, sectors[rw], sectors);
+ part_stat_unlock();

if (atomic_dec_and_test(&mddev->active_io) && mddev->suspended)
wake_up(&mddev->sb_wait);
--
1.7.7


2015-04-08 03:12:10

by NeilBrown

[permalink] [raw]
Subject: Re: [PATCH] md: fix md io stats accounting broken

On Fri, 3 Apr 2015 08:44:47 +0800 Gu Zheng <[email protected]> wrote:

> Simon reported the md io stats accounting issue:
> "
> I'm seeing "iostat -x -k 1" print this after a RAID1 rebuild on 4.0-rc5.
> It's not abnormal other than it's 3-disk, with one being SSD (sdc) and
> the other two being write-mostly:
>
> Device: rrqm/s wrqm/s r/s w/s rkB/s wkB/s avgrq-sz avgqu-sz await r_await w_await svctm %util
> sda 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00
> sdb 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00
> sdc 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00 0.00
> md0 0.00 0.00 0.00 0.00 0.00 0.00 0.00 345.00 0.00 0.00 0.00 0.00 100.00
> md2 0.00 0.00 0.00 0.00 0.00 0.00 0.00 58779.00 0.00 0.00 0.00 0.00 100.00
> md1 0.00 0.00 0.00 0.00 0.00 0.00 0.00 12.00 0.00 0.00 0.00 0.00 100.00
> "
> The cause is commit "18c0b223cf9901727ef3b02da6711ac930b4e5d4" uses the
> generic_start_io_acct to account the disk stats rather than the open code,
> but it also introduced the increase to .in_flight[rw] which is needless to
> md. So we re-use the open code here to fix it.
>
> Reported-by: Simon Kirby <[email protected]>
> Cc: <[email protected]> 3.19
> Signed-off-by: Gu Zheng <[email protected]>
> ---
> drivers/md/md.c | 6 +++++-
> 1 files changed, 5 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 717daad..e617878 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -249,6 +249,7 @@ static void md_make_request(struct request_queue *q, struct bio *bio)
> const int rw = bio_data_dir(bio);
> struct mddev *mddev = q->queuedata;
> unsigned int sectors;
> + int cpu;
>
> if (mddev == NULL || mddev->pers == NULL
> || !mddev->ready) {
> @@ -284,7 +285,10 @@ static void md_make_request(struct request_queue *q, struct bio *bio)
> sectors = bio_sectors(bio);
> mddev->pers->make_request(mddev, bio);
>
> - generic_start_io_acct(rw, sectors, &mddev->gendisk->part0);
> + cpu = part_stat_lock();
> + part_stat_inc(cpu, &mddev->gendisk->part0, ios[rw]);
> + part_stat_add(cpu, &mddev->gendisk->part0, sectors[rw], sectors);
> + part_stat_unlock();
>
> if (atomic_dec_and_test(&mddev->active_io) && mddev->suspended)
> wake_up(&mddev->sb_wait);


Applied, thanks.
Will push to Linus in a day or 2.

NeilBrown


Attachments:
(No filename) (811.00 B)
OpenPGP digital signature