On Tue, Oct 13, 2026 at 8:37 PM <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s)
> to consider:
> - [Low] test_dynamic_debug: classmap offset mismatch for submodule classes
> - [Low] locking: param_set_do_repeats blocks param_lock for up to 60s
> --
>
> commit adf504f365302c7db4e37850cebef69f82ed353f
> Author: Jim Cromie <[email protected]>
>
> dyndbg-test: add do_bulk testpoint, rename do_prints to do_classes
>
> This patch renames do_prints to do_classes to better align with its purpose
> and introduces do_bulk to generate high-volume workloads. It adds support for
> executing these debug operations multiple times or over a timed duration using
> shared module parameter callbacks.
>
> > diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
> > index 01ce07001d4c7..19e27b5747ac2 100644
> > --- a/lib/test_dynamic_debug.c
> > +++ b/lib/test_dynamic_debug.c
>
> [ ... ]
>
> > @@ -28,25 +28,57 @@
> > #endif
> >
> > #include <linux/module.h>
> > +#include <linux/sched.h>
> > +#include <linux/sched/signal.h>
> > +#include <linux/jiffies.h>
> >
> > -/* re-gen output by reading or writing sysfs node: do_prints */
> > +#define TEST_MAX_REPEATS 10000
> > +#define TEST_MAX_SECONDS 60
> >
> > -static void do_prints(void); /* device under test */
> > -static int param_set_do_prints(const char *instr, const struct
> > kernel_param *kp)
> > +/* re-trigger debug output by reading or writing sysfs nodes: do_classes
> > or do_bulk */
> > +static void do_classes(int); /* device under test */
> > +static void do_bulk(int); /* device under test */
> > +
> > +static int param_set_do_repeats(const char *instr, const struct
> > kernel_param *kp)
> > {
> > - do_prints();
> > + int rc, ct;
> > + void (*repeat_fn)(int) = kp->arg;
> > +
> > + rc = kstrtoint(instr, 0, &ct);
> > + if (rc) {
> > + pr_err("expecting numeric input, using 1 instead\n");
> > + ct = 1;
> > + }
> > + if (ct > TEST_MAX_REPEATS) {
> > + pr_warn("%s: clamping %d to %d\n", kp->name, ct,
> > TEST_MAX_REPEATS);
> > + ct = TEST_MAX_REPEATS;
> > + } else if (ct < -TEST_MAX_SECONDS) {
> > + pr_warn("%s: clamping %d seconds to -%d\n",
> > + kp->name, ct, TEST_MAX_SECONDS);
> > + ct = -TEST_MAX_SECONDS;
> > + }
> > +
> > + repeat_fn(ct);
>
> [Severity: Low]
> Does this synchronous loop execution block param_lock system-wide?
>
> When test_dynamic_debug is built-in, writing to its module parameters via
> sysfs triggers param_attr_store() in kernel/params.c, which acquires the
> global param_lock mutex around the .set callback:
>
> kernel/params.c:param_attr_store() {
> ...
> kernel_param_lock(mk->mod);
> if (param_check_unsafe(attribute->param))
> err = attribute->param->ops->set(buf, attribute->param);
> ...
> }
>
> Because param_set_do_repeats() invokes the workload synchronously (which
> can run for up to 60 seconds as shown in do_classes() below), this starves
> param_lock, preventing other processes from reading or writing any
> built-in module parameters for the duration.
In standard usage, test_dynamic_debug is built as a module
(CONFIG_TEST_DYNAMIC_DEBUG=m).
Under kernel/params.c:KPARAM_MUTEX(mod), modular parameters
acquire the module's
private &(mod)->param_lock rather than the global param_lock. The
lock-hold is
strictly local to test_dynamic_debug's own sysfs nodes and has
zero impact on
core kernel or other module parameters.
Even in the hypothetical built-in case
(CONFIG_TEST_DYNAMIC_DEBUG=y in a test
kernel, which I would consider mis-configured),
test_dynamic_debug is exclusively a test harness whose workloads are
triggered on demand by explicit sysfs writes. The loops call
cond_resched() and
check signal_pending(current) on every iteration, keeping the execution
cooperative and immediately abortable via SIGINT.
>
> > +
> > return 0;
> > }
>
> [ ... ]
>
> > @@ -103,7 +135,10 @@ enum cat_disjoint_bits {
> > D2_DRMRES };
> >
> > /* numeric verbosity, V2 > V1 related. V1 is > D2_DRMRES */
> > -enum cat_level_num { V1 = 16, V2, V3, V4, V5, V6, V7 };
> > +enum cat_level_num { V1 = 16, V2, V3, V4, V5, V6, V7, V8 };
> > +
> > +/* test _USE_ w offset */
> > +enum cat_level_offset { Vu1 = V1 + 8, Vu2, Vu3, Vu4, Vu5, Vu6, Vu7, Vu8 };
> >
> > /* recapitulate DRM's multi-classmap setup */
> > #if !defined(TEST_DYNAMIC_DEBUG_SUBMOD)
>
> [ ... ]
>
> > @@ -160,6 +183,19 @@ DYNAMIC_DEBUG_CLASSMAP_DEFINE(fail_emptyclass, 0, 0 /*
> > ,empty */);
> > DYNAMIC_DEBUG_CLASSMAP_DEFINE(fail_maptype, 3, 10, "no such type");
> > DYNAMIC_DEBUG_CLASSMAP_DEFINE(fail_base_len, 0, 60,
> > "base", "plus", "classes", "length", "too-big");
> > +#endif
> > +
> > +#else /* TEST_DYNAMIC_DEBUG_SUBMOD */
> > +
> > +/*
> > + * in submod/drm-drivers, use the classmaps defined in top/parent
> > + * module above.
> > + */
> > +
> > +DYNAMIC_DEBUG_CLASSMAP_USE(map_disjoint_bits);
> > +DYNAMIC_DEBUG_CLASSMAP_USE_(map_level_num, 7);
>
> [Severity: Low]
> Is this registration macro passing an outdated offset?
>
yes - fixed in v13 (forthcoming)
> The patch adds V8 to map_level_num, increasing its length to 8. While
> cat_level_offset correctly shifts the submodule's class definitions by 8
> (Vu1 = V1 + 8) above, this registration macro still tells dyndbg the
> classes start at offset 7.
>
> This mismatch causes submodule debug callsites to map to the wrong class
> IDs, leaving the last class (Vu8) completely out of bounds and
> uncontrollable.
>
> > +
> > +#if defined(DD_MACRO_ARGCHECK)
> > DYNAMIC_DEBUG_CLASSMAP_USE_(fail_offset_big, 100);
> > #endif /* DD_MACRO_ARGCHECK */
>
> [ ... ]
>
> > @@ -210,20 +247,85 @@ static void do_levels(void)
>
> [ ... ]
>
> > -static void do_prints(void)
> > +static void do_classes(int ct)
> > +{
> > + if (ct < 0) {
> > + unsigned long timeout = jiffies + (-ct) * HZ;
> > +
> > + pr_debug("%s for %d seconds:\n", __func__, -ct);
> > + while (time_before(jiffies, timeout)) {
> > + if (signal_pending(current))
> > + break;
> > + do_cats();
> > + do_levels();
> > + cond_resched();
> > + }
> > + return;
> > + }
>
> --
> Sashiko AI review ·
> https://sashiko.dev/#/patchset/[email protected]?part=26
