Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] dyndbg-test: early-boot infinite loop on negative repeat count in do_classes() and do_bulk() --
commit bf6fb515ebe31cab587b06b6c8aeb75be6f0a072 Author: Jim Cromie <[email protected]> dyndbg-test: add do_bulk testpoint, rename do_prints to do_classes This commit renames do_prints to do_classes, adds a new do_bulk module parameter for generating high-volume debug output, and shares a common parameter callback that supports repeat counts and negative duration timeouts. > diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c > index 01ce07001d4c7..f78764025c441 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); > + > return 0; > } > -static int param_get_do_prints(char *buffer, const struct kernel_param *kp) > + > +static int param_get_do_repeats(char *buffer, const struct kernel_param *kp) > { > - do_prints(); > - return scnprintf(buffer, PAGE_SIZE, "did do_prints\n"); > + void (*repeat_fn)(int) = kp->arg; > + > + repeat_fn(1); > + > + return scnprintf(buffer, PAGE_SIZE, "did 1 %s\n", kp->name); > } > -static const struct kernel_param_ops param_ops_do_prints = { > - .set = param_set_do_prints, > - .get = param_get_do_prints, > + > +static const struct kernel_param_ops param_ops_do_repeats = { > + .set = param_set_do_repeats, > + .get = param_get_do_repeats, > }; > -module_param_cb(do_prints, ¶m_ops_do_prints, NULL, 0600); > + > +module_param_cb(do_classes, ¶m_ops_do_repeats, do_classes, 0600); > +module_param_cb(do_bulk, ¶m_ops_do_repeats, do_bulk, 0600); [ ... ] > @@ -210,20 +247,85 @@ static void do_levels(void) > prdbg(Vu5); > prdbg(Vu6); > prdbg(Vu7); > + prdbg(Vu8); > #endif > } > > -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; > + } > + > + pr_debug("do_classes %u times:\n", ct); > + for (; ct; ct--) { > + if (signal_pending(current)) > + break; > + do_cats(); > + do_levels(); > + cond_resched(); > + } > +} > + > +static void do_bulk(int ct) > { > - pr_debug("do_prints:\n"); > - do_cats(); > - do_levels(); > + unsigned int i; > + > + if (ct < 0) { > + unsigned long timeout = jiffies + (-ct) * HZ; > + > + pr_debug("%s for %d seconds:\n", __func__, -ct); > + for (i = 0; time_before(jiffies, timeout); i++) { > + if (signal_pending(current)) > + break; > + pr_debug("bulk msg %u.0\n", i + 1); > + pr_debug("bulk msg %u.1\n", i + 1); > + pr_debug("bulk msg %u.2\n", i + 1); > + pr_debug("bulk msg %u.3\n", i + 1); > + pr_debug("bulk msg %u.4\n", i + 1); > + pr_debug("bulk msg %u.5\n", i + 1); > + pr_debug("bulk msg %u.6\n", i + 1); > + pr_debug("bulk msg %u.7\n", i + 1); > + pr_debug("bulk msg %u.8\n", i + 1); > + pr_debug("bulk msg %u.9\n", i + 1); > + cond_resched(); > + } > + return; > + } [Severity: Medium] Can passing a negative value to do_classes or do_bulk on the kernel command line hang the boot when CONFIG_TEST_DYNAMIC_DEBUG=y (or CONFIG_TEST_DYNAMIC_DEBUG_SUBMOD=y)? Both do_classes and do_bulk are registered with module_param_cb(), which sets the parameter level to -1: include/linux/moduleparam.h: #define module_param_cb(name, ops, arg, perm) \ __module_param_call(MODULE_PARAM_PREFIX, name, ops, arg, perm, -1, 0) When the test driver is built-in, command-line parameters such as test_dynamic_debug.do_classes=-1 or test_dynamic_debug.do_bulk=-1 are parsed in start_kernel() -> parse_args() while local interrupts are disabled and before init_IRQ(), tick_init(), timers_init(), and time_init() have run. At that stage of boot, param_set_do_repeats() calls do_classes(ct) or do_bulk(ct) with ct < 0, which loops waiting for time_before(jiffies, timeout) to become false. Since jiffies does not advance and signal_pending(current) on init_task never becomes true during early boot, will this loop spin forever? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=26
