Re: [PATCH] md: fix md io stats accounting broken

[NeilBrown]

On Fri, 3 Apr 2015 08:44:47 +0800 Gu Zheng <guz.fnst@xxxxxxxxxxxxxx> 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 <sim@xxxxxxxxxx>
> Cc: <stable@xxxxxxxxxxxxxxx> 3.19
> Signed-off-by: Gu Zheng <guz.fnst@xxxxxxxxxxxxxx>
> ---
>  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

Attachment: pgp2wR8wk2td3.pgp
Description: OpenPGP digital signature