public inbox for linux-block@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] block: fix false warning in bdev_count_inflight_rw()
@ 2025-06-26 11:57 Yu Kuai
  2025-06-26 13:33 ` Christoph Hellwig
  2025-06-26 13:35 ` Jens Axboe
  0 siblings, 2 replies; 3+ messages in thread
From: Yu Kuai @ 2025-06-26 11:57 UTC (permalink / raw)
  To: axboe, hare, hch, john.g.garry, yukuai3
  Cc: linux-block, linux-kernel, yi.zhang, calvin, david, yukuai1,
	yi.zhang, yangerkun, johnny.chenyi

While bdev_count_inflight is interating all cpus, if some IOs are issued
from traversed cpu and then completed from the cpu that is not traversed
yet:

cpu0
		cpu1
		bdev_count_inflight
		 //for_each_possible_cpu
		 // cpu0 is 0
		 infliht += 0
// issue a io
blk_account_io_start
// cpu0 inflight ++

				cpu2
				// the io is done
				blk_account_io_done
				// cpu2 inflight --
		 // cpu 1 is 0
		 inflight += 0
		 // cpu2 is -1
		 inflight += -1
		 ...

In this case, the total inflight will be -1, causing lots of false
warning. Fix the problem by removing the warning.

Noted there is still a valid warning for nvme-mpath(From Yi) that is not
fixed yet.

Fixes: f5482ee5edb9 ("block: WARN if bdev inflight counter is negative")
Reported-by: Yi Zhang <yi.zhang@redhat.com>
Closes: https://lore.kernel.org/linux-block/aFtUXy-lct0WxY2w@mozart.vkv.me/T/#mae89155a5006463d0a21a4a2c35ae0034b26a339
Reported-and-tested-by: Calvin Owens <calvin@wbinvd.org>
Closes: https://lore.kernel.org/linux-block/aFtUXy-lct0WxY2w@mozart.vkv.me/T/#m1d935a00070bf95055d0ac84e6075158b08acaef
Reported-by: Dave Chinner <david@fromorbit.com>
Closes: https://lore.kernel.org/linux-block/aFuypjqCXo9-5_En@dread.disaster.area/
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
Changes in v2:
 - fix comments grammar;
 - add signed int for percpu summation to prevent overflow;

 block/genhd.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/block/genhd.c b/block/genhd.c
index 8171a6bc3210..c26733f6324b 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -128,23 +128,27 @@ static void part_stat_read_all(struct block_device *part,
 static void bdev_count_inflight_rw(struct block_device *part,
 		unsigned int inflight[2], bool mq_driver)
 {
+	int write = 0;
+	int read = 0;
 	int cpu;
 
 	if (mq_driver) {
 		blk_mq_in_driver_rw(part, inflight);
-	} else {
-		for_each_possible_cpu(cpu) {
-			inflight[READ] += part_stat_local_read_cpu(
-						part, in_flight[READ], cpu);
-			inflight[WRITE] += part_stat_local_read_cpu(
-						part, in_flight[WRITE], cpu);
-		}
+		return;
+	}
+
+	for_each_possible_cpu(cpu) {
+		read += part_stat_local_read_cpu(part, in_flight[READ], cpu);
+		write += part_stat_local_read_cpu(part, in_flight[WRITE], cpu);
 	}
 
-	if (WARN_ON_ONCE((int)inflight[READ] < 0))
-		inflight[READ] = 0;
-	if (WARN_ON_ONCE((int)inflight[WRITE] < 0))
-		inflight[WRITE] = 0;
+	/*
+	 * While iterating all CPUs, some IOs may be issued from a CPU already
+	 * traversed and complete on a CPU that has not yet been traversed,
+	 * causing the inflight number to be negative.
+	 */
+	inflight[READ] = read > 0 ? read : 0;
+	inflight[WRITE] = write > 0 ? write : 0;
 }
 
 /**
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] block: fix false warning in bdev_count_inflight_rw()
  2025-06-26 11:57 [PATCH v2] block: fix false warning in bdev_count_inflight_rw() Yu Kuai
@ 2025-06-26 13:33 ` Christoph Hellwig
  2025-06-26 13:35 ` Jens Axboe
  1 sibling, 0 replies; 3+ messages in thread
From: Christoph Hellwig @ 2025-06-26 13:33 UTC (permalink / raw)
  To: Yu Kuai
  Cc: axboe, hare, hch, john.g.garry, linux-block, linux-kernel,
	yi.zhang, calvin, david, yukuai1, yi.zhang, yangerkun,
	johnny.chenyi

On Thu, Jun 26, 2025 at 07:57:43PM +0800, Yu Kuai wrote:
>  static void bdev_count_inflight_rw(struct block_device *part,
>  		unsigned int inflight[2], bool mq_driver)

Why not pass the inflight arguments as signed? (and split it for
reads and writes as you did below)

Anyway, I think this is good enough to get a fix queued up ASAP,
but we can probably clean it up incrementally:

Reviewed-by: Christoph Hellwig <hch@lst.de>


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v2] block: fix false warning in bdev_count_inflight_rw()
  2025-06-26 11:57 [PATCH v2] block: fix false warning in bdev_count_inflight_rw() Yu Kuai
  2025-06-26 13:33 ` Christoph Hellwig
@ 2025-06-26 13:35 ` Jens Axboe
  1 sibling, 0 replies; 3+ messages in thread
From: Jens Axboe @ 2025-06-26 13:35 UTC (permalink / raw)
  To: hare, hch, john.g.garry, Yu Kuai
  Cc: linux-block, linux-kernel, yi.zhang, calvin, david, yukuai1,
	yi.zhang, yangerkun, johnny.chenyi


On Thu, 26 Jun 2025 19:57:43 +0800, Yu Kuai wrote:
> While bdev_count_inflight is interating all cpus, if some IOs are issued
> from traversed cpu and then completed from the cpu that is not traversed
> yet:
> 
> cpu0
> 		cpu1
> 		bdev_count_inflight
> 		 //for_each_possible_cpu
> 		 // cpu0 is 0
> 		 infliht += 0
> // issue a io
> blk_account_io_start
> // cpu0 inflight ++
> 
> [...]

Applied, thanks!

[1/1] block: fix false warning in bdev_count_inflight_rw()
      commit: c007062188d8e402c294117db53a24b2bed2b83f

Best regards,
-- 
Jens Axboe




^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-06-26 13:35 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-26 11:57 [PATCH v2] block: fix false warning in bdev_count_inflight_rw() Yu Kuai
2025-06-26 13:33 ` Christoph Hellwig
2025-06-26 13:35 ` Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox