* [PATCH v3 12/24] dyndbg: drop NUM_TYPE_ARGS
From: Jim Cromie @ 2026-06-01 12:04 UTC (permalink / raw)
To: Jonathan Corbet, Shuah Khan, Arnd Bergmann, Jason Baron,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Andrew Morton, Shuah Khan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: linux-doc, linux-kernel, linux-arch, linux-modules,
linux-kselftest, dri-devel, Jim Cromie, Louis Chauvet
In-Reply-To: <20260601-dd-maint-2-v3-0-4a15b241bd3c@gmail.com>
ARRAY_SIZE works here, since array decl is complete.
no functional change
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
v2: include linux/array_size.h, correct commit subject
review after sob
---
include/linux/dynamic_debug.h | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 78c22c6d2312..867e3978675f 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -121,11 +121,9 @@ struct ddebug_class_param {
.mod_name = KBUILD_MODNAME, \
.base = _base, \
.map_type = _maptype, \
- .length = NUM_TYPE_ARGS(char*, __VA_ARGS__), \
+ .length = ARRAY_SIZE(_var##_classnames), \
.class_names = _var##_classnames, \
}
-#define NUM_TYPE_ARGS(eltype, ...) \
- (sizeof((eltype[]) {__VA_ARGS__}) / sizeof(eltype))
extern __printf(2, 3)
void __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...);
--
2.54.0
^ permalink raw reply related
* [PATCH v3 13/24] dyndbg: reduce verbose/debug clutter
From: Jim Cromie @ 2026-06-01 12:04 UTC (permalink / raw)
To: Jonathan Corbet, Shuah Khan, Arnd Bergmann, Jason Baron,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Andrew Morton, Shuah Khan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: linux-doc, linux-kernel, linux-arch, linux-modules,
linux-kselftest, dri-devel, Jim Cromie, Louis Chauvet
In-Reply-To: <20260601-dd-maint-2-v3-0-4a15b241bd3c@gmail.com>
currently, for verbose=3, these are logged (blank lines for clarity):
dyndbg: query 0: "class DRM_UT_CORE +p" mod:*
dyndbg: split into words: "class" "DRM_UT_CORE" "+p"
dyndbg: op='+'
dyndbg: flags=0x1
dyndbg: *flagsp=0x1 *maskp=0xffffffff
dyndbg: parsed: func="" file="" module="" format="" lineno=0-0 class=...
dyndbg: no matches for query
dyndbg: no-match: func="" file="" module="" format="" lineno=0-0 class=...
dyndbg: processed 1 queries, with 0 matches, 0 errs
That is excessive, so this patch:
- shrinks 3 lines of 2nd stanza to single line
- drops 1st 2 lines of 3rd stanza
3rd line is like 1st, with result, not procedure.
2nd line is just status, retold in 4th, with more info.
New output:
dyndbg: query 0: "class DRM_UT_CORE +p"
dyndbg: split into words: "class" "DRM_UT_CORE" "+p"
dyndbg: op='+' flags=0x1 maskp=0xffffffff
dyndbg: processed 1 queries, with 0 matches, 0 errs
Also drop several verbose=3 messages in ddebug_add_module
When modprobing a module, dyndbg currently logs/says "add-module", and
then "skipping" if the module has no prdbgs. Instead just check 1st
and return quietly.
no functional change
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
v2:
RvB after SoB
trivial change to verbose-debug output line to output the actual
"module" keyword rather than "mod:", and do so only when the module is
constrained by the callchain (ie as part of a modprobe).
was: query X: "(keyword value)* [+-=]flags" mod:*
now: query X: "(keyword value)* [+-=]flags"
or query X: module FOO "keyword value)* [+-=]flags"
IOW, adjust output to reflect the input grammar more closely.
drop-info-parsed
vinfo-applied-nomatch
---
lib/dynamic_debug.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 0377d9f8dcd1..bacb55f960fa 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -276,9 +276,6 @@ static int ddebug_change(const struct ddebug_query *query,
}
mutex_unlock(&ddebug_lock);
- if (!nfound && verbose)
- pr_info("no matches for query\n");
-
return nfound;
}
@@ -487,7 +484,6 @@ static int ddebug_parse_query(char *words[], int nwords,
*/
query->module = modname;
- vpr_info_dq(query, "parsed");
return 0;
}
@@ -511,7 +507,6 @@ static int ddebug_parse_flags(const char *str, struct flag_settings *modifiers)
pr_err("bad flag-op %c, at start of %s\n", *str, str);
return -EINVAL;
}
- v3pr_info("op='%c'\n", op);
for (; *str ; ++str) {
for (i = ARRAY_SIZE(opt_array) - 1; i >= 0; i--) {
@@ -525,7 +520,6 @@ static int ddebug_parse_flags(const char *str, struct flag_settings *modifiers)
return -EINVAL;
}
}
- v3pr_info("flags=0x%x\n", modifiers->flags);
/* calculate final flags, mask based upon op */
switch (op) {
@@ -541,7 +535,7 @@ static int ddebug_parse_flags(const char *str, struct flag_settings *modifiers)
modifiers->flags = 0;
break;
}
- v3pr_info("*flagsp=0x%x *maskp=0x%x\n", modifiers->flags, modifiers->mask);
+ v3pr_info("op='%c' flags=0x%x maskp=0x%x\n", op, modifiers->flags, modifiers->mask);
return 0;
}
@@ -593,7 +587,8 @@ static int ddebug_exec_queries(char *query, const char *modname)
if (!query || !*query || *query == '#')
continue;
- vpr_info("query %d: \"%s\" mod:%s\n", i, query, modname ?: "*");
+ vpr_info("query %d: %s%s \"%s\"\n", i,
+ modname ? "module " : "", modname ?: "", query);
rc = ddebug_exec_query(query, modname);
if (rc < 0) {
@@ -1159,11 +1154,10 @@ static int ddebug_add_module(struct _ddebug_info *di, const char *modname)
{
struct ddebug_table *dt;
- v3pr_info("add-module: %s.%d sites\n", modname, di->num_descs);
- if (!di->num_descs) {
- v3pr_info(" skip %s\n", modname);
+ if (!di->num_descs)
return 0;
- }
+
+ v3pr_info("add-module: %s %d sites\n", modname, di->num_descs);
dt = kzalloc_obj(*dt);
if (dt == NULL) {
--
2.54.0
^ permalink raw reply related
* [PATCH v3 14/24] dyndbg: refactor param_set_dyndbg_classes and below
From: Jim Cromie @ 2026-06-01 12:05 UTC (permalink / raw)
To: Jonathan Corbet, Shuah Khan, Arnd Bergmann, Jason Baron,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Andrew Morton, Shuah Khan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: linux-doc, linux-kernel, linux-arch, linux-modules,
linux-kselftest, dri-devel, Jim Cromie, Louis Chauvet
In-Reply-To: <20260601-dd-maint-2-v3-0-4a15b241bd3c@gmail.com>
Refactor callchain below param_set_dyndbg_classes(1) to allow mod-name
specific settings. Split (1) into upper/lower fns, adding modname
param to lower, and passing NULL in from upper. Below that, add the
same param to ddebug_apply_class_bitmap(), and pass it thru to
_ddebug_queries(), replacing NULL with the param.
This allows the callchain to update the classmap in just one module,
vs just all as currently done. While the sysfs param is unlikely to
ever update just one module, the callchain will be used for modprobe
handling, which should update only that just-probed module.
In ddebug_apply_class_bitmap(), also check for actual changes to the
bits before announcing them, to declutter logs.
No functional change.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
v2: move RvB after SoB
---
lib/dynamic_debug.c | 51 +++++++++++++++++++++++++++++++++++++++------------
1 file changed, 39 insertions(+), 12 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index bacb55f960fa..182b84b6f6c5 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -608,9 +608,10 @@ static int ddebug_exec_queries(char *query, const char *modname)
return nfound;
}
-/* apply a new bitmap to the sys-knob's current bit-state */
+/* apply a new class-param setting */
static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
- unsigned long *new_bits, unsigned long *old_bits)
+ unsigned long *new_bits, unsigned long *old_bits,
+ const char *query_modname)
{
#define QUERY_SIZE 128
char query[QUERY_SIZE];
@@ -618,7 +619,9 @@ static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
int matches = 0;
int bi, ct;
- v2pr_info("apply: 0x%lx to: 0x%lx\n", *new_bits, *old_bits);
+ if (*new_bits != *old_bits)
+ v2pr_info("apply bitmap: 0x%lx to: 0x%lx for %s\n", *new_bits,
+ *old_bits, query_modname ?: "'*'");
for (bi = 0; bi < map->length; bi++) {
if (test_bit(bi, new_bits) == test_bit(bi, old_bits))
@@ -627,12 +630,16 @@ static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
snprintf(query, QUERY_SIZE, "class %s %c%s", map->class_names[bi],
test_bit(bi, new_bits) ? '+' : '-', dcp->flags);
- ct = ddebug_exec_queries(query, NULL);
+ ct = ddebug_exec_queries(query, query_modname);
matches += ct;
v2pr_info("bit_%d: %d matches on class: %s -> 0x%lx\n", bi,
ct, map->class_names[bi], *new_bits);
}
+ if (*new_bits != *old_bits)
+ v2pr_info("applied bitmap: 0x%lx to: 0x%lx for %s\n", *new_bits,
+ *old_bits, query_modname ?: "'*'");
+
return matches;
}
@@ -645,6 +652,7 @@ static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
* param_set_dyndbg_classes - class FOO >control
* @instr: string echo>d to sysfs, input depends on map_type
* @kp: kp->arg has state: bits/lvl, map, map_type
+ * @mod_name: module name or null for all modules with the classes
*
* Enable/disable prdbgs by their class, as given in the arguments to
* DECLARE_DYNDBG_CLASSMAP. For LEVEL map-types, enforce relative
@@ -652,7 +660,9 @@ static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
*
* Returns: 0 or <0 if error.
*/
-int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp)
+static int param_set_dyndbg_module_classes(const char *instr,
+ const struct kernel_param *kp,
+ const char *mod_name)
{
const struct ddebug_class_param *dcp = kp->arg;
const struct ddebug_class_map *map = dcp->map;
@@ -675,8 +685,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:
@@ -689,7 +699,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);
*dcp->lvl = inrep;
break;
default:
@@ -699,16 +709,33 @@ int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp)
vpr_info("%s: total matches: %d\n", KP_NAME(kp), totct);
return 0;
}
+
+/**
+ * param_set_dyndbg_classes - classmap kparam setter
+ * @instr: string echo>d to sysfs, input depends on map_type
+ * @kp: kp->arg has state: bits/lvl, map, map_type
+ *
+ * enable/disable all class'd pr_debugs in the classmap. For LEVEL
+ * map-types, enforce * relative levels by bitpos.
+ *
+ * Returns: 0 or <0 if error.
+ */
+int param_set_dyndbg_classes(const char *instr, const struct kernel_param *kp)
+{
+ return param_set_dyndbg_module_classes(instr, kp, NULL);
+}
EXPORT_SYMBOL(param_set_dyndbg_classes);
/**
- * param_get_dyndbg_classes - classes reader
+ * param_get_dyndbg_classes - classmap kparam getter
* @buffer: string description of controlled bits -> classes
* @kp: kp->arg has state: bits, map
*
- * Reads last written state, underlying prdbg state may have been
- * altered by direct >control. Displays 0x for DISJOINT, 0-N for
- * LEVEL Returns: #chars written or <0 on error
+ * Reads last written state, underlying pr_debug states may have been
+ * altered by direct >control. Displays 0x for DISJOINT classmap
+ * types, 0-N for LEVEL types.
+ *
+ * Returns: ct of chars written or <0 on error
*/
int param_get_dyndbg_classes(char *buffer, const struct kernel_param *kp)
{
--
2.54.0
^ permalink raw reply related
* [PATCH v3 15/24] dyndbg: tighten fn-sig of ddebug_apply_class_bitmap
From: Jim Cromie @ 2026-06-01 12:05 UTC (permalink / raw)
To: Jonathan Corbet, Shuah Khan, Arnd Bergmann, Jason Baron,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Andrew Morton, Shuah Khan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: linux-doc, linux-kernel, linux-arch, linux-modules,
linux-kselftest, dri-devel, Jim Cromie, Louis Chauvet
In-Reply-To: <20260601-dd-maint-2-v3-0-4a15b241bd3c@gmail.com>
old_bits arg is currently a pointer to the input bits, but this could
allow inadvertent changes to the input by the fn. Disallow this.
And constify new_bits while here.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
v2: move RvB after SoB
---
lib/dynamic_debug.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 182b84b6f6c5..bb7e16d38f19 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -610,7 +610,8 @@ static int ddebug_exec_queries(char *query, const char *modname)
/* apply a new class-param setting */
static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
- unsigned long *new_bits, unsigned long *old_bits,
+ const unsigned long *new_bits,
+ const unsigned long old_bits,
const char *query_modname)
{
#define QUERY_SIZE 128
@@ -619,12 +620,12 @@ static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
int matches = 0;
int bi, ct;
- if (*new_bits != *old_bits)
+ if (*new_bits != old_bits)
v2pr_info("apply bitmap: 0x%lx to: 0x%lx for %s\n", *new_bits,
- *old_bits, query_modname ?: "'*'");
+ old_bits, query_modname ?: "'*'");
for (bi = 0; bi < map->length; bi++) {
- if (test_bit(bi, new_bits) == test_bit(bi, old_bits))
+ if (test_bit(bi, new_bits) == test_bit(bi, &old_bits))
continue;
snprintf(query, QUERY_SIZE, "class %s %c%s", map->class_names[bi],
@@ -636,9 +637,9 @@ static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
v2pr_info("bit_%d: %d matches on class: %s -> 0x%lx\n", bi,
ct, map->class_names[bi], *new_bits);
}
- if (*new_bits != *old_bits)
+ if (*new_bits != old_bits)
v2pr_info("applied bitmap: 0x%lx to: 0x%lx for %s\n", *new_bits,
- *old_bits, query_modname ?: "'*'");
+ old_bits, query_modname ?: "'*'");
return matches;
}
@@ -686,7 +687,7 @@ static int param_set_dyndbg_module_classes(const char *instr,
inrep &= CLASSMAP_BITMASK(map->length);
}
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);
+ totct += ddebug_apply_class_bitmap(dcp, &inrep, *dcp->bits, mod_name);
*dcp->bits = inrep;
break;
case DD_CLASS_TYPE_LEVEL_NUM:
@@ -699,7 +700,7 @@ static int param_set_dyndbg_module_classes(const char *instr,
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, mod_name);
+ totct += ddebug_apply_class_bitmap(dcp, &new_bits, old_bits, mod_name);
*dcp->lvl = inrep;
break;
default:
--
2.54.0
^ permalink raw reply related
* [PATCH v3 16/24] dyndbg: replace classmap list with an array-slice
From: Jim Cromie @ 2026-06-01 12:05 UTC (permalink / raw)
To: Jonathan Corbet, Shuah Khan, Arnd Bergmann, Jason Baron,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Andrew Morton, Shuah Khan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: linux-doc, linux-kernel, linux-arch, linux-modules,
linux-kselftest, dri-devel, Jim Cromie, Louis Chauvet
In-Reply-To: <20260601-dd-maint-2-v3-0-4a15b241bd3c@gmail.com>
Classmaps are stored in an elf section/array, but currently are
individually list-linked onto dyndbg's per-module ddebug_table for
operation. This is unnecessary.
Just like dyndbg's descriptors, classes are packed in compile order;
so even with many builtin modules employing multiple classmaps, each
modules' maps are packed contiguously, and can be treated as a
array-start-address & array-length.
So this drops the whole list building operation done in
ddebug_attach_module_classes(), and removes the list-head members of
the classmap structs. The "select-by-modname" condition is reused to
find the start,end of the subrange of classmaps belonging to the module.
NOTES:
There are multiple modules named "main" but thats an artifact of how
KBUILD_MODNAME gets its value, and none of those repeats are
contiguous. The legacy code segmenting the builtin descriptors
depends upon this, we are "reusing" that dependency.
The "filter-by-modname" on classmaps should really be done in
ddebug_add_module(1); ie closer to dynamic_debug_init(2), which
already splits up pr-debug descriptors into subranges by modname, then
calls (1) on each. (2) knows nothing of classmaps currently, and
doesn't need to.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
v2: RvB after SoB
---
include/linux/dynamic_debug.h | 1 -
lib/dynamic_debug.c | 65 +++++++++++++++++++++++--------------------
2 files changed, 35 insertions(+), 31 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 867e3978675f..0443781ed95b 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -73,7 +73,6 @@ enum ddebug_class_map_type {
};
struct ddebug_class_map {
- struct list_head link;
struct module *mod;
const char *mod_name; /* needed for builtins */
const char **class_names;
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index bb7e16d38f19..34d002e0a08c 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -45,10 +45,11 @@ extern struct ddebug_class_map __start___dyndbg_classes[];
extern struct ddebug_class_map __stop___dyndbg_classes[];
struct ddebug_table {
- struct list_head link, maps;
+ struct list_head link;
const char *mod_name;
- unsigned int num_ddebugs;
struct _ddebug *ddebugs;
+ struct ddebug_class_map *classes;
+ unsigned int num_ddebugs, num_classes;
};
struct ddebug_query {
@@ -149,12 +150,13 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
}
static struct ddebug_class_map *ddebug_find_valid_class(struct ddebug_table const *dt,
- const char *class_string, int *class_id)
+ const char *class_string,
+ int *class_id)
{
struct ddebug_class_map *map;
- int idx;
+ int i, idx;
- list_for_each_entry(map, &dt->maps, link) {
+ for (map = dt->classes, i = 0; i < dt->num_classes; i++, map++) {
idx = match_string(map->class_names, map->length, class_string);
if (idx >= 0) {
*class_id = idx + map->base;
@@ -165,7 +167,6 @@ static struct ddebug_class_map *ddebug_find_valid_class(struct ddebug_table cons
return NULL;
}
-#define __outvar /* filled by callee */
/*
* Search the tables for _ddebug's which match the given `query' and
* apply the `flags' and `mask' to them. Returns number of matching
@@ -227,7 +228,7 @@ static int ddebug_change(const struct ddebug_query *query,
unsigned int nfound = 0;
struct flagsbuf fbuf, nbuf;
struct ddebug_class_map *map = NULL;
- int __outvar valid_class;
+ int valid_class;
/* search for matching ddebugs */
mutex_lock(&ddebug_lock);
@@ -1062,9 +1063,10 @@ static void *ddebug_proc_next(struct seq_file *m, void *p, loff_t *pos)
static const char *ddebug_class_name(struct ddebug_iter *iter, struct _ddebug *dp)
{
- struct ddebug_class_map *map;
+ struct ddebug_class_map *map = iter->table->classes;
+ int i, nc = iter->table->num_classes;
- list_for_each_entry(map, &iter->table->maps, link)
+ for (i = 0; i < nc; i++, map++)
if (class_in_range(dp->class_id, map))
return map->class_names[dp->class_id - map->base];
@@ -1148,30 +1150,34 @@ static const struct proc_ops proc_fops = {
.proc_write = ddebug_proc_write
};
-static void ddebug_attach_module_classes(struct ddebug_table *dt,
- struct ddebug_class_map *classes,
- int num_classes)
+static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug_info *di)
{
struct ddebug_class_map *cm;
- int i, j, ct = 0;
+ int i, nc = 0;
- for (cm = classes, i = 0; i < num_classes; i++, cm++) {
+ /*
+ * Find this module's classmaps in a subrange/wholerange of
+ * the builtin/modular classmap vector/section. Save the start
+ * and length of the subrange at its edges.
+ */
+ for (cm = di->classes, i = 0; i < di->num_classes; i++, cm++) {
if (!strcmp(cm->mod_name, dt->mod_name)) {
-
- v2pr_info("class[%d]: module:%s base:%d len:%d ty:%d\n", i,
- cm->mod_name, cm->base, cm->length, cm->map_type);
-
- for (j = 0; j < cm->length; j++)
- v3pr_info(" %d: %d %s\n", j + cm->base, j,
- cm->class_names[j]);
-
- list_add(&cm->link, &dt->maps);
- ct++;
+ if (!nc) {
+ v2pr_info("start subrange, class[%d]: module:%s base:%d len:%d ty:%d\n",
+ i, cm->mod_name, cm->base, cm->length, cm->map_type);
+ dt->classes = cm;
+ }
+ nc++;
+ } else if (nc) {
+ /* end of matching classmaps */
+ break;
}
}
- if (ct)
- vpr_info("module:%s attached %d classes\n", dt->mod_name, ct);
+ if (nc) {
+ dt->num_classes = nc;
+ vpr_info("module:%s attached %d classes\n", dt->mod_name, nc);
+ }
}
/*
@@ -1203,10 +1209,9 @@ static int ddebug_add_module(struct _ddebug_info *di, const char *modname)
dt->num_ddebugs = di->num_descs;
INIT_LIST_HEAD(&dt->link);
- INIT_LIST_HEAD(&dt->maps);
if (di->classes && di->num_classes)
- ddebug_attach_module_classes(dt, di->classes, di->num_classes);
+ ddebug_attach_module_classes(dt, di);
mutex_lock(&ddebug_lock);
list_add_tail(&dt->link, &ddebug_tables);
@@ -1319,8 +1324,8 @@ static void ddebug_remove_all_tables(void)
mutex_lock(&ddebug_lock);
while (!list_empty(&ddebug_tables)) {
struct ddebug_table *dt = list_entry(ddebug_tables.next,
- struct ddebug_table,
- link);
+ struct ddebug_table,
+ link);
ddebug_table_free(dt);
}
mutex_unlock(&ddebug_lock);
--
2.54.0
^ permalink raw reply related
* [PATCH v3 17/24] dyndbg: macrofy a 2-index for-loop pattern
From: Jim Cromie @ 2026-06-01 12:05 UTC (permalink / raw)
To: Jonathan Corbet, Shuah Khan, Arnd Bergmann, Jason Baron,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Andrew Morton, Shuah Khan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: linux-doc, linux-kernel, linux-arch, linux-modules,
linux-kselftest, dri-devel, Jim Cromie, Louis Chauvet
In-Reply-To: <20260601-dd-maint-2-v3-0-4a15b241bd3c@gmail.com>
dynamic-debug currently has 2 __sections (__dyndbg, __dyndb_classes),
struct _ddebug_info keeps track of them both, with 2 members each:
_vec and _vec#_len.
We need to loop over these sections, with index and record pointer,
making ref to both _vec and _vec_len. This is already fiddly and
error-prone, and will get worse as we add a 3rd section.
Lets instead embed/abstract the fiddly-ness in the `for_subvec()`
macro, and avoid repeating it going forward.
This is a for-loop macro expander, so it syntactically expects to
precede either a single statement or a { block } of them, and the
usual typeof or do-while-0 tricks are unavailable to fix the
multiple-expansion warning.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
v2: move RvB after SoB
---
lib/dynamic_debug.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 34d002e0a08c..41bbb72ef730 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -129,6 +129,20 @@ do { \
#define v3pr_info(fmt, ...) vnpr_info(3, fmt, ##__VA_ARGS__)
#define v4pr_info(fmt, ...) vnpr_info(4, fmt, ##__VA_ARGS__)
+/*
+ * simplify a repeated for-loop pattern walking N steps in a T _vec
+ * member inside a struct _box. It expects int i and T *_sp to be
+ * declared in the caller.
+ * @_i: caller provided counter.
+ * @_sp: cursor into _vec, to examine each item.
+ * @_box: ptr to a struct containing @_vec member
+ * @_vec: name of a member in @_box
+ */
+#define for_subvec(_i, _sp, _box, _vec) \
+ for ((_i) = 0, (_sp) = (_box)->_vec; \
+ (_i) < (_box)->num_##_vec; \
+ (_i)++, (_sp)++) /* { block } */
+
static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
{
/* trim any trailing newlines */
@@ -156,7 +170,7 @@ static struct ddebug_class_map *ddebug_find_valid_class(struct ddebug_table cons
struct ddebug_class_map *map;
int i, idx;
- for (map = dt->classes, i = 0; i < dt->num_classes; i++, map++) {
+ for_subvec(i, map, dt, classes) {
idx = match_string(map->class_names, map->length, class_string);
if (idx >= 0) {
*class_id = idx + map->base;
@@ -1160,8 +1174,7 @@ 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 (cm = di->classes, i = 0; i < di->num_classes; i++, cm++) {
-
+ for_subvec(i, cm, di, classes) {
if (!strcmp(cm->mod_name, dt->mod_name)) {
if (!nc) {
v2pr_info("start subrange, class[%d]: module:%s base:%d len:%d ty:%d\n",
--
2.54.0
^ permalink raw reply related
* [PATCH v3 18/24] dyndbg: Upgrade class param storage to u64 for 64-bit classmaps
From: Jim Cromie @ 2026-06-01 12:05 UTC (permalink / raw)
To: Jonathan Corbet, Shuah Khan, Arnd Bergmann, Jason Baron,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Andrew Morton, Shuah Khan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: linux-doc, linux-kernel, linux-arch, linux-modules,
linux-kselftest, dri-devel, Jim Cromie
In-Reply-To: <20260601-dd-maint-2-v3-0-4a15b241bd3c@gmail.com>
Currently, `struct ddebug_class_param` uses `unsigned long` pointers
to store the state of `bits` and `lvl`. On 32-bit architectures, this
limits the bit-vector to 32 bits, which truncates 64-bit classmaps
(such as the one needed by `__drm_debug`).
To guarantee support for 64-bit debug categories across all
architectures, upgrade the internal storage types in `struct
_ddebug_class_param` to `u64`. This inherently makes both union
members the same size while safely avoiding truncation.
This includes:
- Changing union members `bits` and `lvl` to `u64 *`.
- Using `kstrtou64()` for sysfs input parsing.
- Replacing array-based `test_bit()` operations with safe `u64` scalar
bitwise logic (`!!(val & (1ULL << bi))`).
- adjusting lib/test_dynamic_debug.c too
- Updating `drm_print`'s `__drm_debug` refs to `u64`
further changes to drm later, when BROKEN is fixed.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
v3:
fix undefd behavior when classmaps is all 64 bits.
change struct ddebug_class_param to _ddebug_class_param
change module_param_named( type-arg from ulong to ullong)
in drivers/gpu/drm/drm_print.{c,h}
api change later
v2:
patch was "make bits & lvl same size"
but that size was unsigned long, only 32 bits on i386 etc
use u64 for all bits, and %llu %llx
u64-fix
u64-drm-dbg
_ddebug-cl-parm-drm
---
drivers/gpu/drm/drm_print.c | 6 +++---
include/drm/drm_print.h | 2 +-
include/linux/dynamic_debug.h | 4 ++--
lib/dynamic_debug.c | 34 ++++++++++++++++++----------------
lib/test_dynamic_debug.c | 2 +-
5 files changed, 25 insertions(+), 23 deletions(-)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index ded9461df5f2..9b4d8e3baacc 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -40,7 +40,7 @@
* __drm_debug: Enable debug output.
* Bitmask of DRM_UT_x. See include/drm/drm_print.h for details.
*/
-unsigned long __drm_debug;
+u64 __drm_debug;
EXPORT_SYMBOL(__drm_debug);
MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug category.\n"
@@ -54,7 +54,7 @@ MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug cat
"\t\tBit 8 (0x100) will enable DP messages (displayport code)");
#if !defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
-module_param_named(debug, __drm_debug, ulong, 0600);
+module_param_named(debug, __drm_debug, ullong, 0600);
#else
/* classnames must match vals of enum drm_debug_category */
DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
@@ -69,7 +69,7 @@ DECLARE_DYNDBG_CLASSMAP(drm_debug_classes, DD_CLASS_TYPE_DISJOINT_BITS, 0,
"DRM_UT_DP",
"DRM_UT_DRMRES");
-static struct ddebug_class_param drm_debug_bitmap = {
+static struct _ddebug_class_param drm_debug_bitmap = {
.bits = &__drm_debug,
.flags = "p",
.map = &drm_debug_classes,
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index ab017b05e175..21a0ced0b2f2 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -39,7 +39,7 @@ struct drm_device;
struct seq_file;
/* Do *not* use outside of drm_print.[ch]! */
-extern unsigned long __drm_debug;
+extern u64 __drm_debug;
/**
* DOC: print
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 0443781ed95b..91546a99014b 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -91,8 +91,8 @@ struct _ddebug_info {
struct ddebug_class_param {
union {
- unsigned long *bits;
- unsigned int *lvl;
+ u64 *bits;
+ u64 *lvl;
};
char flags[8];
const struct ddebug_class_map *map;
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 41bbb72ef730..7298520a89dc 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -625,8 +625,7 @@ static int ddebug_exec_queries(char *query, const char *modname)
/* apply a new class-param setting */
static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
- const unsigned long *new_bits,
- const unsigned long old_bits,
+ const u64 *new_bits, const u64 old_bits,
const char *query_modname)
{
#define QUERY_SIZE 128
@@ -636,24 +635,27 @@ static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
int bi, ct;
if (*new_bits != old_bits)
- v2pr_info("apply bitmap: 0x%lx to: 0x%lx for %s\n", *new_bits,
+ v2pr_info("apply bitmap: 0x%llx to: 0x%llx for %s\n", *new_bits,
old_bits, query_modname ?: "'*'");
for (bi = 0; bi < map->length; bi++) {
- if (test_bit(bi, new_bits) == test_bit(bi, &old_bits))
+ bool new_b = !!(*new_bits & (1ULL << bi));
+ bool old_b = !!(old_bits & (1ULL << bi));
+
+ if (new_b == old_b)
continue;
snprintf(query, QUERY_SIZE, "class %s %c%s", map->class_names[bi],
- test_bit(bi, new_bits) ? '+' : '-', dcp->flags);
+ new_b ? '+' : '-', dcp->flags);
ct = ddebug_exec_queries(query, query_modname);
matches += ct;
- v2pr_info("bit_%d: %d matches on class: %s -> 0x%lx\n", bi,
+ v2pr_info("bit_%d: %d matches on class: %s -> 0x%llx\n", bi,
ct, map->class_names[bi], *new_bits);
}
if (*new_bits != old_bits)
- v2pr_info("applied bitmap: 0x%lx to: 0x%lx for %s\n", *new_bits,
+ v2pr_info("applied bitmap: 0x%llx to: 0x%llx for %s\n", *new_bits,
old_bits, query_modname ?: "'*'");
return matches;
@@ -662,7 +664,7 @@ static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
/* stub to later conditionally add "$module." prefix where not already done */
#define KP_NAME(kp) kp->name
-#define CLASSMAP_BITMASK(width) ((1UL << (width)) - 1)
+#define CLASSMAP_BITMASK(width) ((width) == 64 ? ~0ULL : (1ULL << (width)) - 1)
/**
* param_set_dyndbg_classes - class FOO >control
@@ -682,10 +684,10 @@ static int param_set_dyndbg_module_classes(const char *instr,
{
const struct ddebug_class_param *dcp = kp->arg;
const struct ddebug_class_map *map = dcp->map;
- unsigned long inrep, new_bits, old_bits;
+ u64 inrep, new_bits, old_bits;
int rc, totct = 0;
- rc = kstrtoul(instr, 0, &inrep);
+ rc = kstrtou64(instr, 0, &inrep);
if (rc) {
int len = strcspn(instr, "\n");
pr_err("expecting numeric input, not: %.*s > %s\n",
@@ -697,24 +699,24 @@ static int param_set_dyndbg_module_classes(const char *instr,
case DD_CLASS_TYPE_DISJOINT_BITS:
/* expect bits. mask and warn if too many */
if (inrep & ~CLASSMAP_BITMASK(map->length)) {
- pr_warn("%s: input: 0x%lx exceeds mask: 0x%lx, masking\n",
+ pr_warn("%s: input: 0x%llx exceeds mask: 0x%llx, masking\n",
KP_NAME(kp), inrep, CLASSMAP_BITMASK(map->length));
inrep &= CLASSMAP_BITMASK(map->length);
}
- v2pr_info("bits:0x%lx > %s.%s\n", inrep, mod_name ?: "*", KP_NAME(kp));
+ v2pr_info("bits:0x%llx > %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:
/* input is bitpos, of highest verbosity to be enabled */
if (inrep > map->length) {
- pr_warn("%s: level:%ld exceeds max:%d, clamping\n",
+ pr_warn("%s: level:%llu exceeds max:%d, clamping\n",
KP_NAME(kp), inrep, map->length);
inrep = map->length;
}
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));
+ v2pr_info("lvl:%llu bits:0x%llx > %s\n", inrep, new_bits, KP_NAME(kp));
totct += ddebug_apply_class_bitmap(dcp, &new_bits, old_bits, mod_name);
*dcp->lvl = inrep;
break;
@@ -760,9 +762,9 @@ int param_get_dyndbg_classes(char *buffer, const struct kernel_param *kp)
switch (map->map_type) {
case DD_CLASS_TYPE_DISJOINT_BITS:
- return scnprintf(buffer, PAGE_SIZE, "0x%lx\n", *dcp->bits);
+ return scnprintf(buffer, PAGE_SIZE, "0x%llx\n", *dcp->bits);
case DD_CLASS_TYPE_LEVEL_NUM:
- return scnprintf(buffer, PAGE_SIZE, "%d\n", *dcp->lvl);
+ return scnprintf(buffer, PAGE_SIZE, "%llu\n", *dcp->lvl);
default:
return -1;
}
diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
index 74d183ebf3e0..c049580d2152 100644
--- a/lib/test_dynamic_debug.c
+++ b/lib/test_dynamic_debug.c
@@ -40,7 +40,7 @@ module_param_cb(do_prints, ¶m_ops_do_prints, NULL, 0600);
* - tie together sysname, mapname, bitsname, flagsname
*/
#define DD_SYS_WRAP(_model, _flags) \
- static unsigned long bits_##_model; \
+ static u64 bits_##_model; \
static struct ddebug_class_param _flags##_model = { \
.bits = &bits_##_model, \
.flags = #_flags, \
--
2.54.0
^ permalink raw reply related
* [PATCH v3 19/24] dyndbg,module: make proper substructs in _ddebug_info
From: Jim Cromie @ 2026-06-01 12:05 UTC (permalink / raw)
To: Jonathan Corbet, Shuah Khan, Arnd Bergmann, Jason Baron,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Andrew Morton, Shuah Khan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: linux-doc, linux-kernel, linux-arch, linux-modules,
linux-kselftest, dri-devel, Jim Cromie, Louis Chauvet
In-Reply-To: <20260601-dd-maint-2-v3-0-4a15b241bd3c@gmail.com>
recompose struct _ddebug_info, inserting proper sub-structs.
The struct _ddebug_info has 2 pairs of _vec, num_##_vec fields, for
descs and classes respectively. for_subvec() makes walking these
vectors less cumbersome, now lets move those field pairs into their
own "vec" structs: _ddebug_descs & _ddebug_class_maps, and re-compose
struct _ddebug_info to contain them cleanly. This also lets us get
rid of for_subvec()'s num_##_vec paste-up.
Also recompose struct ddebug_table to contain a _ddebug_info. This
reinforces its use as a cursor into relevant data for a builtin
module, and access to the full _ddebug state for modules.
NOTES:
Fixup names: Normalize all struct names to "struct _ddebug_*"
eliminating the minor/stupid variations created in classmaps-v1.
Change section names together, for more obvious name pairing.
Invariant: These vectors ref a contiguous subrange of __section memory
in builtin/DATA or in loadable modules via mod->dyndbg_info; with
guaranteed life-time for us.
struct module contains a _ddebug_info field and module/main.c sets it
up, so that gets adjusted rather obviously.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
v3: squash in section name changes.
v2:
Move RvB after SoB
In structs _ddebug_descs & _ddebug_class_maps, change int length to unsigned int
No use of <0 vals is contemplated.
dyndbg: improve section names
change __dyndbg to __dyndbg_descs
change __dyndbg_classes to __dyndbg_class_maps
this sets up for adding __dyndbg_class_users
---
include/asm-generic/dyndbg.lds.h | 14 ++---
include/linux/dynamic_debug.h | 40 +++++++++-----
kernel/module/main.c | 12 ++--
lib/dynamic_debug.c | 116 +++++++++++++++++++--------------------
lib/test_dynamic_debug.c | 2 +-
5 files changed, 98 insertions(+), 86 deletions(-)
diff --git a/include/asm-generic/dyndbg.lds.h b/include/asm-generic/dyndbg.lds.h
index 9d8951bef688..ec661f9f3793 100644
--- a/include/asm-generic/dyndbg.lds.h
+++ b/include/asm-generic/dyndbg.lds.h
@@ -3,16 +3,16 @@
#define __ASM_GENERIC_DYNDBG_LDS_H
#include <asm-generic/bounded_sections.lds.h>
-#define DYNDBG_SECTIONS() \
- BOUNDED_SECTION_BY(__dyndbg, ___dyndbg) \
- BOUNDED_SECTION_BY(__dyndbg_classes, ___dyndbg_classes)
+#define DYNDBG_SECTIONS() \
+ BOUNDED_SECTION_BY(__dyndbg_descs, ___dyndbg_descs) \
+ BOUNDED_SECTION_BY(__dyndbg_class_maps, ___dyndbg_class_maps)
#define MOD_DYNDBG_SECTIONS() \
- __dyndbg 0 : ALIGN(8) { \
- KEEP(*(__dyndbg)) \
+ __dyndbg_descs 0 : ALIGN(8) { \
+ KEEP(*(__dyndbg_descs)) \
} \
- __dyndbg_classes 0 : ALIGN(8) { \
- KEEP(*(__dyndbg_classes)) \
+ __dyndbg_class_maps 0 : ALIGN(8) { \
+ KEEP(*(__dyndbg_class_maps)) \
}
#endif /* __ASM_GENERIC_DYNDBG_LDS_H */
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 91546a99014b..8fc315d0e5a5 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -72,8 +72,8 @@ enum ddebug_class_map_type {
*/
};
-struct ddebug_class_map {
- struct module *mod;
+struct _ddebug_class_map {
+ struct module *mod; /* NULL for builtins */
const char *mod_name; /* needed for builtins */
const char **class_names;
const int length;
@@ -81,21 +81,33 @@ struct ddebug_class_map {
enum ddebug_class_map_type map_type;
};
-/* encapsulate linker provided built-in (or module) dyndbg data */
+/*
+ * @_ddebug_info: gathers module/builtin dyndbg_* __sections together.
+ * For builtins, it is used as a cursor, with the inner structs
+ * marking sub-vectors of the builtin __sections in DATA.
+ */
+struct _ddebug_descs {
+ struct _ddebug *start;
+ unsigned int len;
+};
+
+struct _ddebug_class_maps {
+ struct _ddebug_class_map *start;
+ unsigned int len;
+};
+
struct _ddebug_info {
- struct _ddebug *descs;
- struct ddebug_class_map *classes;
- unsigned int num_descs;
- unsigned int num_classes;
+ struct _ddebug_descs descs;
+ struct _ddebug_class_maps maps;
};
-struct ddebug_class_param {
+struct _ddebug_class_param {
union {
u64 *bits;
u64 *lvl;
};
char flags[8];
- const struct ddebug_class_map *map;
+ const struct _ddebug_class_map *map;
};
/*
@@ -114,8 +126,8 @@ struct ddebug_class_param {
*/
#define DECLARE_DYNDBG_CLASSMAP(_var, _maptype, _base, ...) \
static const char *_var##_classnames[] = { __VA_ARGS__ }; \
- static struct ddebug_class_map __aligned(8) __used \
- __section("__dyndbg_classes") _var = { \
+ static struct _ddebug_class_map __aligned(8) __used \
+ __section("__dyndbg_class_maps") _var = { \
.mod = THIS_MODULE, \
.mod_name = KBUILD_MODNAME, \
.base = _base, \
@@ -155,7 +167,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
#define DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, cls, fmt) \
static struct _ddebug __aligned(8) \
- __section("__dyndbg") name = { \
+ __section("__dyndbg_descs") name = { \
.modname = KBUILD_MODNAME, \
.function = __func__, \
.filename = __FILE__, \
@@ -242,7 +254,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
* macro.
*/
#define _dynamic_func_call_cls(cls, fmt, func, ...) \
- __dynamic_func_call_cls(__UNIQUE_ID(ddebug), cls, fmt, func, ##__VA_ARGS__)
+ __dynamic_func_call_cls(__UNIQUE_ID(_ddebug), cls, fmt, func, ##__VA_ARGS__)
#define _dynamic_func_call(fmt, func, ...) \
_dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
@@ -252,7 +264,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
* with precisely the macro's varargs.
*/
#define _dynamic_func_call_cls_no_desc(cls, fmt, func, ...) \
- __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt, \
+ __dynamic_func_call_cls_no_desc(__UNIQUE_ID(_ddebug), cls, fmt, \
func, ##__VA_ARGS__)
#define _dynamic_func_call_no_desc(fmt, func, ...) \
_dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt, \
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 46dd8d25a605..bd7899a91755 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2774,12 +2774,12 @@ static int find_module_sections(struct module *mod, struct load_info *info)
pr_warn("%s: Ignoring obsolete parameters\n", mod->name);
#ifdef CONFIG_DYNAMIC_DEBUG_CORE
- mod->dyndbg_info.descs = section_objs(info, "__dyndbg",
- sizeof(*mod->dyndbg_info.descs),
- &mod->dyndbg_info.num_descs);
- mod->dyndbg_info.classes = section_objs(info, "__dyndbg_classes",
- sizeof(*mod->dyndbg_info.classes),
- &mod->dyndbg_info.num_classes);
+ mod->dyndbg_info.descs.start = section_objs(info, "__dyndbg_descs",
+ sizeof(*mod->dyndbg_info.descs.start),
+ &mod->dyndbg_info.descs.len);
+ mod->dyndbg_info.maps.start = section_objs(info, "__dyndbg_class_maps",
+ sizeof(*mod->dyndbg_info.maps.start),
+ &mod->dyndbg_info.maps.len);
#endif
return 0;
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 7298520a89dc..0995cbef6f47 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -39,17 +39,15 @@
#include <rdma/ib_verbs.h>
-extern struct _ddebug __start___dyndbg[];
-extern struct _ddebug __stop___dyndbg[];
-extern struct ddebug_class_map __start___dyndbg_classes[];
-extern struct ddebug_class_map __stop___dyndbg_classes[];
+extern struct _ddebug __start___dyndbg_descs[];
+extern struct _ddebug __stop___dyndbg_descs[];
+extern struct _ddebug_class_map __start___dyndbg_class_maps[];
+extern struct _ddebug_class_map __stop___dyndbg_class_maps[];
struct ddebug_table {
struct list_head link;
const char *mod_name;
- struct _ddebug *ddebugs;
- struct ddebug_class_map *classes;
- unsigned int num_ddebugs, num_classes;
+ struct _ddebug_info info;
};
struct ddebug_query {
@@ -136,11 +134,11 @@ do { \
* @_i: caller provided counter.
* @_sp: cursor into _vec, to examine each item.
* @_box: ptr to a struct containing @_vec member
- * @_vec: name of a member in @_box
+ * @_vec: name of a vector member in @_box
*/
#define for_subvec(_i, _sp, _box, _vec) \
- for ((_i) = 0, (_sp) = (_box)->_vec; \
- (_i) < (_box)->num_##_vec; \
+ for ((_i) = 0, (_sp) = (_box)->_vec.start; \
+ (_i) < (_box)->_vec.len; \
(_i)++, (_sp)++) /* { block } */
static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
@@ -163,14 +161,14 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
query->first_lineno, query->last_lineno, query->class_string);
}
-static struct ddebug_class_map *ddebug_find_valid_class(struct ddebug_table const *dt,
+static struct _ddebug_class_map *ddebug_find_valid_class(struct ddebug_table const *dt,
const char *class_string,
int *class_id)
{
- struct ddebug_class_map *map;
+ struct _ddebug_class_map *map;
int i, idx;
- for_subvec(i, map, dt, classes) {
+ for_subvec(i, map, &dt->info, maps) {
idx = match_string(map->class_names, map->length, class_string);
if (idx >= 0) {
*class_id = idx + map->base;
@@ -241,7 +239,7 @@ static int ddebug_change(const struct ddebug_query *query,
unsigned int newflags;
unsigned int nfound = 0;
struct flagsbuf fbuf, nbuf;
- struct ddebug_class_map *map = NULL;
+ struct _ddebug_class_map *map = NULL;
int valid_class;
/* search for matching ddebugs */
@@ -262,8 +260,8 @@ static int ddebug_change(const struct ddebug_query *query,
valid_class = _DPRINTK_CLASS_DFLT;
}
- for (i = 0; i < dt->num_ddebugs; i++) {
- struct _ddebug *dp = &dt->ddebugs[i];
+ for (i = 0; i < dt->info.descs.len; i++) {
+ struct _ddebug *dp = &dt->info.descs.start[i];
if (!ddebug_match_desc(query, dp, valid_class))
continue;
@@ -624,13 +622,13 @@ static int ddebug_exec_queries(char *query, const char *modname)
}
/* apply a new class-param setting */
-static int ddebug_apply_class_bitmap(const struct ddebug_class_param *dcp,
+static int ddebug_apply_class_bitmap(const struct _ddebug_class_param *dcp,
const u64 *new_bits, const u64 old_bits,
const char *query_modname)
{
#define QUERY_SIZE 128
char query[QUERY_SIZE];
- const struct ddebug_class_map *map = dcp->map;
+ const struct _ddebug_class_map *map = dcp->map;
int matches = 0;
int bi, ct;
@@ -682,8 +680,8 @@ static int param_set_dyndbg_module_classes(const char *instr,
const struct kernel_param *kp,
const char *mod_name)
{
- const struct ddebug_class_param *dcp = kp->arg;
- const struct ddebug_class_map *map = dcp->map;
+ const struct _ddebug_class_param *dcp = kp->arg;
+ const struct _ddebug_class_map *map = dcp->map;
u64 inrep, new_bits, old_bits;
int rc, totct = 0;
@@ -757,8 +755,8 @@ EXPORT_SYMBOL(param_set_dyndbg_classes);
*/
int param_get_dyndbg_classes(char *buffer, const struct kernel_param *kp)
{
- const struct ddebug_class_param *dcp = kp->arg;
- const struct ddebug_class_map *map = dcp->map;
+ const struct _ddebug_class_param *dcp = kp->arg;
+ const struct _ddebug_class_map *map = dcp->map;
switch (map->map_type) {
case DD_CLASS_TYPE_DISJOINT_BITS:
@@ -1005,8 +1003,8 @@ static struct _ddebug *ddebug_iter_first(struct ddebug_iter *iter)
}
iter->table = list_entry(ddebug_tables.next,
struct ddebug_table, link);
- iter->idx = iter->table->num_ddebugs;
- return &iter->table->ddebugs[--iter->idx];
+ iter->idx = iter->table->info.descs.len;
+ return &iter->table->info.descs.start[--iter->idx];
}
/*
@@ -1027,10 +1025,10 @@ static struct _ddebug *ddebug_iter_next(struct ddebug_iter *iter)
}
iter->table = list_entry(iter->table->link.next,
struct ddebug_table, link);
- iter->idx = iter->table->num_ddebugs;
+ iter->idx = iter->table->info.descs.len;
--iter->idx;
}
- return &iter->table->ddebugs[iter->idx];
+ return &iter->table->info.descs.start[iter->idx];
}
/*
@@ -1074,16 +1072,19 @@ static void *ddebug_proc_next(struct seq_file *m, void *p, loff_t *pos)
return dp;
}
-#define class_in_range(class_id, map) \
- (class_id >= map->base && class_id < map->base + map->length)
+static bool ddebug_class_in_range(const int class_id, const struct _ddebug_class_map *map)
+{
+ return (class_id >= map->base &&
+ class_id < map->base + map->length);
+}
-static const char *ddebug_class_name(struct ddebug_iter *iter, struct _ddebug *dp)
+static const char *ddebug_class_name(struct ddebug_table *dt, struct _ddebug *dp)
{
- struct ddebug_class_map *map = iter->table->classes;
- int i, nc = iter->table->num_classes;
+ struct _ddebug_class_map *map;
+ int i;
- for (i = 0; i < nc; i++, map++)
- if (class_in_range(dp->class_id, map))
+ for_subvec(i, map, &dt->info, maps)
+ if (ddebug_class_in_range(dp->class_id, map))
return map->class_names[dp->class_id - map->base];
return NULL;
@@ -1116,7 +1117,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
seq_putc(m, '"');
if (dp->class_id != _DPRINTK_CLASS_DFLT) {
- class = ddebug_class_name(iter, dp);
+ class = ddebug_class_name(iter->table, dp);
if (class)
seq_printf(m, " class:%s", class);
else
@@ -1168,7 +1169,7 @@ static const struct proc_ops proc_fops = {
static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug_info *di)
{
- struct ddebug_class_map *cm;
+ struct _ddebug_class_map *cm;
int i, nc = 0;
/*
@@ -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)) {
if (!nc) {
v2pr_info("start subrange, class[%d]: module:%s base:%d len:%d ty:%d\n",
i, cm->mod_name, cm->base, cm->length, cm->map_type);
- dt->classes = cm;
+ dt->info.maps.start = cm;
}
nc++;
} else if (nc) {
@@ -1190,7 +1191,7 @@ static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug
}
}
if (nc) {
- dt->num_classes = nc;
+ dt->info.maps.len = nc;
vpr_info("module:%s attached %d classes\n", dt->mod_name, nc);
}
}
@@ -1203,10 +1204,10 @@ static int ddebug_add_module(struct _ddebug_info *di, const char *modname)
{
struct ddebug_table *dt;
- if (!di->num_descs)
+ if (!di->descs.len)
return 0;
- v3pr_info("add-module: %s %d sites\n", modname, di->num_descs);
+ v3pr_info("add-module: %s %d sites\n", modname, di->descs.len);
dt = kzalloc_obj(*dt);
if (dt == NULL) {
@@ -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;
INIT_LIST_HEAD(&dt->link);
- if (di->classes && di->num_classes)
+ if (di->maps.len)
ddebug_attach_module_classes(dt, di);
mutex_lock(&ddebug_lock);
list_add_tail(&dt->link, &ddebug_tables);
mutex_unlock(&ddebug_lock);
- vpr_info("%3u debug prints in module %s\n", di->num_descs, modname);
+ vpr_info("%3u debug prints in module %s\n", di->descs.len, modname);
return 0;
}
@@ -1379,10 +1379,10 @@ static int __init dynamic_debug_init(void)
char *cmdline;
struct _ddebug_info di = {
- .descs = __start___dyndbg,
- .classes = __start___dyndbg_classes,
- .num_descs = __stop___dyndbg - __start___dyndbg,
- .num_classes = __stop___dyndbg_classes - __start___dyndbg_classes,
+ .descs.start = __start___dyndbg_descs,
+ .maps.start = __start___dyndbg_class_maps,
+ .descs.len = __stop___dyndbg_descs - __start___dyndbg_descs,
+ .maps.len = __stop___dyndbg_class_maps - __start___dyndbg_class_maps,
};
#ifdef CONFIG_MODULES
@@ -1393,7 +1393,7 @@ static int __init dynamic_debug_init(void)
}
#endif /* CONFIG_MODULES */
- if (&__start___dyndbg == &__stop___dyndbg) {
+ if (&__start___dyndbg_descs == &__stop___dyndbg_descs) {
if (IS_ENABLED(CONFIG_DYNAMIC_DEBUG)) {
pr_warn("_ddebug table is empty in a CONFIG_DYNAMIC_DEBUG build\n");
return 1;
@@ -1403,16 +1403,16 @@ static int __init dynamic_debug_init(void)
return 0;
}
- iter = iter_mod_start = __start___dyndbg;
+ iter = iter_mod_start = __start___dyndbg_descs;
modname = iter->modname;
i = mod_sites = mod_ct = 0;
- for (; iter < __stop___dyndbg; iter++, i++, mod_sites++) {
+ for (; iter < __stop___dyndbg_descs; iter++, i++, mod_sites++) {
if (strcmp(modname, iter->modname)) {
mod_ct++;
- di.num_descs = mod_sites;
- di.descs = iter_mod_start;
+ di.descs.len = mod_sites;
+ di.descs.start = iter_mod_start;
ret = ddebug_add_module(&di, modname);
if (ret)
goto out_err;
@@ -1422,19 +1422,19 @@ static int __init dynamic_debug_init(void)
iter_mod_start = iter;
}
}
- di.num_descs = mod_sites;
- di.descs = iter_mod_start;
+ di.descs.len = mod_sites;
+ di.descs.start = iter_mod_start;
ret = ddebug_add_module(&di, modname);
if (ret)
goto out_err;
ddebug_init_success = 1;
- vpr_info("%d prdebugs in %d modules, %d KiB in ddebug tables, %d kiB in __dyndbg section\n",
+ vpr_info("%d prdebugs in %d modules, %d KiB in ddebug tables, %d kiB in __dyndbg_descs section\n",
i, mod_ct, (int)((mod_ct * sizeof(struct ddebug_table)) >> 10),
(int)((i * sizeof(struct _ddebug)) >> 10));
- if (di.num_classes)
- v2pr_info(" %d builtin ddebug class-maps\n", di.num_classes);
+ if (di.maps.len)
+ v2pr_info(" %d builtin ddebug class-maps\n", di.maps.len);
/* now that ddebug tables are loaded, process all boot args
* again to find and activate queries given in dyndbg params.
diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
index c049580d2152..e6b2422ca671 100644
--- a/lib/test_dynamic_debug.c
+++ b/lib/test_dynamic_debug.c
@@ -41,7 +41,7 @@ module_param_cb(do_prints, ¶m_ops_do_prints, NULL, 0600);
*/
#define DD_SYS_WRAP(_model, _flags) \
static u64 bits_##_model; \
- static struct ddebug_class_param _flags##_model = { \
+ static struct _ddebug_class_param _flags##_model = { \
.bits = &bits_##_model, \
.flags = #_flags, \
.map = &map_##_model, \
--
2.54.0
^ permalink raw reply related
* [PATCH v3 20/24] dyndbg: move mod_name down from struct ddebug_table to _ddebug_info
From: Jim Cromie @ 2026-06-01 12:05 UTC (permalink / raw)
To: Jonathan Corbet, Shuah Khan, Arnd Bergmann, Jason Baron,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Andrew Morton, Shuah Khan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: linux-doc, linux-kernel, linux-arch, linux-modules,
linux-kselftest, dri-devel, Jim Cromie, Louis Chauvet
In-Reply-To: <20260601-dd-maint-2-v3-0-4a15b241bd3c@gmail.com>
struct _ddebug_info already has most of dyndbg's info for a module;
push debug_table.mod_name down into it, finishing the encapsulation.
This allows refactoring several callchains, passing &_ddebug_info
instead of &ddebug_table, and hoisting the "&dt->info" deref up
instead of repeating it thru the callchans
ddebug_table contains a _ddebug_info member, so its users keep access
to mod_name, just now with "->info." added in.
In static ddebug_add_module(&di), reinforce the cursor-model by
dropping the modname arg, and setting di->mod_name at each caller.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
v2: move RvB after SoB
old-v12
. moved up 1 position in series, ahead of hoist...
---
include/linux/dynamic_debug.h | 1 +
lib/dynamic_debug.c | 50 ++++++++++++++++++++++---------------------
2 files changed, 27 insertions(+), 24 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 8fc315d0e5a5..e4a62cb73267 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -97,6 +97,7 @@ struct _ddebug_class_maps {
};
struct _ddebug_info {
+ const char *mod_name;
struct _ddebug_descs descs;
struct _ddebug_class_maps maps;
};
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 0995cbef6f47..ebfff48c4a6c 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -46,7 +46,6 @@ extern struct _ddebug_class_map __stop___dyndbg_class_maps[];
struct ddebug_table {
struct list_head link;
- const char *mod_name;
struct _ddebug_info info;
};
@@ -245,10 +244,11 @@ static int ddebug_change(const struct ddebug_query *query,
/* search for matching ddebugs */
mutex_lock(&ddebug_lock);
list_for_each_entry(dt, &ddebug_tables, link) {
+ struct _ddebug_info *di = &dt->info;
/* match against the module name */
if (query->module &&
- !match_wildcard(query->module, dt->mod_name))
+ !match_wildcard(query->module, di->mod_name))
continue;
if (query->class_string) {
@@ -260,8 +260,8 @@ static int ddebug_change(const struct ddebug_query *query,
valid_class = _DPRINTK_CLASS_DFLT;
}
- for (i = 0; i < dt->info.descs.len; i++) {
- struct _ddebug *dp = &dt->info.descs.start[i];
+ for (i = 0; i < di->descs.len; i++) {
+ struct _ddebug *dp = &di->descs.start[i];
if (!ddebug_match_desc(query, dp, valid_class))
continue;
@@ -281,7 +281,7 @@ static int ddebug_change(const struct ddebug_query *query,
#endif
v4pr_info("changed %s:%d [%s]%s %s => %s\n",
trim_prefix(dp->filename), dp->lineno,
- dt->mod_name, dp->function,
+ di->mod_name, dp->function,
ddebug_describe_flags(dp->flags, &fbuf),
ddebug_describe_flags(newflags, &nbuf));
dp->flags = newflags;
@@ -1078,12 +1078,12 @@ static bool ddebug_class_in_range(const int class_id, const struct _ddebug_class
class_id < map->base + map->length);
}
-static const char *ddebug_class_name(struct ddebug_table *dt, struct _ddebug *dp)
+static const char *ddebug_class_name(struct _ddebug_info *di, struct _ddebug *dp)
{
struct _ddebug_class_map *map;
int i;
- for_subvec(i, map, &dt->info, maps)
+ for_subvec(i, map, di, maps)
if (ddebug_class_in_range(dp->class_id, map))
return map->class_names[dp->class_id - map->base];
@@ -1111,13 +1111,13 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
seq_printf(m, "%s:%u [%s]%s =%s \"",
trim_prefix(dp->filename), dp->lineno,
- iter->table->mod_name, dp->function,
+ iter->table->info.mod_name, dp->function,
ddebug_describe_flags(dp->flags, &flags));
seq_escape_str(m, dp->format, ESCAPE_SPACE, "\t\r\n\"");
seq_putc(m, '"');
if (dp->class_id != _DPRINTK_CLASS_DFLT) {
- class = ddebug_class_name(iter->table, dp);
+ class = ddebug_class_name(&iter->table->info, dp);
if (class)
seq_printf(m, " class:%s", class);
else
@@ -1178,7 +1178,7 @@ static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug
* and length of the subrange at its edges.
*/
for_subvec(i, cm, di, maps) {
- if (!strcmp(cm->mod_name, dt->mod_name)) {
+ if (!strcmp(cm->mod_name, dt->info.mod_name)) {
if (!nc) {
v2pr_info("start subrange, class[%d]: module:%s base:%d len:%d ty:%d\n",
i, cm->mod_name, cm->base, cm->length, cm->map_type);
@@ -1192,7 +1192,7 @@ static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug
}
if (nc) {
dt->info.maps.len = nc;
- vpr_info("module:%s attached %d classes\n", dt->mod_name, nc);
+ vpr_info("module:%s attached %d classes\n", dt->info.mod_name, nc);
}
}
@@ -1200,27 +1200,26 @@ static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug
* Allocate a new ddebug_table for the given module
* and add it to the global list.
*/
-static int ddebug_add_module(struct _ddebug_info *di, const char *modname)
+static int ddebug_add_module(struct _ddebug_info *di)
{
struct ddebug_table *dt;
if (!di->descs.len)
return 0;
- v3pr_info("add-module: %s %d sites\n", modname, di->descs.len);
+ v3pr_info("add-module: %s %d sites\n", di->mod_name, di->descs.len);
dt = kzalloc_obj(*dt);
if (dt == NULL) {
- pr_err("error adding module: %s\n", modname);
+ pr_err("error adding module: %s\n", di->mod_name);
return -ENOMEM;
}
/*
- * For built-in modules, name lives in .rodata and is
- * immortal. For loaded modules, name points at the name[]
- * member of struct module, which lives at least as long as
- * this struct ddebug_table.
+ * For built-in modules, name (as supplied in di by its
+ * callers) lives in .rodata and is immortal. For loaded
+ * modules, name points at the name[] member of struct module,
+ * which lives at least as long as this struct ddebug_table.
*/
- dt->mod_name = modname;
dt->info = *di;
INIT_LIST_HEAD(&dt->link);
@@ -1232,7 +1231,7 @@ static int ddebug_add_module(struct _ddebug_info *di, const char *modname)
list_add_tail(&dt->link, &ddebug_tables);
mutex_unlock(&ddebug_lock);
- vpr_info("%3u debug prints in module %s\n", di->descs.len, modname);
+ vpr_info("%3u debug prints in module %s\n", di->descs.len, di->mod_name);
return 0;
}
@@ -1295,7 +1294,7 @@ static int ddebug_remove_module(const char *mod_name)
mutex_lock(&ddebug_lock);
list_for_each_entry_safe(dt, nextdt, &ddebug_tables, link) {
- if (dt->mod_name == mod_name) {
+ if (dt->info.mod_name == mod_name) {
ddebug_table_free(dt);
ret = 0;
break;
@@ -1315,7 +1314,8 @@ static int ddebug_module_notify(struct notifier_block *self, unsigned long val,
switch (val) {
case MODULE_STATE_COMING:
- ret = ddebug_add_module(&mod->dyndbg_info, mod->name);
+ mod->dyndbg_info.mod_name = mod->name;
+ ret = ddebug_add_module(&mod->dyndbg_info);
if (ret)
WARN(1, "Failed to allocate memory: dyndbg may not work properly.\n");
break;
@@ -1413,7 +1413,8 @@ static int __init dynamic_debug_init(void)
mod_ct++;
di.descs.len = mod_sites;
di.descs.start = iter_mod_start;
- ret = ddebug_add_module(&di, modname);
+ di.mod_name = modname;
+ ret = ddebug_add_module(&di);
if (ret)
goto out_err;
@@ -1424,7 +1425,8 @@ static int __init dynamic_debug_init(void)
}
di.descs.len = mod_sites;
di.descs.start = iter_mod_start;
- ret = ddebug_add_module(&di, modname);
+ di.mod_name = modname;
+ ret = ddebug_add_module(&di);
if (ret)
goto out_err;
--
2.54.0
^ permalink raw reply related
* [PATCH v3 21/24] dyndbg: hoist classmap-filter-by-modname up to ddebug_add_module
From: Jim Cromie @ 2026-06-01 12:05 UTC (permalink / raw)
To: Jonathan Corbet, Shuah Khan, Arnd Bergmann, Jason Baron,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Andrew Morton, Shuah Khan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: linux-doc, linux-kernel, linux-arch, linux-modules,
linux-kselftest, dri-devel, Jim Cromie, Louis Chauvet
In-Reply-To: <20260601-dd-maint-2-v3-0-4a15b241bd3c@gmail.com>
The body of ddebug_attach_module_classes() is just a code-block that
finds the contiguous subrange of classmaps matching on modname, and
saves it into the ddebug_table's info record.
Implement this block in a macro to accommodate different component
vectors in the "box" (as named in the for_subvec macro). We will
reuse this macro shortly.
And hoist its invocation out of ddebug_attach_module_classes() up into
ddebug_add_module(). This moves the filtering step up closer to
dynamic_debug_init(), which already segments the builtin pr_debug
descriptors on their mod_name boundaries.
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
v3: expand block-comment in ddebug_add_module
v2: move RvB after SoB
finish hoist - drop old fn - ddebug_attach_module_classes
the v1 rev left the old ddebug_attach_module_classes in place, but it
is completely redundant now, since it already lost the list-linking
job it was doing.
It was being cut out later in the patchset (in the unsent API
adaptation phase), but for cleaner review, lets excise it now.
OLD all-in-1-series (pre split into reviewable chunks)
v10?- reordered params to match kdoc
v12- refactor/rename: s/dd_mark_vector_subrange/dd_set_module_subrange/
1. Renamed the macro from dd_mark_vector_subrange to
dd_set_module_subrange to better reflect its purpose of narrowing a
vector to a module-specific subrange.
2. Simplified the arguments by removing the redundant _dst, as the _di
pointer already provides access to the target _ddebug_info struct.
3. Refactored for Clarity: Instead of overwriting the struct's start
pointer while the for_subvec loop is using it to iterate, I
introduced a temporary __start variable. This avoids the "subtle"
side effect and makes the logic easier to follow.
4. Updated Documentation: Improved the comment block to explicitly
state that the macro scans for the first match and counts
contiguous elements.
fiuxp
---
lib/dynamic_debug.c | 84 ++++++++++++++++++++++++++++-------------------------
1 file changed, 45 insertions(+), 39 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index ebfff48c4a6c..fdb730db385e 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -161,8 +161,8 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
}
static struct _ddebug_class_map *ddebug_find_valid_class(struct ddebug_table const *dt,
- const char *class_string,
- int *class_id)
+ const char *class_string,
+ int *class_id)
{
struct _ddebug_class_map *map;
int i, idx;
@@ -1167,34 +1167,34 @@ static const struct proc_ops proc_fops = {
.proc_write = ddebug_proc_write
};
-static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug_info *di)
-{
- struct _ddebug_class_map *cm;
- int i, nc = 0;
-
- /*
- * Find this module's classmaps in a subrange/wholerange of
- * the builtin/modular classmap vector/section. Save the start
- * and length of the subrange at its edges.
- */
- for_subvec(i, cm, di, maps) {
- if (!strcmp(cm->mod_name, dt->info.mod_name)) {
- if (!nc) {
- v2pr_info("start subrange, class[%d]: module:%s base:%d len:%d ty:%d\n",
- i, cm->mod_name, cm->base, cm->length, cm->map_type);
- dt->info.maps.start = cm;
- }
- nc++;
- } else if (nc) {
- /* end of matching classmaps */
- break;
- }
- }
- if (nc) {
- dt->info.maps.len = nc;
- vpr_info("module:%s attached %d classes\n", dt->info.mod_name, nc);
- }
-}
+/*
+ * dd_set_module_subrange - find matching subrange of classmaps
+ * @_i: caller-provided index var
+ * @_sp: cursor into @_vec
+ * @_di: pointer to the struct _ddebug_info to be narrowed
+ * @_vec: name of the vector member (must have .start and .len)
+ *
+ * Narrow a _ddebug_info's vector (@_vec) of classmaps to the
+ * contiguous subrange of elements where ->mod_name matches
+ * @__di->mod_name. This is primarily for builtins, loadable modules
+ * have only their classmaps, and dont need this sub-selection.
+ */
+#define dd_set_module_subrange(_i, _sp, _di, _vec) ({ \
+ struct _ddebug_info *__di = (_di); \
+ typeof(__di->_vec.start) __start = NULL; \
+ int __nc = 0; \
+ for_subvec(_i, _sp, __di, _vec) { \
+ if (!strcmp((_sp)->mod_name, __di->mod_name)) { \
+ if (!__nc++) \
+ __start = (_sp); \
+ } else if (__nc) { \
+ break; /* end of consecutive matches */ \
+ } \
+ } \
+ if (__nc) \
+ __di->_vec.start = __start; \
+ __di->_vec.len = __nc; \
+})
/*
* Allocate a new ddebug_table for the given module
@@ -1203,6 +1203,8 @@ static void ddebug_attach_module_classes(struct ddebug_table *dt, struct _ddebug
static int ddebug_add_module(struct _ddebug_info *di)
{
struct ddebug_table *dt;
+ struct _ddebug_class_map *cm;
+ int i;
if (!di->descs.len)
return 0;
@@ -1215,17 +1217,21 @@ static int ddebug_add_module(struct _ddebug_info *di)
return -ENOMEM;
}
/*
- * For built-in modules, name (as supplied in di by its
- * callers) lives in .rodata and is immortal. For loaded
- * modules, name points at the name[] member of struct module,
- * which lives at least as long as this struct ddebug_table.
+ * For built-in modules, di is a partial cursor into the
+ * builtin dyndbg data; the descriptors are the subrange
+ * matching the modname, but the classmaps are the full set.
+ * We find and set the relevant subrange of classmaps here.
+ *
+ * The modname string is in .rodata, the descriptors and
+ * classmaps are in writable .data. All are immortal.
+ *
+ * For loaded modules, mod_name points at the name[] member
+ * of struct module, and the descriptors and classmaps point
+ * at the module's ELF sections; all have lifetimes matching
+ * the module's presence.
*/
dt->info = *di;
-
- INIT_LIST_HEAD(&dt->link);
-
- if (di->maps.len)
- ddebug_attach_module_classes(dt, di);
+ dd_set_module_subrange(i, cm, &dt->info, maps);
mutex_lock(&ddebug_lock);
list_add_tail(&dt->link, &ddebug_tables);
--
2.54.0
^ permalink raw reply related
* [PATCH v3 22/24] selftests-dyndbg: add a dynamic_debug run_tests target
From: Jim Cromie @ 2026-06-01 12:05 UTC (permalink / raw)
To: Jonathan Corbet, Shuah Khan, Arnd Bergmann, Jason Baron,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Andrew Morton, Shuah Khan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: linux-doc, linux-kernel, linux-arch, linux-modules,
linux-kselftest, dri-devel, Jim Cromie, Łukasz Bartosik,
Louis Chauvet
In-Reply-To: <20260601-dd-maint-2-v3-0-4a15b241bd3c@gmail.com>
Add a selftest script for dynamic-debug. The config requires
CONFIG_TEST_DYNAMIC_DEBUG=m and CONFIG_TEST_DYNAMIC_DEBUG_SUBMOD=m,
which tacitly requires either CONFIG_DYNAMIC_DEBUG=y or
CONFIG_DYNAMIC_DEBUG_CORE=y
ATM this has just basic_tests(), which modify pr_debug() flags in the
builtin params module. This means they're available to manipulate and
observe the effects in "cat control".
This is backported from another feature branch; the support-fns (thx
Lukas) have unused features at the moment, they'll get used shortly.
The script enables simple virtme-ng testing:
[jimc@gandalf b0-ftrace]$ vrun_t
virtme-ng 1.32+115.g07b109d
doing: vng --name v6.14-rc4-60-gd5f48427de0c \
--user root -v -p 4 -a dynamic_debug.verbose=3 V=1 \
-- ../tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
virtme: waiting for virtiofsd to start
..
And add dynamic_debug to TARGETS, so `make run_tests` sees it properly.
For the impatient, set TARGETS explicitly:
[root@v6 selftests]# make TARGETS=dynamic_debug run_tests
make[1]: Nothing to be done for 'all'.
TAP version 13
1..1
# timeout set to 45
# selftests: dynamic_debug: dyndbg_selftest.sh
# # BASIC_TESTS 95.422122] dyndbg: query 0: 0"=_" mod:*
...
NOTES
check KCONFIG_CONFIG to avoid silly fails. Several tests are
dependent upon config choices. Lets avoid failing where that is noise.
The KCONFIG_CONFIG var exists to convey the config-file around. If
the var names a file, read it and extract the relevant CONFIG items,
and use them to skip the dependent tests, thus avoiding the fails that
would follow, and the disruption to whatever CI is running these
selftests.
If the envar doesn't name a config-file, ".config" is assumed.
CONFIG_DYNAMIC_DEBUG=y:
basic-tests() and comma-terminator-tests() test for the presence of
the builtin pr_debugs in module/main.c, which I deemed stable and
therefore safe to count. That said, the test fails if only
CONFIG_DYNAMIC_DEBUG_CORE=y is set. It could be rewritten to test
against test-dynamic-debug.ko, but that just trades one config
dependence for another.
Co-developed-by: Łukasz Bartosik <ukaszb@chromium.org>
Signed-off-by: Łukasz Bartosik <ukaszb@chromium.org>
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
v3: use ksft_* exit codes
v2:
drop commit-msg mention of yet-to-be-submitted tests
move RvB after SoB
script fixups per sashiko review
1. CONFIG_DYNAMIC_DEBUG=y is set correctly.
2. All subshell captures $( ( ... ) 2>&1 ) are fixed.
3. All echo variables are safely quoted to prevent word-splitting.
4. Standardized on modern /sys/kernel/tracing/ paths.
5. exit $exp_exit_code correctly propagates failure status.
---
MAINTAINERS | 1 +
tools/testing/selftests/Makefile | 1 +
tools/testing/selftests/dynamic_debug/Makefile | 9 +
tools/testing/selftests/dynamic_debug/config | 8 +
.../selftests/dynamic_debug/dyndbg_selftest.sh | 258 +++++++++++++++++++++
5 files changed, 277 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 6cf80e7ac039..b3d41824220a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -9087,6 +9087,7 @@ F: include/asm-generic/dyndbg.lds.h
F: include/linux/dynamic_debug.h
F: lib/dynamic_debug.c
F: lib/test_dynamic_debug.c
+F: tools/testing/selftests/dynamic_debug/*
DYNAMIC INTERRUPT MODERATION
M: Tal Gilboa <talgi@nvidia.com>
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 6e59b8f63e41..17c4ddbcee89 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -27,6 +27,7 @@ TARGETS += drivers/net/team
TARGETS += drivers/net/virtio_net
TARGETS += drivers/platform/x86/intel/ifs
TARGETS += dt
+TARGETS += dynamic_debug
TARGETS += efivarfs
TARGETS += exec
TARGETS += fchmodat2
diff --git a/tools/testing/selftests/dynamic_debug/Makefile b/tools/testing/selftests/dynamic_debug/Makefile
new file mode 100644
index 000000000000..6d06fa7f1040
--- /dev/null
+++ b/tools/testing/selftests/dynamic_debug/Makefile
@@ -0,0 +1,9 @@
+# SPDX-License-Identifier: GPL-2.0-only
+# borrowed from Makefile for user memory selftests
+
+# No binaries, but make sure arg-less "make" doesn't trigger "run_tests"
+all:
+
+TEST_PROGS := dyndbg_selftest.sh
+
+include ../lib.mk
diff --git a/tools/testing/selftests/dynamic_debug/config b/tools/testing/selftests/dynamic_debug/config
new file mode 100644
index 000000000000..ec478b17873d
--- /dev/null
+++ b/tools/testing/selftests/dynamic_debug/config
@@ -0,0 +1,8 @@
+
+# basic tests ref the builtin params module
+CONFIG_DYNAMIC_DEBUG=y
+
+# more testing is possible with these,
+# but insisting on them here skips testing entirely for such configs
+# CONFIG_TEST_DYNAMIC_DEBUG=m
+# CONFIG_TEST_DYNAMIC_DEBUG_SUBMOD=m
diff --git a/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
new file mode 100755
index 000000000000..8b2b7388678e
--- /dev/null
+++ b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
@@ -0,0 +1,258 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0-only
+
+V=${V:=0} # invoke as V=1 $0 for global verbose
+RED="\033[0;31m"
+GREEN="\033[0;32m"
+YELLOW="\033[0;33m"
+BLUE="\033[0;34m"
+MAGENTA="\033[0;35m"
+CYAN="\033[0;36m"
+NC="\033[0;0m"
+error_msg=""
+
+# Standard kselftest exit codes
+ksft_pass=0
+ksft_fail=1
+ksft_skip=4
+
+[ -e /proc/dynamic_debug/control ] || {
+ echo -e "${RED}: this test requires CONFIG_DYNAMIC_DEBUG=y ${NC}"
+ exit $ksft_skip # nothing to test here, no good reason to fail.
+}
+
+# need info to avoid failures due to untestable configs
+
+[ -f "$KCONFIG_CONFIG" ] || KCONFIG_CONFIG=".config"
+if [ -f "$KCONFIG_CONFIG" ]; then
+ echo "# consulting KCONFIG_CONFIG: $KCONFIG_CONFIG"
+ grep -q "CONFIG_DYNAMIC_DEBUG=y" $KCONFIG_CONFIG ; LACK_DD_BUILTIN=$?
+ grep -q "CONFIG_TEST_DYNAMIC_DEBUG=m" $KCONFIG_CONFIG ; LACK_TMOD=$?
+else
+ # if no config, try runtime probes
+ modprobe -n test_dynamic_debug 2>/dev/null ; LACK_TMOD=$?
+ # assume builtin dyndbg if control exists (checked above)
+ LACK_DD_BUILTIN=0
+fi
+
+function vx () {
+ echo "$1" > /sys/module/dynamic_debug/parameters/verbose
+}
+
+function ddgrep () {
+ grep "$1" /proc/dynamic_debug/control
+}
+
+function doprints () {
+ cat /sys/module/test_dynamic_debug/parameters/do_prints
+}
+
+function ddcmd () {
+ exp_exit_code=0
+ num_args=$#
+ if [ "${@:$#}" = "pass" ]; then
+ num_args=$#-1
+ elif [ "${@:$#}" = "fail" ]; then
+ num_args=$#-1
+ exp_exit_code=1
+ fi
+ args=${@:1:$num_args}
+ output=$( (echo "$args" > /proc/dynamic_debug/control) 2>&1)
+ exit_code=$?
+ error_msg=$(echo "$output" | cut -d ":" -f 5 | sed -e 's/^[[:space:]]*//')
+ handle_exit_code $BASH_LINENO $FUNCNAME $exit_code $exp_exit_code
+}
+
+function handle_exit_code() {
+ local exp_exit_code=0
+ [ $# == 4 ] && exp_exit_code=$4
+ if [ $3 -ne $exp_exit_code ]; then
+ echo -e "${RED}: $BASH_SOURCE:$1 $2() expected to exit with code $exp_exit_code, got $3"
+ [ $3 == 1 ] && echo "Error: '$error_msg'"
+ exit $ksft_fail
+ fi
+}
+
+# $1 - pattern to match, pattern in $1 is enclosed by spaces for a match ""\s$1\s"
+# $2 - number of times the pattern passed in $1 is expected to match
+# $3 - optional can be set either to "-r" or "-v"
+# "-r" means relaxed matching in this case pattern provided in $1 is passed
+# as is without enclosing it with spaces
+# "-v" prints matching lines
+# $4 - optional when $3 is set to "-r" then $4 can be used to pass "-v"
+function check_match_ct {
+ pattern="\s$1\s"
+ exp_cnt=0
+
+ [ "$3" == "-r" ] && pattern="$1"
+ let cnt=$(ddgrep "$pattern" | wc -l)
+ if [ $V -eq 1 ] || [ "$3" == "-v" ] || [ "$4" == "-v" ]; then
+ echo -ne "${BLUE}" && ddgrep "$pattern" && echo -ne "${NC}"
+ fi
+ [ $# -gt 1 ] && exp_cnt=$2
+ if [ $cnt -ne $exp_cnt ]; then
+ echo -e "${RED}: $BASH_SOURCE:$BASH_LINENO check failed expected $exp_cnt on $1, got $cnt"
+ exit $ksft_fail
+ else
+ echo ": $cnt matches on $1"
+ fi
+}
+
+# $1 - trace instance name
+# #2 - if > 0 then directory is expected to exist, if <= 0 then otherwise
+# $3 - "-v" for verbose
+function check_trace_instance_dir {
+ if [ -e /sys/kernel/tracing/instances/$1 ]; then
+ if [ "$3" == "-v" ] ; then
+ echo "ls -l /sys/kernel/tracing/instances/$1: "
+ ls -l /sys/kernel/tracing/instances/$1
+ fi
+ if [ $2 -le 0 ]; then
+ echo -e "${RED}: $BASH_SOURCE:$BASH_LINENO error trace instance \
+ '/sys/kernel/tracing/instances/$1' does exist"
+ exit $ksft_fail
+ fi
+ else
+ if [ $2 -gt 0 ]; then
+ echo -e "${RED}: $BASH_SOURCE:$BASH_LINENO error trace instance \
+ '/sys/kernel/tracing/instances/$1' does not exist"
+ exit $ksft_fail
+ fi
+ fi
+}
+
+function tmark {
+ echo $* > /sys/kernel/tracing/trace_marker
+}
+
+# $1 - trace instance name
+# $2 - line number
+# $3 - if > 0 then the instance is expected to be opened, otherwise
+# the instance is expected to be closed
+function check_trace_instance {
+ output=$(tail -n9 /proc/dynamic_debug/control | grep ": Opened trace instances" \
+ | xargs -n1 | grep $1)
+ if [ "$output" != $1 ] && [ $3 -gt 0 ]; then
+ echo -e "${RED}: $BASH_SOURCE:$2 trace instance $1 is not opened"
+ exit $ksft_fail
+ fi
+ if [ "$output" == $1 ] && [ $3 -le 0 ]; then
+ echo -e "${RED}: $BASH_SOURCE:$2 trace instance $1 is not closed"
+ exit $ksft_fail
+ fi
+}
+
+function is_trace_instance_opened {
+ check_trace_instance $1 $BASH_LINENO 1
+}
+
+function is_trace_instance_closed {
+ check_trace_instance $1 $BASH_LINENO 0
+}
+
+# $1 - trace instance directory to delete
+# $2 - if > 0 then directory is expected to be deleted successfully, if <= 0 then otherwise
+function del_trace_instance_dir() {
+ exp_exit_code=1
+ [ $2 -gt 0 ] && exp_exit_code=0
+ output=$( (rmdir /sys/kernel/tracing/instances/$1) 2>&1)
+ exit_code=$?
+ error_msg=$(echo "$output" | cut -d ":" -f 3 | sed -e 's/^[[:space:]]*//')
+ handle_exit_code $BASH_LINENO $FUNCNAME $exit_code $exp_exit_code
+}
+
+function error_log_ref {
+ # to show what I got
+ : echo "# error-log-ref: $1"
+ : echo cat \$2
+}
+
+function ifrmmod {
+ lsmod | grep $1 2>&1>/dev/null && rmmod $1
+}
+
+# $1 - text to search for
+function search_trace() {
+ search_trace_name 0 1 $1
+}
+
+# $1 - trace instance name, 0 for global event trace
+# $2 - line number counting from the bottom
+# $3 - text to search for
+function search_trace_name() {
+ if [ "$1" = "0" ]; then
+ buf=$(cat /sys/kernel/tracing/trace)
+ line=$(tail -$2 /sys/kernel/tracing/trace | head -1 | sed -e 's/^[[:space:]]*//')
+ else
+ buf=$(cat /sys/kernel/tracing/instances/$1/trace)
+ line=$(tail -$2 /sys/kernel/tracing/instances/$1/trace | head -1 | \
+ sed -e 's/^[[:space:]]*//')
+ fi
+ if [ $2 = 0 ]; then
+ # whole-buf check
+ output=$(echo "$buf" | grep "$3")
+ else
+ output=$(echo "$line" | grep "$3")
+ fi
+ if [ "$output" = "" ]; then
+ echo -e "${RED}: $BASH_SOURCE:$BASH_LINENO search for '$3' failed \
+ in line '$line' or '$buf'"
+ exit $ksft_fail
+ fi
+ if [ $V = 1 ]; then
+ echo -e "${MAGENTA}: search_trace_name in $1 found: \n$output \nin:${BLUE} $buf ${NC}"
+ fi
+}
+
+# $1 - error message to check
+function check_err_msg() {
+ if [ "$error_msg" != "$1" ]; then
+ echo -e "${RED}: $BASH_SOURCE:$BASH_LINENO error message '$error_msg' \
+ does not match with '$1'"
+ exit $ksft_fail
+ fi
+}
+
+function basic_tests {
+ echo -e "${GREEN}# BASIC_TESTS ${NC}"
+ if [ $LACK_DD_BUILTIN -eq 1 ]; then
+ echo "SKIP"
+ exit $ksft_skip
+ fi
+ ddcmd =_ # zero everything
+ check_match_ct =p 0
+
+ # module params are builtin to handle boot args
+ check_match_ct '\[params\]' 4 -r
+ ddcmd module params +mpf
+ check_match_ct =pmf 4
+
+ # multi-cmd input, newline separated, with embedded comments
+ cat <<"EOF" > /proc/dynamic_debug/control
+ module params =_ # clear params
+ module params +mf # set flags
+ module params func parse_args +sl # other flags
+EOF
+ check_match_ct =mf 3
+ check_match_ct =mfsl 1
+ ddcmd =_
+}
+
+tests_list=(
+ basic_tests
+)
+
+# Run tests
+
+ifrmmod test_dynamic_debug
+
+for test in "${tests_list[@]}"
+do
+ $test
+ echo ""
+done
+echo -en "${GREEN}# Done on: "
+date
+echo -en "${NC}"
+
+exit $ksft_pass
--
2.54.0
^ permalink raw reply related
* [PATCH v3 23/24] dyndbg: change __dynamic_func_call_cls* macros into expressions
From: Jim Cromie @ 2026-06-01 12:05 UTC (permalink / raw)
To: Jonathan Corbet, Shuah Khan, Arnd Bergmann, Jason Baron,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Andrew Morton, Shuah Khan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: linux-doc, linux-kernel, linux-arch, linux-modules,
linux-kselftest, dri-devel, Jim Cromie, Louis Chauvet
In-Reply-To: <20260601-dd-maint-2-v3-0-4a15b241bd3c@gmail.com>
The Xe driver's XE_IOCTL_DBG macro calls drm_dbg() from inside an if
(expression). This breaks when CONFIG_DRM_USE_DYNAMIC_DEBUG=y because
the invoked macro has a do-while-0 wrapper, and is not an expression.
if (cond && (drm_dbg("expr-form"),1)) {
... do some more stuff
}
Fix for this usage by changing __dynamic_func_call_cls{,_no_desc}
macros into expressions, by replacing the do-while-0s with a ({ })
wrapper. In the common usage, the trailing ';' converts the
expression into a statement.
drm_dbg("statement form");
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>
---
v2:
fix statement-expressions to return 0 (not void) like their respective fallbacks
1. Add 0; to __dynamic_func_call_cls
2. Add 0; to __dynamic_func_call_cls_no_desc
3. Convert the disabled fallback of dynamic_hex_dump from do { ... } while(0) to ({ ... 0; })
move RvB after SoB
---
include/linux/dynamic_debug.h | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index e4a62cb73267..2d6983186f37 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -224,24 +224,26 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
* (|_cls): adds in _DPRINT_CLASS_DFLT as needed
* (|_no_desc): former gets callsite descriptor as 1st arg (for prdbgs)
*/
-#define __dynamic_func_call_cls(id, cls, fmt, func, ...) do { \
+#define __dynamic_func_call_cls(id, cls, fmt, func, ...) ({ \
DEFINE_DYNAMIC_DEBUG_METADATA_CLS(id, cls, fmt); \
if (DYNAMIC_DEBUG_BRANCH(id)) { \
func(&id, ##__VA_ARGS__); \
__dynamic_dump_stack(id); \
} \
-} while (0)
+ 0; /* match no_printk return value */ \
+})
#define __dynamic_func_call(id, fmt, func, ...) \
__dynamic_func_call_cls(id, _DPRINTK_CLASS_DFLT, fmt, \
func, ##__VA_ARGS__)
-#define __dynamic_func_call_cls_no_desc(id, cls, fmt, func, ...) do { \
+#define __dynamic_func_call_cls_no_desc(id, cls, fmt, func, ...) ({ \
DEFINE_DYNAMIC_DEBUG_METADATA_CLS(id, cls, fmt); \
if (DYNAMIC_DEBUG_BRANCH(id)) { \
func(__VA_ARGS__); \
__dynamic_dump_stack(id); \
} \
-} while (0)
+ 0; /* match no_printk return value */ \
+})
#define __dynamic_func_call_no_desc(id, fmt, func, ...) \
__dynamic_func_call_cls_no_desc(id, _DPRINTK_CLASS_DFLT, \
fmt, func, ##__VA_ARGS__)
@@ -321,10 +323,12 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
dev_no_printk(KERN_DEBUG, dev, fmt, ##__VA_ARGS__)
#define dynamic_hex_dump(prefix_str, prefix_type, rowsize, \
groupsize, buf, len, ascii) \
- do { if (0) \
+({ \
+ if (0) \
print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, \
- rowsize, groupsize, buf, len, ascii); \
- } while (0)
+ rowsize, groupsize, buf, len, ascii); \
+ 0; \
+})
#endif /* CONFIG_DYNAMIC_DEBUG || (CONFIG_DYNAMIC_DEBUG_CORE && DYNAMIC_DEBUG_MODULE) */
--
2.54.0
^ permalink raw reply related
* [PATCH v3 24/24] dynamic_debug: use KBUILD_MODFILE for unique builtin module names
From: Jim Cromie @ 2026-06-01 12:05 UTC (permalink / raw)
To: Jonathan Corbet, Shuah Khan, Arnd Bergmann, Jason Baron,
Luis Chamberlain, Petr Pavlu, Daniel Gomez, Sami Tolvanen,
Aaron Tomlin, Andrew Morton, Shuah Khan, Maarten Lankhorst,
Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter
Cc: linux-doc, linux-kernel, linux-arch, linux-modules,
linux-kselftest, dri-devel, Jim Cromie
In-Reply-To: <20260601-dd-maint-2-v3-0-4a15b241bd3c@gmail.com>
Historically dynamic-debug gets its module names from KBUILD_MODNAME.
This works well for loadable modules, as the module loader has always
required them to have unique names, but for builtins it is basically
kbasename(srcfile), which sadly gives us many modules named "main".
This makes the following ambiguous:
bash-5.3# echo module main +m > /proc/dynamic_debug/control
since it would affect 4 independent modules named main:
bash-5.3# ddgrep =m
init/main.c:1265 [main]initcall_blacklist =m "blacklisting initcall %s\n"
kernel/power/main.c:49 [main]pm_restore_gfp_mask =m "GFP mask restored\n"
kernel/module/main.c:2862 [main]move_module =m "\t0x%lx 0x%.8lx %s\n"
drivers/base/power/main.c:149 [main]device_pm_add =m "Adding info for %s:%s\n"
We can improve this by using KBUILD_MODFILE for dyndbg's modname in
builtins, and KBUILD_MODNAME for loadables.
The above control-file entries then become:
init/main.c:1265 [init/main]initcall_blacklist ...
kernel/power/main.c:49 [kernel/power/main]pm_restore_gfp_mask ...
kernel/module/main.c:2862 [kernel/module/main]move_module ...
drivers/base/power/main.c:149 [drivers/base/power/main]device_pm_add ...
While this is a user visible change; [params] becomes [kernel/params],
it is not a behavior change; we now match the query-module against the
subsystem/module name or its kbasename (the simple-modname), which as
before, matches all 4 modules.
This allows queries to be specific when desired: "module init/main",
while preserving the existing meaning of "module main"
The deeper reason for this change is not obvious. If any builtin
"main" module were to add a classmap, it would attach to all "main"
modules. If 2 "main" modules defined separate classmaps, both modules
would inadvertently share both classmaps. Since classmaps map
classnames to 0..62, and independently defined classmaps are most
likely to start at 0 (unless author is planning to share the 0..62
range with other classmaps), we have a setup for later reserved range
conflicts. Having unique names prevents future conflicts.
This solution isn't perfect:
1. it changes displayed [params] to [kernel/params] etc
2. its mostly redundant with "filename */main.*"
3. "module power", "module module", "module base/power" might be better
but would break old queries.
Adapt dynamic-debug selftest:
1- Add 'test_subsystem_module_queries' to verify path-based module matching.
2- Use dynamic counting with precise regexes to determine expectations.
3- Reorder tests to run slash-query verification immediately after
basic tests.
4- Update basic_tests and comma_terminator_tests to use 'kernel/params'
instead of 'params' to match new path-based names for built-ins.
And adjust Documentation
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
v3: new patch in rev-3
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
Documentation/admin-guide/dynamic-debug-howto.rst | 40 ++++++++++----------
include/linux/dynamic_debug.h | 17 +++++++--
lib/dynamic_debug.c | 3 +-
.../selftests/dynamic_debug/dyndbg_selftest.sh | 44 ++++++++++++++++++++--
4 files changed, 77 insertions(+), 27 deletions(-)
diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
index 9c2f096ed1d8..8befb69575b7 100644
--- a/Documentation/admin-guide/dynamic-debug-howto.rst
+++ b/Documentation/admin-guide/dynamic-debug-howto.rst
@@ -38,12 +38,12 @@ You can view the currently configured behaviour in the *prdbg* catalog::
:#> head -n7 /proc/dynamic_debug/control
# filename:lineno [module]function flags format
- init/main.c:1179 [main]initcall_blacklist =_ "blacklisting initcall %s\n"
- init/main.c:1218 [main]initcall_blacklisted =_ "initcall %s blacklisted\n"
- init/main.c:1424 [main]run_init_process =_ " with arguments:\n"
- init/main.c:1426 [main]run_init_process =_ " %s\n"
- init/main.c:1427 [main]run_init_process =_ " with environment:\n"
- init/main.c:1429 [main]run_init_process =_ " %s\n"
+ init/main.c:1179 [init/main]initcall_blacklist =_ "blacklisting initcall %s\n"
+ init/main.c:1218 [init/main]initcall_blacklisted =_ "initcall %s blacklisted\n"
+ init/main.c:1424 [init/main]run_init_process =_ " with arguments:\n"
+ init/main.c:1426 [init/main]run_init_process =_ " %s\n"
+ init/main.c:1427 [init/main]run_init_process =_ " with environment:\n"
+ init/main.c:1429 [init/main]run_init_process =_ " %s\n"
The 3rd space-delimited column shows the current flags, preceded by
a ``=`` for easy use with grep/cut. ``=p`` shows enabled callsites.
@@ -59,10 +59,10 @@ query/commands to the control file. Example::
:#> ddcmd '-p; module main func run* +p'
:#> grep =p /proc/dynamic_debug/control
- init/main.c:1424 [main]run_init_process =p " with arguments:\n"
- init/main.c:1426 [main]run_init_process =p " %s\n"
- init/main.c:1427 [main]run_init_process =p " with environment:\n"
- init/main.c:1429 [main]run_init_process =p " %s\n"
+ init/main.c:1424 [init/main]run_init_process =p " with arguments:\n"
+ init/main.c:1426 [init/main]run_init_process =p " %s\n"
+ init/main.c:1427 [init/main]run_init_process =p " with environment:\n"
+ init/main.c:1429 [init/main]run_init_process =p " %s\n"
Error messages go to console/syslog::
@@ -161,17 +161,19 @@ file
file kernel/freezer.c # ie column 1 of control file
file drivers/usb/* # all callsites under it
file inode.c:start_* # parse :tail as a func (above)
- file inode.c:1-100 # parse :tail as a line-range (above)
+ file inode.c:1-100 # parse :tail as a line-range (below)
module
- The given string is compared against the module name
- of each callsite. The module name is the string as
- seen in ``lsmod``, i.e. without the directory or the ``.ko``
- suffix and with ``-`` changed to ``_``. Examples::
-
- module sunrpc
- module nfsd
- module drm* # both drm, drm_kms_helper
+ The query string is compared against the subsystem module name of
+ each callsite, as shown in the control file. The simple module
+ name is the string as seen in ``lsmod``, i.e. without the
+ directory or the ``.ko`` suffix and with ``-`` changed to ``_``.
+
+ Examples::
+
+ module nfsd # simple modname (as from lsmod)
+ module init/main # subsystem modname (as in control file)
+ module drm* # both drm, drm_kms_helper
format
The given string is searched for in the dynamic debug format
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 2d6983186f37..aee6f3d0916f 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -8,6 +8,17 @@
#include <linux/build_bug.h>
+/*
+ * Pick the best name for the module:
+ * KBUILD_MODFILE includes the path (e.g., drivers/usb/core/usbcore) for built-ins.
+ * Fall back to KBUILD_MODNAME for modules (loader requires unique names).
+ */
+#ifdef KBUILD_MODFILE
+# define DDEBUG_MODNAME KBUILD_MODFILE
+#else
+# define DDEBUG_MODNAME KBUILD_MODNAME
+#endif
+
/*
* An instance of this structure is created in a special
* ELF section at every dynamic debug callsite. At runtime,
@@ -128,9 +139,9 @@ struct _ddebug_class_param {
#define DECLARE_DYNDBG_CLASSMAP(_var, _maptype, _base, ...) \
static const char *_var##_classnames[] = { __VA_ARGS__ }; \
static struct _ddebug_class_map __aligned(8) __used \
- __section("__dyndbg_class_maps") _var = { \
+ __section("__dyndbg_class_maps") _var = { \
.mod = THIS_MODULE, \
- .mod_name = KBUILD_MODNAME, \
+ .mod_name = DDEBUG_MODNAME, \
.base = _base, \
.map_type = _maptype, \
.length = ARRAY_SIZE(_var##_classnames), \
@@ -169,7 +180,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
#define DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, cls, fmt) \
static struct _ddebug __aligned(8) \
__section("__dyndbg_descs") name = { \
- .modname = KBUILD_MODNAME, \
+ .modname = DDEBUG_MODNAME, \
.function = __func__, \
.filename = __FILE__, \
.format = (fmt), \
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index fdb730db385e..7f78c6b3eeaf 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -248,7 +248,8 @@ static int ddebug_change(const struct ddebug_query *query,
/* match against the module name */
if (query->module &&
- !match_wildcard(query->module, di->mod_name))
+ !match_wildcard(query->module, di->mod_name) &&
+ !match_wildcard(query->module, kbasename(di->mod_name)))
continue;
if (query->class_string) {
diff --git a/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
index 8b2b7388678e..541a2ea7bcb3 100755
--- a/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
+++ b/tools/testing/selftests/dynamic_debug/dyndbg_selftest.sh
@@ -76,9 +76,9 @@ function handle_exit_code() {
# $1 - pattern to match, pattern in $1 is enclosed by spaces for a match ""\s$1\s"
# $2 - number of times the pattern passed in $1 is expected to match
# $3 - optional can be set either to "-r" or "-v"
-# "-r" means relaxed matching in this case pattern provided in $1 is passed
-# as is without enclosing it with spaces
-# "-v" prints matching lines
+# "-r" means relaxed matching in this case pattern provided in
+# $1 is passed as is without enclosing it with spaces "-v"
+# prints matching lines
# $4 - optional when $3 is set to "-r" then $4 can be used to pass "-v"
function check_match_ct {
pattern="\s$1\s"
@@ -223,7 +223,7 @@ function basic_tests {
check_match_ct =p 0
# module params are builtin to handle boot args
- check_match_ct '\[params\]' 4 -r
+ check_match_ct '\[kernel/params\]' 4 -r
ddcmd module params +mpf
check_match_ct =pmf 4
@@ -238,8 +238,44 @@ EOF
ddcmd =_
}
+function test_subsystem_module_queries {
+ echo -e "${GREEN}# TEST_SUBSYTEM_MODULE_QUERIES ${NC}"
+ ddcmd =_
+
+ # Find how many 'main' modules we have in total (by basename)
+ # Use a more precise regex to avoid false positives like [irqdomain]
+ local total_main=$(grep -c "\[\([^]]*/\)\?main\]" /proc/dynamic_debug/control)
+ echo "# found $total_main total 'main' modules"
+
+ if [ $total_main -eq 0 ]; then
+ echo "SKIP - no 'main' modules found to test slashes"
+ return
+ fi
+
+ echo "# testing 'module */main'"
+ ddcmd module "*/main" +p
+ # This should match modules that HAVE a slash and end in /main
+ local slash_main=$(grep -c "\[[^]]*/main\]" /proc/dynamic_debug/control)
+ check_match_ct =p $slash_main -r
+
+ echo "# testing 'module init/main' (specific path)"
+ ddcmd =_
+ ddcmd module "init/main" +p
+ local init_main=$(grep -c "\[init/main\]" /proc/dynamic_debug/control)
+ check_match_ct =p $init_main
+
+ echo "# testing 'module main' (basename match)"
+ ddcmd =_
+ ddcmd module main +p
+ # This should match ALL $total_main entries due to kbasename matching
+ check_match_ct =p $total_main
+
+ ddcmd =_
+}
+
tests_list=(
basic_tests
+ test_subsystem_module_queries
)
# Run tests
--
2.54.0
^ permalink raw reply related
* Re: [PATCH v4 0/5] selftests/mm: separate GUP microbenchmarking from functional testing
From: Sarthak Sharma @ 2026-06-01 12:04 UTC (permalink / raw)
To: John Hubbard, Andrew Morton
Cc: David Hildenbrand, Lorenzo Stoakes, Liam R . Howlett,
Vlastimil Babka, Mike Rapoport, Suren Baghdasaryan, Michal Hocko,
Shuah Khan, Zi Yan, Baolin Wang, Nico Pache, Ryan Roberts,
Dev Jain, Barry Song, Lance Yang, Jason Gunthorpe, Peter Xu,
Leon Romanovsky, Jonathan Corbet, Shuah Khan, Mark Brown,
linux-mm, linux-kselftest, linux-doc, linux-kernel
In-Reply-To: <13b63b4f-6a4b-4433-93e8-78035ab62b83@nvidia.com>
Hi John!
On 5/30/26 1:04 AM, John Hubbard wrote:
> On 5/28/26 10:57 PM, Sarthak Sharma wrote:
>> On 5/28/26 12:17 AM, Andrew Morton wrote:
>>> On Wed, 27 May 2026 19:54:27 +0530 Sarthak Sharma <sarthak.sharma@arm.com> wrote:
> ...
>> Thanks! I went through Sashiko's review.
>>
>> Both the points are valid but look very minor to me: one is a temporary
>> x86 build issue fixed by the next patch and the other is stricter
>
> This is the sort of claim that an AI will make, in kernel code: "it's
> just a temporary build issue". I've seen this happen during sessions
> when I'm interrogating an AI about something.
>
> Don't be misled by AI's evaluations, as unless it is guided very
> firmly (sashiko for example has fairly firm guidance, although still
> a work in progress) it will provide routinely terrible advice about
> how to deal with the kernel.
>
> I'm sure we have something in the "how to contribute" docs that
> explain that patchsets must be bisectable and that there are no
> "minor" build "issues"!
>
> I'd request that you study those docs and the surrounding ones,
> it won't take long and it will help immunize you against the
> various AI suggestions that seem to be unavoidable problems in
> 2026.
>
> thanks,
Thanks for the guidance. You're right, I shouldn't have called it a
minor issue just because it was fixed in the next patch.
I have sent a fixup that moves the Makefile hunk from patch 3 to patch
2, so every patch is buildable on its own. If I need to send a v5 after
reviewer feedback, I'll fold this fixup into patch 2 directly.
^ permalink raw reply
* Re: [PATCH mm-unstable v18 11/14] mm/khugepaged: Introduce mTHP collapse support
From: David Hildenbrand (Arm) @ 2026-06-01 12:06 UTC (permalink / raw)
To: Nico Pache, Lance Yang
Cc: linux-doc, linux-kernel, linux-mm, linux-trace-kernel, aarcange,
akpm, anshuman.khandual, apopple, baohua, baolin.wang, byungchul,
catalin.marinas, cl, corbet, dave.hansen, dev.jain, gourry,
hannes, hughd, jack, jackmanb, jannh, jglisse, joshua.hahnjy, kas,
liam, ljs, mathieu.desnoyers, matthew.brost, mhiramat, mhocko,
peterx, pfalcato, rakie.kim, raquini, rdunlap, richard.weiyang,
rientjes, rostedt, rppt, ryan.roberts, shivankg, sunnanyong,
surenb, thomas.hellstrom, tiwai, usamaarif642, vbabka,
vishal.moola, wangkefeng.wang, will, willy, yang, ying.huang, ziy,
zokeefe
In-Reply-To: <CAA1CXcCfww9X-f2Vb=ipz8tY2nMNnMx5_58Ozz63tvZTz_wpOw@mail.gmail.com>
On 6/1/26 14:01, Nico Pache wrote:
> On Sun, May 31, 2026 at 2:48 AM Lance Yang <lance.yang@linux.dev> wrote:
>>
>>
>>
>> On 2026/5/31 15:18, Lance Yang wrote:
>>>
>>> [...]
>>>
>>> Hmm ... don't we lose the allocation-failure result here?
>>>
>>> Previously collapse_scan_pmd() propagated SCAN_ALLOC_HUGE_PAGE_FAIL from
>>> collapse_huge_page(), so khugepaged would call khugepaged_alloc_sleep()
>>> in khugepaged_do_scan().
>>>
>>> Now if allocation fails and nr_collapsed stays 0, we just return
>>> SCAN_FAIL. So we won't back off via khugepaged_alloc_sleep() anymore?
>>
>> Looks like this is a more general issue with mthp_collapse() only
>> returning nr_collapsed.
>>
>> For example, SCAN_PMD_MAPPED used to be propagated too, and
>> madvise_collapse() treats that as success. With the new code, if
>> nothing was collapsed by this call, that can also become SCAN_FAIL ...
>>
>> So I think we should keep both.
>
> Yeah I thought about this before, but more regarding the "incorrect"
> propagation of errors; I didn't consider that those results were
> actually being considered.
>
> I actually had a patch to track the last_failure (with some
> prioritization on certain results). I think that would solve this
> issue.
>
> Thanks for reminding me to improve this.
>
> Depending on how the rest of the reviews go, I can either send up a
> follow up series to do some more cleanups and improvements of the
> current approach or we can send out a v19.
Let's do a v19.
Patch #11 might need a bit of work, we can discuss offline if you want.
If we could get a v19 by the end of the week, that would be nice.
--
Cheers,
David
^ permalink raw reply
* Re: [PATCH v4 01/13] liveupdate: change file_set->count type to u64 for type safety
From: Pratyush Yadav @ 2026-06-01 12:08 UTC (permalink / raw)
To: Pasha Tatashin
Cc: linux-kselftest, rppt, shuah, akpm, linux-mm, skhan, linux-doc,
linux-kernel, corbet, dmatlack, kexec, pratyush, skhawaja, graf
In-Reply-To: <20260530221938.115978-2-pasha.tatashin@soleen.com>
On Sat, May 30 2026, Pasha Tatashin wrote:
> This improves type safety and aligns the in-memory file_set->count with
> the serialized count type. It avoids potential truncation or sign
> conversion mismatch issues.
>
> Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
Reviewed-by: Pratyush Yadav (Google) <pratyush@kernel.org>
[...]
--
Regards,
Pratyush Yadav
^ permalink raw reply
* RE: [PATCH 06/11] net: wwan: t9xx: Add AT & MBIM WWAN ports
From: Jagielski, Jedrzej @ 2026-06-01 12:09 UTC (permalink / raw)
To: jackbb_wu@compal.com, Loic Poulain, Sergey Ryazanov,
Johannes Berg, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Wen-Zhi Huang, Shi-Wei Yeh,
Minano Tseng, Matthias Brugger, AngeloGioacchino Del Regno,
Simon Horman, Jonathan Corbet, Shuah Khan
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-mediatek@lists.infradead.org, linux-doc@vger.kernel.org
In-Reply-To: <20260529-t9xx_driver_v1-v1-6-bdbfe2c01e57@compal.com>
From: Jack Wu via B4 Relay <devnull+jackbb_wu.compal.com@kernel.org>
Sent: Friday, May 29, 2026 12:32 PM
>From: Jack Wu <jackbb_wu@compal.com>
>
>Adds AT & MBIM ports to the port infrastructure.
please use imperative mode in commit msg
>The WWAN initialization method is responsible for creating the
>corresponding ports using the WWAN framework infrastructure. The
>implemented WWAN port operations are start, stop, tx, tx_blocking
>and tx_poll.
>
>Signed-off-by: Jack Wu <jackbb_wu@compal.com>
>---
> drivers/net/wwan/t9xx/mtk_port.c | 27 ++
> drivers/net/wwan/t9xx/mtk_port.h | 15 ++
> drivers/net/wwan/t9xx/mtk_port_io.c | 332 ++++++++++++++++++++++++-
> drivers/net/wwan/t9xx/mtk_port_io.h | 5 +
> drivers/net/wwan/t9xx/pcie/mtk_ctrl_cfg_m9xx.c | 8 +
> 5 files changed, 386 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/net/wwan/t9xx/mtk_port.c b/drivers/net/wwan/t9xx/mtk_port.c
>index dbd279cf2a14..4032df99b5b0 100644
>--- a/drivers/net/wwan/t9xx/mtk_port.c
>+++ b/drivers/net/wwan/t9xx/mtk_port.c
>@@ -630,6 +630,7 @@ static int mtk_port_rx_dispatch(struct sk_buff *skb, void *priv, bool force_recv
> /* Support scatter gather transmission */
> if (port->rx_mtu > port->rx_frag_size) {
> ret = mtk_port_rx_dispatch_frag_skb(port, skb);
>+ /* -EIO means partial data dispatch complete, does not goto drop flow */
unclear how adding this comment is related to the patch
> if (ret < 0 && ret != -EIO)
> goto drop_frag_skb;
> } else {
>@@ -818,6 +819,29 @@ int mtk_port_ch_disable(struct mtk_port *port)
> return ret;
> }
>
>+static int mtk_port_enable_by_type(struct mtk_port_mngr *port_mngr, int tbl_type)
>+{
>+ struct mtk_port **ports;
>+ int ret, idx;
>+
>+ if (tbl_type < 0 || tbl_type >= PORT_TBL_MAX)
>+ return -EINVAL;
>+
>+ ports = kcalloc(port_mngr->port_cnt, sizeof(struct mtk_port *), GFP_KERNEL);
>+ if (!ports)
>+ return -ENOMEM;
>+
>+ ret = radix_tree_gang_lookup(&port_mngr->port_tbl[tbl_type],
>+ (void **)ports, 0, port_mngr->port_cnt);
>+ for (idx = 0; idx < ret; idx++) {
>+ if (ports[idx]->enable)
>+ ports_ops[ports[idx]->info.type]->enable(ports[idx]);
>+ }
>+
>+ kfree(ports);
>+ return 0;
>+}
>+
> static void mtk_port_disable(struct mtk_port_mngr *port_mngr)
> {
> struct mtk_port **ports;
>@@ -851,6 +875,9 @@ void mtk_port_mngr_fsm_state_handler(struct mtk_fsm_param *fsm_param, void *arg)
> case FSM_STATE_OFF:
> mtk_port_disable(port_mngr);
> break;
>+ case FSM_STATE_READY:
>+ mtk_port_enable_by_type(port_mngr, PORT_TBL_MD);
>+ break;
> default:
> break;
> }
>diff --git a/drivers/net/wwan/t9xx/mtk_port.h b/drivers/net/wwan/t9xx/mtk_port.h
>index a201c0007878..cf561add6318 100644
>--- a/drivers/net/wwan/t9xx/mtk_port.h
>+++ b/drivers/net/wwan/t9xx/mtk_port.h
>@@ -56,6 +56,10 @@ enum mtk_ccci_ch {
> /* to MD */
> CCCI_CONTROL_RX = 0x2000,
> CCCI_CONTROL_TX = 0x2001,
>+ CCCI_UART2_RX = 0x200A,
>+ CCCI_UART2_TX = 0x200C,
>+ CCCI_MBIM_RX = 0x20D0,
>+ CCCI_MBIM_TX = 0x20D1,
> };
>
> enum mtk_port_flag {
>@@ -73,6 +77,7 @@ enum mtk_port_tbl {
>
> enum mtk_port_type {
> PORT_TYPE_INTERNAL,
>+ PORT_TYPE_WWAN,
> PORT_TYPE_MAX
> };
>
>@@ -81,6 +86,13 @@ struct mtk_internal_port {
> int (*recv_cb)(void *arg, struct sk_buff *skb);
> };
>
>+struct mtk_wwan_port {
>+ /* w_lock protects wwan_port when recv data and disable port at the same time */
>+ struct mutex w_lock;
>+ int w_type;
>+ void *w_port;
>+};
>+
> struct mtk_port_cfg {
> enum mtk_ccci_ch tx_ch;
> enum mtk_ccci_ch rx_ch;
>@@ -108,8 +120,11 @@ struct mtk_port {
> wait_queue_head_t rx_wq;
> struct list_head stale_entry;
> char dev_str[MTK_DEV_STR_LEN];
>+ /* Serializes port write operations */
>+ struct mutex write_lock;
> struct mtk_port_mngr *port_mngr;
> struct mtk_internal_port i_priv;
>+ struct mtk_wwan_port w_priv;
> };
>
> struct mtk_port_mngr {
>diff --git a/drivers/net/wwan/t9xx/mtk_port_io.c b/drivers/net/wwan/t9xx/mtk_port_io.c
>index 9e7a1207cc03..ab8b1c5157ec 100644
>--- a/drivers/net/wwan/t9xx/mtk_port_io.c
>+++ b/drivers/net/wwan/t9xx/mtk_port_io.c
>@@ -3,6 +3,10 @@
> * Copyright (c) 2022, MediaTek Inc.
> */
> #include <linux/netdevice.h>
>+#include <linux/poll.h>
>+#include <linux/slab.h>
>+#include <linux/wait.h>
>+#include <linux/wwan.h>
>
> #include "mtk_port_io.h"
>
>@@ -39,6 +43,149 @@ static void mtk_port_struct_init(struct mtk_port *port)
> port->rx_buf_size = MTK_RX_BUF_SIZE;
> init_waitqueue_head(&port->trb_wq);
> init_waitqueue_head(&port->rx_wq);
>+ mutex_init(&port->write_lock);
>+}
>+
>+static int mtk_port_copy_data_from(void *to, union user_buf from, unsigned int len,
>+ unsigned int offset, bool from_user_space)
>+{
>+ int ret = 0;
like for the previous commits - please do not zeroinit when don't required
returbning 0 at the end is completely fine here
>+
>+ if (from_user_space) {
>+ ret = copy_from_user(to, from.ubuf + offset, len);
>+ if (ret)
>+ ret = -EFAULT;
#define EFAULT 14 /* Bad address */
i believe there are better suiting codes
>+ } else {
>+ memcpy(to, from.kbuf + offset, len);
>+ }
>+
>+ return ret;
>+}
>+
>+static int mtk_port_common_write_frag_skb(struct mtk_port *port, struct sk_buff *skb,
>+ union user_buf buf, u32 packet_size,
>+ u32 cur_pos, bool from_user_space)
>+{
>+ struct sk_buff *frag_skb, *tmp = NULL;
>+ u32 frag_size;
>+ int ret;
>+
>+ frag_size = min(packet_size, port->tx_frag_size);
>+ ret = mtk_port_copy_data_from(skb_put(skb, frag_size),
>+ buf, frag_size,
>+ cur_pos, from_user_space);
>+ if (ret) {
>+ dev_err(port->port_mngr->ctrl_blk->mdev->dev,
>+ "Failed to copy skb for port(%s)\n", port->info.name);
>+ goto err_reset_skb;
>+ }
>+ cur_pos += frag_size;
>+ packet_size -= frag_size;
>+ if (!packet_size)
>+ return cur_pos;
>+
>+ while (packet_size > 0) {
>+ frag_skb = __dev_alloc_skb(port->tx_mtu, GFP_KERNEL);
>+ if (!frag_skb) {
>+ ret = -ENOMEM;
>+ goto err_free_frag_list;
>+ }
>+
>+ frag_size = min(packet_size, port->tx_frag_size);
>+ ret = mtk_port_copy_data_from(skb_put(frag_skb, frag_size),
>+ buf, frag_size,
>+ cur_pos, from_user_space);
>+ if (ret) {
>+ dev_err(port->port_mngr->ctrl_blk->mdev->dev,
>+ "Failed to copy frag_skb for port(%s)\n", port->info.name);
>+ dev_kfree_skb_any(frag_skb);
>+ goto err_free_frag_list;
>+ }
>+ skb->data_len += frag_size;
>+ skb->len += frag_size;
>+ cur_pos += frag_size;
>+ packet_size -= frag_size;
>+ if (!tmp)
>+ skb_shinfo(skb)->frag_list = frag_skb;
>+ else
>+ tmp->next = frag_skb;
>+ tmp = frag_skb;
>+ }
>+ return cur_pos;
>+
>+err_free_frag_list:
>+ frag_skb = skb_shinfo(skb)->frag_list;
>+ while (frag_skb) {
>+ tmp = frag_skb->next;
>+ frag_skb->next = NULL;
>+ dev_kfree_skb_any(frag_skb);
>+ frag_skb = tmp;
>+ }
>+ skb_shinfo(skb)->frag_list = NULL;
>+err_reset_skb:
>+ skb->data_len = 0;
>+ return ret;
>+}
>+
>+static int mtk_port_common_write(struct mtk_port *port, union user_buf buf, unsigned int len,
>+ bool from_user_space)
>+{
>+ u32 packet_size, left_cnt = len, cur_pos;
>+ struct sk_buff *skb;
>+ int ret;
>+
>+ if (len == 0)
that's really successful path?
>+ return 0;
>+
>+start_write:
>+ ret = mtk_port_status_check(port);
>+ if (ret)
>+ goto end_write;
>+
>+ skb = __dev_alloc_skb(port->tx_mtu, GFP_KERNEL);
>+ if (!skb) {
>+ ret = -ENOMEM;
>+ goto end_write;
>+ }
>+
>+ skb_reserve(skb, sizeof(struct mtk_ccci_header));
>+
>+ packet_size = min(left_cnt, port->tx_mtu);
>+ cur_pos = len - left_cnt;
>+ /* Support scatter gather transmission */
>+ if (port->tx_mtu > port->tx_frag_size) {
>+ ret = mtk_port_common_write_frag_skb(port, skb, buf, packet_size,
>+ cur_pos, from_user_space);
>+ if (ret < 0)
>+ goto err_free_skb;
>+ } else {
>+ ret = mtk_port_copy_data_from(skb_put(skb, packet_size),
>+ buf, packet_size,
>+ cur_pos, from_user_space);
>+ if (ret) {
>+ dev_err(port->port_mngr->ctrl_blk->mdev->dev,
>+ "Failed to copy data for port(%s)\n", port->info.name);
>+ goto err_free_skb;
>+ }
>+ }
>+
>+ ret = mtk_port_send_data(port, skb);
>+ if (ret < 0) {
>+ if (ret == -EINTR)
>+ left_cnt -= packet_size;
>+ goto end_write;
>+ }
>+
>+ left_cnt -= ret;
>+ if (left_cnt)
>+ goto start_write;
>+ else
>+ goto end_write;
>+
>+err_free_skb:
>+ dev_kfree_skb_any(skb);
>+end_write:
>+ return (len > left_cnt) ? (len - left_cnt) : ret;
> }
>
> static int mtk_port_internal_init(struct mtk_port *port)
>@@ -109,7 +256,6 @@ static int mtk_port_internal_recv(struct mtk_port *port, struct sk_buff *skb)
> return ret;
>
> drop_data:
>- dev_kfree_skb_any(skb);
> return ret;
> }
>
>@@ -241,6 +387,190 @@ static const struct port_ops port_internal_ops = {
> .recv = mtk_port_internal_recv,
> };
>
>+static int mtk_port_wwan_open(struct wwan_port *w_port)
>+{
>+ struct mtk_port *port;
>+ int ret;
>+
>+ port = wwan_port_get_drvdata(w_port);
>+ ret = mtk_port_get_locked(port);
>+ if (ret)
>+ return ret;
>+
>+ ret = mtk_port_common_open(port);
>+ if (ret)
>+ mtk_port_put_locked(port);
>+
>+ return ret;
>+}
>+
>+static void mtk_port_wwan_close(struct wwan_port *w_port)
>+{
>+ struct mtk_port *port = wwan_port_get_drvdata(w_port);
>+
>+ mtk_port_common_close(port);
>+ mtk_port_put_locked(port);
>+}
>+
>+static int mtk_port_wwan_write(struct wwan_port *w_port, struct sk_buff *skb)
>+{
>+ struct mtk_port *port = wwan_port_get_drvdata(w_port);
>+ union user_buf user_buf;
>+
>+ if (unlikely(!skb->len)) {
>+ kfree_skb(skb);
>+ return 0;
>+ }
>+
>+ port->info.flags &= ~PORT_F_BLOCKING;
>+ user_buf.kbuf = (void *)skb->data;
>+ return mtk_port_common_write(port, user_buf, skb->len, false);
>+}
>+
>+static int mtk_port_wwan_write_blocking(struct wwan_port *w_port, struct sk_buff *skb)
>+{
>+ struct mtk_port *port = wwan_port_get_drvdata(w_port);
>+ union user_buf user_buf;
>+
>+ if (unlikely(!skb->len)) {
>+ kfree_skb(skb);
>+ return 0;
>+ }
>+
>+ port->info.flags |= PORT_F_BLOCKING;
>+ user_buf.kbuf = (void *)skb->data;
>+ return mtk_port_common_write(port, user_buf, skb->len, false);
>+}
>+
>+static __poll_t mtk_port_wwan_poll(struct wwan_port *w_port, struct file *file,
>+ struct poll_table_struct *poll)
>+{
>+ struct mtk_port *port = wwan_port_get_drvdata(w_port);
>+ union ctrl_hif_cmd_data hif_cmd;
>+ struct mtk_ctrl_blk *ctrl_blk;
>+ __poll_t mask = 0;
>+
>+ poll_wait(file, &port->trb_wq, poll);
>+ if (mtk_port_status_check(port))
>+ return EPOLLERR | EPOLLHUP;
>+
>+ ctrl_blk = port->port_mngr->ctrl_blk;
>+ hif_cmd.rx_ch = port->info.rx_ch;
>+ if (!ctrl_blk->ops->send_cmd(ctrl_blk->mdev, HIF_CTRL_CMD_CHECK_TX_FULL, &hif_cmd))
>+ mask |= EPOLLOUT | EPOLLWRNORM;
>+
>+ return mask;
>+}
>+
>+static const struct wwan_port_ops wwan_ops = {
>+ .start = mtk_port_wwan_open,
>+ .stop = mtk_port_wwan_close,
>+ .tx = mtk_port_wwan_write,
>+ .tx_blocking = mtk_port_wwan_write_blocking,
>+ .tx_poll = mtk_port_wwan_poll,
>+};
>+
>+static int mtk_port_wwan_init(struct mtk_port *port)
for the whole series - please assess where int over void
is really required
>+{
>+ mtk_port_struct_init(port);
>+ port->enable = false;
>+
>+ mutex_init(&port->w_priv.w_lock);
>+
>+ switch (port->info.rx_ch) {
>+ case CCCI_MBIM_RX:
>+ port->w_priv.w_type = WWAN_PORT_MBIM;
>+ break;
>+ case CCCI_UART2_RX:
>+ port->w_priv.w_type = WWAN_PORT_AT;
>+ break;
>+ default:
>+ port->w_priv.w_type = WWAN_PORT_UNKNOWN;
>+ break;
>+ }
>+
>+ return 0;
>+}
>+
>+static int mtk_port_wwan_exit(struct mtk_port *port)
>+{
>+ if (test_bit(PORT_S_ENABLE, &port->status))
>+ ports_ops[port->info.type]->disable(port);
>+
>+ return 0;
>+}
>+
>+static int mtk_port_wwan_enable(struct mtk_port *port)
>+{
>+ struct mtk_port_mngr *port_mngr;
>+ int ret = 0;
>+
>+ port_mngr = port->port_mngr;
>+
>+ if (test_bit(PORT_S_ENABLE, &port->status))
>+ return 0;
>+
>+ ret = mtk_port_ch_enable(port);
>+ if (ret && ret != -EBUSY)
>+ return ret;
>+
>+ port->w_priv.w_port = wwan_create_port(port_mngr->ctrl_blk->mdev->dev,
>+ port->w_priv.w_type,
>+ &wwan_ops, NULL, port);
>+ if (IS_ERR(port->w_priv.w_port)) {
>+ dev_warn(port_mngr->ctrl_blk->mdev->dev,
>+ "Failed to create wwan port for (%s)\n", port->info.name);
>+ return PTR_ERR(port->w_priv.w_port);
>+ }
>+
>+ set_bit(PORT_S_WR, &port->status);
>+ set_bit(PORT_S_ENABLE, &port->status);
>+
>+ return 0;
>+}
>+
>+static int mtk_port_wwan_disable(struct mtk_port *port)
>+{
>+ struct wwan_port *w_port;
>+
>+ if (!test_and_clear_bit(PORT_S_ENABLE, &port->status))
>+ return 0;
>+
>+ clear_bit(PORT_S_WR, &port->status);
>+ w_port = port->w_priv.w_port;
>+ mutex_lock(&port->w_priv.w_lock);
>+ port->w_priv.w_port = NULL;
>+ mutex_unlock(&port->w_priv.w_lock);
>+
>+ mtk_port_ch_disable(port);
>+ wwan_remove_port(w_port);
>+
>+ return 0;
>+}
>+
>+static int mtk_port_wwan_recv(struct mtk_port *port, struct sk_buff *skb)
>+{
>+ mutex_lock(&port->w_priv.w_lock);
>+ if (!port->w_priv.w_port) {
>+ mutex_unlock(&port->w_priv.w_lock);
>+ return -ENXIO;
>+ }
>+
>+ wwan_port_rx(port->w_priv.w_port, skb);
>+ mutex_unlock(&port->w_priv.w_lock);
>+ return 0;
>+}
>+
>+static const struct port_ops port_wwan_ops = {
>+ .init = mtk_port_wwan_init,
>+ .exit = mtk_port_wwan_exit,
>+ .reset = mtk_port_reset,
>+ .enable = mtk_port_wwan_enable,
>+ .disable = mtk_port_wwan_disable,
>+ .recv = mtk_port_wwan_recv,
>+};
>+
> const struct port_ops *ports_ops[PORT_TYPE_MAX] = {
> &port_internal_ops,
>+ &port_wwan_ops,
> };
>diff --git a/drivers/net/wwan/t9xx/mtk_port_io.h b/drivers/net/wwan/t9xx/mtk_port_io.h
>index 7d2cfe90334c..12f26d244f1f 100644
>--- a/drivers/net/wwan/t9xx/mtk_port_io.h
>+++ b/drivers/net/wwan/t9xx/mtk_port_io.h
>@@ -23,6 +23,11 @@ struct port_ops {
> int (*recv)(struct mtk_port *port, struct sk_buff *skb);
> };
>
>+union user_buf {
>+ void __user *ubuf;
>+ void *kbuf;
>+};
>+
> void *mtk_port_internal_open(struct mtk_md_dev *mdev, char *name, int flag);
> int mtk_port_internal_close(void *i_port);
> int mtk_port_internal_write(void *i_port, struct sk_buff *skb);
>diff --git a/drivers/net/wwan/t9xx/pcie/mtk_ctrl_cfg_m9xx.c b/drivers/net/wwan/t9xx/pcie/mtk_ctrl_cfg_m9xx.c
>index 8611561dd67c..aab09cab360c 100644
>--- a/drivers/net/wwan/t9xx/pcie/mtk_ctrl_cfg_m9xx.c
>+++ b/drivers/net/wwan/t9xx/pcie/mtk_ctrl_cfg_m9xx.c
>@@ -16,6 +16,10 @@ static const int mtk_srv_cfg_m9xx[NR_CLDMA][HW_QUE_NUM] = {
>
> /* the number of RX GPDs should be at last two */
> static const struct queue_info mtk_queue_info_m9xx[] = {
>+ {CCCI_UART2_TX, CCCI_UART2_RX, CLDMA1, TXQ(5), RXQ(5),
>+ Q_MTU_3_5K, Q_MTU_3_5K, TX_GPD_NUM, RX_GPD_NUM, Q_FRAG_3_5K, Q_FRAG_3_5K, 0},
>+ {CCCI_MBIM_TX, CCCI_MBIM_RX, CLDMA1, TXQ(2), RXQ(2),
>+ Q_MTU_3_5K, Q_MTU_3_5K, TX_GPD_NUM, RX_GPD_NUM, Q_FRAG_3_5K, Q_FRAG_3_5K, 0},
> {CCCI_CONTROL_TX, CCCI_CONTROL_RX, CLDMA1, TXQ(0), RXQ(0),
> Q_MTU_3_5K, Q_MTU_3_5K, TX_GPD_NUM, RX_GPD_NUM, Q_FRAG_3_5K, Q_FRAG_3_5K, 0},
> {CCCI_SAP_CONTROL_TX, CCCI_SAP_CONTROL_RX, CLDMA0, TXQ(0), RXQ(0),
>@@ -23,6 +27,10 @@ static const struct queue_info mtk_queue_info_m9xx[] = {
> };
>
> static const struct mtk_port_cfg port_cfg_m9xx[] = {
>+ {CCCI_UART2_TX, CCCI_UART2_RX, PORT_TYPE_WWAN, "AT",
>+ PORT_F_ALLOW_DROP},
>+ {CCCI_MBIM_TX, CCCI_MBIM_RX, PORT_TYPE_WWAN, "MBIM",
>+ PORT_F_ALLOW_DROP},
> {CCCI_CONTROL_TX, CCCI_CONTROL_RX, PORT_TYPE_INTERNAL, "MDCTRL",
> PORT_F_ALLOW_DROP},
> {CCCI_SAP_CONTROL_TX, CCCI_SAP_CONTROL_RX, PORT_TYPE_INTERNAL, "SAPCTRL",
>
>--
>2.34.1
^ permalink raw reply
* Re: [PATCH v4 02/13] liveupdate: avoid mixing cleanup guards with goto in luo_session_retrieve_fd
From: Pratyush Yadav @ 2026-06-01 12:15 UTC (permalink / raw)
To: Pasha Tatashin
Cc: linux-kselftest, rppt, shuah, akpm, linux-mm, skhan, linux-doc,
linux-kernel, corbet, dmatlack, kexec, pratyush, skhawaja, graf
In-Reply-To: <20260530221938.115978-3-pasha.tatashin@soleen.com>
On Sat, May 30 2026, Pasha Tatashin wrote:
> Refactoring luo_session_retrieve_fd() to avoid mixing automated
> cleanup-style guards with goto-based resource release, which is not
> recommended under the Linux kernel coding style.
>
> Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
Perhaps we would be better off moving to FD_ADD() at some point, which
should make this a little bit simpler?
Anyway, this patch is still an improvement, so
Reviewed-by: Pratyush Yadav (Google) <pratyush@kernel.org>
[...]
--
Regards,
Pratyush Yadav
^ permalink raw reply
* RE: [PATCH 09/11] net: wwan: t9xx: Introduce WWAN interface
From: Jagielski, Jedrzej @ 2026-06-01 12:19 UTC (permalink / raw)
To: jackbb_wu@compal.com, Loic Poulain, Sergey Ryazanov,
Johannes Berg, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Wen-Zhi Huang, Shi-Wei Yeh,
Minano Tseng, Matthias Brugger, AngeloGioacchino Del Regno,
Simon Horman, Jonathan Corbet, Shuah Khan
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-mediatek@lists.infradead.org, linux-doc@vger.kernel.org
In-Reply-To: <20260529-t9xx_driver_v1-v1-9-bdbfe2c01e57@compal.com>
From: Jack Wu via B4 Relay <devnull+jackbb_wu.compal.com@kernel.org>
Sent: Friday, May 29, 2026 12:32 PM
>From: Jack Wu <jackbb_wu@compal.com>
>
>Creates the WWAN interface which implements the wwan_ops
>for registration with the WWAN framework. WWAN interface
>also implements the net_device_ops functions used by the
>network devices. Network device operations include open,
>stop, start transmission and get states.
>
>Signed-off-by: Jack Wu <jackbb_wu@compal.com>
>---
> drivers/net/wwan/t9xx/Makefile | 3 +-
> drivers/net/wwan/t9xx/mtk_data_plane.c | 14 +-
> drivers/net/wwan/t9xx/mtk_data_plane.h | 2 +
> drivers/net/wwan/t9xx/mtk_wwan.c | 475 ++++++++++++++++++++++++++++
> drivers/net/wwan/t9xx/mtk_wwan.h | 17 +
> drivers/net/wwan/t9xx/pcie/mtk_dpmaif.c | 15 +-
> drivers/net/wwan/t9xx/pcie/mtk_dpmaif_drv.c | 1 -
> 7 files changed, 521 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/net/wwan/t9xx/Makefile b/drivers/net/wwan/t9xx/Makefile
>index 3776ccb4952f..6927216021cd 100644
>--- a/drivers/net/wwan/t9xx/Makefile
>+++ b/drivers/net/wwan/t9xx/Makefile
>@@ -12,4 +12,5 @@ mtk_t9xx-y := \
> mtk_port.o \
> mtk_port_io.o \
> mtk_fsm.o \
>- mtk_data_plane.o
>+ mtk_data_plane.o \
>+ mtk_wwan.o
>diff --git a/drivers/net/wwan/t9xx/mtk_data_plane.c b/drivers/net/wwan/t9xx/mtk_data_plane.c
>index d11c7de45e17..6266915f08bb 100644
>--- a/drivers/net/wwan/t9xx/mtk_data_plane.c
>+++ b/drivers/net/wwan/t9xx/mtk_data_plane.c
>@@ -6,10 +6,13 @@
> #include "mtk_data_plane.h"
> #include "mtk_dev.h"
> #include "mtk_fsm.h"
>+#include "mtk_wwan.h"
>
> static void mtk_data_stop(struct mtk_data_blk *data_blk, struct mtk_md_dev *mdev)
> {
>+ mtk_wwan_notify(data_blk, DATA_EVT_TX_STOP, 0xff);
> data_blk->hif_ops->stop(mdev);
>+ mtk_wwan_notify(data_blk, DATA_EVT_UNREG_DEV, 0);
> data_blk->hif_ops->clear(mdev);
> }
>
>@@ -31,6 +34,7 @@ static void mtk_data_fsm_callback(struct mtk_fsm_param *fsm_param, void *data)
> data_blk->hif_ops->start(data);
> break;
> case FSM_STATE_READY:
>+ mtk_wwan_notify(data_blk, DATA_EVT_REG_DEV, 0);
> break;
> default:
> break;
>@@ -54,15 +58,21 @@ int mtk_data_init(struct mtk_md_dev *mdev, struct mtk_data_hif_ops *ops)
> if (ret < 0)
> goto data_blk_free;
>
>+ ret = mtk_wwan_init(data_blk);
>+ if (ret < 0)
>+ goto hif_exit;
>+
> ret = mtk_fsm_notifier_register(mdev, MTK_USER_DATA, mtk_data_fsm_callback, mdev,
> FSM_PRIO_1, false);
> if (ret < 0) {
> dev_err(mdev->dev, "Failed to register FSM notifier\n");
>- goto hif_exit;
>+ goto wwan_exit;
> }
>
> return 0;
>
>+wwan_exit:
>+ mtk_wwan_exit(data_blk);
> hif_exit:
> data_blk->hif_ops->exit(mdev);
> data_blk_free:
>@@ -82,6 +92,8 @@ int mtk_data_exit(struct mtk_md_dev *mdev)
>
> mtk_fsm_notifier_unregister(mdev, MTK_USER_DATA);
>
>+ mtk_wwan_exit(data_blk);
>+
> data_blk->hif_ops->exit(mdev);
>
> devm_kfree(mdev->dev, data_blk);
>diff --git a/drivers/net/wwan/t9xx/mtk_data_plane.h b/drivers/net/wwan/t9xx/mtk_data_plane.h
>index 1464fab544f1..351b30ebbbe5 100644
>--- a/drivers/net/wwan/t9xx/mtk_data_plane.h
>+++ b/drivers/net/wwan/t9xx/mtk_data_plane.h
>@@ -31,6 +31,7 @@ struct mtk_data_trans_info {
>
> struct mtk_data_blk {
> struct mtk_md_dev *mdev;
>+ struct mtk_wwan_ctlb *wcb;
> void *dcb;
> struct mtk_data_hif_ops *hif_ops;
> struct mtk_data_trans_info trans_info;
>@@ -80,6 +81,7 @@ enum mtk_data_evt {
> DATA_EVT_MIN,
> DATA_EVT_TX_START,
> DATA_EVT_TX_STOP,
>+ DATA_EVT_RX_START,
> DATA_EVT_RX_STOP,
> DATA_EVT_REG_DEV,
> DATA_EVT_UNREG_DEV,
>diff --git a/drivers/net/wwan/t9xx/mtk_wwan.c b/drivers/net/wwan/t9xx/mtk_wwan.c
>new file mode 100644
>index 000000000000..a1534c312358
>--- /dev/null
>+++ b/drivers/net/wwan/t9xx/mtk_wwan.c
>@@ -0,0 +1,475 @@
>+// SPDX-License-Identifier: GPL-2.0-only
>+/*
>+ * Copyright (c) 2022, MediaTek Inc.
>+ */
>+
>+#include <linux/if_arp.h>
>+#include <linux/netdevice.h>
>+#include <linux/skbuff.h>
>+#include <linux/types.h>
>+#include <linux/wwan.h>
>+#include <net/pkt_sched.h>
>+
>+#include "mtk_data_plane.h"
>+#include "mtk_dev.h"
>+#include "mtk_wwan.h"
>+
>+#define MTK_NETDEV_MAX 20
>+#define MTK_DFLT_INTF_ID 0
>+#define MTK_NETDEV_WDT (HZ)
>+#define MTK_CMD_WDT (HZ)
>+#define MTK_MAX_INTF_ID (MTK_NETDEV_MAX - 1)
>+#define MTK_NAPI_POLL_WEIGHT 128
>+
>+static unsigned int napi_budget = MTK_NAPI_POLL_WEIGHT;
>+
>+struct mtk_wwan_instance {
>+ struct mtk_wwan_ctlb *wcb;
>+ struct net_device *netdev;
>+ unsigned int intf_id;
>+ struct mtk_data_hif_ops *hif_ops;
>+};
>+
>+struct mtk_wwan_ctlb {
>+ struct mtk_data_blk *data_blk;
>+ struct mtk_md_dev *mdev;
>+ struct mtk_wwan_instance __rcu *wwan_inst[MTK_NETDEV_MAX];
>+ struct net_device *dummy_dev;
>+ struct napi_struct **gro_napis;
>+ atomic_t napi_enabled;
>+ unsigned int active_cnt;
>+ bool reg_done;
>+};
>+
please add kdoc as exporting the func
>+int mtk_wwan_recv(struct mtk_data_blk *data_blk, struct sk_buff *skb)
>+{
>+ union mtk_data_pkt_info *pkt_info = DATA_SKB_CB(skb);
>+ struct mtk_wwan_instance *wwan_inst;
>+ unsigned char q_id;
>+
>+ if (unlikely(pkt_info->rx.ch_id > MTK_MAX_INTF_ID)) {
>+ dev_warn(data_blk->mdev->dev,
>+ "Invalid interface id=%d\n", pkt_info->rx.ch_id);
>+ goto free_skb;
>+ }
>+
>+ q_id = pkt_info->rx.q_id;
>+
>+ rcu_read_lock();
>+ wwan_inst = rcu_dereference(data_blk->wcb->wwan_inst[pkt_info->rx.ch_id]);
>+ if (unlikely(!wwan_inst)) {
>+ rcu_read_unlock();
>+ goto free_skb;
>+ }
>+
>+ skb->dev = wwan_inst->netdev;
>+
>+ napi_gro_receive(data_blk->wcb->gro_napis[q_id], skb);
>+
>+ rcu_read_unlock();
>+ return 0;
>+
>+free_skb:
>+ dev_kfree_skb_any(skb);
>+ return -EINVAL;
>+}
>+EXPORT_SYMBOL(mtk_wwan_recv);
>+
>+static void mtk_wwan_napi_enable(struct mtk_wwan_ctlb *wcb)
>+{
>+ int i;
>+
>+ if (atomic_cmpxchg(&wcb->napi_enabled, 0, 1) == 0) {
>+ for (i = 0; i < wcb->data_blk->trans_info.rxq_cnt; i++)
>+ napi_enable(wcb->data_blk->trans_info.napis[i]);
>+ }
>+}
>+
>+static void mtk_wwan_napi_disable(struct mtk_wwan_ctlb *wcb)
>+{
>+ int i;
>+
>+ if (atomic_cmpxchg(&wcb->napi_enabled, 1, 0) == 1) {
>+ for (i = 0; i < wcb->data_blk->trans_info.rxq_cnt; i++) {
>+ napi_synchronize(wcb->data_blk->trans_info.napis[i]);
>+ napi_disable(wcb->data_blk->trans_info.napis[i]);
>+ }
>+ }
>+}
>+
>+static int mtk_wwan_open(struct net_device *dev)
>+{
>+ struct mtk_wwan_instance *wwan_inst = wwan_netdev_drvpriv(dev);
>+ struct mtk_wwan_ctlb *wcb = wwan_inst->wcb;
>+ struct mtk_data_trans_ctl trans_ctl;
>+ int ret;
>+
>+ if (wcb->active_cnt == 0) {
>+ trans_ctl.enable = true;
>+ ret = mtk_wwan_cmd_execute(dev, DATA_CMD_TRANS_CTL, &trans_ctl);
>+ if (ret < 0) {
>+ dev_err(wcb->mdev->dev, "Failed to enable trans\n");
>+ return ret;
>+ }
>+ }
>+
>+ wcb->active_cnt++;
>+
>+ netif_tx_start_all_queues(dev);
>+ netif_carrier_on(dev);
>+
>+ return 0;
>+}
>+
>+static int mtk_wwan_stop(struct net_device *dev)
>+{
>+ struct mtk_wwan_instance *wwan_inst = wwan_netdev_drvpriv(dev);
>+ struct mtk_wwan_ctlb *wcb = wwan_inst->wcb;
>+ struct mtk_data_trans_ctl trans_ctl;
>+ int ret;
>+
>+ netif_carrier_off(dev);
>+ netif_tx_disable(dev);
>+
>+ if (wcb->active_cnt == 1) {
>+ trans_ctl.enable = false;
>+ ret = mtk_wwan_cmd_execute(dev, DATA_CMD_TRANS_CTL, &trans_ctl);
>+ if (ret < 0)
>+ dev_err(wcb->mdev->dev, "Failed to disable trans\n");
>+ }
>+ wcb->active_cnt--;
>+
>+ return 0;
>+}
>+
>+static netdev_tx_t mtk_wwan_start_xmit(struct sk_buff *skb, struct net_device *dev)
>+{
>+ struct mtk_wwan_instance *wwan_inst = wwan_netdev_drvpriv(dev);
>+ union mtk_data_pkt_info *pkt_info = DATA_SKB_CB(skb);
>+ int ret;
>+
>+ skb_set_queue_mapping(skb, 0);
>+ pkt_info->tx.intf_id = wwan_inst->intf_id;
>+
>+ ret = wwan_inst->hif_ops->send(wwan_inst->wcb->data_blk, DATA_PKT, skb);
>+ if (ret == -EBUSY)
>+ return NETDEV_TX_BUSY;
>+ else if (ret == -EINVAL)
>+ dev_kfree_skb_any(skb);
>+
>+ return NETDEV_TX_OK;
>+}
>+
>+static const struct net_device_ops mtk_netdev_ops = {
>+ .ndo_open = mtk_wwan_open,
>+ .ndo_stop = mtk_wwan_stop,
>+ .ndo_start_xmit = mtk_wwan_start_xmit,
>+};
>+
>+static int mtk_wwan_cmd_check(struct net_device *dev, enum mtk_data_cmd_type cmd)
>+{
>+ int ret = 0;
>+
>+ switch (cmd) {
>+ case DATA_CMD_TRANS_CTL:
>+ break;
>+ default:
>+ ret = -EOPNOTSUPP;
>+ break;
just return here
>+ }
>+
>+ return ret;
>+}
>+
>+static struct sk_buff *mtk_wwan_cmd_alloc(enum mtk_data_cmd_type cmd, unsigned int len)
>+{
>+ struct mtk_data_cmd *event;
>+ struct sk_buff *skb;
>+
>+ skb = dev_alloc_skb(sizeof(*event) + len);
>+ if (unlikely(!skb))
>+ return NULL;
>+
>+ skb_put(skb, len + sizeof(*event));
>+ event = (struct mtk_data_cmd *)skb->data;
>+ event->cmd = cmd;
>+ event->len = len;
>+
>+ return skb;
>+}
>+
>+static int mtk_wwan_cmd_send(struct net_device *dev, struct sk_buff *skb)
>+{
>+ struct mtk_wwan_instance *wwan_inst = wwan_netdev_drvpriv(dev);
>+
>+ return wwan_inst->hif_ops->send(wwan_inst->wcb->data_blk, DATA_CMD, skb);
>+}
>+
>+int mtk_wwan_cmd_execute(struct net_device *dev,
>+ enum mtk_data_cmd_type cmd, void *data)
>+{
>+ struct mtk_wwan_instance *wwan_inst;
>+ struct sk_buff *skb;
>+ int ret;
>+
>+ if (mtk_wwan_cmd_check(dev, cmd))
>+ return -EOPNOTSUPP;
>+
>+ skb = mtk_wwan_cmd_alloc(cmd, sizeof(void *));
>+ if (unlikely(!skb))
>+ return -ENOMEM;
>+
>+ SKB_TO_CMD_DATA(skb) = data;
>+
>+ ret = mtk_wwan_cmd_send(dev, skb);
>+ if (ret < 0) {
>+ wwan_inst = wwan_netdev_drvpriv(dev);
>+ dev_err(wwan_inst->wcb->mdev->dev,
>+ "Failed to excute command:ret=%d,cmd=%d\n", ret, cmd);
>+ }
>+
>+ dev_consume_skb_any(skb);
>+
>+ return ret;
>+}
>+
>+static int mtk_wwan_start_txq(struct mtk_wwan_ctlb *wcb, u32 qmask)
>+{
>+ struct mtk_wwan_instance *wwan_inst;
>+ struct net_device *dev;
>+ int i;
>+
>+ rcu_read_lock();
>+ for (i = 0; i < MTK_NETDEV_MAX; i++) {
>+ wwan_inst = rcu_dereference(wcb->wwan_inst[i]);
>+ if (!wwan_inst)
>+ continue;
>+
>+ dev = wwan_inst->netdev;
>+
>+ if (!(dev->flags & IFF_UP))
>+ continue;
>+
>+ netif_tx_wake_all_queues(dev);
>+ netif_carrier_on(dev);
>+ }
>+ rcu_read_unlock();
>+
>+ return 0;
>+}
>+
>+static int mtk_wwan_stop_txq(struct mtk_wwan_ctlb *wcb, u32 qmask)
>+{
>+ struct mtk_wwan_instance *wwan_inst;
>+ struct net_device *dev;
>+ int i;
>+
>+ rcu_read_lock();
>+ for (i = 0; i < MTK_NETDEV_MAX; i++) {
>+ wwan_inst = rcu_dereference(wcb->wwan_inst[i]);
>+ if (!wwan_inst)
>+ continue;
>+
>+ dev = wwan_inst->netdev;
>+
>+ if (!(dev->flags & IFF_UP))
>+ continue;
>+
>+ netif_carrier_off(dev);
>+ netif_tx_stop_all_queues(dev);
>+ }
>+ rcu_read_unlock();
>+
>+ return 0;
>+}
>+
>+static void mtk_wwan_napi_exit(struct mtk_wwan_ctlb *wcb)
>+{
>+ int i;
>+
>+ for (i = 0; i < wcb->data_blk->trans_info.rxq_cnt; i++) {
>+ if (!wcb->data_blk->trans_info.napis[i])
>+ continue;
>+ netif_napi_del(wcb->data_blk->trans_info.napis[i]);
>+ }
>+}
>+
>+static int mtk_wwan_napi_init(struct mtk_wwan_ctlb *wcb, struct net_device *dev)
>+{
>+ int i;
>+
>+ for (i = 0; i < wcb->data_blk->trans_info.rxq_cnt; i++) {
>+ if (!wcb->data_blk->trans_info.napis[i]) {
>+ dev_err(wcb->mdev->dev, "Invalid napi pointer, napi=%d\n", i);
>+ goto out;
>+ }
>+ netif_napi_add_weight(dev, wcb->data_blk->trans_info.napis[i],
>+ wcb->data_blk->hif_ops->poll, napi_budget);
>+ }
>+
>+ return 0;
>+
>+out:
>+ for (--i; i >= 0; i--)
>+ netif_napi_del(wcb->data_blk->trans_info.napis[i]);
>+ return -EINVAL;
>+}
>+
>+static void mtk_wwan_setup(struct net_device *dev)
>+{
>+ dev->watchdog_timeo = MTK_NETDEV_WDT;
>+ dev->mtu = ETH_DATA_LEN;
>+ dev->min_mtu = ETH_MIN_MTU;
>+
>+ dev->features = NETIF_F_SG;
>+ dev->hw_features = NETIF_F_SG;
>+
>+ dev->features |= NETIF_F_GRO;
>+ dev->hw_features |= NETIF_F_GRO;
could be squashed
>+
>+ dev->tx_queue_len = DEFAULT_TX_QUEUE_LEN;
>+
>+ dev->flags = IFF_NOARP;
>+ dev->type = ARPHRD_NONE;
>+
>+ dev->needs_free_netdev = true;
>+
>+ dev->netdev_ops = &mtk_netdev_ops;
>+}
>+
>+static int mtk_wwan_newlink(void *ctxt, struct net_device *dev, u32 intf_id,
>+ struct netlink_ext_ack *extack)
>+{
>+ struct mtk_wwan_instance *wwan_inst = wwan_netdev_drvpriv(dev);
>+ struct mtk_wwan_ctlb *wcb = ctxt;
>+ int ret;
>+
>+ if (intf_id > MTK_MAX_INTF_ID)
>+ return -EINVAL;
>+
>+ if (rcu_access_pointer(wcb->wwan_inst[intf_id]))
>+ return -EBUSY;
>+
>+ dev->max_mtu = wcb->data_blk->trans_info.max_mtu;
>+
>+ wwan_inst->wcb = wcb;
>+ wwan_inst->netdev = dev;
>+ wwan_inst->intf_id = intf_id;
>+ wwan_inst->hif_ops = wcb->data_blk->hif_ops;
>+
>+ ret = register_netdevice(dev);
>+ if (ret)
>+ return ret;
>+
>+ rcu_assign_pointer(wcb->wwan_inst[intf_id], wwan_inst);
>+
>+ netif_device_attach(dev);
>+
>+ return 0;
>+}
>+
>+static void mtk_wwan_dellink(void *ctxt, struct net_device *dev,
>+ struct list_head *head)
>+{
>+ struct mtk_wwan_instance *wwan_inst = wwan_netdev_drvpriv(dev);
>+ int intf_id = wwan_inst->intf_id;
>+ struct mtk_wwan_ctlb *wcb = ctxt;
>+
>+ if (WARN_ON(rcu_access_pointer(wcb->wwan_inst[intf_id]) != wwan_inst))
>+ return;
>+
>+ RCU_INIT_POINTER(wcb->wwan_inst[intf_id], NULL);
>+ unregister_netdevice_queue(dev, head);
>+}
>+
>+static const struct wwan_ops mtk_wwan_ops = {
>+ .priv_size = sizeof(struct mtk_wwan_instance),
>+ .setup = mtk_wwan_setup,
>+ .newlink = mtk_wwan_newlink,
>+ .dellink = mtk_wwan_dellink,
>+};
>+
>+void mtk_wwan_notify(struct mtk_data_blk *data_blk, enum mtk_data_evt evt, u64 data)
>+{
>+ struct mtk_wwan_ctlb *wcb;
>+
>+ if (unlikely(!data_blk || !data_blk->wcb))
>+ return;
>+
>+ wcb = data_blk->wcb;
>+
>+ switch (evt) {
>+ case DATA_EVT_TX_START:
>+ mtk_wwan_start_txq(wcb, data);
>+ break;
>+ case DATA_EVT_TX_STOP:
>+ mtk_wwan_stop_txq(wcb, data);
>+ break;
>+ case DATA_EVT_RX_START:
>+ mtk_wwan_napi_enable(wcb);
>+ break;
>+ case DATA_EVT_RX_STOP:
>+ mtk_wwan_napi_disable(wcb);
>+ break;
>+ case DATA_EVT_REG_DEV:
>+ if (!wcb->reg_done) {
>+ wwan_register_ops(wcb->mdev->dev, &mtk_wwan_ops, wcb, MTK_DFLT_INTF_ID);
>+ wcb->reg_done = true;
>+ }
>+ break;
>+ case DATA_EVT_UNREG_DEV:
>+ if (wcb->reg_done) {
>+ wwan_unregister_ops(wcb->mdev->dev);
>+ wcb->reg_done = false;
>+ }
>+ break;
>+ default:
no need to at least log that received an unsupported evt?
>+ break;
>+ }
>+}
>+EXPORT_SYMBOL(mtk_wwan_notify);
>+
>+int mtk_wwan_init(struct mtk_data_blk *data_blk)
>+{
>+ struct mtk_wwan_ctlb *wcb;
>+ int ret;
>+
>+ wcb = devm_kzalloc(data_blk->mdev->dev, sizeof(*wcb), GFP_KERNEL);
>+ if (unlikely(!wcb))
>+ return -ENOMEM;
>+
>+ wcb->mdev = data_blk->mdev;
>+ wcb->data_blk = data_blk;
>+
>+ wcb->dummy_dev = alloc_netdev_dummy(0);
>+ if (!wcb->dummy_dev) {
>+ devm_kfree(data_blk->mdev->dev, wcb);
>+ return -ENOMEM;
>+ }
>+
>+ data_blk->wcb = wcb;
>+
>+ wcb->gro_napis = data_blk->trans_info.napis;
>+ ret = mtk_wwan_napi_init(wcb, wcb->dummy_dev);
>+ if (ret < 0) {
>+ free_netdev(wcb->dummy_dev);
>+ devm_kfree(data_blk->mdev->dev, wcb);
>+ data_blk->wcb = NULL;
>+ return ret;
>+ }
>+
>+ return 0;
>+}
>+
>+void mtk_wwan_exit(struct mtk_data_blk *data_blk)
>+{
>+ struct mtk_wwan_ctlb *wcb = data_blk->wcb;
>+
>+ if (unlikely(!wcb))
was it verified how adding unlikely impacts performance?
>+ return;
>+
>+ mtk_wwan_napi_exit(wcb);
>+ free_netdev(wcb->dummy_dev);
>+ devm_kfree(data_blk->mdev->dev, wcb);
>+ data_blk->wcb = NULL;
>+}
>diff --git a/drivers/net/wwan/t9xx/mtk_wwan.h b/drivers/net/wwan/t9xx/mtk_wwan.h
>new file mode 100644
>index 000000000000..8005aeb1ed97
>--- /dev/null
>+++ b/drivers/net/wwan/t9xx/mtk_wwan.h
>@@ -0,0 +1,17 @@
>+/* SPDX-License-Identifier: GPL-2.0-only
>+ *
>+ * Copyright (c) 2022, MediaTek Inc.
>+ */
>+
>+#ifndef __MTK_WWAN_H__
>+#define __MTK_WWAN_H__
>+#include <linux/netdevice.h>
>+#include "mtk_data_plane.h"
>+
>+int mtk_wwan_init(struct mtk_data_blk *data_blk);
>+void mtk_wwan_exit(struct mtk_data_blk *data_blk);
>+int mtk_wwan_recv(struct mtk_data_blk *data_blk, struct sk_buff *skb);
>+void mtk_wwan_notify(struct mtk_data_blk *data_blk, enum mtk_data_evt evt, u64 data);
>+int mtk_wwan_cmd_execute(struct net_device *dev, enum mtk_data_cmd_type cmd, void *data);
>+
>+#endif /* __MTK_WWAN_H__ */
>diff --git a/drivers/net/wwan/t9xx/pcie/mtk_dpmaif.c b/drivers/net/wwan/t9xx/pcie/mtk_dpmaif.c
>index 1c58dba738fa..43803587bfc3 100644
>--- a/drivers/net/wwan/t9xx/pcie/mtk_dpmaif.c
>+++ b/drivers/net/wwan/t9xx/pcie/mtk_dpmaif.c
>@@ -20,9 +20,11 @@
> #include "mtk_dev.h"
> #include "mtk_dpmaif.h"
> #include "mtk_dpmaif_drv.h"
>+#include "mtk_dpmaif_reg.h"
> #include "mtk_dpmaif_ring.h"
> #include "mtk_pci_reg.h"
> #include "mtk_pci.h"
>+#include "mtk_wwan.h"
>
> #define MTK_DATA_WS_NAME_LEN 32
> #define DPMAIF_PIT_CNT_UPDATE_THRESHOLD 60
>@@ -741,6 +743,7 @@ static void mtk_dpmaif_sw_stop_rx(struct mtk_dpmaif_ctlb *dcb)
> }
>
> /* Stop PIT polling NAPI. */
>+ mtk_wwan_notify(dcb->data_blk, DATA_EVT_RX_STOP, 0xff);
> }
>
> static void mtk_dpmaif_sw_start_rx(struct mtk_dpmaif_ctlb *dcb)
>@@ -749,6 +752,7 @@ static void mtk_dpmaif_sw_start_rx(struct mtk_dpmaif_ctlb *dcb)
> int i;
>
> /* Start PIT polling NAPI. */
>+ mtk_wwan_notify(dcb->data_blk, DATA_EVT_RX_START, 0xff);
>
> for (i = 0; i < dcb->rxq_cnt; i++) {
> rxq = &dcb->rxqs[i];
>@@ -910,6 +914,7 @@ static int mtk_dpmaif_tx_rel_internal(struct dpmaif_txq *txq,
> tx_srv = &dcb->tx_srvs[srv_id];
> clear_bit(txq->id, &tx_srv->txq_drb_lack_sta);
> wake_up(&tx_srv->wait);
>+ mtk_wwan_notify(dcb->data_blk, DATA_EVT_TX_START, (u64)1 << txq->id);
> }
>
> return 0;
>@@ -1426,6 +1431,7 @@ static void mtk_dpmaif_tx_update_ring(struct mtk_dpmaif_ctlb *dcb, struct dpmaif
> if (drb_available_cnt < skb_drb_cnt) {
> skb_queue_head(&vq->list, skb);
> set_bit(q_id, &tx_srv->txq_drb_lack_sta);
>+ mtk_wwan_notify(dcb->data_blk, DATA_EVT_TX_STOP, (u64)1 << q_id);
> break;
> }
>
>@@ -2346,6 +2352,7 @@ static int mtk_dpmaif_update_rx_skb_info(struct sk_buff *skb,
>
> static int mtk_dpmaif_rx_skb(struct dpmaif_rxq *rxq, struct dpmaif_rx_record *rx_record)
> {
>+ struct mtk_dpmaif_ctlb *dcb = rxq->dcb;
> struct sk_buff *new_skb;
> int ret;
>
>@@ -2362,7 +2369,7 @@ static int mtk_dpmaif_rx_skb(struct dpmaif_rxq *rxq, struct dpmaif_rx_record *rx
> continue;
>
> /* Send skb to data port. */
>- /* Data would be sent to network stack here */
>+ mtk_wwan_recv(dcb->data_blk, new_skb);
> } while (!skb_queue_empty(&rx_record->rx_list));
>
> return ret;
>@@ -2600,10 +2607,12 @@ static int mtk_dpmaif_send_pkt(struct mtk_dpmaif_ctlb *dcb, struct sk_buff *skb)
> int ret = 0;
>
> vq = &dcb->tx_vqs[vq_id];
>- if (likely(skb_queue_len(&vq->list) < vq->max_len))
>+ if (likely(skb_queue_len(&vq->list) < vq->max_len)) {
> skb_queue_tail(&vq->list, skb);
>- else
>+ } else {
>+ mtk_wwan_notify(dcb->data_blk, DATA_EVT_TX_STOP, (u64)1 << vq_id);
> ret = -EBUSY;
>+ }
>
> wake_up(&dcb->tx_srvs[vq->srv_id].wait);
>
>diff --git a/drivers/net/wwan/t9xx/pcie/mtk_dpmaif_drv.c b/drivers/net/wwan/t9xx/pcie/mtk_dpmaif_drv.c
>index 077600389ab4..3fd2b33d1199 100644
>--- a/drivers/net/wwan/t9xx/pcie/mtk_dpmaif_drv.c
>+++ b/drivers/net/wwan/t9xx/pcie/mtk_dpmaif_drv.c
>@@ -8,7 +8,6 @@
> #include <linux/delay.h>
>
> #include "mtk_dev.h"
>-#include "mtk_data_plane.h"
> #include "mtk_dpmaif_drv.h"
> #include "mtk_dpmaif_reg.h"
> #include "mtk_pci.h"
>
>--
>2.34.1
^ permalink raw reply
* Re: [PATCH v4 03/13] liveupdate: centralize state management into struct luo_ser
From: Pratyush Yadav @ 2026-06-01 12:19 UTC (permalink / raw)
To: Pasha Tatashin
Cc: linux-kselftest, rppt, shuah, akpm, linux-mm, skhan, linux-doc,
linux-kernel, corbet, dmatlack, kexec, pratyush, skhawaja, graf
In-Reply-To: <20260530221938.115978-4-pasha.tatashin@soleen.com>
On Sat, May 30 2026, Pasha Tatashin wrote:
> Transition the LUO to ABI v2, which centralizes state management into a
> single struct luo_ser header.
>
> Previously, LUO state was spread across multiple FDT properties and
> subnodes. ABI v2 simplifies this by placing all core state, including
> the liveupdate number and physical addresses for sessions and FLB
> headers into a centralized struct luo_ser.
>
> Note that this change introduces a semantic difference: the sessions
> and FLB serialization formats are no longer completely independent of
> the core LUO. Their metadata (such as physical addresses for sessions
> and FLB headers) is now coupled to and managed via the centralized
> struct luo_ser.
>
> Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
Reviewed-by: Pratyush Yadav (Google) <pratyush@kernel.org>
[...]
--
Regards,
Pratyush Yadav
^ permalink raw reply
* RE: [PATCH 10/11] net: wwan: t9xx: Add power management support
From: Jagielski, Jedrzej @ 2026-06-01 12:26 UTC (permalink / raw)
To: jackbb_wu@compal.com, Loic Poulain, Sergey Ryazanov,
Johannes Berg, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Wen-Zhi Huang, Shi-Wei Yeh,
Minano Tseng, Matthias Brugger, AngeloGioacchino Del Regno,
Simon Horman, Jonathan Corbet, Shuah Khan
Cc: linux-kernel@vger.kernel.org, netdev@vger.kernel.org,
linux-arm-kernel@lists.infradead.org,
linux-mediatek@lists.infradead.org, linux-doc@vger.kernel.org
In-Reply-To: <20260529-t9xx_driver_v1-v1-10-bdbfe2c01e57@compal.com>
From: Jack Wu via B4 Relay <devnull+jackbb_wu.compal.com@kernel.org>
Sent: Friday, May 29, 2026 12:32 PM
>From: Jack Wu <jackbb_wu@compal.com>
>
>Add s2idle (S0ix) power management support for the t9xx WWAN driver.
>
>In s2idle the modem remains powered. The driver must quiesce host-side
>DMA engines and service threads before the platform enters low-power
>state, then restore them on resume.
>
>- Suspend: park TRB service threads, stop CLDMA TX/RX queues,
> disable DPMAIF data path, mask MHCCIF and MSIX interrupts,
> save PCIe state
>- Resume: restore PCIe state, re-initialize ATR, unmask MHCCIF,
> resume CLDMA queues, re-enable DPMAIF data path, unpark TRB
> service threads
>
>Signed-off-by: Jack Wu <jackbb_wu@compal.com>
>---
> drivers/net/wwan/t9xx/pcie/mtk_cldma.c | 94 +++++++++++++++++++++++++++++++++
> drivers/net/wwan/t9xx/pcie/mtk_cldma.h | 3 ++
> drivers/net/wwan/t9xx/pcie/mtk_dpmaif.c | 35 +++++++++++-
> drivers/net/wwan/t9xx/pcie/mtk_dpmaif.h | 2 +
> drivers/net/wwan/t9xx/pcie/mtk_pci.c | 85 ++++++++++++++++++++++++++++-
> 5 files changed, 216 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/net/wwan/t9xx/pcie/mtk_cldma.c b/drivers/net/wwan/t9xx/pcie/mtk_cldma.c
>index aacb4177d914..a5227eb546f4 100644
>--- a/drivers/net/wwan/t9xx/pcie/mtk_cldma.c
>+++ b/drivers/net/wwan/t9xx/pcie/mtk_cldma.c
>@@ -1113,6 +1113,100 @@ int mtk_cldma_exit(struct mtk_ctrl_trans *trans)
> return 0;
> }
>
>+void mtk_cldma_pm_suspend(struct mtk_md_dev *mdev)
>+{
>+ struct mtk_ctrl_trans *trans = ((struct mtk_ctrl_blk *)mdev->ctrl_blk)->ctrl_hw_priv;
>+ struct cldma_dev *cd = trans->dev;
>+ struct cldma_drv_info *drv_info;
>+ struct cldma_drv_ops *drv_ops;
>+ struct rxq *rxq;
>+ struct txq *txq;
>+ int i, q;
>+
>+ for (i = 0; i < NR_CLDMA; i++) {
>+ drv_info = cd->cldma_drv_info[i];
>+ if (!drv_info)
>+ continue;
>+
>+ drv_ops = drv_info->drv_ops;
>+
>+ /* Stop TX queues and flush pending tx_done_work (suspend phase) */
>+ drv_ops->cldma_stop_queue(drv_info, DIR_TX, ALLQ);
>+ for (q = 0; q < HW_QUEUE_NUM; q++) {
>+ txq = drv_info->txq[q];
>+ if (txq)
>+ flush_work(&txq->tx_done_work);
>+ }
>+
>+ for (q = 0; q < HW_QUEUE_NUM; q++) {
>+ rxq = drv_info->rxq[q];
>+ if (!rxq)
>+ continue;
>+ atomic_set(&rxq->need_exit, 1);
>+ drv_ops->cldma_stop_queue(drv_info, DIR_RX, q);
>+ flush_work(&rxq->rx_done_work);
>+ }
>+ mtk_pci_mask_irq(mdev, drv_info->pci_ext_irq_id);
>+ }
>+}
>+
>+void mtk_cldma_pm_resume_early(struct mtk_md_dev *mdev)
>+{
>+ struct mtk_ctrl_trans *trans = ((struct mtk_ctrl_blk *)mdev->ctrl_blk)->ctrl_hw_priv;
>+ struct cldma_dev *cd = trans->dev;
>+ struct cldma_drv_info *drv_info;
>+ struct cldma_drv_ops *drv_ops;
>+ struct rxq *rxq;
>+ int i, q;
>+
>+ for (i = 0; i < NR_CLDMA; i++) {
>+ drv_info = cd->cldma_drv_info[i];
>+ if (!drv_info)
>+ continue;
>+
>+ drv_ops = drv_info->drv_ops;
>+
>+ /* Resume RX queues from current HW ring position (no addr reset) */
>+ for (q = 0; q < HW_QUEUE_NUM; q++) {
>+ rxq = drv_info->rxq[q];
>+ if (!rxq)
>+ continue;
>+ atomic_set(&rxq->need_exit, 0);
>+ drv_ops->cldma_resume_queue(drv_info, DIR_RX, q);
>+ }
>+
>+ /* Unmask CLDMA L1 interrupt */
>+ mtk_pci_unmask_irq(mdev, drv_info->pci_ext_irq_id);
>+ }
>+}
>+
>+void mtk_cldma_pm_resume(struct mtk_md_dev *mdev)
>+{
>+ struct mtk_ctrl_trans *trans = ((struct mtk_ctrl_blk *)mdev->ctrl_blk)->ctrl_hw_priv;
>+ struct cldma_dev *cd = trans->dev;
>+ struct cldma_drv_info *drv_info;
>+ struct cldma_drv_ops *drv_ops;
>+ struct txq *txq;
>+ int i, q;
>+
>+ for (i = 0; i < NR_CLDMA; i++) {
>+ drv_info = cd->cldma_drv_info[i];
>+ if (!drv_info)
>+ continue;
>+
>+ drv_ops = drv_info->drv_ops;
>+
>+ /* Restart TX queues that have pending descriptors */
>+ for (q = 0; q < HW_QUEUE_NUM; q++) {
>+ txq = drv_info->txq[q];
>+ if (!txq)
>+ continue;
>+ if (atomic_read(&txq->req_budget) < txq->nr_gpds)
>+ mtk_cldma_start_xfer(drv_info, q);
>+ }
>+ }
>+}
>+
> static int mtk_cldma_open(struct cldma_dev *cd, struct sk_buff *skb)
> {
> struct trb_open_priv *trb_open_priv = (struct trb_open_priv *)skb->data;
>diff --git a/drivers/net/wwan/t9xx/pcie/mtk_cldma.h b/drivers/net/wwan/t9xx/pcie/mtk_cldma.h
>index 04f83ff0e37d..fd39985f75e7 100644
>--- a/drivers/net/wwan/t9xx/pcie/mtk_cldma.h
>+++ b/drivers/net/wwan/t9xx/pcie/mtk_cldma.h
>@@ -163,6 +163,9 @@ int mtk_cldma_get_tx_budget(void *dev, enum mtk_hif_id hif_id, u32 qno);
> int mtk_cldma_trb_process(void *dev, struct sk_buff *skb);
> void mtk_cldma_fsm_state_listener(struct mtk_fsm_param *param, struct mtk_ctrl_trans *trans);
> int mtk_cldma_check_ch_cfg(void *dev, struct queue_info *que);
>+void mtk_cldma_pm_suspend(struct mtk_md_dev *mdev);
>+void mtk_cldma_pm_resume_early(struct mtk_md_dev *mdev);
>+void mtk_cldma_pm_resume(struct mtk_md_dev *mdev);
>
> #define drv_ops_name(NAME) cldma_drv_ops_##NAME
> #define cldma_regs_name(NAME) mtk_cldma_regs_##NAME
>diff --git a/drivers/net/wwan/t9xx/pcie/mtk_dpmaif.c b/drivers/net/wwan/t9xx/pcie/mtk_dpmaif.c
>index 43803587bfc3..63273a85e532 100644
>--- a/drivers/net/wwan/t9xx/pcie/mtk_dpmaif.c
>+++ b/drivers/net/wwan/t9xx/pcie/mtk_dpmaif.c
>@@ -205,6 +205,7 @@ struct mtk_dpmaif_ctlb {
> struct dpmaif_irq_param *irq_params;
>
> bool dpmaif_sw_reset;
>+ bool trans_enabled;
> unsigned char rxq_cnt;
> unsigned char txq_cnt;
> };
>@@ -1687,10 +1688,16 @@ static void mtk_dpmaif_trans_disable(struct mtk_dpmaif_ctlb *dcb)
> static void mtk_dpmaif_trans_ctl(struct mtk_dpmaif_ctlb *dcb, bool enable)
> {
> if (enable) {
>- if (dcb->dpmaif_state == DPMAIF_STATE_PWRON)
>+ if (!dcb->trans_enabled &&
>+ dcb->dpmaif_state == DPMAIF_STATE_PWRON) {
>+ dcb->trans_enabled = true;
> mtk_dpmaif_trans_enable(dcb);
>+ }
> } else {
>- mtk_dpmaif_trans_disable(dcb);
>+ if (dcb->trans_enabled) {
should it be done unconditionally from DPMAIF_STATE_PWRON?
>+ dcb->trans_enabled = false;
>+ mtk_dpmaif_trans_disable(dcb);
>+ }
> }
> }
>
>@@ -2060,6 +2067,30 @@ static int mtk_dpmaif_stop(struct mtk_md_dev *mdev)
> return 0;
> }
>
>+void mtk_dpmaif_pm_suspend(struct mtk_md_dev *mdev)
>+{
>+ struct mtk_dpmaif_ctlb *dcb = ((struct mtk_data_blk *)(mdev->data_blk))->dcb;
>+
>+ if (!dcb)
>+ return;
>+
>+ mutex_lock(&dcb->trans_ctl_lock);
>+ mtk_dpmaif_trans_ctl(dcb, false);
>+ mutex_unlock(&dcb->trans_ctl_lock);
>+}
>+
>+void mtk_dpmaif_pm_resume(struct mtk_md_dev *mdev)
>+{
>+ struct mtk_dpmaif_ctlb *dcb = ((struct mtk_data_blk *)(mdev->data_blk))->dcb;
>+
>+ if (!dcb)
>+ return;
>+
>+ mutex_lock(&dcb->trans_ctl_lock);
>+ mtk_dpmaif_trans_ctl(dcb, true);
>+ mutex_unlock(&dcb->trans_ctl_lock);
>+}
>+
> static void mtk_dpmaif_clear(struct mtk_md_dev *mdev)
> {
> struct mtk_dpmaif_ctlb *dcb = ((struct mtk_data_blk *)(mdev->data_blk))->dcb;
>diff --git a/drivers/net/wwan/t9xx/pcie/mtk_dpmaif.h b/drivers/net/wwan/t9xx/pcie/mtk_dpmaif.h
>index e7e2f333141c..20fd53fd44b5 100644
>--- a/drivers/net/wwan/t9xx/pcie/mtk_dpmaif.h
>+++ b/drivers/net/wwan/t9xx/pcie/mtk_dpmaif.h
>@@ -10,5 +10,7 @@
>
> int mtk_pcie_data_init(struct mtk_md_dev *mdev);
> int mtk_pcie_data_exit(struct mtk_md_dev *mdev);
>+void mtk_dpmaif_pm_suspend(struct mtk_md_dev *mdev);
>+void mtk_dpmaif_pm_resume(struct mtk_md_dev *mdev);
>
> #endif /* __MTK_DPMAIF_H__ */
>diff --git a/drivers/net/wwan/t9xx/pcie/mtk_pci.c b/drivers/net/wwan/t9xx/pcie/mtk_pci.c
>index baac3692f1e3..f659c9a7aa96 100644
>--- a/drivers/net/wwan/t9xx/pcie/mtk_pci.c
>+++ b/drivers/net/wwan/t9xx/pcie/mtk_pci.c
>@@ -11,8 +11,10 @@
> #include <linux/device.h>
> #include <linux/dma-mapping.h>
> #include <linux/kernel.h>
>+#include <linux/kthread.h>
> #include <linux/module.h>
>
>+#include "mtk_cldma.h"
> #include "mtk_dev.h"
> #include "mtk_dpmaif.h"
> #include "mtk_trans_ctrl.h"
>@@ -199,7 +201,6 @@ int mtk_pci_register_irq(struct mtk_md_dev *mdev, int irq_id,
> }
> priv->irq_cb_list[irq_id] = irq_cb;
> priv->irq_cb_data[irq_id] = data;
>-
> return 0;
> }
>
>@@ -970,11 +971,93 @@ static const struct pci_error_handlers mtk_pci_err_handler = {
> .error_detected = mtk_pci_error_detected,
> };
>
>+static void mtk_pci_pm_trb_park(struct mtk_md_dev *mdev)
>+{
>+ struct mtk_ctrl_trans *trans = ((struct mtk_ctrl_blk *)mdev->ctrl_blk)->ctrl_hw_priv;
>+ int i;
>+
>+ for (i = 0; i < trans->trb_srv_num; i++)
>+ kthread_park(trans->trb_srv[i]->trb_thread);
>+}
>+
>+static void mtk_pci_pm_trb_unpark(struct mtk_md_dev *mdev)
>+{
>+ struct mtk_ctrl_trans *trans = ((struct mtk_ctrl_blk *)mdev->ctrl_blk)->ctrl_hw_priv;
>+ int i;
>+
>+ for (i = 0; i < trans->trb_srv_num; i++)
>+ kthread_unpark(trans->trb_srv[i]->trb_thread);
>+}
>+
>+static int __maybe_unused mtk_pci_pm_suspend(struct device *dev)
>+{
>+ struct pci_dev *pdev = to_pci_dev(dev);
>+ struct mtk_md_dev *mdev = pci_get_drvdata(pdev);
>+ struct mtk_pci_priv *priv = mdev->hw_priv;
RCT
>+
>+ mtk_pci_pm_trb_park(mdev);
>+
>+ mtk_cldma_pm_suspend(mdev);
>+
>+ mtk_dpmaif_pm_suspend(mdev);
>+
>+ /* Mask MHCCIF interrupt */
>+ mtk_pci_mask_irq(mdev, priv->mhccif_irq_id);
>+
>+ /* Mask all MSI-X interrupts at the device level */
>+ mtk_pci_mac_write32(priv, REG_IMASK_HOST_MSIX_CLR_GRP0_0, U32_MAX);
>+
>+ /* Save PCI configuration space */
>+ pci_save_state(pdev);
>+
>+ return 0;
>+}
>+
>+static int __maybe_unused mtk_pci_pm_resume(struct device *dev)
>+{
>+ struct pci_dev *pdev = to_pci_dev(dev);
>+ struct mtk_md_dev *mdev = pci_get_drvdata(pdev);
>+ struct mtk_pci_priv *priv = mdev->hw_priv;
>+ int ret;
>+
>+ /* Restore PCIe configuration space (including MSI-X enable bits) */
these comments are rather obvious
>+ pci_restore_state(pdev);
>+
>+ /* Re-enable bus mastering for DMA */
>+ pci_set_master(pdev);
>+
>+ /* Restore ATR (address translation registers in MMIO BAR space) */
>+ ret = priv->cfg->atr_init(mdev);
>+ if (ret) {
>+ dev_err(mdev->dev, "PM: failed to re-init ATR on resume\n");
>+ return ret;
>+ }
>+
>+ /* Unmask MHCCIF interrupt */
>+ mtk_pci_unmask_irq(mdev, priv->mhccif_irq_id);
>+
>+ mtk_cldma_pm_resume_early(mdev);
>+
>+ /* Restart CLDMA TX queues that have pending descriptors */
>+ mtk_cldma_pm_resume(mdev);
>+
>+ mtk_dpmaif_pm_resume(mdev);
>+
>+ mtk_pci_pm_trb_unpark(mdev);
>+
>+ return 0;
>+}
>+
>+static const struct dev_pm_ops mtk_pci_pm_ops = {
>+ SET_SYSTEM_SLEEP_PM_OPS(mtk_pci_pm_suspend, mtk_pci_pm_resume)
>+};
>+
> static struct pci_driver mtk_pci_drv = {
> .name = "mtk_pci_drv",
> .id_table = t9xx_pci_table,
> .probe = mtk_pci_probe,
> .remove = mtk_pci_remove,
>+ .driver.pm = &mtk_pci_pm_ops,
> .err_handler = &mtk_pci_err_handler
> };
>
>
>--
>2.34.1
^ permalink raw reply
* Re: [PATCH v8 2/6] mm/memory-failure: surface unhandlable kernel pages as -ENOTRECOVERABLE
From: Miaohe Lin @ 2026-06-01 12:28 UTC (permalink / raw)
To: Breno Leitao
Cc: linux-mm, linux-kernel, linux-doc, linux-kselftest,
linux-trace-kernel, kernel-team, Lance Yang, Andrew Morton,
David Hildenbrand, Lorenzo Stoakes, Vlastimil Babka,
Mike Rapoport, Suren Baghdasaryan, Michal Hocko, Shuah Khan,
Naoya Horiguchi, Steven Rostedt, Masami Hiramatsu,
Mathieu Desnoyers, Jonathan Corbet, Shuah Khan, Liam R. Howlett
In-Reply-To: <20260527-ecc_panic-v8-2-9ea0cfa16bb0@debian.org>
On 2026/5/27 22:06, Breno Leitao wrote:
> get_any_page() collapses every HWPoisonHandlable() rejection into a
> single -EIO via the __get_hwpoison_page() -> -EBUSY -> shake_page()
> -> retry path. That is correct for the transient case (a userspace
> folio briefly off LRU during migration or compaction, which a later
> shake can drag back), but wrong for stable kernel-owned pages: slab,
> page-table, large-kmalloc and PG_reserved pages will never become
> HWPoisonHandlable(), so the retry loop is wasted work and the final
> -EIO loses the "this is structurally unrecoverable" information.
> memory_failure() then maps -EIO into MF_MSG_GET_HWPOISON, which the
> panic-on-unrecoverable sysctl deliberately does not act on.
>
> Introduce HWPoisonKernelOwned(), a small predicate that positively
> identifies pages the hwpoison handler cannot recover from:
>
> HWPoisonKernelOwned(p, flags) :=
> !(MF_SOFT_OFFLINE && page_has_movable_ops(p)) &&
> (PageReserved(p) || PageSlab(p) ||
> PageTable(p) || PageLargeKmalloc(p))
>
> The MF_SOFT_OFFLINE / page_has_movable_ops() opt-out mirrors the
> same exception in HWPoisonHandlable(): soft-offline is allowed to
> migrate movable_ops pages even though they are not on the LRU, and
> we must not pre-empt that with an unrecoverable verdict.
>
> The list is intentionally not exhaustive. vmalloc and kernel-stack
> pages, for example, do not carry a page_type bit and would need a
> different oracle; they keep going through the existing retry path
> unchanged. This is the smallest set we can identify with certainty
> by page type.
>
> Wire the helper into the top of get_any_page() to short-circuit
> those pages before the retry loop runs. On a hit, drop the caller's
> MF_COUNT_INCREASED reference (if any) and return -ENOTRECOVERABLE
> straight away. Pages outside the helper's positive list still take
> the existing retry path and return -EIO, leaving operator-visible
> behaviour for those cases unchanged.
>
> Extend the unhandlable-page pr_err() to fire for either errno and
> update the get_hwpoison_page() kerneldoc to document the new return.
>
> memory_failure() still folds every negative return into
> MF_MSG_GET_HWPOISON via its existing "else if (res < 0)" branch, so
> this patch on its own only changes the errno that soft_offline_page()
> can propagate to its callers. A follow-up wires -ENOTRECOVERABLE
> through memory_failure() and reports MF_MSG_KERNEL for the
> unrecoverable cases, which is what the
> panic_on_unrecoverable_memory_failure sysctl observes.
Thanks for your patch.
>
> Suggested-by: David Hildenbrand <david@kernel.org>
> Suggested-by: Lance Yang <lance.yang@linux.dev>
> Signed-off-by: Breno Leitao <leitao@debian.org>
> ---
> mm/memory-failure.c | 42 ++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 40 insertions(+), 2 deletions(-)
>
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index f4d3e6e20e13..8f63bdfeff8f 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -1325,6 +1325,28 @@ static inline bool HWPoisonHandlable(struct page *page, unsigned long flags)
> return PageLRU(page) || is_free_buddy_page(page);
> }
>
> +/*
> + * Positive identification of pages the hwpoison handler cannot recover.
> + * These page types are owned by kernel internals (no userspace mapping
> + * to unmap, no file mapping to invalidate, no migration target), so the
> + * shake_page() / retry loop in get_any_page() can never turn them into
> + * something HWPoisonHandlable() will accept. Short-circuit them to
> + * -ENOTRECOVERABLE so callers can panic on operator request instead of
> + * spinning through retries that exit as a transient-looking -EIO.
> + *
> + * The MF_SOFT_OFFLINE / page_has_movable_ops() opt-out mirrors
> + * HWPoisonHandlable(): soft-offline is allowed to migrate movable_ops
> + * pages even though they are not on the LRU.
> + */
> +static inline bool HWPoisonKernelOwned(struct page *page, unsigned long flags)
> +{
> + if ((flags & MF_SOFT_OFFLINE) && page_has_movable_ops(page))
> + return false;
> +
> + return PageReserved(page) || PageSlab(page) ||
Once shake_page finds a lightweight range-based way to shrink slab, slab pages could be freed
into buddy and above PageSlab test should be removed then. Maybe add a TODO or XXX here?
> + PageTable(page) || PageLargeKmalloc(page);
I'm not sure but is it safe or a common way to test PageReserved, PageSlab,
PageTable and PageLargeKmalloc without extra page refcnt?
Apart from the above nits, this patch looks good to me.
Thanks.
.
^ permalink raw reply
* Re: [PATCH mm-unstable v18 11/14] mm/khugepaged: Introduce mTHP collapse support
From: Nico Pache @ 2026-06-01 12:40 UTC (permalink / raw)
To: David Hildenbrand (Arm), Usama Arif, usamaarif642
Cc: linux-doc, linux-kernel, linux-mm, linux-trace-kernel, aarcange,
akpm, anshuman.khandual, apopple, baohua, baolin.wang, byungchul,
catalin.marinas, cl, corbet, dave.hansen, dev.jain, gourry,
hannes, hughd, jack, jackmanb, jannh, jglisse, joshua.hahnjy, kas,
lance.yang, liam, ljs, mathieu.desnoyers, matthew.brost, mhiramat,
mhocko, peterx, pfalcato, rakie.kim, raquini, rdunlap,
richard.weiyang, rientjes, rostedt, rppt, ryan.roberts, shivankg,
sunnanyong, surenb, thomas.hellstrom, tiwai, vbabka, vishal.moola,
wangkefeng.wang, will, willy, yang, ying.huang, ziy, zokeefe
In-Reply-To: <b8380eb3-096a-49f1-9ace-99c1e75888b4@kernel.org>
On Mon, Jun 1, 2026 at 2:11 AM David Hildenbrand (Arm) <david@kernel.org> wrote:
>
> On 5/22/26 17:00, Nico Pache wrote:
>
> Finally time for the core piece :)
*music intensifies* :p
>
> > Enable khugepaged to collapse to mTHP orders. This patch implements the
> > main scanning logic using a bitmap to track occupied pages and a stack
> > structure that allows us to find optimal collapse sizes.
> >
> > Previous to this patch, PMD collapse had 3 main phases, a light weight
> > scanning phase (mmap_read_lock) that determines a potential PMD
> > collapse, an alloc phase (mmap unlocked), then finally heavier collapse
> > phase (mmap_write_lock).
> >
> > To enabled mTHP collapse we make the following changes:
> >
> > During PMD scan phase, track occupied pages in a bitmap. When mTHP
> > orders are enabled, we remove the restriction of max_ptes_none during the
> > scan phase to avoid missing potential mTHP collapse candidates. Once we
> > have scanned the full PMD range and updated the bitmap to track occupied
> > pages, we use the bitmap to find the optimal mTHP size.
> >
> > Implement collapse_scan_bitmap() to perform binary recursion on the bitmap
> > and determine the best eligible order for the collapse. A stack structure
> > is used instead of traditional recursion to manage the search. This also
> > prevents a traditional recursive approach when the kernel stack struct is
> > limited. The algorithm recursively splits the bitmap into smaller chunks to
> > find the highest order mTHPs that satisfy the collapse criteria. We start
> > by attempting the PMD order, then moved on the consecutively lower orders
> > (mTHP collapse). The stack maintains a pair of variables (offset, order),
> > indicating the number of PTEs from the start of the PMD, and the order of
> > the potential collapse candidate.
> >
> > The algorithm for consuming the bitmap works as such:
> > 1) push (0, HPAGE_PMD_ORDER) onto the stack
> > 2) pop the stack
> > 3) check if the number of set bits in that (offset,order) pair
> > statisfy the max_ptes_none threshold for that order
> > 4) if yes, attempt collapse
> > 5) if no (or collapse fails), push two new stack items representing
> > the left and right halves of the current bitmap range, at the
> > next lower order
> > 6) repeat at step (2) until stack is empty.
> >
> > Below is a diagram representing the algorithm and stack items:
> >
> > offset mid_offset
> > | |
> > | |
> > v v
> > ____________________________________
> > | PTE Page Table |
> > --------------------------------------
> > <-------><------->
> > order-1 order-1
>
>
> Reading this, it is unclear why exactly do we need the stack.
So I looked into your items below. It seems logical, and I think it
works the same way; however, your method seems slightly harder to
understand due to all the edge cases and more error-prone to future
changes (the stack holds implicit knowledge of the offset/order that
must now be tracked in the edge cases).
Given the stack is 24 bytes, I'm not sure if the extra complexity is
worth saving that small amount of memory. Although we would also be
getting rid of (3?) functions, so both approaches have pros and cons.
I will implement a patch comparing your solution against mine and send
it here, then we can decide which approach is better.
>
> Why can't you work with offset + cur_order?
>
> Initially,
>
> offset = 0;
> cur_order = HPAGE_PMD_ORDER;
>
> If collapse succeeded, advance to next range.
> If collapse failed, try next smaller order, keeping offset unchanged.
>
> if (failed && cur_order > KHUGEPAGED_MIN_MTHP_ORDER) {
> /* Try next smaller order. */
> cur_order = cur_order - 1;
> } else {
> /* Skip to next chunk. */
> offset += 1 << cur_order;
> cur_order = max_order_from_offset(offset);
> }
>
> Of course, handling disabled orders. max_order_from_offset() is rather trivial
> (natural buddy order, capped at HPAGE_PMD_ORDER).
>
> What's the benefit of the stack?
>
> >
> > mTHP collapses reject regions containing swapped out or shared pages.
> > This is because adding new entries can lead to new none pages, and these
> > may lead to constant promotion into a higher order mTHP. A similar
> > issue can occur with "max_ptes_none > HPAGE_PMD_NR/2" due to a collapse
> > introducing at least 2x the number of pages, and on a future scan will
> > satisfy the promotion condition once again. This issue is prevented via
> > the collapse_max_ptes_none() function which imposes the max_ptes_none
> > restrictions above.
> >
> > We currently only support mTHP collapse for max_ptes_none values of 0
> > and HPAGE_PMD_NR - 1. resulting in the following behavior:
> >
> > - max_ptes_none=0: Never introduce new empty pages during collapse
> > - max_ptes_none=HPAGE_PMD_NR-1: Always try collapse to the highest
> > available mTHP order
> >
> > Any other max_ptes_none value will emit a warning and default mTHP
> > collapse to max_ptes_none=0. There should be no behavior change for PMD
> > collapse.
> >
> > Once we determine what mTHP sizes fits best in that PMD range a collapse
> > is attempted. A minimum collapse order of 2 is used as this is the lowest
> > order supported by anon memory as defined by THP_ORDERS_ALL_ANON.
> >
> > Currently madv_collapse is not supported and will only attempt PMD
> > collapse.
> >
> > We can also remove the check for is_khugepaged inside the PMD scan as
> > the collapse_max_ptes_none() function handles this logic now.
> >
> > Signed-off-by: Nico Pache <npache@redhat.com>
> > ---
> > mm/khugepaged.c | 181 +++++++++++++++++++++++++++++++++++++++++++++---
> > 1 file changed, 172 insertions(+), 9 deletions(-)
> >
> > diff --git a/mm/khugepaged.c b/mm/khugepaged.c
> > index 64ceebc9d8a7..d3d7db8be26c 100644
> > --- a/mm/khugepaged.c
> > +++ b/mm/khugepaged.c
> > @@ -99,6 +99,30 @@ static DEFINE_READ_MOSTLY_HASHTABLE(mm_slots_hash, MM_SLOTS_HASH_BITS);
> >
> > static struct kmem_cache *mm_slot_cache __ro_after_init;
> >
> > +#define KHUGEPAGED_MIN_MTHP_ORDER 2
> > +/*
> > + * mthp_collapse() does an iterative DFS over a binary tree, from
> > + * HPAGE_PMD_ORDER down to KHUGEPAGED_MIN_MTHP_ORDER. The max stack
> > + * size needed for a DFS on a binary tree is height + 1, where
> > + * height = HPAGE_PMD_ORDER - KHUGEPAGED_MIN_MTHP_ORDER.
> > + *
> > + * ilog2 is used in place of HPAGE_PMD_ORDER because some architectures
> > + * (e.g. ppc64le) do not define HPAGE_PMD_ORDER until after build time.
>
> I was confused there for a second why you mention ilog2, when it's really "We
> cannot use HPAGE_PMD_ORDER.".
>
> Best to simplify to:
>
> "Note that we cannot use HPAGE_PMD_ORDER, because it is variable on some
> architectures".
Ok thank you i can clear that up.
>
> > + */
> > +#define MTHP_STACK_SIZE (ilog2(MAX_PTRS_PER_PTE) - KHUGEPAGED_MIN_MTHP_ORDER + 1)
> > +
> > +/*
> > + * Defines a range of PTE entries in a PTE page table which are being
> > + * considered for mTHP collapse.
> > + *
> > + * @offset: the offset of the first PTE entry in a PMD range.
> > + * @order: the order of the PTE entries being considered for collapse.
> > + */
> > +struct mthp_range {
> > + u16 offset;
> > + u8 order;
> > +};
> > +
> > struct collapse_control {
> > bool is_khugepaged;
> >
> > @@ -110,6 +134,12 @@ struct collapse_control {
> >
> > /* nodemask for allocation fallback */
> > nodemask_t alloc_nmask;
> > +
> > + /* Each bit represents a single occupied (!none/zero) page. */
> > + DECLARE_BITMAP(mthp_bitmap, MAX_PTRS_PER_PTE);
>
> This should just be called something like "present_ptes"
yeah not a bad idea.
>
> > + /* A mask of the current range being considered for mTHP collapse. */
> > + DECLARE_BITMAP(mthp_bitmap_mask, MAX_PTRS_PER_PTE);
> > + struct mthp_range mthp_bitmap_stack[MTHP_STACK_SIZE];
>
> This is really just a temporary bitmap used for collapse_mthp_count_present()
> only. Either rename it, or better, avoid it completely.
yeah when i first started this we didnt have bitmap_weight_from()
thanks for the pointer to that, I no longer need a temp bitmap.
>
> > };
> >
> > /**
> > @@ -1411,20 +1441,137 @@ static enum scan_result collapse_huge_page(struct mm_struct *mm, unsigned long s
> > return result;
> > }
> >
> > +static void collapse_mthp_stack_push(struct collapse_control *cc, int *stack_size,
> > + u16 offset, u8 order)
> > +{
> > + const int size = *stack_size;
> > + struct mthp_range *stack = &cc->mthp_bitmap_stack[size];
> > +
> > + VM_WARN_ON_ONCE(size >= MTHP_STACK_SIZE);
> > + stack->order = order;
> > + stack->offset = offset;
> > + (*stack_size)++;
> > +}
> > +
> > +static struct mthp_range collapse_mthp_stack_pop(struct collapse_control *cc,
> > + int *stack_size)
> > +{
> > + const int size = *stack_size;
> > +
> > + VM_WARN_ON_ONCE(size <= 0);
> > + (*stack_size)--;
> > + return cc->mthp_bitmap_stack[size - 1];
> > +}
> > +
> > +static unsigned int collapse_mthp_count_present(struct collapse_control *cc,
> > + u16 offset, unsigned int nr_ptes)
> > +{
> > + bitmap_zero(cc->mthp_bitmap_mask, MAX_PTRS_PER_PTE);
> > + bitmap_set(cc->mthp_bitmap_mask, offset, nr_ptes);
> > + return bitmap_weight_and(cc->mthp_bitmap, cc->mthp_bitmap_mask, MAX_PTRS_PER_PTE);
>
> You really just want to count the number of set bits? You don't need a temporary
> bitmap for that.
>
> Assume you want to check an order-2 (4 bits), bitmap_weight_and() would check
> all bits ...
>
> I'd suggest starting simple here, and avoiding the temporary bitmap.
>
> Can we simply use bitmap_weight_from(cc->mthp_bitmap, offset, nr_ptes)?
Yes! Thank you :)
>
> > +}
> > +
> > +/*
> > + * mthp_collapse() consumes the bitmap that is generated during
> > + * collapse_scan_pmd() to determine what regions and mTHP orders fit best.
> > + *
> > + * Each bit in cc->mthp_bitmap represents a single occupied (!none/zero) page.
> > + * A stack structure cc->mthp_bitmap_stack is used to check different regions
> > + * of the bitmap for collapse eligibility. The stack maintains a pair of
> > + * variables (offset, order), indicating the number of PTEs from the start of
> > + * the PMD, and the order of the potential collapse candidate respectively. We
> > + * start at the PMD order and check if it is eligible for collapse; if not, we
> > + * add two entries to the stack at a lower order to represent the left and right
> > + * halves of the PTE page table we are examining.
> > + *
> > + * offset mid_offset
> > + * | |
> > + * | |
> > + * v v
> > + * --------------------------------------
> > + * | cc->mthp_bitmap |
> > + * --------------------------------------
> > + * <-------><------->
> > + * order-1 order-1
> > + *
> > + * For each of these, we determine how many PTE entries are occupied in the
> > + * range of PTE entries we propose to collapse, then we compare this to a
> > + * threshold number of PTE entries which would need to be occupied for a
> > + * collapse to be permitted at that order (accounting for max_ptes_none).
> > + *
> > + * If a collapse is permitted, we attempt to collapse the PTE range into a
> > + * mTHP.
> > + */
> > +static int mthp_collapse(struct mm_struct *mm, struct vm_area_struct *vma,
> > + unsigned long address, int referenced, int unmapped,
> > + struct collapse_control *cc, unsigned long enabled_orders)
> > +{
> > + unsigned int nr_occupied_ptes, nr_ptes, max_ptes_none;
> > + int collapsed = 0, stack_size = 0;
> > + unsigned long collapse_address;
> > + struct mthp_range range;
> > + u16 offset;
> > + u8 order;
> > +
> > + collapse_mthp_stack_push(cc, &stack_size, 0, HPAGE_PMD_ORDER);
> > +
> > + while (stack_size) {
> > + range = collapse_mthp_stack_pop(cc, &stack_size);
> > + order = range.order;
> > + offset = range.offset;
> > + nr_ptes = 1UL << order;
> > +
> > + if (!test_bit(order, &enabled_orders))
> > + goto next_order;
> > +
> > + max_ptes_none = collapse_max_ptes_none(cc, vma, order);
> > +
> > + nr_occupied_ptes = collapse_mthp_count_present(cc, offset,
> > + nr_ptes);
> > +
> > + if (nr_occupied_ptes >= nr_ptes - max_ptes_none) {
> > + int ret;
> > +
> > + collapse_address = address + offset * PAGE_SIZE;
> > + ret = collapse_huge_page(mm, collapse_address, referenced,
> > + unmapped, cc, order);
> > + if (ret == SCAN_SUCCEED) {
> > + collapsed += nr_ptes;
> > + continue;
> > + }
> > + }
> > +
> > +next_order:
> > + if ((BIT(order) - 1) & enabled_orders) {
> > + const u8 next_order = order - 1;
> > + const u16 mid_offset = offset + (nr_ptes / 2);
> > +
> > + collapse_mthp_stack_push(cc, &stack_size, mid_offset,
> > + next_order);
> > + collapse_mthp_stack_push(cc, &stack_size, offset,
> > + next_order);
> > + }
> > + }
> > + return collapsed;
> > +}
> > +
> > static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> > struct vm_area_struct *vma, unsigned long start_addr,
> > bool *lock_dropped, struct collapse_control *cc)
> > {
> > - const unsigned int max_ptes_none = collapse_max_ptes_none(cc, vma, HPAGE_PMD_ORDER);
> > const unsigned int max_ptes_shared = collapse_max_ptes_shared(cc, HPAGE_PMD_ORDER);
> > const unsigned int max_ptes_swap = collapse_max_ptes_swap(cc, HPAGE_PMD_ORDER);
> > + unsigned int max_ptes_none = collapse_max_ptes_none(cc, vma, HPAGE_PMD_ORDER);
> > + enum tva_type tva_flags = cc->is_khugepaged ? TVA_KHUGEPAGED : TVA_FORCED_COLLAPSE;
> > pmd_t *pmd;
> > - pte_t *pte, *_pte;
> > - int none_or_zero = 0, shared = 0, referenced = 0;
> > + pte_t *pte, *_pte, pteval;
> > + int i;
> > + int none_or_zero = 0, shared = 0, nr_collapsed = 0, referenced = 0;
> > enum scan_result result = SCAN_FAIL;
> > struct page *page = NULL;
> > struct folio *folio = NULL;
> > unsigned long addr;
> > + unsigned long enabled_orders;
> > spinlock_t *ptl;
> > int node = NUMA_NO_NODE, unmapped = 0;
> >
> > @@ -1436,8 +1583,19 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> > goto out;
> > }
> >
> > + bitmap_zero(cc->mthp_bitmap, MAX_PTRS_PER_PTE);
> > memset(cc->node_load, 0, sizeof(cc->node_load));
> > nodes_clear(cc->alloc_nmask);
> > +
> > + enabled_orders = collapse_allowable_orders(vma, vma->vm_flags, tva_flags);
> > +
> > + /*
> > + * If PMD is the only enabled order, enforce max_ptes_none, otherwise
> > + * scan all pages to populate the bitmap for mTHP collapse.
> > + */
>
> You should note here, that we re-verify in mthp_collapse().
>
> But the question is, whether we should relocate the check completely into
> mthp_collapse(), instead of conditionally duplicating it.
>
> What speaks against always populating the bitmap and making the decision in
> mthp_collapse()?
>
> Sure, we might scan a page table a bit longer, but the code gets clearer ... and
> I am not sure if scanning some more page table entries is really that critical here.
Someone asked me to preserve the legacy behavior (PMD only). Although
rather trivial, if you set max_ptes_none=0 for example, we'd still
have to do 511 iterations for no reason if PMD collapse is the only
enabled order rather than bailing immediately.
I'm ok with dropping it, but I think its the correct approach (despite
the extra complexity). @Usama Arif brought up this point here
https://lore.kernel.org/all/f8f7bb71-ca31-46ee-a62d-7ddfd83e0ead@gmail.com/
>
>
> > + if (enabled_orders != BIT(HPAGE_PMD_ORDER))
> > + max_ptes_none = KHUGEPAGED_MAX_PTES_LIMIT;
> > +
> > pte = pte_offset_map_lock(mm, pmd, start_addr, &ptl);
> > if (!pte) {
> > cc->progress++;
> > @@ -1445,11 +1603,13 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> > goto out;
> > }
> >
> > - for (addr = start_addr, _pte = pte; _pte < pte + HPAGE_PMD_NR;
> > - _pte++, addr += PAGE_SIZE) {
> > + for (i = 0; i < HPAGE_PMD_NR; i++) {
> > + _pte = pte + i;
> > + addr = start_addr + i * PAGE_SIZE;
> > + pteval = ptep_get(_pte);
> > +
> > cc->progress++;
> >
> > - pte_t pteval = ptep_get(_pte);
> > if (pte_none_or_zero(pteval)) {
> > if (++none_or_zero > max_ptes_none) {
> > result = SCAN_EXCEED_NONE_PTE;
> > @@ -1529,6 +1689,8 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> > }
> > }
> >
> > + /* Set bit for occupied pages */
> > + __set_bit(i, cc->mthp_bitmap);
> > /*
> > * Record which node the original page is from and save this
> > * information to cc->node_load[].
> > @@ -1587,10 +1749,11 @@ static enum scan_result collapse_scan_pmd(struct mm_struct *mm,
> > if (result == SCAN_SUCCEED) {
> > /* collapse_huge_page expects the lock to be dropped before calling */
> > mmap_read_unlock(mm);
> > - result = collapse_huge_page(mm, start_addr, referenced,
> > - unmapped, cc, HPAGE_PMD_ORDER);
> > - /* collapse_huge_page will return with the mmap_lock released */
> > + nr_collapsed = mthp_collapse(mm, vma, start_addr, referenced,
> > + unmapped, cc, enabled_orders);
> > + /* mmap_lock was released above, set lock_dropped */
> > *lock_dropped = true;
> > + result = nr_collapsed ? SCAN_SUCCEED : SCAN_FAIL;
>
> As Lance says, this error handling likely needs some thought.
Yes I agree, I'm working on that now. I had a WIP patch for this some
time ago, but I never gave it the full thought it needed, and it fell
into the abyss.
Thanks :)
-- Nico
>
> --
> Cheers,
>
> David
>
^ permalink raw reply
* Re: [PATCH v4 04/13] liveupdate: register luo_ser as KHO subtree
From: Pratyush Yadav @ 2026-06-01 12:39 UTC (permalink / raw)
To: Pasha Tatashin
Cc: linux-kselftest, rppt, shuah, akpm, linux-mm, skhan, linux-doc,
linux-kernel, corbet, dmatlack, kexec, pratyush, skhawaja, graf
In-Reply-To: <20260530221938.115978-5-pasha.tatashin@soleen.com>
On Sat, May 30 2026, Pasha Tatashin wrote:
> Entirely remove the LUO FDT wrapper since the FDT only carries the
> compatible string and the pointer to the centralized struct luo_ser.
> Instead, register the struct luo_ser via the KHO raw subtree
> API, placing the compatibility string inside the structure itself.
>
> Signed-off-by: Pasha Tatashin <pasha.tatashin@soleen.com>
> ---
> include/linux/kho/abi/luo.h | 57 +++++++++---------------
> kernel/liveupdate/luo_core.c | 85 +++++++++++-------------------------
> 2 files changed, 46 insertions(+), 96 deletions(-)
>
> diff --git a/include/linux/kho/abi/luo.h b/include/linux/kho/abi/luo.h
> index 1b2f865a771a..9a4fe491812b 100644
> --- a/include/linux/kho/abi/luo.h
> +++ b/include/linux/kho/abi/luo.h
> @@ -10,11 +10,11 @@
> *
> * Live Update Orchestrator uses the stable Application Binary Interface
> * defined below to pass state from a pre-update kernel to a post-update
> - * kernel. The ABI is built upon the Kexec HandOver framework and uses a
> - * Flattened Device Tree to describe the preserved data.
> + * kernel. The ABI is built upon the Kexec HandOver framework and registers
> + * the central `struct luo_ser` via the KHO raw subtree API.
> *
> - * This interface is a contract. Any modification to the FDT structure, node
> - * properties, compatible strings, or the layout of the `__packed` serialization
> + * This interface is a contract. Any modification to the structure fields,
> + * compatible strings, or the layout of the `__packed` serialization
> * structures defined here constitutes a breaking change. Such changes require
> * incrementing the version number in the relevant `_COMPATIBLE` string to
> * prevent a new kernel from misinterpreting data from an old kernel.
> @@ -23,31 +23,15 @@
> * however, backward/forward compatibility is only guaranteed for kernels
> * supporting the same ABI version.
> *
> - * FDT Structure Overview:
> + * KHO Structure Overview:
> * The entire LUO state is encapsulated within a single KHO entry named "LUO".
> - * This entry contains an FDT with the following layout:
> - *
> - * .. code-block:: none
> - *
> - * / {
> - * compatible = "luo-v2";
> - * luo-abi-header = <phys_addr_of_luo_ser>;
> - * };
> - *
> - * Main LUO Node (/):
> - *
> - * - compatible: "luo-v2"
> - * Identifies the overall LUO ABI version.
> - * - luo-abi-header: u64
> - * The physical address of `struct luo_ser`.
> + * This entry contains the `struct luo_ser` structure.
> *
> * Serialization Structures:
> - * The FDT properties point to memory regions containing arrays of simple,
> - * `__packed` structures. These structures contain the actual preserved state.
> - *
> * - struct luo_ser:
> * The central ABI structure that contains the overall state of the LUO.
> - * It includes the liveupdate-number and pointers to sessions and FLBs.
> + * It includes the compatibility string, the liveupdate-number, and pointers
> + * to sessions and FLBs.
> *
> * - struct luo_session_header_ser:
> * Header for the session array. Contains the total page count of the
> @@ -78,26 +62,27 @@
> #ifndef _LINUX_KHO_ABI_LUO_H
> #define _LINUX_KHO_ABI_LUO_H
>
> +#include <linux/align.h>
> #include <uapi/linux/liveupdate.h>
>
> /*
> - * The LUO FDT hooks all LUO state for sessions, fds, etc.
> + * The LUO state is registered under this KHO entry name.
> */
> -#define LUO_FDT_SIZE PAGE_SIZE
> -#define LUO_FDT_KHO_ENTRY_NAME "LUO"
> -#define LUO_FDT_COMPATIBLE "luo-v2"
> -#define LUO_FDT_ABI_HEADER "luo-abi-header"
> +#define LUO_KHO_ENTRY_NAME "LUO"
> +#define LUO_ABI_COMPATIBLE "luo-v3"
> +#define LUO_ABI_COMPAT_LEN ALIGN(sizeof(LUO_ABI_COMPATIBLE), 8)
The length of the compatible field will change depending on the length
of the string. While that is technically fine since a new ABI version is
allowed to change the layout, it feels odd. I think it would be better
if we define a static size here, say 64 bytes. This way you can avoid
all the weirdness that can happen when you move from one version to
another.
>
> /**
> * struct luo_ser - Centralized LUO ABI header.
> + * @compatible: Compatibility string identifying the LUO ABI version.
> * @liveupdate_num: A counter tracking the number of successful live updates.
> * @sessions_pa: Physical address of the first session block header.
> * @flbs_pa: Physical address of the FLB header.
> *
> - * This structure is the root of all preserved LUO state. It is pointed to by
> - * the "luo-abi-header" property in the LUO FDT.
> + * This structure is the root of all preserved LUO state.
> */
> struct luo_ser {
> + char compatible[LUO_ABI_COMPAT_LEN];
> u64 liveupdate_num;
> u64 sessions_pa;
> u64 flbs_pa;
[...]
> @@ -94,40 +91,29 @@ static int __init luo_early_startup(void)
> return 0;
> }
>
> - /* Retrieve LUO subtree, and verify its format. */
> - err = kho_retrieve_subtree(LUO_FDT_KHO_ENTRY_NAME, &fdt_phys, NULL);
> + /* Retrieve LUO state from KHO. */
> + err = kho_retrieve_subtree(LUO_KHO_ENTRY_NAME, &luo_ser_phys, &len);
> if (err) {
> if (err != -ENOENT) {
> - pr_err("failed to retrieve FDT '%s' from KHO: %pe\n",
> - LUO_FDT_KHO_ENTRY_NAME, ERR_PTR(err));
> + pr_err("failed to retrieve LUO state '%s' from KHO: %pe\n",
> + LUO_KHO_ENTRY_NAME, ERR_PTR(err));
> return err;
> }
>
> return 0;
> }
>
> - luo_global.fdt_in = phys_to_virt(fdt_phys);
> - err = fdt_node_check_compatible(luo_global.fdt_in, 0,
> - LUO_FDT_COMPATIBLE);
> - if (err) {
> - pr_err("FDT '%s' is incompatible with '%s' [%d]\n",
> - LUO_FDT_KHO_ENTRY_NAME, LUO_FDT_COMPATIBLE, err);
> -
> + if (len < sizeof(*luo_ser)) {
len != sizeof(*luo_ser) here?
> + pr_err("LUO state is too small (%zu < %zu)\n", len, sizeof(*luo_ser));
> return -EINVAL;
> }
>
> - header_size = 0;
> - ptr = fdt_getprop(luo_global.fdt_in, 0, LUO_FDT_ABI_HEADER, &header_size);
> - if (!ptr || header_size != sizeof(u64)) {
> - pr_err("Unable to get ABI header '%s' [%d]\n",
> - LUO_FDT_ABI_HEADER, header_size);
> -
> + luo_ser = phys_to_virt(luo_ser_phys);
> + if (strncmp(luo_ser->compatible, LUO_ABI_COMPATIBLE, LUO_ABI_COMPAT_LEN)) {
> + pr_err("LUO state is incompatible with '%s'\n", LUO_ABI_COMPATIBLE);
> return -EINVAL;
> }
>
> - luo_ser_pa = get_unaligned((u64 *)ptr);
> - luo_ser = phys_to_virt(luo_ser_pa);
> -
> luo_global.liveupdate_num = luo_ser->liveupdate_num;
> pr_info("Retrieved live update data, liveupdate number: %lld\n",
> luo_global.liveupdate_num);
[...]
--
Regards,
Pratyush Yadav
^ permalink raw reply
* Re: [PATCH mm-unstable v18 11/14] mm/khugepaged: Introduce mTHP collapse support
From: David Hildenbrand (Arm) @ 2026-06-01 13:15 UTC (permalink / raw)
To: Nico Pache, Usama Arif, usamaarif642
Cc: linux-doc, linux-kernel, linux-mm, linux-trace-kernel, aarcange,
akpm, anshuman.khandual, apopple, baohua, baolin.wang, byungchul,
catalin.marinas, cl, corbet, dave.hansen, dev.jain, gourry,
hannes, hughd, jack, jackmanb, jannh, jglisse, joshua.hahnjy, kas,
lance.yang, liam, ljs, mathieu.desnoyers, matthew.brost, mhiramat,
mhocko, peterx, pfalcato, rakie.kim, raquini, rdunlap,
richard.weiyang, rientjes, rostedt, rppt, ryan.roberts, shivankg,
sunnanyong, surenb, thomas.hellstrom, tiwai, vbabka, vishal.moola,
wangkefeng.wang, will, willy, yang, ying.huang, ziy, zokeefe
In-Reply-To: <CAA1CXcBg1su-bk3i_H+TW4-nTgvGSGqRNeC9MpQo7sGeH8ejnA@mail.gmail.com>
>>
>> Reading this, it is unclear why exactly do we need the stack.
>
> So I looked into your items below. It seems logical, and I think it
> works the same way; however, your method seems slightly harder to
> understand due to all the edge cases and more error-prone to future
> changes (the stack holds implicit knowledge of the offset/order that
> must now be tracked in the edge cases).
>
> Given the stack is 24 bytes, I'm not sure if the extra complexity is
> worth saving that small amount of memory. Although we would also be
> getting rid of (3?) functions, so both approaches have pros and cons.
I consider a simple forward loop over the offset ... less complexity compared to
a stack structure :)
>
> I will implement a patch comparing your solution against mine and send
> it here, then we can decide which approach is better.
Right, throw it over the fence and I'll see how to improve it further.
[...]
>>> + bitmap_zero(cc->mthp_bitmap, MAX_PTRS_PER_PTE);
>>> memset(cc->node_load, 0, sizeof(cc->node_load));
>>> nodes_clear(cc->alloc_nmask);
>>> +
>>> + enabled_orders = collapse_allowable_orders(vma, vma->vm_flags, tva_flags);
>>> +
>>> + /*
>>> + * If PMD is the only enabled order, enforce max_ptes_none, otherwise
>>> + * scan all pages to populate the bitmap for mTHP collapse.
>>> + */
>>
>> You should note here, that we re-verify in mthp_collapse().
>>
>> But the question is, whether we should relocate the check completely into
>> mthp_collapse(), instead of conditionally duplicating it.
>>
>> What speaks against always populating the bitmap and making the decision in
>> mthp_collapse()?
>>
>> Sure, we might scan a page table a bit longer, but the code gets clearer ... and
>> I am not sure if scanning some more page table entries is really that critical here.
>
> Someone asked me to preserve the legacy behavior (PMD only). Although
> rather trivial, if you set max_ptes_none=0 for example, we'd still
> have to do 511 iterations for no reason if PMD collapse is the only
> enabled order rather than bailing immediately.
>
> I'm ok with dropping it, but I think its the correct approach (despite
> the extra complexity). @Usama Arif brought up this point here
> https://lore.kernel.org/all/f8f7bb71-ca31-46ee-a62d-7ddfd83e0ead@gmail.com/
We talk about regressions, but I am not sure if we care about scanning speed
within a page table that much?
After all, we locked it and already read some entries.
Having the same check at two places to optimize for PMD order might right now
feel like a good optimization, but likely an irrelevant one in a near future?
Anyhow, won't push back, as long as we document why we are special casing things
here.
--
Cheers,
David
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox