On Tue, 13 Oct 2026 13:33:10 +0200 Petr Mladek <[email protected]> wrote:
> On Mon 2026-10-12 20:37:04, Jim Cromie via B4 Relay wrote: > > From: Philipp Hahn <[email protected]> > > > > pr_fmt can be used to add a common prefix to any output from a module: > > #define pr_fmt(fmt) KBUILD_MODNAME ".%s " fmt, __func__ > > > > But adding additional arguments breaks dynamic debug: > > > error: macro "DEFINE_DYNAMIC_DEBUG_METADATA_CLS" passed 4 arguments, but > > > takes just 3 > > > | pr_debug_ratelimited("%s", "Hello world!"); > > > | ^ > > > note: macro "DEFINE_DYNAMIC_DEBUG_METADATA_CLS" defined here > > > | #define DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, cls, fmt) \ > > > | > > > error: ‘DEFINE_DYNAMIC_DEBUG_METADATA_CLS’ undeclared (first use in this > > > function) > > > | DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, _DPRINTK_CLASS_DFLT, > > > fmt) > > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > I wonder what this error about undeclared macro means. > > > > note: in expansion of macro ‘DEFINE_DYNAMIC_DEBUG_METADATA’ > > > | DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, pr_fmt(fmt)); > > > \ > > > | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > > note: in expansion of macro ‘pr_debug_ratelimited’ > > > | pr_debug_ratelimited("%s", "Hello world!"); > > > | ^~~~~~~~~~~~~~~~~~~~ > > > > Add an additional ', ...' to DEFINE_DYNAMIC_DEBUG_METADATA_CLS to slurp > > any additional argument, which `pr_fmt` might add. > > > > --- a/include/linux/dynamic_debug.h > > +++ b/include/linux/dynamic_debug.h > > @@ -351,7 +351,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor, > > dump_stack(); \ > > } > > > > -#define DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, cls, fmt) \ > > +#define DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, cls, fmt, ...) \ > > static struct _ddebug __aligned(8) \ > > __section("__dyndbg_descs") name = { \ > > .modname = KBUILD_MODNAME, \ > > diff --git a/include/linux/printk.h b/include/linux/printk.h > > index f594c1266bfd..5d701952e739 100644 > > --- a/include/linux/printk.h > > +++ b/include/linux/printk.h > > @@ -742,7 +742,7 @@ do { > > \ > > static DEFINE_RATELIMIT_STATE(_rs, \ > > DEFAULT_RATELIMIT_INTERVAL, \ > > DEFAULT_RATELIMIT_BURST); \ > > - DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, pr_fmt(fmt)); \ > > + DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, fmt); \ > > IMHO, this changes the existing behavior. The prefix added by > pr_fmt() will no longer be stored in DEFINE_DYNAMIC_DEBUG_METADATA(). > Is this intentional, please? > > Anyway, if we do not longer pass pr_fmt(fmt) then we should not > need to add "..." into DEFINE_DYNAMIC_DEBUG_METADATA_CLS(). > > By other words, we newly pass just the format "fmt" without the parametes. > The 4th parameter was added by pr_fmt(). > > > IMHO, the right solution is to keep pr_fmt(fmt) here and extend both > BEFINE_DYNAMIC_DEBUG_METADATA() and > DEFINE_DYNAMIC_DEBUG_METADATA_CLS() > to accept var args. I would expect something like: > > #define DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, cls, fmt, ...) \ > ... > > #define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt, ...) \ > DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, _DPRINTK_CLASS_DFLT, fmt, > ##__VA_ARGS__) > > > or maybe, we could "eat" the extra parameters in > DEFINE_DYNAMIC_DEBUG_METADATA. It would mean to keep > DEFINE_DYNAMIC_DEBUG_METADATA_CLS() as is and just add the "..." into > DEFINE_DYNAMIC_DEBUG_METADATA(), aka > > #define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt, ...) \ > DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, _DPRINTK_CLASS_DFLT, fmt) > > > Or do I miss something, please? Don't think that will work. Won't you end up with ".%s" fmt as the string without the extra argument. So you need to keep the expansion of pr_fmt() together until is is finally substituted into a printf-like call. It is possible to enclose it in () so it is passed as one parameter and remove them on the final expansion. David > > It is possible that the macros work another way. Or this depends > on another change. I checked only this patch... > > Best Regards, > Petr > > PS: I was curious what Sashiko says about this change and it seems > that it does not review this patchset from some reasons. > > > if (DYNAMIC_DEBUG_BRANCH(descriptor) && \ > > __ratelimit(&_rs)) \ > > __dynamic_pr_debug(&descriptor, pr_fmt(fmt), ##__VA_ARGS__); > > \ >
