[v4,0/7] per-cpu in_flight counters for bio-based drivers

Message ID	20181206164122.2166-1-snitzer@redhat.com (mailing list archive)
Headers	show Return-Path: <linux-block-owner@kernel.org> From: Mike Snitzer <snitzer@redhat.com> To: Jens Axboe <axboe@kernel.dk> Cc: linux-block@vger.kernel.org, dm-devel@redhat.com, Mikulas Patocka <mpatocka@redhat.com> Subject: [PATCH v4 0/7] per-cpu in_flight counters for bio-based drivers Date: Thu, 6 Dec 2018 11:41:15 -0500 Message-Id: <20181206164122.2166-1-snitzer@redhat.com> Sender: linux-block-owner@vger.kernel.org Precedence: bulk
Series	per-cpu in_flight counters for bio-based drivers \| expand [v4,0/7] per-cpu in_flight counters for bio-based drivers [v4,1/7] dm: dont rewrite dm_disk(md)->part0.in_flight [v4,2/7] dm rq: leverage blk_mq_queue_busy() to check for outstanding IO [v4,3/7] block: stop passing 'cpu' to all percpu stats methods [v4,4/7] block: delete part_round_stats and switch to less precise counting [v4,5/7] block: switch to per-cpu in-flight counters [v4,6/7] block: return just one value from part_in_flight [v4,7/7] dm: remove the pending IO accounting

Message ID

20181206164122.2166-1-snitzer@redhat.com (mailing list archive)

Headers

From: Mike Snitzer <snitzer@redhat.com>
To: Jens Axboe <axboe@kernel.dk>
Cc: linux-block@vger.kernel.org, dm-devel@redhat.com,
        Mikulas Patocka <mpatocka@redhat.com>
Subject: [PATCH v4 0/7] per-cpu in_flight counters for bio-based drivers
Date: Thu,  6 Dec 2018 11:41:15 -0500
Message-Id: <20181206164122.2166-1-snitzer@redhat.com>
Sender: linux-block-owner@vger.kernel.org
Precedence: bulk

Series

per-cpu in_flight counters for bio-based drivers | expand

Message

Mike Snitzer Dec. 6, 2018, 4:41 p.m. UTC

Hey,

This v4 addresses the compile issues on various archs when CONFIG_SMP
isn't set (by introducing appropriate wrappers in genhd.h)

Testing with this v4 I was unable to reproduce the issue you reported
where iostat always reports 0 for avgqu-sz -- but please let me know
if you still see problems like that.

Thanks,
Mike

Mike Snitzer (2):
  dm rq: leverage blk_mq_queue_busy() to check for outstanding IO
  block: stop passing 'cpu' to all percpu stats methods

Mikulas Patocka (5):
  dm: dont rewrite dm_disk(md)->part0.in_flight
  block: delete part_round_stats and switch to less precise counting
  block: switch to per-cpu in-flight counters
  block: return just one value from part_in_flight
  dm: remove the pending IO accounting

 block/bio.c               | 36 ++++++++++++++++-----
 block/blk-core.c          | 80 +++++++----------------------------------------
 block/blk-merge.c         |  4 +--
 block/blk-mq.c            | 12 +++----
 block/blk-mq.h            |  3 +-
 block/genhd.c             | 55 ++++++++++++++++++--------------
 block/partition-generic.c | 10 ++----
 drivers/md/dm-core.h      |  2 --
 drivers/md/dm-rq.c        |  9 +++---
 drivers/md/dm.c           | 36 +++++++++------------
 drivers/md/md.c           |  7 ++---
 include/linux/genhd.h     | 55 +++++++++++++++++++-------------
 12 files changed, 137 insertions(+), 172 deletions(-)

Comments

Mike Snitzer Dec. 6, 2018, 6 p.m. UTC | #1

On Thu, Dec 06 2018 at 11:41am -0500,
Mike Snitzer <snitzer@redhat.com> wrote:

