Linux Device Mapper development
 help / color / mirror / Atom feed
From: Mike Snitzer <snitzer@redhat.com>
To: Mikulas Patocka <mpatocka@redhat.com>
Cc: Mike Snitzer <msnitzer@redhat.com>, dm-devel@redhat.com
Subject: Re: [dm-devel] dm: fix dm_stats accounting
Date: Fri, 11 Feb 2022 11:08:05 -0500	[thread overview]
Message-ID: <YgaJ5bqSIR+OtYJt@redhat.com> (raw)
In-Reply-To: <alpine.LRH.2.02.2202110959350.30005@file01.intranet.prod.int.rdu2.redhat.com>

On Fri, Feb 11 2022 at 10:01P -0500,
Mikulas Patocka <mpatocka@redhat.com> wrote:

> Exactly the same sector number and length must be passed to both calls of
> dm_stats_account_io, otherwise its per-zone in_flight counters go out of
> sync. This patch fixes it.
> 
> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
> 
> ---
>  drivers/md/dm.c |   10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
> 
> Index: linux-dm/drivers/md/dm.c
> ===================================================================
> --- linux-dm.orig/drivers/md/dm.c	2022-02-11 15:32:56.000000000 +0100
> +++ linux-dm/drivers/md/dm.c	2022-02-11 15:40:11.000000000 +0100
> @@ -487,16 +487,20 @@ EXPORT_SYMBOL_GPL(dm_start_time_ns_from_
>  
>  static void start_io_acct(struct dm_io *io, struct bio *bio)
>  {
> +	struct bio *orig_bio;
> +
>  	/* Ensure IO accounting is only ever started once */
>  	if (xchg(&io->was_accounted, 1) == 1)
>  		return;
>  
> +	orig_bio = io->orig_bio;
> +
>  	bio_start_io_acct_remapped(bio, io->start_time,
> -				   io->orig_bio->bi_bdev);
> +				   orig_bio->bi_bdev);
>  
>  	if (unlikely(dm_stats_used(&io->md->stats)))
> -		dm_stats_account_io(&io->md->stats, bio_data_dir(bio),
> -				    bio->bi_iter.bi_sector, bio_sectors(bio),
> +		dm_stats_account_io(&io->md->stats, bio_data_dir(orig_bio),
> +				    orig_bio->bi_iter.bi_sector, bio_sectors(orig_bio),
>  				    false, 0, &io->stats_aux);
>  }
>  
> 

You cannot account using the orig_bio because it doesn't reflect
splits that occurred.  orig_bio will be updated to reflect the split
after __map_bio returns all the way back to dm_split_and_process_bio.

So there is no imbalance between start_io_acct and end_io_acct's calls
to dm_stats_account_io.

Also, in_flight counters don't (or shouldn't) care about the payload.

So again, I'm not following the reasoning on this patch.

Mike

--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel


      reply	other threads:[~2022-02-11 16:08 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-11 15:01 [dm-devel] [PATCH] dm: fix dm_stats accounting Mikulas Patocka
2022-02-11 16:08 ` Mike Snitzer [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YgaJ5bqSIR+OtYJt@redhat.com \
    --to=snitzer@redhat.com \
    --cc=dm-devel@redhat.com \
    --cc=mpatocka@redhat.com \
    --cc=msnitzer@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox