All of lore.kernel.org
 help / color / mirror / Atom feed
From: Breno Leitao <leitao@debian.org>
To: "Michal Koutný" <mkoutny@suse.com>
Cc: Jens Axboe <axboe@kernel.dk>, Tejun Heo <tj@kernel.org>,
	 Josef Bacik <josef@toxicpanda.com>,
	linux-block@vger.kernel.org, linux-kernel@vger.kernel.org,
	 cgroups@vger.kernel.org, thevlad@meta.com, kernel-team@meta.com
Subject: Re: [PATCH] blk-cgroup: add CONFIG_BLK_CGROUP_DEBUG_STATS option
Date: Tue, 3 Mar 2026 05:59:29 -0800	[thread overview]
Message-ID: <aabo8xm1S0Zxw3gD@gmail.com> (raw)
In-Reply-To: <jf2bkbvk3h5j3mqfldu66egbvbeq62mzdenuimpgn7d4tfkrpx@b2s6zzdgmgyh>

hello Michal,

First of all, sorry for being late here, I had some days off.

On Thu, Feb 12, 2026 at 11:26:29AM +0100, Michal Koutný wrote:
> On Wed, Feb 04, 2026 at 08:15:12AM -0800, Breno Leitao <leitao@debian.org> wrote:
> > Add a Kconfig option to enable blkcg_debug_stats by default at compile
> > time. When CONFIG_BLK_CGROUP_DEBUG_STATS is enabled, additional debugging
> > information is shown in the cgroup io.stat file, including cost.wait,
> > cost.indebt, and cost.indelay for iocost, as well as latency statistics
> > for iolatency.
> 
> This seems to be toggleable quite easily anytime at runtime (not sysctl
> but modprobe config), not a boot cmdline where CONFIG_ default could
> step in.
> 
> This only guards printing of already collected stats (sometimes even
> without kernel consumers), not sure if it's that useful.
> 
> blk-cgroup isn't modularized since 32e380aedc3de ("blkcg: make
> CONFIG_BLK_CGROUP bool") v3.5-rc1~42^2~92 exposing it like a module
> parameter is historical artifact.
> 
> So I'd dare to propose removing it altogether and print those stats
> everytime. Readers of the nested-keys format should survive that.
> (I don't even see the param documented.)
> 
> And if there were eager readers that'd be affected performance-wise,
> more conventional would be to make this only boot cmdline parameter that
> could static-branch also the stat collection spots (for some more
> benefit). And then would also a CONFIG_urable default make sense.
> 
> WDYT?

That seems to make sense and it also simplify kernel management. I am sending a
V2 with it, let's see if there is any other concern.

      reply	other threads:[~2026-03-03 13:59 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-04 16:15 [PATCH] blk-cgroup: add CONFIG_BLK_CGROUP_DEBUG_STATS option Breno Leitao
2026-02-12 10:26 ` Michal Koutný
2026-03-03 13:59   ` Breno Leitao [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aabo8xm1S0Zxw3gD@gmail.com \
    --to=leitao@debian.org \
    --cc=axboe@kernel.dk \
    --cc=cgroups@vger.kernel.org \
    --cc=josef@toxicpanda.com \
    --cc=kernel-team@meta.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mkoutny@suse.com \
    --cc=thevlad@meta.com \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.