> Hey,
> 
> This v4 addresses the compile issues on various archs when CONFIG_SMP
> isn't set (by introducing appropriate wrappers in genhd.h)
> 
> Testing with this v4 I was unable to reproduce the issue you reported
> where iostat always reports 0 for avgqu-sz -- but please let me know
> if you still see problems like that.

I ran blktests and both block/017 and block/018 fail due to this
patchset's changes, I'm still trying to grok what the test result is
_saying_ but I'll hopefully get there soon:

block/017 (do I/O and check the inflight counter)            [failed]
    runtime    ...  77.675s
    --- tests/block/017.out     2018-12-06 12:31:21.457936217 -0500
    +++ /home/git/blktests/results/nodev/block/017.out.bad      2018-12-06 12:36:15.735053586 -0500
    @@ -1,14 +1,14 @@
     Running block/017
    -sysfs inflight reads 1
    +sysfs inflight reads 2
     sysfs inflight writes 0
    -sysfs stat 1
    -diskstats 1
    -sysfs inflight reads 1
    ...
    (Run 'diff -u tests/block/017.out /home/git/blktests/results/nodev/block/017.out.bad' to see the entire diff)

# diff -u tests/block/017.out /home/git/blktests/results/nodev/block/017.out.bad
--- tests/block/017.out 2018-12-06 12:31:21.457936217 -0500
+++ /home/git/blktests/results/nodev/block/017.out.bad  2018-12-06 12:51:36.695542332 -0500
@@ -1,14 +1,14 @@
 Running block/017
-sysfs inflight reads 1
+sysfs inflight reads 2
 sysfs inflight writes 0
-sysfs stat 1
-diskstats 1
-sysfs inflight reads 1
-sysfs inflight writes 1
 sysfs stat 2
 diskstats 2
-sysfs inflight reads 0
+sysfs inflight reads 2
+sysfs inflight writes 1
+sysfs stat 3
+diskstats 3
+sysfs inflight reads 1
 sysfs inflight writes 0
-sysfs stat 0
-diskstats 0
+sysfs stat 1
+diskstats 1
 Test complete

block/018 (do I/O and check iostats times)                   [failed]
    runtime    ...  156.073s
    --- tests/block/018.out     2018-12-06 12:31:21.458936217 -0500
    +++ /home/git/blktests/results/nodev/block/018.out.bad      2018-12-06 12:38:51.831116369 -0500
    @@ -1,10 +1,10 @@
     Running block/018
     read 0 s
     write 0 s
    -read 1 s
    +read 2 s
     write 0 s
    -read 1 s
    ...
    (Run 'diff -u tests/block/018.out /home/git/blktests/results/nodev/block/018.out.bad' to see the entire diff)

# diff -u tests/block/018.out /home/git/blktests/results/nodev/block/018.out.bad
--- tests/block/018.out 2018-12-06 12:31:21.458936217 -0500
+++ /home/git/blktests/results/nodev/block/018.out.bad  2018-12-06 12:38:51.831116369 -0500
@@ -1,10 +1,10 @@
 Running block/018
 read 0 s
 write 0 s
-read 1 s
+read 2 s
 write 0 s
-read 1 s
-write 1 s
 read 2 s
+write 1 s
+read 6 s
 write 3 s
 Test complete

Mike Snitzer Dec. 6, 2018, 7:04 p.m. UTC | #2

On Thu, Dec 06 2018 at  1:00pm -0500,
Mike Snitzer <snitzer@redhat.com> wrote:

