Re: [PATCH v12 33/40] dyndbg: Ignore additional arguments from pr_fmt
flat view
From: Petr Mladek <pmladek@suse.com>
Date: 2026-10-13 11:33:27
Also in:
dri-devel, linux-arch, linux-doc, linux-kbuild, linux-kselftest, linux-pm, lkml
On Mon 2026-10-12 20:37:04, Jim Cromie via B4 Relay wrote:
From: Philipp Hahn <redacted> 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:quoted
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.
quoted hunk ↗ jump to hunk
quoted
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__); \