block: switch to per-cpu in-flight counters

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 <[email protected]>

---
 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(-)

Index: linux-block/drivers/md/dm.c
===================================================================

drivers/md/dm.c +1 -2

@@ -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),

Quoted 22 lines · Mikulas Patocka

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 <[email protected]>

---
 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 16 lines

Index: linux-block/drivers/md/dm.c
===================================================================

drivers/md/dm.c +1 -2

@@ -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),

drivers/md/dm.c static void end_io_acct(struct dm_io *io · line 697

Quoted 7 lines

drivers/md/dm.c +0 -1

@@ -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

Quoted 60 lines · Mike Snitzer

Quoted 22 lines · Mikulas Patocka

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 <[email protected]>

---
 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 16 lines

Index: linux-block/drivers/md/dm.c
===================================================================

drivers/md/dm.c +1 -2

@@ -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),

drivers/md/dm.c static void end_io_acct(struct dm_io *io · line 697

Quoted 7 lines

drivers/md/dm.c +0 -1

@@ -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.

Quoted 1 line

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.

Quoted 60 lines · Mike Snitzer

Quoted 22 lines · Mikulas Patocka

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 <[email protected]>

---
 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 16 lines

Index: linux-block/drivers/md/dm.c
===================================================================

drivers/md/dm.c +1 -2

@@ -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),

drivers/md/dm.c static void end_io_acct(struct dm_io *io · line 697

Quoted 7 lines

drivers/md/dm.c +0 -1

@@ -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.

Quoted 1 line

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.

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 <[email protected]>

---
 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(-)

Index: linux-block/block/genhd.c
===================================================================

block/genhd.c +4 -4

@@ -45,53 +45,76 @@ static void disk_add_events(struct gendi
 static void disk_del_events(struct gendisk *disk);
 static void disk_release_events(struct gendisk *disk);
 
-void part_inc_in_flight(struct request_queue *q, struct hd_struct *part, int rw)
+void part_inc_in_flight(struct request_queue *q, int cpu, struct hd_struct *part, int rw)
 {
 	if (queue_is_mq(q))
 		return;
 
-	atomic_inc(&part->in_flight[rw]);
+	local_inc(per_cpu_ptr(part->dkstats, cpu)->in_flight);
 	if (part->partno)
-		atomic_inc(&part_to_disk(part)->part0.in_flight[rw]);
+		local_inc(per_cpu_ptr(part_to_disk(part)->part0.dkstats, cpu)->in_flight);
 }
 
-void part_dec_in_flight(struct request_queue *q, struct hd_struct *part, int rw)
+void part_dec_in_flight(struct request_queue *q, int cpu, struct hd_struct *part, int rw)

Quoted 49 lines · Mikulas Patocka

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 <[email protected]>

---
 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(-)

Index: linux-block/block/genhd.c
===================================================================

block/genhd.c +4 -4

@@ -45,53 +45,76 @@ static void disk_add_events(struct gendi
 static void disk_del_events(struct gendisk *disk);
 static void disk_release_events(struct gendisk *disk);
 
-void part_inc_in_flight(struct request_queue *q, struct hd_struct *part, int rw)
+void part_inc_in_flight(struct request_queue *q, int cpu, struct hd_struct *part, int rw)
 {
 	if (queue_is_mq(q))
 		return;
 
-	atomic_inc(&part->in_flight[rw]);
+	local_inc(per_cpu_ptr(part->dkstats, cpu)->in_flight);
 	if (part->partno)
-		atomic_inc(&part_to_disk(part)->part0.in_flight[rw]);
+		local_inc(per_cpu_ptr(part_to_disk(part)->part0.dkstats, cpu)->in_flight);
 }
 
-void part_dec_in_flight(struct request_queue *q, struct hd_struct *part, int rw)
+void part_dec_in_flight(struct request_queue *q, int cpu, struct hd_struct *part, int rw)
 {
 	if (queue_is_mq(q))
 		return;
 

block/genhd.c static void disk_add_events(struct gendi · line 66

Quoted 6 lines

block/genhd.c +2 -2

-	atomic_dec(&part->in_flight[rw]);
+	local_dec(per_cpu_ptr(part->dkstats, cpu)->in_flight);
 	if (part->partno)
-		atomic_dec(&part_to_disk(part)->part0.in_flight[rw]);
+		local_dec(per_cpu_ptr(part_to_disk(part)->part0.dkstats, cpu)->in_flight);
 }

The above is wrong, needs this incremental fix:

Quoted 52 lines · Mike Snitzer

Quoted 49 lines · Mikulas Patocka

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 <[email protected]>

---
 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(-)

Index: linux-block/block/genhd.c
===================================================================

block/genhd.c +4 -4

@@ -45,53 +45,76 @@ static void disk_add_events(struct gendi
 static void disk_del_events(struct gendisk *disk);
 static void disk_release_events(struct gendisk *disk);
 
-void part_inc_in_flight(struct request_queue *q, struct hd_struct *part, int rw)
+void part_inc_in_flight(struct request_queue *q, int cpu, struct hd_struct *part, int rw)
 {
 	if (queue_is_mq(q))
 		return;
 
-	atomic_inc(&part->in_flight[rw]);
+	local_inc(per_cpu_ptr(part->dkstats, cpu)->in_flight);
 	if (part->partno)
-		atomic_inc(&part_to_disk(part)->part0.in_flight[rw]);
+		local_inc(per_cpu_ptr(part_to_disk(part)->part0.dkstats, cpu)->in_flight);
 }
 
-void part_dec_in_flight(struct request_queue *q, struct hd_struct *part, int rw)
+void part_dec_in_flight(struct request_queue *q, int cpu, struct hd_struct *part, int rw)
 {
 	if (queue_is_mq(q))
 		return;
 

Quoted 9 lines

block/genhd.c static void disk_add_events(struct gendi · line 66

Quoted 6 lines

block/genhd.c +2 -2

-	atomic_dec(&part->in_flight[rw]);
+	local_dec(per_cpu_ptr(part->dkstats, cpu)->in_flight);
 	if (part->partno)
-		atomic_dec(&part_to_disk(part)->part0.in_flight[rw]);
+		local_dec(per_cpu_ptr(part_to_disk(part)->part0.dkstats, cpu)->in_flight);
 }

The above is wrong, needs this incremental fix:

Should also be using this_cpu_inc/dec instead.