Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] param_set_dyndbg_module_classes() fails to initialize debug queries for a newly probed module because it incorrectly uses the global class parameter state as the baseline (`old_bits`). --
--- Patch [15]: [PATCH v6 15/24] dyndbg: refactor param_set_dyndbg_classes and below --- commit 041ecc33e02248dbfd09801bededdf6047b80f6e Author: Jim Cromie <[email protected]> dyndbg: refactor param_set_dyndbg_classes and below This commit refactors the param_set_dyndbg_classes callchain to allow module-specific settings by passing a modname parameter down to ddebug_apply_class_bitmap and ddebug_exec_queries. It also adds a check to announce bit changes only when they actually change. > diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c > index ce42e03f1600a..0fc9cd14e2d20 100644 > --- a/lib/dynamic_debug.c > +++ b/lib/dynamic_debug.c [ ... ] > @@ -682,8 +692,8 @@ int param_set_dyndbg_classes(const char *instr, const > struct kernel_param *kp) > KP_NAME(kp), inrep, > CLASSMAP_BITMASK(map->length)); > inrep &= CLASSMAP_BITMASK(map->length); > } > - v2pr_info("bits:%lx > %s\n", inrep, KP_NAME(kp)); > - totct += ddebug_apply_class_bitmap(dcp, &inrep, dcp->bits); > + v2pr_info("bits:0x%lx > %s.%s\n", inrep, mod_name ?: "*", > KP_NAME(kp)); > + totct += ddebug_apply_class_bitmap(dcp, &inrep, dcp->bits, > mod_name); > *dcp->bits = inrep; > break; > case DD_CLASS_TYPE_LEVEL_NUM: > @@ -696,7 +706,7 @@ int param_set_dyndbg_classes(const char *instr, const > struct kernel_param *kp) > old_bits = CLASSMAP_BITMASK(*dcp->lvl); > new_bits = CLASSMAP_BITMASK(inrep); > v2pr_info("lvl:%ld bits:0x%lx > %s\n", inrep, new_bits, > KP_NAME(kp)); > - totct += ddebug_apply_class_bitmap(dcp, &new_bits, &old_bits); > + totct += ddebug_apply_class_bitmap(dcp, &new_bits, &old_bits, > mod_name); [Severity: Medium] Does this skip enabling debug queries for newly probed modules if the global state already has those bits set? When param_set_dyndbg_module_classes() is called for a newly probed module (mod_name != NULL), it passes the global class parameter state as the old state (dcp->bits and old_bits respectively) instead of 0. If a global parameter was previously set (e.g., drm.debug=2), those bits are already present in the global state. The optimization loop in ddebug_apply_class_bitmap() checks this: lib/dynamic_debug.c:ddebug_apply_class_bitmap() { ... for (bi = 0; bi < map->length; bi++) { if (test_bit(bi, new_bits) == test_bit(bi, old_bits)) continue; ... } Because the new module's target state matches the global state, it skips executing the queries, even though the newly loaded module actually starts with a default state of 0. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=15
