Re: [PATCH v12 33/40] dyndbg: Ignore additional arguments from pr_fmt
From: jim . cromie
Date: Wed Oct 28 2026 - 14:42:16 EST
On Tue, Oct 27, 2026 at 7:22 AM David Laight
<david.laight.linux@xxxxxxxxx> wrote:
>
> On Tue, 13 Oct 2026 13:33:10 +0200
> Petr Mladek <pmladek@xxxxxxxx> wrote:
>
> > On Mon 2026-10-12 20:37:04, Jim Cromie via B4 Relay wrote:
> > > From: Philipp Hahn <phahn-oss@xxxxxx>
> > >
> > > 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.
Im not sure who youre responding to here,
in any case I sent v13, with adjustments based on Petr's observations.
here it is, in action
bash-5.3# history
1 modprobe test_dynamic_debug dyndbg=+p
2 dmesg
3 modprobe test_dynamic_debug_submod dyndbg=+pmf
4 dmesg
5 history
[ 0.707538] virtme-ng-init: initialization done
[ 1.060923] clocksource: Marking clocksource tsc unstable due to
inter CPU skew
[ 1.060929] clocksource: CPU0 4136478715 < CPU1
4136478900 (cycles)
[ 1.060932] tsc: Marking TSC unstable due to clocksource watchdog
[ 4.703188] (udev-worker) (78) used greatest stack depth: 12376 bytes left
[ 44.598698] dyndbg: 47 debug prints in module test_dynamic_debug
[ 44.598706] dyndbg: module: test_dynamic_debug dyndbg="+p"
[ 44.599208] test_dd: init start
[ 44.599210] test_dd: do_classes 1 times:
[ 44.599212] test_dd: doing categories
[ 44.599213] test_dd: doing levels
[ 44.599214] test_dd: do_bulk 1 times:
[ 44.599215] test_dd: bulk msg 1.0
[ 44.599216] test_dd: bulk msg 1.1
[ 44.599217] test_dd: bulk msg 1.2
[ 44.599219] test_dd: bulk msg 1.3
[ 44.599220] test_dd: bulk msg 1.4
[ 44.599221] test_dd: bulk msg 1.5
[ 44.599222] test_dd: bulk msg 1.6
[ 44.599223] test_dd: bulk msg 1.7
[ 44.599224] test_dd: bulk msg 1.8
[ 44.599225] test_dd: bulk msg 1.9
[ 44.599226] test_dd: init done
[ 527.233033] dyndbg: 47 debug prints in module test_dynamic_debug_submod
[ 527.233039] dyndbg: module: test_dynamic_debug_submod dyndbg="+pmf"
[ 527.233458] test_dynamic_debug_submod:test_dynamic_debug_init:
test_dd_submod: test_dynamic_debug_init init start
[ 527.233460] test_dynamic_debug_submod:do_classes: test_dd_submod:
do_classes do_classes 1 times:
[ 527.233461] test_dynamic_debug_submod:do_cats: test_dd_submod:
do_cats doing categories
[ 527.233463] test_dynamic_debug_submod:do_levels: test_dd_submod:
do_levels doing levels
[ 527.233464] test_dynamic_debug_submod:do_bulk: test_dd_submod:
do_bulk do_bulk 1 times:
[ 527.233465] test_dynamic_debug_submod:do_bulk: test_dd_submod:
do_bulk bulk msg 1.0
[ 527.233467] test_dynamic_debug_submod:do_bulk: test_dd_submod:
do_bulk bulk msg 1.1
[ 527.233468] test_dynamic_debug_submod:do_bulk: test_dd_submod:
do_bulk bulk msg 1.2
[ 527.233469] test_dynamic_debug_submod:do_bulk: test_dd_submod:
do_bulk bulk msg 1.3
[ 527.233471] test_dynamic_debug_submod:do_bulk: test_dd_submod:
do_bulk bulk msg 1.4
[ 527.233472] test_dynamic_debug_submod:do_bulk: test_dd_submod:
do_bulk bulk msg 1.5
[ 527.233473] test_dynamic_debug_submod:do_bulk: test_dd_submod:
do_bulk bulk msg 1.6
[ 527.233475] test_dynamic_debug_submod:do_bulk: test_dd_submod:
do_bulk bulk msg 1.7
[ 527.233476] test_dynamic_debug_submod:do_bulk: test_dd_submod:
do_bulk bulk msg 1.8
[ 527.233477] test_dynamic_debug_submod:do_bulk: test_dd_submod:
do_bulk bulk msg 1.9
[ 527.233478] test_dynamic_debug_submod:test_dynamic_debug_init:
test_dd_submod: test_dynamic_debug_init init done
bash-5.3#
NOTE that the dynamic-prefix precedes the one added by pr_fmt,
(the parent and submod differ, latter adds __func__,
in both the dynamic-prefix, and the one from pr_fmt())
This is perhaps vaguely sub-optimal, but it has been this way for ages.
> 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
>
hi David,
Im not sure what / who youre responding to with "
> >
> > 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__); \
> >
>