[PATCH v2] blk-cgroup: always display debug stats in io.stat

[PATCH v2] blk-cgroup: always display debug stats in io.stat

[Breno Leitao]

Remove the blkcg_debug_stats toggle and always display detailed
statistics in the cgroup io.stat file. This includes use_delay and
delay_nsec information, cost.wait/cost.indebt/cost.indelay for iocost,
and latency statistics for iolatency.

The stats are already being collected regardless of the toggle, so
gating their display provides no real benefit. Additionally, blk-cgroup
has not been modularized since commit 32e380aedc3de ("blkcg: make
CONFIG_BLK_CGROUP bool"), making the module parameter a historical
artifact. Readers of the nested-keys format should be able to handle
additional fields.

Before (without blkcg_debug_stats enabled):
  253:0 rbytes=6273024 wbytes=0 rios=20 wios=0 dbytes=0 dios=0 cost.usage=0

After:
  253:0 rbytes=6273024 wbytes=0 rios=20 wios=0 dbytes=0 dios=0 cost.usage=0 cost.wait=0 cost.indebt=0 cost.indelay=0

Suggested-by: Michal Koutný <mkoutny@suse.com>
Signed-off-by: Breno Leitao <leitao@debian.org>
---
Changes in v2:
- Remove blkcg_debug_stats instead of creating a CONFIG_ to set it. (Michal)
- Link to v1: https://patch.msgid.link/20260204-blk_cgroup_debug_stats-v1-1-09c0754b4242@debian.org
---
 block/blk-cgroup.c    | 6 +-----
 block/blk-cgroup.h    | 1 -
 block/blk-iocost.c    | 9 ++++-----
 block/blk-iolatency.c | 3 ---
 4 files changed, 5 insertions(+), 14 deletions(-)

diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index b70096497d389..8b89a1ce6927a 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -56,8 +56,6 @@ static struct blkcg_policy *blkcg_policy[BLKCG_MAX_POLS];
 
 static LIST_HEAD(all_blkcgs);		/* protected by blkcg_pol_mutex */
 
-bool blkcg_debug_stats = false;
-
 static DEFINE_RAW_SPINLOCK(blkg_stat_lock);
 
 #define BLKG_DESTROY_BATCH_SIZE  64
@@ -1209,7 +1207,7 @@ static void blkcg_print_one_stat(struct blkcg_gq *blkg, struct seq_file *s)
 			dbytes, dios);
 	}
 