> On Thu, Dec 06 2018 at 11:41am -0500,
> Mike Snitzer <snitzer@redhat.com> wrote:
> 
> > Hey,
> > 
> > This v4 addresses the compile issues on various archs when CONFIG_SMP
> > isn't set (by introducing appropriate wrappers in genhd.h)
> > 
> > Testing with this v4 I was unable to reproduce the issue you reported
> > where iostat always reports 0 for avgqu-sz -- but please let me know
> > if you still see problems like that.
> 
> I ran blktests and both block/017 and block/018 fail due to this
> patchset's changes, I'm still trying to grok what the test result is
> _saying_ but I'll hopefully get there soon:
> 
> block/017 (do I/O and check the inflight counter)            [failed]
>     runtime    ...  77.675s
>     --- tests/block/017.out     2018-12-06 12:31:21.457936217 -0500
>     +++ /home/git/blktests/results/nodev/block/017.out.bad      2018-12-06 12:36:15.735053586 -0500
>     @@ -1,14 +1,14 @@
>      Running block/017
>     -sysfs inflight reads 1
>     +sysfs inflight reads 2
>      sysfs inflight writes 0
>     -sysfs stat 1
>     -diskstats 1
>     -sysfs inflight reads 1
>     ...
>     (Run 'diff -u tests/block/017.out /home/git/blktests/results/nodev/block/017.out.bad' to see the entire diff)
> 
> # diff -u tests/block/017.out /home/git/blktests/results/nodev/block/017.out.bad
> --- tests/block/017.out 2018-12-06 12:31:21.457936217 -0500
> +++ /home/git/blktests/results/nodev/block/017.out.bad  2018-12-06 12:51:36.695542332 -0500
> @@ -1,14 +1,14 @@
>  Running block/017
> -sysfs inflight reads 1
> +sysfs inflight reads 2
>  sysfs inflight writes 0
> -sysfs stat 1
> -diskstats 1
> -sysfs inflight reads 1
> -sysfs inflight writes 1
>  sysfs stat 2
>  diskstats 2
> -sysfs inflight reads 0
> +sysfs inflight reads 2
> +sysfs inflight writes 1
> +sysfs stat 3
> +diskstats 3
> +sysfs inflight reads 1
>  sysfs inflight writes 0
> -sysfs stat 0
> -diskstats 0
> +sysfs stat 1
> +diskstats 1
>  Test complete
> 

Interestingly the 017 test is only testing blk-mq (using null_blk's
queue_mode=2).  This patchset's changes are specific to bio-based.

If I revert this patchset I still get the failures.

So it would seem something regressed in latest block for-next (I've been
testing up through linux-block commit c754a9bf7ee8 ("blk-mq: remove
QUEUE_FLAG_POLL from default MQ flags").

> block/018 (do I/O and check iostats times)                   [failed]
>     runtime    ...  156.073s
>     --- tests/block/018.out     2018-12-06 12:31:21.458936217 -0500
>     +++ /home/git/blktests/results/nodev/block/018.out.bad      2018-12-06 12:38:51.831116369 -0500
>     @@ -1,10 +1,10 @@
>      Running block/018
>      read 0 s
>      write 0 s
>     -read 1 s
>     +read 2 s
>      write 0 s
>     -read 1 s
>     ...
>     (Run 'diff -u tests/block/018.out /home/git/blktests/results/nodev/block/018.out.bad' to see the entire diff)
> 
> # diff -u tests/block/018.out /home/git/blktests/results/nodev/block/018.out.bad
> --- tests/block/018.out 2018-12-06 12:31:21.458936217 -0500
> +++ /home/git/blktests/results/nodev/block/018.out.bad  2018-12-06 12:38:51.831116369 -0500
> @@ -1,10 +1,10 @@
>  Running block/018
>  read 0 s
>  write 0 s
> -read 1 s
> +read 2 s
>  write 0 s
> -read 1 s
> -write 1 s
>  read 2 s
> +write 1 s
> +read 6 s
>  write 3 s
>  Test complete

Same goes for this block/018 test.

Mike

Jens Axboe Dec. 10, 2018, 3:35 p.m. UTC | #3

On Thu, Dec 6, 2018 at 9:41 AM Mike Snitzer <snitzer@redhat.com> wrote:
>
> Hey,
>
> This v4 addresses the compile issues on various archs when CONFIG_SMP
> isn't set (by introducing appropriate wrappers in genhd.h)
>
> Testing with this v4 I was unable to reproduce the issue you reported
> where iostat always reports 0 for avgqu-sz -- but please let me know
> if you still see problems like that.

Looks good to me, and also passes my testing. Dropped the mq special
cases and used this in general, also works fine. I was hoping we could
get rid of the tag iteration with this, but that still seems to be more
efficient for blk-mq. But at least we're better off for stacked devices
with this.