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?
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__);
> \