-	if (blkcg_debug_stats && atomic_read(&blkg->use_delay)) {
+	if (atomic_read(&blkg->use_delay)) {
 		seq_printf(s, " use_delay=%d delay_nsec=%llu",
 			atomic_read(&blkg->use_delay),
 			atomic64_read(&blkg->delay_nsec));
@@ -2246,5 +2244,3 @@ bool blk_cgroup_congested(void)
 	return ret;
 }
 
-module_param(blkcg_debug_stats, bool, 0644);
-MODULE_PARM_DESC(blkcg_debug_stats, "True if you want debug stats, false if not");
diff --git a/block/blk-cgroup.h b/block/blk-cgroup.h
index 1cce3294634d1..ac38bc3e2486b 100644
--- a/block/blk-cgroup.h
+++ b/block/blk-cgroup.h
@@ -189,7 +189,6 @@ struct blkcg_policy {
 };
 
 extern struct blkcg blkcg_root;
-extern bool blkcg_debug_stats;
 
 void blkg_init_queue(struct request_queue *q);
 int blkcg_init_disk(struct gendisk *disk);
diff --git a/block/blk-iocost.c b/block/blk-iocost.c
index d145db61e5c31..a913e67d0f695 100644
--- a/block/blk-iocost.c
+++ b/block/blk-iocost.c
@@ -3060,11 +3060,10 @@ static void ioc_pd_stat(struct blkg_policy_data *pd, struct seq_file *s)
 
 	seq_printf(s, " cost.usage=%llu", iocg->last_stat.usage_us);
 
-	if (blkcg_debug_stats)
-		seq_printf(s, " cost.wait=%llu cost.indebt=%llu cost.indelay=%llu",
-			iocg->last_stat.wait_us,
-			iocg->last_stat.indebt_us,
-			iocg->last_stat.indelay_us);
+	seq_printf(s, " cost.wait=%llu cost.indebt=%llu cost.indelay=%llu",
+		iocg->last_stat.wait_us,
+		iocg->last_stat.indebt_us,
+		iocg->last_stat.indelay_us);
 }
 
 static u64 ioc_weight_prfill(struct seq_file *sf, struct blkg_policy_data *pd,
diff --git a/block/blk-iolatency.c b/block/blk-iolatency.c
index 53e8dd2dfa8ad..e99abc97050a0 100644
--- a/block/blk-iolatency.c
+++ b/block/blk-iolatency.c
@@ -946,9 +946,6 @@ static void iolatency_pd_stat(struct blkg_policy_data *pd, struct seq_file *s)
 	unsigned long long avg_lat;
 	unsigned long long cur_win;
 
-	if (!blkcg_debug_stats)
-		return;
-
 	if (iolat->ssd)
 		return iolatency_ssd_stat(iolat, s);
 

---
base-commit: d517cb8cea012f43b069617fc8179b45404f8018
change-id: 20260204-blk_cgroup_debug_stats-e81b887f9c30

Best regards,
--  
Breno Leitao <leitao@debian.org>

Re: [PATCH v2] blk-cgroup: always display debug stats in io.stat

[Tejun Heo]

Hello,

On Tue, Mar 03, 2026 at 06:11:15AM -0800, Breno Leitao wrote:
> Remove the blkcg_debug_stats toggle and always display detailed
> statistics in the cgroup io.stat file. This includes use_delay and
> delay_nsec information, cost.wait/cost.indebt/cost.indelay for iocost,
> and latency statistics for iolatency.
>
> The stats are already being collected regardless of the toggle, so
> gating their display provides no real benefit. Additionally, blk-cgroup

The reason for gating is that these are more internal implementation details
and might change in the future.

> has not been modularized since commit 32e380aedc3de ("blkcg: make
> CONFIG_BLK_CGROUP bool"), making the module parameter a historical
> artifact. Readers of the nested-keys format should be able to handle
> additional fields.

I'm not sure what the above para means. Module param works just fine for
built-in modules on both boot command line and through sysfs.

> Before (without blkcg_debug_stats enabled):
>   253:0 rbytes=6273024 wbytes=0 rios=20 wios=0 dbytes=0 dios=0 cost.usage=0
> 
> After:
>   253:0 rbytes=6273024 wbytes=0 rios=20 wios=0 dbytes=0 dios=0 cost.usage=0 cost.wait=0 cost.indebt=0 cost.indelay=0

Given that they haven't changed for a long time, maybe it's okay to expose
them by default, but why? This is something which can be toggled on easily
at any time. What's the benefit of exposing these extra numbers which
probably don't mean much for most people?

Thanks.

-- 
tejun

Re: [PATCH v2] blk-cgroup: always display debug stats in io.stat

[Breno Leitao]

Hello Tejun,

On Tue, Mar 03, 2026 at 07:46:46AM -1000, Tejun Heo wrote:
> On Tue, Mar 03, 2026 at 06:11:15AM -0800, Breno Leitao wrote:

> > Before (without blkcg_debug_stats enabled):
> >   253:0 rbytes=6273024 wbytes=0 rios=20 wios=0 dbytes=0 dios=0 cost.usage=0
> >
> > After:
> >   253:0 rbytes=6273024 wbytes=0 rios=20 wios=0 dbytes=0 dios=0 cost.usage=0 cost.wait=0 cost.indebt=0 cost.indelay=0
>
> Given that they haven't changed for a long time, maybe it's okay to expose
> them by default, but why? This is something which can be toggled on easily
> at any time.

My goal is to ship a kernel that exposes these detailed io stats by
default, without requiring any runtime configuration. The stats should
simply be available out of the box.

> What's the benefit of exposing these extra numbers which
> probably don't mean much for most people?

My original plan was to introduce a Kconfig option for this. In v1 (about a
month ago), Michal suggested removing the toggle entirely and always exposing
the stats, which seemed reasonable to me and received no objections, so I
went ahead with that approach.

To be clear: is your position that we should not support building a kernel
that always exposes the detailed stats in io.stat?

Thanks for your review,
--breno

Re: [PATCH v2] blk-cgroup: always display debug stats in io.stat

[Tejun Heo]

Hello, Breno.

On Wed, Mar 04, 2026 at 01:56:43AM -0800, Breno Leitao wrote:
> > Given that they haven't changed for a long time, maybe it's okay to expose
> > them by default, but why? This is something which can be toggled on easily
> > at any time.
> 
> My goal is to ship a kernel that exposes these detailed io stats by
> default, without requiring any runtime configuration. The stats should
> simply be available out of the box.

This is a pretty trivial patch to carry, right? Or just do it as a part of
boot system config?

> > What's the benefit of exposing these extra numbers which
> > probably don't mean much for most people?
> 
> My original plan was to introduce a Kconfig option for this. In v1 (about a
> month ago), Michal suggested removing the toggle entirely and always exposing
> the stats, which seemed reasonable to me and received no objections, so I
> went ahead with that approach.
> 
> To be clear: is your position that we should not support building a kernel
> that always exposes the detailed stats in io.stat?

As a debug option maybe but I'd prefer if vendors (including us) that want
to permanently enable these debug stats carried the patch in their trees.

I think that these are a bit too tied to the specifics of the current
implementation and it's not very difficult to imagine at least some of the
keys changing or getting dropped in the future, so I'm reluctant to export
them by default.

Thanks.

-- 
tejun

Re: [PATCH v2] blk-cgroup: always display debug stats in io.stat

[Breno Leitao]

On Wed, Mar 04, 2026 at 06:48:17AM -1000, Tejun Heo wrote:
> Hello, Breno.
> 
> On Wed, Mar 04, 2026 at 01:56:43AM -0800, Breno Leitao wrote:
> > > Given that they haven't changed for a long time, maybe it's okay to expose
> > > them by default, but why? This is something which can be toggled on easily
> > > at any time.
> > 
> > My goal is to ship a kernel that exposes these detailed io stats by
> > default, without requiring any runtime configuration. The stats should
> > simply be available out of the box.
> 
> This is a pretty trivial patch to carry, right? Or just do it as a part of
> boot system config?

Yes, a very trivial one, but, which requires some work to be done at
every rebase. Mainly when trying to use pristine stable trees.

> > > What's the benefit of exposing these extra numbers which
> > > probably don't mean much for most people?
> > 
> > My original plan was to introduce a Kconfig option for this. In v1 (about a
> > month ago), Michal suggested removing the toggle entirely and always exposing
> > the stats, which seemed reasonable to me and received no objections, so I
> > went ahead with that approach.
> > 
> > To be clear: is your position that we should not support building a kernel
> > that always exposes the detailed stats in io.stat?
> 
> As a debug option maybe but I'd prefer if vendors (including us) that want
> to permanently enable these debug stats carried the patch in their trees.

That is fair, thanks for the direction!
--breno

Re: [PATCH v2] blk-cgroup: always display debug stats in io.stat

[Michal Koutný]

[-- Attachment #1: Type: text/plain, Size: 1431 bytes --]

On Tue, Mar 03, 2026 at 07:46:46AM -1000, Tejun Heo <tj@kernel.org> wrote:
> > has not been modularized since commit 32e380aedc3de ("blkcg: make
> > CONFIG_BLK_CGROUP bool"), making the module parameter a historical
> > artifact. Readers of the nested-keys format should be able to handle
> > additional fields.
> 
> I'm not sure what the above para means. Module param works just fine for
> built-in modules on both boot command line and through sysfs.

Yeah, it works but BLK_CGROUP is not a module/built-in it's config
option affecting builds. I find the module_param() in blk-cgroup.c to be
a residual, I admit it's convenient way how to expose a tunable to
userspace.

(Contemporary way of implementing the option could also be a cgroupfs
mount option/feature or maybe sysctl for which tooling is available.)

Thus I was mainly motivated by bundling this change with cleanup of the
module_param().
I may have misjudged importance/usefulness of the data (not using them
myself).

On Wed, Mar 04, 2026 at 01:56:43AM -0800, Breno Leitao <leitao@debian.org> wrote:
> My goal is to ship a kernel that exposes these detailed io stats by    
> default, without requiring any runtime configuration. The stats should 
> simply be available out of the box.

Does it mean the information is useful to you since early boot and
adjusting the param with a userspace tool in boot sequence is too late?

Michal

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 265 bytes --]

Re: [PATCH v2] blk-cgroup: always display debug stats in io.stat

[Breno Leitao]

Hello Michal,

On Wed, Mar 04, 2026 at 02:28:03PM +0100, Michal Koutný wrote:
>
> On Wed, Mar 04, 2026 at 01:56:43AM -0800, Breno Leitao <leitao@debian.org> wrote:
> > My goal is to ship a kernel that exposes these detailed io stats by
> > default, without requiring any runtime configuration. The stats should
> > simply be available out of the box.
>
> Does it mean the information is useful to you since early boot and
> adjusting the param with a userspace tool in boot sequence is too late?

Not exactly. The goal is to have a kernel binary that, regardless of
where it is deployed and how it is configured, I know know it provides
full io.stat details without requiring any tunable to be changed.

Re: [PATCH v2] blk-cgroup: always display debug stats in io.stat

[Tejun Heo]

Hello,

On Wed, Mar 04, 2026 at 02:28:03PM +0100, Michal Koutný wrote:
> On Tue, Mar 03, 2026 at 07:46:46AM -1000, Tejun Heo <tj@kernel.org> wrote:
> > > has not been modularized since commit 32e380aedc3de ("blkcg: make
> > > CONFIG_BLK_CGROUP bool"), making the module parameter a historical
> > > artifact. Readers of the nested-keys format should be able to handle
> > > additional fields.
> > 
> > I'm not sure what the above para means. Module param works just fine for
> > built-in modules on both boot command line and through sysfs.
> 
> Yeah, it works but BLK_CGROUP is not a module/built-in it's config
> option affecting builds. I find the module_param() in blk-cgroup.c to be
> a residual, I admit it's convenient way how to expose a tunable to
> userspace.
> 
> (Contemporary way of implementing the option could also be a cgroupfs
> mount option/feature or maybe sysctl for which tooling is available.)

I don't konw whether I'm the only one doing it but I use moduleparams as an
easy way to get non-API boot and runtime toggles whether the target code is
actually module or not. It's easy to use and the params are in a pretty gray
area in terms of API stability, so if you wanna throw in a debug option, it
works.

Thanks.

-- 
tejun