[PATCH] blk-cgroup-rwstat: add missing DISCARD counter in blkg_rwstat_total()

[PATCH] blk-cgroup-rwstat: add missing DISCARD counter in blkg_rwstat_total()

[Tao Cui]

Since commit 636620b66d5d ("blkcg: Track DISCARD statistics and output
them in cgroup io.stat"), blkg_rwstat_add() routes discard IOs into
BLKG_RWSTAT_DISCARD, but blkg_rwstat_total() still only sums READ and
WRITE.  This makes the total inconsistent with the per-type counters
whenever discard IOs are present.

On mixed read/write/discard workloads, the total undercounts the
actual IO activity.  If only discard requests are queued, the total
returns 0 outright.  In BFQ this causes avg_queue_size to be
underestimated and the group to be incorrectly marked empty.

The affected BFQ code paths are all gated by CONFIG_BFQ_CGROUP_DEBUG
and used only for statistics display (bfq.sectors, bfq.avg_queue_size,
bfq.empty_time).  No scheduling decisions depend on these values.

Signed-off-by: Tao Cui <cuitao@kylinos.cn>
---
 block/blk-cgroup-rwstat.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/blk-cgroup-rwstat.h b/block/blk-cgroup-rwstat.h
index 703a16fe1404..b7908d877e2b 100644
--- a/block/blk-cgroup-rwstat.h
+++ b/block/blk-cgroup-rwstat.h
@@ -110,7 +110,8 @@ static inline uint64_t blkg_rwstat_total(struct blkg_rwstat *rwstat)
 	struct blkg_rwstat_sample tmp = { };
 
 	blkg_rwstat_read(rwstat, &tmp);
-	return tmp.cnt[BLKG_RWSTAT_READ] + tmp.cnt[BLKG_RWSTAT_WRITE];
+	return tmp.cnt[BLKG_RWSTAT_READ] + tmp.cnt[BLKG_RWSTAT_WRITE] +
+	       tmp.cnt[BLKG_RWSTAT_DISCARD];
 }
 
 /**
-- 
2.43.0

Re: [PATCH] blk-cgroup-rwstat: add missing DISCARD counter in blkg_rwstat_total()

[Tejun Heo]

On Sat, May 16, 2026 at 12:47:52PM +0800, Tao Cui wrote:
> Since commit 636620b66d5d ("blkcg: Track DISCARD statistics and output
> them in cgroup io.stat"), blkg_rwstat_add() routes discard IOs into
> BLKG_RWSTAT_DISCARD, but blkg_rwstat_total() still only sums READ and
> WRITE.  This makes the total inconsistent with the per-type counters
> whenever discard IOs are present.
> 
> On mixed read/write/discard workloads, the total undercounts the
> actual IO activity.  If only discard requests are queued, the total
> returns 0 outright.  In BFQ this causes avg_queue_size to be
> underestimated and the group to be incorrectly marked empty.
> 
> The affected BFQ code paths are all gated by CONFIG_BFQ_CGROUP_DEBUG
> and used only for statistics display (bfq.sectors, bfq.avg_queue_size,
> bfq.empty_time).  No scheduling decisions depend on these values.

I don't think including discard in total io amount is an improvement. This
will throw off a lot of people.

Thanks.

-- 
tejun

Re: [PATCH] blk-cgroup-rwstat: add missing DISCARD counter in blkg_rwstat_total()

[Tao Cui]

在 2026/5/19 3:19, Tejun Heo 写道:
> On Sat, May 16, 2026 at 12:47:52PM +0800, Tao Cui wrote:
>> Since commit 636620b66d5d ("blkcg: Track DISCARD statistics and output
>> them in cgroup io.stat"), blkg_rwstat_add() routes discard IOs into
>> BLKG_RWSTAT_DISCARD, but blkg_rwstat_total() still only sums READ and
>> WRITE.  This makes the total inconsistent with the per-type counters
>> whenever discard IOs are present.
>>
>> On mixed read/write/discard workloads, the total undercounts the
>> actual IO activity.  If only discard requests are queued, the total
>> returns 0 outright.  In BFQ this causes avg_queue_size to be
>> underestimated and the group to be incorrectly marked empty.
>>
>> The affected BFQ code paths are all gated by CONFIG_BFQ_CGROUP_DEBUG
>> and used only for statistics display (bfq.sectors, bfq.avg_queue_size,
>> bfq.empty_time).  No scheduling decisions depend on these values.
> 
> I don't think including discard in total io amount is an improvement. This
> will throw off a lot of people.
> 
Thanks for the review. 
You're right - for stats.bytes (displayed as
bfq.sectors), including discard doesn't make semantic sense since
discard doesn't transfer data.

The original motivation was the stats.queued case in BFQ: when only
discard requests are pending, blkg_rwstat_total() returns 0, so
bfqg_stats_set_start_empty_time() incorrectly marks the group as empty.
However, this code path is gated by CONFIG_BFQ_CGROUP_DEBUG and is
purely for debug display, so the practical impact is minimal.

I'll drop this patch.

Thanks.
--
Tao