Re: [PATCH v3 19/24] dyndbg,module: make proper substructs in _ddebug_info
From: sashiko-bot@kernel.org
Date: 2026-06-01 12:20:29
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] Built-in modules without class maps incorrectly inherit the global built-in class maps array, violating state invariants and causing performance regressions on queries. -- commit 97c08d5dc7f7ec501676f73addba8ad9f6c67352 Author: Jim Cromie [off-list ref] dyndbg,module: make proper substructs in _ddebug_info This patch recomposes struct _ddebug_info by inserting proper sub-structs for descriptors and class maps. It updates the usage of this struct as a cursor for built-in module data and module state access. Note that the regression detailed below was fixed later in the patch series by commit b54eb6f1ef1ecdd4f503388db9a2426bb84d49d1.
quoted hunk ↗ jump to hunk
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c@@ -1176,12 +1177,12 @@ static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug * the builtin/modular classmap vector/section. Save the start * and length of the subrange at its edges. */ - for_subvec(i, cm, di, classes) { + for_subvec(i, cm, di, maps) { if (!strcmp(cm->mod_name, dt->mod_name)) {
[ ... ]
} else if (nc) {
/* end of matching classmaps */
break;
}
}
if (nc) {
- dt->num_classes = nc;
+ dt->info.maps.len = nc;[Severity: Medium] When a built-in module does not have any class maps (so nc remains 0), this function will exit without setting dt->info.maps.len to 0. Because dt->info was populated by a shallow copy of *di earlier in ddebug_add_module(), this leaves dt->info.maps.len set to the global built-in class maps array length. Does this cause modules without class maps to incorrectly retain the entire global class maps array bounds? This appears to degrade query time complexity, as ddebug_change() will falsely find class maps in these modules and needlessly iterate over all their descriptors.
vpr_info("module:%s attached %d classes\n", dt->mod_name, nc);
}
}[ ... ]
quoted hunk ↗ jump to hunk
@@ -1220,19 +1221,18 @@ static int ddebug_add_module(struct _ddebug_info *di, const char *modname) * this struct ddebug_table. */ dt->mod_name = modname; - dt->ddebugs = di->descs; - dt->num_ddebugs = di->num_descs; + dt->info = *di;
[Severity: Medium] This shallow struct copy initializes the module's maps with the global array bounds. Before this patch, dt->num_classes was correctly initialized to 0 by kzalloc_obj(*dt) and only updated if classes were found. If ddebug_attach_module_classes() doesn't overwrite dt->info.maps.len for modules with no class maps, does this break the contiguous subrange invariant?
INIT_LIST_HEAD(&dt->link); - if (di->classes && di->num_classes) + if (di->maps.len) ddebug_attach_module_classes(dt, di);
-- Sashiko AI review · https://sashiko.dev/#/patchset/20260601-dd-maint-2-v3-0-4a15b241bd3c@gmail.com?part=19