Re: [PATCH v9 29/39] dyndbg-API: promote DYNAMIC_DEBUG_CLASSMAP_PARAM to API
flat view
From: sashiko-bot@kernel.org
Date: 2026-09-09 01:08:47
Also in:
dri-devel
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] When CONFIG_DYNAMIC_DEBUG is disabled, DYNAMIC_DEBUG_CLASSMAP_PARAM_REF evaluates to an empty macro, which will silently drop the module parameter for consumers like DRM. - [Medium] The commit message claims a modification to `ddebug_sync_classbits()` that does not exist in the patch. - [Medium] DYNAMIC_DEBUG_CLASSMAP_PARAM zeroes the bitvector regardless of the `DEBUG` macro, causing state desynchronization and an inability to disable debug classes when compiled with DEBUG. -- commit a5fb641199d77a5e65f10185304cd8aae102e99f Author: Jim Cromie [off-list ref] dyndbg-API: promote DYNAMIC_DEBUG_CLASSMAP_PARAM to API move the DYNAMIC_DEBUG_CLASSMAP_PARAM macro from test-dynamic-debug.c into the header, and refine it, by distinguishing the 2 use cases.
Modify ddebug_sync_classbits() argtype deref inside the fn, to give access to all kp members.
[Severity: Medium] The commit message claims a modification to ddebug_sync_classbits() that does not exist in the patch. Is this an oversight or an artifact from an older revision?
quoted hunk ↗ jump to hunk
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h index b53e7488b7f7d..cc203b6c1b565 100644 --- a/include/linux/dynamic_debug.h +++ b/include/linux/dynamic_debug.h@@ -293,6 +293,44 @@ struct ddebug_class_param { .offset = _offset \ } +/** + * DYNAMIC_DEBUG_CLASSMAP_PARAM - control a ddebug-classmap from a sys-param + * @_name: sysfs node name + * @_var: name of the classmap var defining the controlled classes/bits + * @_flags: flags to be toggled, typically just 'p' + * + * Creates a sysfs-param to control the classes defined by the + * exported classmap, with bits 0..N-1 mapped to the classes named. + * This version keeps class-state in a private long int. + */ +#define DYNAMIC_DEBUG_CLASSMAP_PARAM(_name, _var, _flags) \ + static u32 _name##_bvec; \ + __DYNAMIC_DEBUG_CLASSMAP_PARAM(_name, _name##_bvec, _var, _flags)
[Severity: Medium] Does this macro unconditionally zero-initialize the state bitvector, breaking synchronization with the actual enabled state of call sites if the module is compiled with DEBUG? If a module using this macro is compiled with -DDEBUG, its dynamic debug call sites are enabled by default. However, the static state vector _name##_bvec is forcibly zero-initialized. If a user writes 0 to the sysfs parameter to disable the logs, param_set_dyndbg_module_classes() compares the new 0 against the old 0 state and skips sending the -p queries, leaving the logs permanently enabled. [ ... ]
quoted hunk ↗ jump to hunk
@@ -476,6 +514,8 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor, #define DYNAMIC_DEBUG_CLASSMAP_DEFINE(_var, _mapty, _base, ...) #define DYNAMIC_DEBUG_CLASSMAP_USE(_var) #define DYNAMIC_DEBUG_CLASSMAP_USE_(_var, _offset) +#define DYNAMIC_DEBUG_CLASSMAP_PARAM(_name, _var, _flags) +#define DYNAMIC_DEBUG_CLASSMAP_PARAM_REF(_name, _bits, _var, _flags) #define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt)
[Severity: High] When CONFIG_DYNAMIC_DEBUG is disabled, DYNAMIC_DEBUG_CLASSMAP_PARAM_REF evaluates to an empty macro. Will this silently drop the module parameter for consumers like DRM? Consumers of this macro will completely lose their sysfs module parameters (e.g., /sys/module/drm/parameters/debug) on kernels built without CONFIG_DYNAMIC_DEBUG. Does the macro need to provide a fallback like module_param_named() to avoid breaking kernel ABI expectations for these debugging parameters? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260908-dd-cmap-part2-clean-v9-0-3a3f621f4a10@gmail.com?part=29