Thread (8 messages) 8 messages, 3 authors, 2018-11-30

Re: [PATCH 2/3] block: switch to per-cpu in-flight counters

From: Mike Snitzer <hidden>
Date: 2018-11-30 00:21:53
Also in: dm-devel

On Thu, Nov 29 2018 at  5:05pm -0500,
Mikulas Patocka [off-list ref] wrote:

On Thu, 29 Nov 2018, Mike Snitzer wrote:
quoted
On Tue, Nov 27 2018 at  7:42pm -0500,
Mikulas Patocka [off-list ref] wrote:
quoted
Now when part_round_stats is gone, we can switch to per-cpu in-flight
counters.

We use the local-atomic type local_t, so that if part_inc_in_flight or
part_dec_in_flight is reentrantly called from an interrupt, the value will
be correct.

The other counters could be corrupted due to reentrant interrupt, but the
corruption only results in slight counter skew - the in_flight counter
must be exact, so it needs local_t.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

---
 block/bio.c           |    4 ++--
 block/blk-core.c      |    4 ++--
 block/blk-merge.c     |    2 +-
 block/genhd.c         |   47 +++++++++++++++++++++++++++++++++++------------
 drivers/md/dm.c       |    4 +---
 include/linux/genhd.h |    7 ++++---
 6 files changed, 45 insertions(+), 23 deletions(-)
...
quoted
Index: linux-block/drivers/md/dm.c
===================================================================
--- linux-block.orig/drivers/md/dm.c	2018-11-28 00:09:59.000000000 +0100
+++ linux-block/drivers/md/dm.c	2018-11-28 00:09:59.000000000 +0100
@@ -663,8 +663,7 @@ static void start_io_acct(struct dm_io *
 	generic_start_io_acct(md->queue, bio_op(bio), bio_sectors(bio),
 			      &dm_disk(md)->part0);
 
-	atomic_set(&dm_disk(md)->part0.in_flight[rw],
-		   atomic_inc_return(&md->pending[rw]));
+	atomic_inc(&md->pending[rw]);
 
 	if (unlikely(dm_stats_used(&md->stats)))
 		dm_stats_account_io(&md->stats, bio_data_dir(bio),
@@ -693,7 +692,6 @@ static void end_io_acct(struct dm_io *io
 	 * a flush.
 	 */
 	pending = atomic_dec_return(&md->pending[rw]);
-	atomic_set(&dm_disk(md)->part0.in_flight[rw], pending);
 	pending += atomic_read(&md->pending[rw^0x1]);
 
 	/* nudge anyone waiting on suspend queue */

These dm.c hunks conflict with changes from you that I already staged in
dm-4.21, see:

https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.21&id=b5616f7a11592cc74860f4ec3e3c4fba6688eefa
https://git.kernel.org/pub/scm/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.21&id=78f95b53c203c969bbe6b86e405f7a891a43b6be

I'd really like to get away from DM maintaining its own ->pending
counters.

Mike
I know.

It depends on whether Jens takes these patches or not. If he doesn't take 
them, dm will use percpu counters on its own and it will return zero in 
the "/sys/block/dm-*/inflight" file.
I see, yeah, I could easily rebase the changes I referenced.

I had a quick look at your proposed changes, they seem pretty clean to
me.  The fact that disk_stats is already percpu for other counters makes
this change straight forward.

But I'll review closer.

Mike
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help