DYNAMIC_DEBUG plants a vector/table of struct _ddebug's in kernel
data; its (module,filename,function) columns have ordered, repeating
data (90%,55%,45%), which is readily compressible.
This patchset restructures the table to isolate that redundant data,
then packs out the duplicates, achieving 40% reduction in the memory
needed for that data, about 30kb on my laptop. No de/compression used.
On my i7 laptop kernel:
dyndbg: 62 44 2899 1728 mptcp.check_fully_established.895 <== 1728/2899 compresion
dyndbg: 63 debug prints in module mptcp (in 44 functions)
dyndbg: 2900 prdebugs in 277 modules, \
12 KiB in ddebug tables, \
90 KiB in __dyndbg section, \
67 KiB in __dyndbg_sites section <== on this data
It should be practical to actually reclaim that memory; RFC on how
that could be done. On a hail-mary, I kreallocd sites on a module,
it panic'd.
To isolate the compressible part, we split the table/struct in 2. The
redundant columns (modname, filename, function) are moved to struct
_ddebug_site ("__dyndbg_sites" elf-section), and refd by a
new/temporary _ddebug.site pointer. The site data can then be made
optional (6 patches), managed separately, and "compressed".
The pointer adds 8/56 to the memory footprint, we want to replace it
with indexing; the _index only needs to span ~10k callsites, and comes
"free" from unused bits in struct _ddebug.
The difficulty with this, and the reason for the multi-commit
adaptation, is that we have only a struct _ddebug pointer.
We know its in __dyndbgs[], but we dont know &__dyndbg_sites[],
which we need to finally get __dyndbg_sites[_index].
We get it with an "up-over-and-down" maneuver;
1. __dyndbgs[._index] ==> __dyndbgs[0]
pretty trivial with ._index
2. __dyndbgs[0].site
indirectly use __dyndbgs[0].site instead of [._index].site
3. insert "header" record - just _ddebug, renamed
uses .gnu.linkonce to create headers, linker script to insert it - RFC
becomes __dyndbgs[0]
provides .site for [2]
4. __dyndbgs[0].site->__dyndbg_sites[_index]
trivial
With "up-over-and-down" working, we can get rid of .site pointer
Next is specialization and unionization:
We create a header union, containing a struct _ddebug to make it "fit"
in __dyndbgs[], and an anonymous struct with .site and a several other
struct _ddebug fields checked to validate headers.
This allows the header to keep its .site pointer, while struct _ddebug
drops it, shrinking memory usage 8/56 back to parity.
It is working, but Im certain it can be restructured more tightly.
In particular, [header,__dyndbgs[]] fits well in a flex-array.
My anonymous struct/union fu is insufficient, hints or examples
are solicited.
Finally comes "consolidation" (not compression). To do this, we
split/clone ._index to its two tasks (1,4 above); ._back (to n=0), then
._map (to soon to be repacked site). When adding each module, we scan
__dyndbg_sites[], repack it with its unique entries, and ._map it and
the function's remaining _ddebugs to use that unique entry in the
repacked vector.
Version-5 is here:
https://lore.kernel.org/lkml/CAJfuBxyjfopQMzMyFrrZK7YppsL8kh0VVWySrJDXeUB15uwFag@mail.gmail.com/#t
Version-6 differs from -v5 by:
It works so far..
it's been surviving 0day tests:
It compresses, can save 30kb of 70kb table, of 200kb total data overhead.
Ive resolved most of my previous QsRFCs, but Im using .gnu.linkonce
sections and linker script tweaks for the 1st time and figured it
needs more scrutiny than usual. Several of the commit messages say
"RFC" where I have most uncertainty.
The "most broken" part of the patchset is the specialization &
unionization, Im certain theres a tighter way to construct it. ISTM
theres a good chance that [header,__dyndbgs[]] fits well in a
flex-array, and container_of could help. Hints, tips, pointers to
examples are welcome.
Patch 34 - prototype pr_debug_once(), pr_debug_ratelimited()
= print-once.
$> echo function foo lineno 1234 +po > control
Doable with just a new flag, and a new "did_print" data bit, both
which will fit in struct _ddebug, no separate storage needed. Its
just one more flag check for dynamic_pr_debug, AFTER its +p enabled.
Unsetting flag and reseting it would clear the "did_print" bit.
Maybe just resetting "+o" too.
= Rate-Limiting
We don't need this often, so allocate RLs only when enabled & needed,
and store in hash on [ &builtin|&module, ._back|._map ]
$> echo module serio_raw +rp > control
By default, we could allocate a rate-limiting (RL) structure for every
pr-debug flagged, store them in a RLs-hash (on ._back + module), and
retrieve and evaluate them only when the pr-debug is enabled. We
could even defer RL alloc/attach until the pr-debug is actually
called.
If we also (optionally) hash RLs on (._map + module), then they are
shared across a function. This could be useful behavior, and would
save lots of memory where it is appropriate.
$> echo function foo +rg > control
- on function alone, RL-group capable
- flag modifier. use _map not _back.
Next ? More Dieting
_ddebug as 3 vectors of components.
We have split _ddebug in 2, maybe also split out static-keys to a 3rd
vector ? This would smoothe the path towards a single struct _ddebug
per function, which controls all N pr_debugs in that function.
We could recapitulate the repack, with some changes in details;
- we're packing the referring table, not the referred table.
ie: we'd condense __dyndbgs[], keep the vector of static-keys.
- we'd lose meaning of lineno, could reclaim bits for something else.
New API elements ?
. pr_debug_once() etal
. pr_debug_ratelimited() etal
. pr_debug_rl_grouped() etal ??
This is good for signaling programmer intent. A tweak to DD_METADATA
could set the corresponding bits at compile-time, causing its
provisioning when prdebug'd.
RL contol - Some speculation:
$> echo module serio_raw +r rate 10 burst 3 > control
- after required flags.
- only legal with +r
- suggests sharing across group (by default)
- should rates scale per callsite count ? or only by user ?
$> echo module serio_raw lineno 333 +r rate 100 burst 3 > control
- individual RLs are possible
Jim Cromie (33):
Jim Cromie (34):
3 trivial patches, 2 are in -rc3:
dyndbg: avoid calling dyndbg_emit_prefix when it has no work
dyndbg: drop uninformative vpr_info
dyndbg: display KiB of data memory used.
split struct _ddebug in 2:
dyndbg: split struct _ddebug's display fields to new _ddebug_site
dyndbg: __init iterate over __dyndbg & __dyndbg_site in parallel
dyndbg+module: expose ddebug_sites to modules
make site info optional:
Even though !.site is impossible, by linker init + DD_METADATA,
lets allow it, to shorten codepaths and work towards dropping site data.
dyndbg: refactor part of ddebug_change to ddebug_match_site
dyndbg: accept null site in ddebug_match_site
dyndbg: hoist ->site out of ddebug_match_site
dyndbg: accept null site in ddebug_change
dyndbg: accept null site in dynamic_emit_prefix
dyndbg: accept null site in ddebug_proc_show
dyndbg: refactor ddebug_alter_site out of ddebug_change
dyndbg: allow deleting site info via control interface
decoupling:
dyndbg: add ddebug_site(_get|_put) abstraction
cleanup, tidy:
dyndbg: ddebug_add_module avoid adding empty modules
do the "up-over-down" maneuver:
dyndbg: add _index to struct _ddebug
dyndbg: prevent build bugs via -DNO_DYNAMIC_DEBUG_TABLE
dyndbg: RFC - DEFINE_DYNAMIC_DEBUG_TABLE
dyndbg: RFC handle __dyndbg* sections in module.lds.h
dyndbg: ddebug_add_module() handle headers.
dyndbg: validate ddebug_site_get invariants
dyndbg: fix NULL deref after deleting sites
dyndbg: dont show header records in control
dyndbg: make site pointer and checks on it optional (almost)
dyndbg: swap WARN_ON for BUG_ON see what 0-day says
dyndbg: fixup protect header when deleting site
dyndbg: unionize _ddebug*_headers with struct _ddebug*
dyndbg: RFC drop _ddebug.site pointer
condense unique site data:
dyndbg: split/copy ._index into 2 new fields: ._back, ._map
dyndbg: detect repeated site recs in add_module
dyndbg: pack module pr_debug sites
dyndbg: pack pr-debug site-recs in builtin modules
new features - _once, _ratelimited:
dyndbg: prototype print-once and print-ratelimited RFC
arch/arm/boot/compressed/Makefile | 2 +
arch/sparc/vdso/Makefile | 2 +
arch/x86/boot/compressed/Makefile | 1 +
arch/x86/entry/vdso/Makefile | 3 +
arch/x86/purgatory/Makefile | 1 +
drivers/firmware/efi/libstub/Makefile | 3 +-
include/asm-generic/module.lds.h | 9 +-
include/asm-generic/vmlinux.lds.h | 24 +-
include/linux/dynamic_debug.h | 143 +++++++--
kernel/module-internal.h | 1 +
kernel/module.c | 11 +-
lib/dynamic_debug.c | 421 ++++++++++++++++++++------
12 files changed, 499 insertions(+), 122 deletions(-)
--
2.31.1
Migrate 3 fields: module, function, file into a new struct _ddebug_site.
These reflect the code's hierarchy, and are inherently ordered by the
compile/link; they exhibit 90%,55%,45% repetition (on my fedora based,
localmodconfig'd, virtme-configkernel --update'd conf).
Since struct _ddebugs are placed in their own elf section, this means
splitting its section also. The new section is named "__dyndbg_sites"
(the plural name conveys the vector nature of the contents), and
"__dyndbg" is renamed "__dyndbgs" to reinforce this.
Struct _ddebug gets a new .site, pointing to a struct _ddebug_site.
The new ptr increases memory footprint, but it is temporary
scaffolding; once we can map from _ddebugs[N] --> _ddebug_sites[N]
indirectly, we can drop site pointer, regaining worst case memory
parity with master.
The indirection gives several advantages:
- site ptr lets us decouple the 2 arrays
we can properly isolate dependencies by allowing null site
- the moved display fields are inherently hierarchical, and the linker
section is ordered; so (module, file, function) have repeating
values (90%, 85%, 45%). This is readily compressible, even with a
simple field-wise run length encoding. Since I'm splitting the struct,
I also reordered the fields to match the hierarchy.
- the separate linker section sets up naturally for block compression.
section compression at build time is practical - how ?
include/linux/decompress/generic.h has no corresponding compression
- we could drop sites and their storage opportunistically.
this could reduce per-site mem by 24/56.
Subsystems may not need/want "module:func:line:" in their logs.
If they already use format-prefixes such as "drm:kms:",
they can select on those, and don't need the site info for that.
forex:
#> echo module drm format "^drm:kms: " +p >control
ie: dynamic_debug_exec_queries("format '^drm:kms: '", "drm");
Once we can map: ddebugs[N] -> ddebug_sites[N], we can:
- compress __dyndbg_sites during __init, and mark section __initdata
- store the compressed block instead.
- decompress on-demand, stream for `cat control`
- save chunks of decompressed buffer for enabled callsites
- free chunks on site disable, or on memory pressure.
Whats actually done here is ths rather mechanical, and preparatory.
dynamic_debug.h:
I cut struct _ddebug in half, renamed the optional top-half to
_ddebug_site, kept __align(8) for both halves. I added a forward decl
for a unified comment for both head & body, and added _ddebug.site to
point at body.
DEFINE_DYNAMIC_DEBUG_METADATA now declares and initializes a 2nd
static struct var holding the _ddebug_site, and refs _ddebug to it.
dynamic_debug.c:
dynamic_debug_init() mem-usage now also counts sites.
dynamic_emit_prefix() & ddebug_change() use those moved fields; they
get a new initialized auto-var, and the field refs get adjusted as
needed to follow the field moves from one struct to the other.
struct _ddebug_site *dc = dp->site;
ddebug_proc_show() differs slightly; it assigns to (not initializes)
the autovar, to avoid a panic when p == SEQ_START_TOKEN.
vmlinux.lds.h:
1st, move the original set of __dyndbg___start|stop directives into a
new macro; this puts a name (and a block of blamelog) on this and
future changes. Also, since compressed elf sections are supported,
maybe we can use it on our "__dyndbg_sites" section; the macro better
isolates the items involved in such a move.
Then add in new "__dyndbg_sites" section and its start|stop symbols.
I kept the same align(8) and KEEP as used in the "__dyndbgs" section,
but I think this can be relaxed; I suspect the align is there for the
static-key member, which has state encoded in 2 LSBs, and _ddebug_site
doesnt have a key. That said, I dont think anything is gained by
changing.
Signed-off-by: Jim Cromie <[email protected]>
---
include/asm-generic/vmlinux.lds.h | 22 +++++++++++-----
include/linux/dynamic_debug.h | 37 +++++++++++++++++---------
kernel/module.c | 2 +-
lib/dynamic_debug.c | 44 ++++++++++++++++++-------------
4 files changed, 66 insertions(+), 39 deletions(-)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 0331d5d49551..f4e861282ed7 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -335,6 +335,20 @@
KEEP(*(.dtb.init.rodata)) \
__dtb_end = .;
+/* implement dynamic printk debug */
+#if defined(CONFIG_DYNAMIC_DEBUG) || defined(CONFIG_DYNAMIC_DEBUG_CORE)
+#define DYNAMIC_DEBUG_DATA() \
+ . = ALIGN(8); \
+ __start___dyndbg_sites = .; \
+ KEEP(*(__dyndbg_sites)) \
+ __stop___dyndbg_sites = .; \
+ __start___dyndbg = .; \
+ KEEP(*(__dyndbgs)) \
+ __stop___dyndbg = .;
+#else
+#define DYNAMIC_DEBUG_DATA()
+#endif
+
/*
* .data section
*/
@@ -351,12 +365,8 @@
__end_once = .; \
STRUCT_ALIGN(); \
*(__tracepoints) \
- /* implement dynamic printk debug */ \
- . = ALIGN(8); \
- __start___dyndbg = .; \
- KEEP(*(__dyndbg)) \
- __stop___dyndbg = .; \
- LIKELY_PROFILE() \
+ DYNAMIC_DEBUG_DATA() \
+ LIKELY_PROFILE() \
BRANCH_PROFILE() \
TRACE_PRINTKS() \
BPF_RAW_TP() \
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index dce631e678dd..130d5b36e08e 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -7,20 +7,28 @@
#endif
/*
- * An instance of this structure is created in a special
- * ELF section at every dynamic debug callsite. At runtime,
- * the special section is treated as an array of these.
+ * A pair of these structs are created in 2 special ELF sections
+ * (__dyndbg, __dyndbg_sites) for every dynamic debug callsite.
+ * At runtime, the sections are treated as arrays.
*/
-struct _ddebug {
+struct _ddebug;
+struct _ddebug_site {
/*
- * These fields are used to drive the user interface
- * for selecting and displaying debug callsites.
+ * These fields (and lineno) are used to:
+ * - decorate log messages per _ddebug.flags
+ * - select callsites for modification via >control
+ * - display callsites & settings in `cat control`
*/
const char *modname;
- const char *function;
const char *filename;
+ const char *function;
+} __aligned(8);
+
+struct _ddebug {
+ struct _ddebug_site *site;
+ /* format is always needed, lineno shares word with flags */
const char *format;
- unsigned int lineno:18;
+ const unsigned lineno:18;
/*
* The flags field controls the behaviour at the callsite.
* The bits here are changed dynamically when the user
@@ -49,8 +57,7 @@ struct _ddebug {
struct static_key_false dd_key_false;
} key;
#endif
-} __attribute__((aligned(8)));
-
+} __aligned(8);
#if defined(CONFIG_DYNAMIC_DEBUG_CORE)
@@ -88,11 +95,15 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
const char *fmt, ...);
#define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt) \
- static struct _ddebug __aligned(8) \
- __section("__dyndbg") name = { \
+ static struct _ddebug_site __aligned(8) \
+ __section("__dyndbg_sites") name##_site = { \
.modname = KBUILD_MODNAME, \
- .function = __func__, \
.filename = __FILE__, \
+ .function = __func__, \
+ }; \
+ static struct _ddebug __aligned(8) \
+ __section("__dyndbgs") name = { \
+ .site = &name##_site, \
.format = (fmt), \
.lineno = __LINE__, \
.flags = _DPRINTK_FLAGS_DEFAULT, \
diff --git a/kernel/module.c b/kernel/module.c
index 30479355ab85..592e71f9f99d 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -3331,7 +3331,7 @@ static int find_module_sections(struct module *mod, struct load_info *info)
if (section_addr(info, "__obsparm"))
pr_warn("%s: Ignoring obsolete parameters\n", mod->name);
- info->debug = section_objs(info, "__dyndbg",
+ info->debug = section_objs(info, "__dyndbgs",
sizeof(*info->debug), &info->num_debug);
return 0;
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index ab8fc16911ae..691e43d58eef 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -165,19 +165,20 @@ static int ddebug_change(const struct ddebug_query *query,
for (i = 0; i < dt->num_ddebugs; i++) {
struct _ddebug *dp = &dt->ddebugs[i];
+ struct _ddebug_site *dc = dp->site;
/* match against the source filename */
if (query->filename &&
- !match_wildcard(query->filename, dp->filename) &&
+ !match_wildcard(query->filename, dc->filename) &&
!match_wildcard(query->filename,
- kbasename(dp->filename)) &&
+ kbasename(dc->filename)) &&
!match_wildcard(query->filename,
- trim_prefix(dp->filename)))
+ trim_prefix(dc->filename)))
continue;
/* match against the function */
if (query->function &&
- !match_wildcard(query->function, dp->function))
+ !match_wildcard(query->function, dc->function))
continue;
/* match against the format */
@@ -214,8 +215,8 @@ static int ddebug_change(const struct ddebug_query *query,
#endif
dp->flags = newflags;
v2pr_info("changed %s:%d [%s]%s =%s\n",
- trim_prefix(dp->filename), dp->lineno,
- dt->mod_name, dp->function,
+ trim_prefix(dc->filename), dp->lineno,
+ dt->mod_name, dc->function,
ddebug_describe_flags(dp->flags, &fbuf));
}
}
@@ -586,12 +587,13 @@ static int remaining(int wrote)
return 0;
}
-static char *__dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
+static char *__dynamic_emit_prefix(const struct _ddebug *dp, char *buf)
{
int pos_after_tid;
int pos = 0;
+ const struct _ddebug_site *desc = dp->site;
- if (desc->flags & _DPRINTK_FLAGS_INCL_TID) {
+ if (dp->flags & _DPRINTK_FLAGS_INCL_TID) {
if (in_interrupt())
pos += snprintf(buf + pos, remaining(pos), "<intr> ");
else
@@ -599,15 +601,15 @@ static char *__dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
task_pid_vnr(current));
}
pos_after_tid = pos;
- if (desc->flags & _DPRINTK_FLAGS_INCL_MODNAME)
+ if (dp->flags & _DPRINTK_FLAGS_INCL_MODNAME)
pos += snprintf(buf + pos, remaining(pos), "%s:",
desc->modname);
- if (desc->flags & _DPRINTK_FLAGS_INCL_FUNCNAME)
+ if (dp->flags & _DPRINTK_FLAGS_INCL_FUNCNAME)
pos += snprintf(buf + pos, remaining(pos), "%s:",
desc->function);
- if (desc->flags & _DPRINTK_FLAGS_INCL_LINENO)
+ if (dp->flags & _DPRINTK_FLAGS_INCL_LINENO)
pos += snprintf(buf + pos, remaining(pos), "%d:",
- desc->lineno);
+ dp->lineno);
if (pos - pos_after_tid)
pos += snprintf(buf + pos, remaining(pos), " ");
if (pos >= PREFIX_SIZE)
@@ -884,6 +886,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
{
struct ddebug_iter *iter = m->private;
struct _ddebug *dp = p;
+ struct _ddebug_site *dc;
struct flagsbuf flags;
if (p == SEQ_START_TOKEN) {
@@ -892,9 +895,11 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
return 0;
}
+ dc = dp->site;
+
seq_printf(m, "%s:%u [%s]%s =%s \"",
- trim_prefix(dp->filename), dp->lineno,
- iter->table->mod_name, dp->function,
+ trim_prefix(dc->filename), dp->lineno,
+ iter->table->mod_name, dc->function,
ddebug_describe_flags(dp->flags, &flags));
seq_escape(m, dp->format, "\t\r\n\"");
seq_puts(m, "\"\n");
@@ -1097,17 +1102,17 @@ static int __init dynamic_debug_init(void)
return 0;
}
iter = __start___dyndbg;
- modname = iter->modname;
+ modname = iter->site->modname;
iter_start = iter;
for (; iter < __stop___dyndbg; iter++) {
entries++;
- if (strcmp(modname, iter->modname)) {
+ if (strcmp(modname, iter->site->modname)) {
modct++;
ret = ddebug_add_module(iter_start, n, modname);
if (ret)
goto out_err;
n = 0;
- modname = iter->modname;
+ modname = iter->site->modname;
iter_start = iter;
}
n++;
@@ -1117,9 +1122,10 @@ static int __init dynamic_debug_init(void)
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 section, %d KiB in __dyndbg_sites section\n",
entries, modct, (int)((modct * sizeof(struct ddebug_table)) >> 10),
- (int)((entries * sizeof(struct _ddebug)) >> 10));
+ (int)((entries * sizeof(struct _ddebug)) >> 10),
+ (int)((entries * sizeof(struct _ddebug_site)) >> 10));
/* apply ddebug_query boot param, dont unload tables on err */
if (ddebug_setup_string[0] != '\0') {
--
2.31.1
Wrap function in a static-inline one, which checks flags to avoid
calling the function unnecessarily.
And hoist its output-buffer initialization to the grand-caller, which
is already allocating the buffer on the stack, and can trivially
initialize it too.
Signed-off-by: Jim Cromie <[email protected]>
---
include/linux/dynamic_debug.h | 5 +++++
lib/dynamic_debug.c | 19 ++++++++++++-------
2 files changed, 17 insertions(+), 7 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index a57ee75342cf..dce631e678dd 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -32,6 +32,11 @@ struct _ddebug {
#define _DPRINTK_FLAGS_INCL_FUNCNAME (1<<2)
#define _DPRINTK_FLAGS_INCL_LINENO (1<<3)
#define _DPRINTK_FLAGS_INCL_TID (1<<4)
+
+#define _DPRINTK_FLAGS_INCL_ANY \
+ (_DPRINTK_FLAGS_INCL_MODNAME | _DPRINTK_FLAGS_INCL_FUNCNAME |\
+ _DPRINTK_FLAGS_INCL_LINENO | _DPRINTK_FLAGS_INCL_TID)
+
#if defined DEBUG
#define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINT
#else
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index c70d6347afa2..ede4a491ee87 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -586,13 +586,11 @@ static int remaining(int wrote)
return 0;
}
-static char *dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
+static char *__dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
{
int pos_after_tid;
int pos = 0;
- *buf = '\0';
-
if (desc->flags & _DPRINTK_FLAGS_INCL_TID) {
if (in_interrupt())
pos += snprintf(buf + pos, remaining(pos), "<intr> ");
@@ -618,11 +616,18 @@ static char *dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
return buf;
}
+static inline char *dynamic_emit_prefix(struct _ddebug *desc, char *buf)
+{
+ if (unlikely(desc->flags & _DPRINTK_FLAGS_INCL_ANY))
+ return __dynamic_emit_prefix(desc, buf);
+ return buf;
+}
+
void __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
{
va_list args;
struct va_format vaf;
- char buf[PREFIX_SIZE];
+ char buf[PREFIX_SIZE] = "";
BUG_ON(!descriptor);
BUG_ON(!fmt);
@@ -655,7 +660,7 @@ void __dynamic_dev_dbg(struct _ddebug *descriptor,
if (!dev) {
printk(KERN_DEBUG "(NULL device *): %pV", &vaf);
} else {
- char buf[PREFIX_SIZE];
+ char buf[PREFIX_SIZE] = "";
dev_printk_emit(LOGLEVEL_DEBUG, dev, "%s%s %s: %pV",
dynamic_emit_prefix(descriptor, buf),
@@ -684,7 +689,7 @@ void __dynamic_netdev_dbg(struct _ddebug *descriptor,
vaf.va = &args;
if (dev && dev->dev.parent) {
- char buf[PREFIX_SIZE];
+ char buf[PREFIX_SIZE] = "";
dev_printk_emit(LOGLEVEL_DEBUG, dev->dev.parent,
"%s%s %s %s%s: %pV",
@@ -720,7 +725,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
vaf.va = &args;
if (ibdev && ibdev->dev.parent) {
- char buf[PREFIX_SIZE];
+ char buf[PREFIX_SIZE] = "";
dev_printk_emit(LOGLEVEL_DEBUG, ibdev->dev.parent,
"%s%s %s %s: %pV",
--
2.31.1
Move all the site-match logic into a separate function, reindent the
code, and replace the continues with return falses.
No functional changes.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 75 ++++++++++++++++++++++++++-------------------
1 file changed, 43 insertions(+), 32 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 78d4a9020600..839dd128ba20 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -143,6 +143,48 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
query->first_lineno, query->last_lineno);
}
+static int ddebug_match_site(const struct ddebug_query *query,
+ const struct _ddebug *dp)
+{
+ struct _ddebug_site *dc = dp->site;
+
+ /* match against the source filename */
+ if (query->filename &&
+ !match_wildcard(query->filename, dc->filename) &&
+ !match_wildcard(query->filename,
+ kbasename(dc->filename)) &&
+ !match_wildcard(query->filename,
+ trim_prefix(dc->filename)))
+ return false;
+
+ /* match against the function */
+ if (query->function &&
+ !match_wildcard(query->function, dc->function))
+ return false;
+
+ /* match against the format */
+ if (query->format) {
+ if (*query->format == '^') {
+ char *p;
+ /* anchored search. match must be at beginning */
+ p = strstr(dp->format, query->format+1);
+ if (p != dp->format)
+ return false;
+ } else if (!strstr(dp->format, query->format))
+ return false;
+ }
+
+ /* match against the line number range */
+ if (query->first_lineno &&
+ dp->lineno < query->first_lineno)
+ return false;
+ if (query->last_lineno &&
+ dp->lineno > query->last_lineno)
+ return false;
+
+ return true;
+}
+
/*
* Search the tables for _ddebug's which match the given `query' and
* apply the `flags' and `mask' to them. Returns number of matching
@@ -171,38 +213,7 @@ static int ddebug_change(const struct ddebug_query *query,
struct _ddebug *dp = &dt->ddebugs[i];
struct _ddebug_site *dc = dp->site;
- /* match against the source filename */
- if (query->filename &&
- !match_wildcard(query->filename, dc->filename) &&
- !match_wildcard(query->filename,
- kbasename(dc->filename)) &&
- !match_wildcard(query->filename,
- trim_prefix(dc->filename)))
- continue;
-
- /* match against the function */
- if (query->function &&
- !match_wildcard(query->function, dc->function))
- continue;
-
- /* match against the format */
- if (query->format) {
- if (*query->format == '^') {
- char *p;
- /* anchored search. match must be at beginning */
- p = strstr(dp->format, query->format+1);
- if (p != dp->format)
- continue;
- } else if (!strstr(dp->format, query->format))
- continue;
- }
-
- /* match against the line number range */
- if (query->first_lineno &&
- dp->lineno < query->first_lineno)
- continue;
- if (query->last_lineno &&
- dp->lineno > query->last_lineno)
+ if (!ddebug_match_site(query, dp))
continue;
nfound++;
--
2.31.1
Up until now, the "__dyndbg_sites" section was kinda dangling.
Unlike the "__dyndbgs" section, it was not explicitly loaded by
module.c:load_info(); but it came along quietly, perhaps because of
the per .site initialization (by DEFINE_DYNAMIC_DEBUG_METADATA) across
the section boundary. ISTM we need to treat "__dyndbg_sites" section
just like "__dyndbg", on basic principles.
Doing so starts in module.c:load_info(), which attaches "__dyndbgs"
section, and now does "__dyndbg_sites" too. These changes propagated
out to ddebug_add_module's prototype, which needs to pass the 2nd
section like the 1st.
-v5 fix !CONFIG_DYNAMIC_DEBUG fn-stub
Reported-by: kernel test robot <[email protected]>
Signed-off-by: Jim Cromie <[email protected]>
---
include/linux/dynamic_debug.h | 8 ++++----
kernel/module-internal.h | 1 +
kernel/module.c | 9 ++++++---
lib/dynamic_debug.c | 20 ++++++++++++--------
4 files changed, 23 insertions(+), 15 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 130d5b36e08e..40ea086853ff 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -65,8 +65,8 @@ struct _ddebug {
/* exported for module authors to exercise >control */
int dynamic_debug_exec_queries(const char *query, const char *modname);
-int ddebug_add_module(struct _ddebug *tab, unsigned int n,
- const char *modname);
+int ddebug_add_module(struct _ddebug *tab, struct _ddebug_site *sites,
+ unsigned int numdbgs, const char *modname);
extern int ddebug_remove_module(const char *mod_name);
extern __printf(2, 3)
void __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...);
@@ -198,8 +198,8 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
#include <linux/errno.h>
#include <linux/printk.h>
-static inline int ddebug_add_module(struct _ddebug *tab, unsigned int n,
- const char *modname)
+static inline int ddebug_add_module(struct _ddebug *tab, struct _ddebug_site *sites,
+ unsigned int numdbgs, const char *modname)
{
return 0;
}
diff --git a/kernel/module-internal.h b/kernel/module-internal.h
index 33783abc377b..fb61eb2f8032 100644
--- a/kernel/module-internal.h
+++ b/kernel/module-internal.h
@@ -18,6 +18,7 @@ struct load_info {
char *secstrings, *strtab;
unsigned long symoffs, stroffs, init_typeoffs, core_typeoffs;
struct _ddebug *debug;
+ struct _ddebug_site *sites;
unsigned int num_debug;
bool sig_ok;
#ifdef CONFIG_KALLSYMS
diff --git a/kernel/module.c b/kernel/module.c
index 592e71f9f99d..c4f8c0b5cf07 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -2780,11 +2780,12 @@ static void add_kallsyms(struct module *mod, const struct load_info *info)
}
#endif /* CONFIG_KALLSYMS */
-static void dynamic_debug_setup(struct module *mod, struct _ddebug *debug, unsigned int num)
+static void dynamic_debug_setup(struct module *mod, struct _ddebug *debug,
+ struct _ddebug_site *sites, unsigned int num)
{
if (!debug)
return;
- ddebug_add_module(debug, num, mod->name);
+ ddebug_add_module(debug, sites, num, mod->name);
}
static void dynamic_debug_remove(struct module *mod, struct _ddebug *debug)
@@ -3333,6 +3334,8 @@ static int find_module_sections(struct module *mod, struct load_info *info)
info->debug = section_objs(info, "__dyndbgs",
sizeof(*info->debug), &info->num_debug);
+ info->sites = section_objs(info, "__dyndbg_sites",
+ sizeof(*info->sites), &info->num_debug);
return 0;
}
@@ -4004,7 +4007,7 @@ static int load_module(struct load_info *info, const char __user *uargs,
goto free_arch_cleanup;
}
- dynamic_debug_setup(mod, info->debug, info->num_debug);
+ dynamic_debug_setup(mod, info->debug, info->sites, info->num_debug);
/* Ftrace init must be called in the MODULE_STATE_UNFORMED state */
ftrace_module_init(mod);
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index f7ece35bc722..78d4a9020600 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -49,6 +49,7 @@ struct ddebug_table {
const char *mod_name;
unsigned int num_ddebugs;
struct _ddebug *ddebugs;
+ struct _ddebug_site *sites;
};
struct ddebug_query {
@@ -953,14 +954,14 @@ static const struct proc_ops proc_fops = {
* Allocate a new ddebug_table for the given module
* and add it to the global list.
*/
-int ddebug_add_module(struct _ddebug *tab, unsigned int n,
- const char *name)
+int ddebug_add_module(struct _ddebug *tab, struct _ddebug_site *sites,
+ unsigned int numdbgs, const char *modname)
{
struct ddebug_table *dt;
dt = kzalloc(sizeof(*dt), GFP_KERNEL);
if (dt == NULL) {
- pr_err("error adding module: %s\n", name);
+ pr_err("error adding module: %s\n", modname);
return -ENOMEM;
}
/*
@@ -969,15 +970,16 @@ int ddebug_add_module(struct _ddebug *tab, unsigned int n,
* member of struct module, which lives at least as long as
* this struct ddebug_table.
*/
- dt->mod_name = name;
- dt->num_ddebugs = n;
+ dt->mod_name = modname;
+ dt->num_ddebugs = numdbgs;
dt->ddebugs = tab;
+ dt->sites = sites;
mutex_lock(&ddebug_lock);
list_add(&dt->link, &ddebug_tables);
mutex_unlock(&ddebug_lock);
- v2pr_info("%3u debug prints in module %s\n", n, dt->mod_name);
+ v2pr_info("%3u debug prints in module %s\n", numdbgs, modname);
return 0;
}
@@ -1117,7 +1119,9 @@ static int __init dynamic_debug_init(void)
if (strcmp(modname, site->modname)) {
modct++;
- ret = ddebug_add_module(iter_mod_start, site_ct, modname);
+
+ ret = ddebug_add_module(iter_mod_start, site_mod_start,
+ site_ct, modname);
if (ret)
goto out_err;
site_ct = 0;
@@ -1127,7 +1131,7 @@ static int __init dynamic_debug_init(void)
}
site_ct++;
}
- ret = ddebug_add_module(iter_mod_start, site_ct, modname);
+ ret = ddebug_add_module(iter_mod_start, site_mod_start, site_ct, modname);
if (ret)
goto out_err;
--
2.31.1
A coming change adds _get/_put abstraction on the site pointer, to
allow managing site info more flexibly. The get/put pattern is best
done at a single lexical scope, where its more obviously correct, so
hoist the ->site ref out of ddebug_match_site, and pass it in instead.
no functional changes
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index d62dc471bc1f..e40ce438636a 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -144,10 +144,9 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
}
static int ddebug_match_site(const struct ddebug_query *query,
- const struct _ddebug *dp)
+ const struct _ddebug *dp,
+ const struct _ddebug_site *dc)
{
- struct _ddebug_site *dc;
-
/* match against the format */
if (query->format) {
if (*query->format == '^') {
@@ -168,7 +167,6 @@ static int ddebug_match_site(const struct ddebug_query *query,
dp->lineno > query->last_lineno)
return false;
- dc = dp->site;
if (!dc) {
/* site info has been dropped, so query cannot test these fields */
if (query->filename || query->function)
@@ -220,9 +218,9 @@ static int ddebug_change(const struct ddebug_query *query,
for (i = 0; i < dt->num_ddebugs; i++) {
struct _ddebug *dp = &dt->ddebugs[i];
- struct _ddebug_site *dc;
+ struct _ddebug_site *dc = dp->site;
- if (!ddebug_match_site(query, dp))
+ if (!ddebug_match_site(query, dp, dc))
continue;
nfound++;
--
2.31.1
Since all query terms must be satisfied, we can reorder for clarity,
moving !!site dependent checks after those on _ddebug itself.
1- move format and line-number check code to the top of the function,
since they don't use/check site info.
2- test site pointer:
If its null, we return early, skipping 3:
If the query tests against missing site info, fail the match.
otherwize site matches.
3- rest of function (checking site vs query) is unchanged.
ddebug_match_site ignores module, because it's tested already
by the caller, where it is known from debug_tables.
no functional changes
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 41 +++++++++++++++++++++++++----------------
1 file changed, 25 insertions(+), 16 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 839dd128ba20..d62dc471bc1f 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -146,21 +146,7 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
static int ddebug_match_site(const struct ddebug_query *query,
const struct _ddebug *dp)
{
- struct _ddebug_site *dc = dp->site;
-
- /* match against the source filename */
- if (query->filename &&
- !match_wildcard(query->filename, dc->filename) &&
- !match_wildcard(query->filename,
- kbasename(dc->filename)) &&
- !match_wildcard(query->filename,
- trim_prefix(dc->filename)))
- return false;
-
- /* match against the function */
- if (query->function &&
- !match_wildcard(query->function, dc->function))
- return false;
+ struct _ddebug_site *dc;
/* match against the format */
if (query->format) {
@@ -182,6 +168,29 @@ static int ddebug_match_site(const struct ddebug_query *query,
dp->lineno > query->last_lineno)
return false;
+ dc = dp->site;
+ if (!dc) {
+ /* site info has been dropped, so query cannot test these fields */
+ if (query->filename || query->function)
+ return false;
+ else
+ return true;
+ }
+
+ /* match against the source filename */
+ if (query->filename &&
+ !match_wildcard(query->filename, dc->filename) &&
+ !match_wildcard(query->filename,
+ kbasename(dc->filename)) &&
+ !match_wildcard(query->filename,
+ trim_prefix(dc->filename)))
+ return false;
+
+ /* match against the function */
+ if (query->function &&
+ !match_wildcard(query->function, dc->function))
+ return false;
+
return true;
}
@@ -211,7 +220,7 @@ static int ddebug_change(const struct ddebug_query *query,
for (i = 0; i < dt->num_ddebugs; i++) {
struct _ddebug *dp = &dt->ddebugs[i];
- struct _ddebug_site *dc = dp->site;
+ struct _ddebug_site *dc;
if (!ddebug_match_site(query, dp))
continue;
--
2.31.1
2 prints use site->member, protect them with if site.
no functional changes.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index b2fd7c181bd5..868f56edf831 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -630,15 +630,19 @@ static char *__dynamic_emit_prefix(const struct _ddebug *dp, char *buf)
task_pid_vnr(current));
}
pos_after_tid = pos;
- if (dp->flags & _DPRINTK_FLAGS_INCL_MODNAME)
- pos += snprintf(buf + pos, remaining(pos), "%s:",
- desc->modname);
- if (dp->flags & _DPRINTK_FLAGS_INCL_FUNCNAME)
- pos += snprintf(buf + pos, remaining(pos), "%s:",
- desc->function);
+
+ if (desc) {
+ if (dp->flags & _DPRINTK_FLAGS_INCL_MODNAME)
+ pos += snprintf(buf + pos, remaining(pos), "%s:",
+ desc->modname);
+ if (dp->flags & _DPRINTK_FLAGS_INCL_FUNCNAME)
+ pos += snprintf(buf + pos, remaining(pos), "%s:",
+ desc->function);
+ }
if (dp->flags & _DPRINTK_FLAGS_INCL_LINENO)
pos += snprintf(buf + pos, remaining(pos), "%d:",
dp->lineno);
+
if (pos - pos_after_tid)
pos += snprintf(buf + pos, remaining(pos), " ");
if (pos >= PREFIX_SIZE)
--
2.31.1
fix a debug-print that includes site info, by adding an alternate
debug message that does not.
no functional changes.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index e40ce438636a..b2fd7c181bd5 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -236,10 +236,17 @@ static int ddebug_change(const struct ddebug_query *query,
static_branch_enable(&dp->key.dd_key_true);
#endif
dp->flags = newflags;
- v2pr_info("changed %s:%d [%s]%s =%s\n",
- trim_prefix(dc->filename), dp->lineno,
- dt->mod_name, dc->function,
- ddebug_describe_flags(dp->flags, &fbuf));
+
+ if (dc)
+ v2pr_info("changed %s:%d [%s]%s =%s\n",
+ trim_prefix(dc->filename), dp->lineno,
+ dt->mod_name, dc->function,
+ ddebug_describe_flags(dp->flags, &fbuf));
+ else
+ v2pr_info("changed %s:%d =%s \"%s\"\n",
+ dt->mod_name, dp->lineno,
+ ddebug_describe_flags(dp->flags, &fbuf),
+ dp->format);
}
}
mutex_unlock(&ddebug_lock);
--
2.31.1
Accept a ddebug record with a null site pointer, and write abbreviated
output for that record that doesn't include site info (but does
include line-number, since that can be used in >control queries).
Also add a 2nd header line with a template for the new output.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 25 +++++++++++++++++--------
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 868f56edf831..592aaaf79fd7 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -924,18 +924,27 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
if (p == SEQ_START_TOKEN) {
seq_puts(m,
- "# filename:lineno [module]function flags format\n");
+ "#: filename:lineno [module]function flags format\n"
+ "#| [module]:lineno flags format\n"
+ );
return 0;
}
dc = dp->site;
-
- seq_printf(m, "%s:%u [%s]%s =%s \"",
- trim_prefix(dc->filename), dp->lineno,
- iter->table->mod_name, dc->function,
- ddebug_describe_flags(dp->flags, &flags));
- seq_escape(m, dp->format, "\t\r\n\"");
- seq_puts(m, "\"\n");
+ if (dc) {
+ seq_printf(m, "%s:%u [%s]%s =%s \"",
+ trim_prefix(dc->filename), dp->lineno,
+ iter->table->mod_name, dc->function,
+ ddebug_describe_flags(dp->flags, &flags));
+ seq_escape(m, dp->format, "\t\r\n\"");
+ seq_puts(m, "\"\n");
+ } else {
+ seq_printf(m, "[%s]:%u =%s \"",
+ iter->table->mod_name, dp->lineno,
+ ddebug_describe_flags(dp->flags, &flags));
+ seq_escape(m, dp->format, "\t\r\n\"");
+ seq_puts(m, "\"\n");
+ }
return 0;
}
--
2.31.1
Move the JUMP_LABEL/static-key code to a separate function.
no functional changes.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 22 +++++++++++++++-------
1 file changed, 15 insertions(+), 7 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 592aaaf79fd7..ad7cda840733 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -192,6 +192,18 @@ static int ddebug_match_site(const struct ddebug_query *query,
return true;
}
+static void ddebug_alter_site(struct _ddebug *dp,
+ struct flag_settings *modifiers)
+{
+#ifdef CONFIG_JUMP_LABEL
+ if (dp->flags & _DPRINTK_FLAGS_PRINT) {
+ if (!(modifiers->flags & _DPRINTK_FLAGS_PRINT))
+ static_branch_disable(&dp->key.dd_key_true);
+ } else if (modifiers->flags & _DPRINTK_FLAGS_PRINT)
+ static_branch_enable(&dp->key.dd_key_true);
+#endif
+}
+
/*
* Search the tables for _ddebug's which match the given `query' and
* apply the `flags' and `mask' to them. Returns number of matching
@@ -228,13 +240,9 @@ static int ddebug_change(const struct ddebug_query *query,
newflags = (dp->flags & modifiers->mask) | modifiers->flags;
if (newflags == dp->flags)
continue;
-#ifdef CONFIG_JUMP_LABEL
- if (dp->flags & _DPRINTK_FLAGS_PRINT) {
- if (!(modifiers->flags & _DPRINTK_FLAGS_PRINT))
- static_branch_disable(&dp->key.dd_key_true);
- } else if (modifiers->flags & _DPRINTK_FLAGS_PRINT)
- static_branch_enable(&dp->key.dd_key_true);
-#endif
+
+ ddebug_alter_site(dp, modifiers);
+
dp->flags = newflags;
if (dc)
--
2.31.1
Replace direct ->site refs with _get(),_put() internal API. Right
now, _get() just returns ->site and _put() does nothing. Later we can
replace this implementation with one using ->_index to fetch site data
then forget or keep it selectively.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 31 +++++++++++++++++++++++++------
1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index a4cb048357fb..7a15216e6bef 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -144,6 +144,14 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
query->first_lineno, query->last_lineno);
}
+static struct _ddebug_site *ddebug_site_get(struct _ddebug *dp)
+{
+ return dp->site; /* passthru abstraction */
+}
+static inline void ddebug_site_put(struct _ddebug *dp)
+{
+}
+
static int ddebug_match_site(const struct ddebug_query *query,
const struct _ddebug *dp,
const struct _ddebug_site *dc)
@@ -239,16 +247,18 @@ static int ddebug_change(const struct ddebug_query *query,
for (i = 0; i < dt->num_ddebugs; i++) {
struct _ddebug *dp = &dt->ddebugs[i];
- struct _ddebug_site *dc = dp->site;
+ struct _ddebug_site *dc;
+
+ dc = ddebug_site_get(dp);
if (!ddebug_match_site(query, dp, dc))
- continue;
+ goto skipsite;
nfound++;
newflags = (dp->flags & modifiers->mask) | modifiers->flags;
if (newflags == dp->flags)
- continue;
+ goto skipsite;
ddebug_alter_site(dp, modifiers);
@@ -264,6 +274,9 @@ static int ddebug_change(const struct ddebug_query *query,
dt->mod_name, dp->lineno,
ddebug_describe_flags(dp->flags, &fbuf),
dp->format);
+
+skipsite:
+ ddebug_site_put(dp);
}
}
mutex_unlock(&ddebug_lock);
@@ -633,11 +646,11 @@ static int remaining(int wrote)
return 0;
}
-static char *__dynamic_emit_prefix(const struct _ddebug *dp, char *buf)
+static char *__dynamic_emit_prefix(struct _ddebug *dp, char *buf)
{
int pos_after_tid;
int pos = 0;
- const struct _ddebug_site *desc = dp->site;
+ const struct _ddebug_site *desc;
if (dp->flags & _DPRINTK_FLAGS_INCL_TID) {
if (in_interrupt())
@@ -648,6 +661,7 @@ static char *__dynamic_emit_prefix(const struct _ddebug *dp, char *buf)
}
pos_after_tid = pos;
+ desc = ddebug_site_get(dp);
if (desc) {
if (dp->flags & _DPRINTK_FLAGS_INCL_MODNAME)
pos += snprintf(buf + pos, remaining(pos), "%s:",
@@ -665,6 +679,8 @@ static char *__dynamic_emit_prefix(const struct _ddebug *dp, char *buf)
if (pos >= PREFIX_SIZE)
buf[PREFIX_SIZE - 1] = '\0';
+ ddebug_site_put(dp);
+
return buf;
}
@@ -947,7 +963,8 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
return 0;
}
- dc = dp->site;
+ dc = ddebug_site_get(dp);
+
if (dc) {
seq_printf(m, "%s:%u [%s]%s =%s \"",
trim_prefix(dc->filename), dp->lineno,
@@ -963,6 +980,8 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
seq_puts(m, "\"\n");
}
+ ddebug_site_put(dp);
+
return 0;
}
--
2.31.1
Don't create ddebug_table's for modules with 0 callsites. This saves
memory, and avoids creating ddebug_tables with questionable contents,
which would then be iterated over for 'cat control'.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 7a15216e6bef..db7563a88d8a 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1033,6 +1033,12 @@ int ddebug_add_module(struct _ddebug *tab, struct _ddebug_site *sites,
{
struct ddebug_table *dt;
+ v3pr_info("add-module: %s.%d sites\n", modname, numdbgs);
+ if (!numdbgs) {
+ v3pr_info(" skip %s\n", modname);
+ return 0;
+ }
+
dt = kzalloc(sizeof(*dt), GFP_KERNEL);
if (dt == NULL) {
pr_err("error adding module: %s\n", modname);
--
2.31.1
The next patch adds DEFINE_DYNAMIC_DEBUG_TABLE(), which broke several
subtrees, including efi, vdso, and some of arch/*/boot/compressed,
with various relocation errors, iirc.
Avoid those problems by adding a define to suppress the "transparent"
DEFINE_DYNAMIC_DEBUG_TABLE() invocation. I found the x86 problems
myself, [email protected] found arm & sparc problems, and may yet find
others.
Reported-by: <[email protected]> # on [jimc:lkp-test/dyndbg-diet] recently
Signed-off-by: Jim Cromie <[email protected]>
---
arch/arm/boot/compressed/Makefile | 2 ++
arch/sparc/vdso/Makefile | 2 ++
arch/x86/boot/compressed/Makefile | 1 +
arch/x86/entry/vdso/Makefile | 3 +++
arch/x86/purgatory/Makefile | 1 +
drivers/firmware/efi/libstub/Makefile | 3 ++-
6 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
index fd94e27ba4fa..72f056a00ad4 100644
--- a/arch/arm/boot/compressed/Makefile
+++ b/arch/arm/boot/compressed/Makefile
@@ -82,6 +82,8 @@ compress-$(CONFIG_KERNEL_LZMA) = lzma
compress-$(CONFIG_KERNEL_XZ) = xzkern
compress-$(CONFIG_KERNEL_LZ4) = lz4
+KBUILD_CFLAGS += -DNO_DYNAMIC_DEBUG_TABLE
+
libfdt_objs := fdt_rw.o fdt_ro.o fdt_wip.o fdt.o
ifeq ($(CONFIG_ARM_ATAG_DTB_COMPAT),y)
diff --git a/arch/sparc/vdso/Makefile b/arch/sparc/vdso/Makefile
index c5e1545bc5cf..960ed0fb6804 100644
--- a/arch/sparc/vdso/Makefile
+++ b/arch/sparc/vdso/Makefile
@@ -30,6 +30,8 @@ obj-y += $(vdso_img_objs)
targets += $(vdso_img_cfiles)
targets += $(vdso_img_sodbg) $(vdso_img-y:%=vdso%.so)
+KBUILD_CFLAGS += -DNO_DYNAMIC_DEBUG_TABLE
+
CPPFLAGS_vdso.lds += -P -C
VDSO_LDFLAGS_vdso.lds = -m elf64_sparc -soname linux-vdso.so.1 --no-undefined \
diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
index e0bc3988c3fa..ada4eb960d95 100644
--- a/arch/x86/boot/compressed/Makefile
+++ b/arch/x86/boot/compressed/Makefile
@@ -31,6 +31,7 @@ targets := vmlinux vmlinux.bin vmlinux.bin.gz vmlinux.bin.bz2 vmlinux.bin.lzma \
KBUILD_CFLAGS := -m$(BITS) -O2
KBUILD_CFLAGS += -fno-strict-aliasing -fPIE
KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING
+KBUILD_CFLAGS += -DNO_DYNAMIC_DEBUG_TABLE
cflags-$(CONFIG_X86_32) := -march=i386
cflags-$(CONFIG_X86_64) := -mcmodel=small -mno-red-zone
KBUILD_CFLAGS += $(cflags-y)
diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile
index 05c4abc2fdfd..619878f2c427 100644
--- a/arch/x86/entry/vdso/Makefile
+++ b/arch/x86/entry/vdso/Makefile
@@ -29,6 +29,9 @@ vobjs32-y := vdso32/note.o vdso32/system_call.o vdso32/sigreturn.o
vobjs32-y += vdso32/vclock_gettime.o
vobjs-$(CONFIG_X86_SGX) += vsgx.o
+# avoid a x86_64_RELATIVE error
+KBUILD_CFLAGS += -DNO_DYNAMIC_DEBUG_TABLE
+
# files to link into kernel
obj-y += vma.o extable.o
KASAN_SANITIZE_vma.o := y
diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
index 95ea17a9d20c..95ba7b18410f 100644
--- a/arch/x86/purgatory/Makefile
+++ b/arch/x86/purgatory/Makefile
@@ -35,6 +35,7 @@ PURGATORY_CFLAGS_REMOVE := -mcmodel=kernel
PURGATORY_CFLAGS := -mcmodel=large -ffreestanding -fno-zero-initialized-in-bss -g0
PURGATORY_CFLAGS += $(DISABLE_STACKLEAK_PLUGIN) -DDISABLE_BRANCH_PROFILING
PURGATORY_CFLAGS += -fno-stack-protector
+PURGATORY_CFLAGS += -DNO_DYNAMIC_DEBUG_TABLE
# Default KBUILD_CFLAGS can have -pg option set when FTRACE is enabled. That
# in turn leaves some undefined symbols like __fentry__ in purgatory and not
diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index c23466e05e60..def8febefbd3 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -13,7 +13,8 @@ cflags-$(CONFIG_X86) += -m$(BITS) -D__KERNEL__ \
-Wno-pointer-sign \
$(call cc-disable-warning, address-of-packed-member) \
$(call cc-disable-warning, gnu) \
- -fno-asynchronous-unwind-tables
+ -fno-asynchronous-unwind-tables \
+ -DNO_DYNAMIC_DEBUG_TABLE
# arm64 uses the full KBUILD_CFLAGS so it's necessary to explicitly
# disable the stackleak plugin
--
2.31.1
we have: dp->site doing mapping: &__dyndbgs[N] -> &__dyndbg_sites[N].
We want to drop site, and do an indirect "up-over-down" maneuver (or
"back-over-and-map") to access sites[N]; this new _ddebug._index field
is that "N" above. This is a debug/unlikely path, so access cost is
unimportant compared to memory savings.
Once we can go from any N to 0, we have &array, and can use it in
container_of(), along with array-member name, to get the containing
struct (which doesnt exist yet).
The ._index is free, from unused padding in struct _ddebug (at least
on 64 bit machines). This commit just adds the ._index member, and
initializes it. That job falls to ddebug_add_module(). 2 cases:
loadable modules: ddebug_add_module( &whole-elf-section )
builtin modules: ddebug_add_module( &slice-of-DATA ), iteratively.
Previously, it didnt care which slice of "__dyndbgs" it was working,
but now we need to know the origin of the allocated memory, to
initialize our way back to it. Simplest way is to pass in the base
index of the current slice.
Since ddebug_add_module() is used indirectly by module.c, the new arg
is hidden in __ddebug_add_module(), and defaults to 0 in the wrapper.
Signed-off-by: Jim Cromie <[email protected]>
---
include/linux/dynamic_debug.h | 2 ++
lib/dynamic_debug.c | 25 ++++++++++++++++++++-----
2 files changed, 22 insertions(+), 5 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index f789608ab935..c866b4a9760a 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -29,6 +29,7 @@ struct _ddebug {
/* format is always needed, lineno shares word with flags */
const char *format;
const unsigned lineno:18;
+ unsigned _index:14;
/*
* The flags field controls the behaviour at the callsite.
* The bits here are changed dynamically when the user
@@ -52,6 +53,7 @@ struct _ddebug {
#define _DPRINTK_FLAGS_DEFAULT 0
#endif
unsigned int flags:8;
+
#ifdef CONFIG_JUMP_LABEL
union {
struct static_key_true dd_key_true;
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index db7563a88d8a..ce134c939f01 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1028,10 +1028,12 @@ static const struct proc_ops proc_fops = {
* Allocate a new ddebug_table for the given module
* and add it to the global list.
*/
-int ddebug_add_module(struct _ddebug *tab, struct _ddebug_site *sites,
- unsigned int numdbgs, const char *modname)
+static int __ddebug_add_module(struct _ddebug *tab, struct _ddebug_site *sites,
+ unsigned int numdbgs, unsigned int base,
+ const char *modname)
{
struct ddebug_table *dt;
+ int i;
v3pr_info("add-module: %s.%d sites\n", modname, numdbgs);
if (!numdbgs) {
@@ -1055,6 +1057,12 @@ int ddebug_add_module(struct _ddebug *tab, struct _ddebug_site *sites,
dt->ddebugs = tab;
dt->sites = sites;
+ for (i = 0; i < numdbgs; i++) {
+ tab[i]._index = base + i;
+ v3pr_info(" %d %d %s.%s.%d\n", i, tab[i]._index,
+ modname, sites[i].function, tab[i].lineno);
+ }
+
mutex_lock(&ddebug_lock);
list_add(&dt->link, &ddebug_tables);
mutex_unlock(&ddebug_lock);
@@ -1063,6 +1071,12 @@ int ddebug_add_module(struct _ddebug *tab, struct _ddebug_site *sites,
return 0;
}
+int ddebug_add_module(struct _ddebug *tab, struct _ddebug_site *sites,
+ unsigned int numdbgs, const char *modname)
+{
+ return __ddebug_add_module(tab, sites, numdbgs, 0, modname);
+}
+
/* helper for ddebug_dyndbg_(boot|module)_param_cb */
static int ddebug_dyndbg_param_cb(char *param, char *val,
const char *modname, int on_err)
@@ -1177,6 +1191,7 @@ static int __init dynamic_debug_init(void)
char *cmdline;
int ret = 0;
int site_ct = 0, entries = 0, modct = 0;
+ int mod_index = 0;
if (&__start___dyndbg == &__stop___dyndbg) {
if (IS_ENABLED(CONFIG_DYNAMIC_DEBUG)) {
@@ -1200,8 +1215,8 @@ static int __init dynamic_debug_init(void)
if (strcmp(modname, site->modname)) {
modct++;
- ret = ddebug_add_module(iter_mod_start, site_mod_start,
- site_ct, modname);
+ ret = __ddebug_add_module(iter_mod_start, site_mod_start,
+ site_ct, mod_index, modname);
if (ret)
goto out_err;
site_ct = 0;
@@ -1211,7 +1226,7 @@ static int __init dynamic_debug_init(void)
}
site_ct++;
}
- ret = ddebug_add_module(iter_mod_start, site_mod_start, site_ct, modname);
+ ret = __ddebug_add_module(iter_mod_start, site_mod_start, site_ct, mod_index, modname);
if (ret)
goto out_err;
--
2.31.1
DEFINE_DYNAMIC_DEBUG_TABLE is based on DEFINE_DYNAMIC_DEBUG_METADATA.
Like its model, it creates/allocates a pair of structs: _ddebug &
_ddebug_site. It inits them distinctively, so is_dyndbg_header_pair()
macro can verify them.
Its purpose is to reserve a single pair of header records in the front
of the "__dyndbgs" & "__dyndbg_sites" sections. This has several parts;
- the pair of structs are defined in 2 .gnu.linkonce.dyndbg* sections
corresponding to the 2 dyndbg* sections populated by _METADATA
- vmlinux.lds.h places these 2 new sections into the dyndbg* sections,
right after the start___dyndbg*s, before existing contents.
- the pair has "__used __weak" to qualm compiler concerns.
explicit externs were problematic with initialization, static decls too.
With this, the header record is now really __dyndbg[0].
Notes:
DYNAMIC_DEBUG is a 'transparent' facility, in that pr_debug() users
get extra features without additional api. Because of this,
DEFINE_DYNAMIC_DEBUG_TABLE is invoked by dynamic_debug.h on behalf of
all its (indirect) users, including printk.h.
IOW this has wide effects; it resulted in multiple redundant
definitions of header records. By hunch then trial and error, using
.gnu.linkonce. sections and "__used __weak " modifiers resolved the
problems. RFC.
In vmlinux-lds.h, I added 2 more KEEPs to place the .gnu.linkonce.*
headers at the front of their respective __dyndbg* sections. I moved
the __dyndbg lines into a new macro, which maybe should be done as a
separate commit, or in the earlier commit that touched it.
The headers are really just struct _ddebug*s, they are initialized
distinctively so that they're recogizable by code, using macro
is_dyndbg_header_pair(dbg, dbgsite).
DECLARE_DYNAMIC_DEBUG_TABLE() initializes the header record pairs with
values which are "impossible" for pr_debug()s: (and therefor distinctive)
- lineno == 0
- site == iter->site
- modname == function not possible in proper linkage
- modname == format not possible in normal linkage
- filename = (void*) iter forced loopback
Next:
Get __dyndbg[0] from any *dp within __dyndbg[N]. Then with
__dyndbg[0].site, we can get __dyndbg_sites[N]. This is a little
slower than a direct pointer, but this is an unlikely debug path, so
this 'up-N-over-down-N' access is ok.
Eventually we can adapt the header (as a new struct/union) to keep its
pointer to the __dyndbg_sites[] section/block, while allowing us to
drop it from struct _ddebug, regaining memory parity with master. But
for now, we keep .site, and will soon use it to validate the
'up-N-over-down-N' computation.
For clarity, N is _ddebug._index. For both builtins & loadable
modules, it is init'd to remember the fixed offset from the beginning
of the section/block/memory-allocation (actual elf sections for *ko's,
and a __start/__stop delineated part of .data for builtins).
N/_index will be used solely to get to __debugs[0] and over to
__debug_sites[N]. It is distinct from a module's numdbgs, which is
used mainly when walking control, and is kept in struct _ddeug_table.
Signed-off-by: Jim Cromie <[email protected]>
---
include/asm-generic/vmlinux.lds.h | 2 ++
include/linux/dynamic_debug.h | 45 +++++++++++++++++++++++++++++++
lib/dynamic_debug.c | 18 ++++++-------
3 files changed, 56 insertions(+), 9 deletions(-)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index f4e861282ed7..11a194fe3cd9 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -340,9 +340,11 @@
#define DYNAMIC_DEBUG_DATA() \
. = ALIGN(8); \
__start___dyndbg_sites = .; \
+ KEEP(*(.gnu.linkonce.dyndbg_site)) \
KEEP(*(__dyndbg_sites)) \
__stop___dyndbg_sites = .; \
__start___dyndbg = .; \
+ KEEP(*(.gnu.linkonce.dyndbg)) \
KEEP(*(__dyndbgs)) \
__stop___dyndbg = .;
#else
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index c866b4a9760a..a99973221f94 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -113,6 +113,43 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
_DPRINTK_KEY_INIT \
}
+/*
+ * DEFINE_DYNAMIC_DEBUG_TABLE(): This is a special version of
+ * DEFINE_DYNAMIC_DEBUG_METADATA(). It creates a single pair of
+ * header records, which linker scripts place into __dyndbg[0] &
+ * __dyndbg_sites[0].
+ * To allow a robust runtime test for is_dyndbg_header_pair(I,S),
+ * force-initialize S->filename to point back to I, an otherwize
+ * pathological condition.
+ */
+#define DEFINE_DYNAMIC_DEBUG_TABLE() \
+ /* forward decl, allowing loopback pointer */ \
+ __weak struct _ddebug __used __aligned(8) \
+ __section(".gnu.linkonce.dyndbg") \
+ _LINKONCE_dyndbg_header; \
+ __weak struct _ddebug_site __used __aligned(8) \
+ __section(".gnu.linkonce.dyndbg_site") \
+ _LINKONCE_dyndbg_site_header = { \
+ .modname = KBUILD_MODNAME, \
+ .function = KBUILD_MODNAME, \
+ /* forced pointer loopback, for distinction */ \
+ .filename = (void *) &_LINKONCE_dyndbg_header \
+ }; \
+ __weak struct _ddebug __used __aligned(8) \
+ __section(".gnu.linkonce.dyndbg") \
+ _LINKONCE_dyndbg_header = { \
+ .site = &_LINKONCE_dyndbg_site_header, \
+ .format = KBUILD_MODNAME \
+ }
+
+/* The header initializations as a distinguishing predicate */
+#define is_dyndbg_header_pair(iter, sitep) \
+ (sitep == iter->site \
+ && (!iter->lineno) \
+ && (iter->format == sitep->modname) \
+ && (sitep->modname == sitep->function) \
+ && ((void *)sitep->filename == (void *)iter))
+
#ifdef CONFIG_JUMP_LABEL
#ifdef DEBUG
@@ -243,4 +280,12 @@ static inline int dynamic_debug_exec_queries(const char *query, const char *modn
#endif /* !CONFIG_DYNAMIC_DEBUG_CORE */
+#if ((defined(CONFIG_DYNAMIC_DEBUG) || \
+ (defined(CONFIG_DYNAMIC_DEBUG_CORE) && defined(DYNAMIC_DEBUG_MODULE))) \
+ && defined(KBUILD_MODNAME) && !defined(NO_DYNAMIC_DEBUG_TABLE))
+
+/* transparently invoked, except when -DNO_DYNAMIC_DEBUG_TABLE */
+DEFINE_DYNAMIC_DEBUG_TABLE();
+#endif
+
#endif
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index ce134c939f01..e4a22f7b153f 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1190,8 +1190,7 @@ static int __init dynamic_debug_init(void)
const char *modname = NULL;
char *cmdline;
int ret = 0;
- int site_ct = 0, entries = 0, modct = 0;
- int mod_index = 0;
+ int i, site_ct = 0, modct = 0, mod_index = 0;
if (&__start___dyndbg == &__stop___dyndbg) {
if (IS_ENABLED(CONFIG_DYNAMIC_DEBUG)) {
@@ -1207,10 +1206,9 @@ static int __init dynamic_debug_init(void)
site = site_mod_start = __start___dyndbg_sites;
modname = site->modname;
- for (; iter < __stop___dyndbg; iter++, site++) {
+ for (i = 0; iter < __stop___dyndbg; iter++, site++, i++) {
- BUG_ON(site != iter->site);
- entries++;
+ WARN_ON(site != iter->site);
if (strcmp(modname, site->modname)) {
modct++;
@@ -1219,10 +1217,12 @@ static int __init dynamic_debug_init(void)
site_ct, mod_index, modname);
if (ret)
goto out_err;
- site_ct = 0;
+
modname = site->modname;
iter_mod_start = iter;
site_mod_start = site;
+ mod_index += site_ct;
+ site_ct = 0;
}
site_ct++;
}
@@ -1232,9 +1232,9 @@ static int __init dynamic_debug_init(void)
ddebug_init_success = 1;
vpr_info("%d prdebugs in %d modules, %d KiB in ddebug tables, %d KiB in __dyndbg section, %d KiB in __dyndbg_sites section\n",
- entries, modct, (int)((modct * sizeof(struct ddebug_table)) >> 10),
- (int)((entries * sizeof(struct _ddebug)) >> 10),
- (int)((entries * sizeof(struct _ddebug_site)) >> 10));
+ i, modct, (int)((modct * sizeof(struct ddebug_table)) >> 10),
+ (int)((i * sizeof(struct _ddebug)) >> 10),
+ (int)((i * sizeof(struct _ddebug_site)) >> 10));
/* apply ddebug_query boot param, dont unload tables on err */
if (ddebug_setup_string[0] != '\0') {
--
2.31.1
Up until now, loadable modules have tacitly linked the 2 __dyndbg*
sections into the .ko, as observable in log by enabling module.c's
pr_debugs and loading a module.
But now, we need to explicitly compose our named output sections,
placing the header sections in front of their respective __dyndbg*
input sections. We reuse input section names for the output.
This gives us the placement we need for the header record, which we
can see in the "add-module:"s and elements "0 0" below:
"0 0" lines are headers: predicate (function==module && !lineno)
"X debug prints in" are 1 too high, they count headers.
we are adding tables for empty modules (1st 2 below)
[ 7.578873] dyndbg: add-module: ghash_clmulni_intel
[ 7.579716] dyndbg: 0 0 ghash_clmulni_intel.ghash_clmulni_intel.0
[ 7.608995] dyndbg: 1 debug prints in module ghash_clmulni_intel
[ 8.078085] dyndbg: add-module: rapl
[ 8.078977] dyndbg: 0 0 rapl.rapl.0
[ 8.079584] dyndbg: 1 debug prints in module rapl
[ 8.082009] RAPL PMU: API unit is 2^-32 Joules, 0 fixed counters, 10737418240 ms ovfl timer
[ 8.099294] dyndbg: add-module: intel_rapl_common
[ 8.100177] dyndbg: 0 0 intel_rapl_common.intel_rapl_common.0
[ 8.101026] dyndbg: 1 1 intel_rapl_common.rapl_remove_package.1279
[ 8.101931] dyndbg: 2 2 intel_rapl_common.rapl_detect_domains.1245
[ 8.102836] dyndbg: 3 3 intel_rapl_common.rapl_detect_domains.1242
[ 8.103778] dyndbg: 4 4 intel_rapl_common.rapl_package_register_powercap.1159
[ 8.104960] dyndbg: 5 5 intel_rapl_common.rapl_package_register_powercap.1145
[ 8.106246] dyndbg: 6 6 intel_rapl_common.rapl_package_register_powercap.1114
[ 8.107302] dyndbg: 7 7 intel_rapl_common.rapl_package_register_powercap.1108
[ 8.108338] dyndbg: 8 8 intel_rapl_common.rapl_update_domain_data.1083
[ 8.109278] dyndbg: 9 9 intel_rapl_common.rapl_check_unit_atom.824
[ 8.110255] dyndbg: 10 10 intel_rapl_common.rapl_check_unit_core.796
[ 8.111361] dyndbg: 11 11 intel_rapl_common.rapl_read_data_raw.722
[ 8.112301] dyndbg: 12 12 intel_rapl_common.contraint_to_pl.303
[ 8.113276] dyndbg: 13 debug prints in module intel_rapl_common
[ 8.130452] dyndbg: add-module: intel_rapl_msr
[ 8.131140] dyndbg: 0 0 intel_rapl_msr.intel_rapl_msr.0
[ 8.132026] dyndbg: 1 1 intel_rapl_msr.rapl_msr_probe.172
[ 8.132818] dyndbg: 2 2 intel_rapl_msr.rapl_msr_read_raw.104
[ 8.133794] dyndbg: 3 debug prints in module intel_rapl_msr
This gives us the property we need:
fixed offset from &__dyndbg[N] to &__dyndbg[0]
container_of gets &header
header has ptr to __dyndbg_sites[]
we can (in principle) drop __dyndbg.site ptr
(after we adapt header to keep it)
TODO:
At this point, for loaded modules, ddebug_add_module() sees the header
as 0'th element, as we need in order to drop site (and regain worst
case footprint parity).
It could/should properly init this header to support the _sites[n]
lookup for loaded mods. Or at least handle it explicitly. Or at
least see what proc-show does with it currently.
Note that for builtins, decided by (__start <= dp < __stop), we use
__start___dyndbg_sites[N] directly, and dont need the header.
But maybe we should use it anyway, double-checking/BUGing when wrong.
Signed-off-by: Jim Cromie <[email protected]>
---
include/asm-generic/module.lds.h | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/include/asm-generic/module.lds.h b/include/asm-generic/module.lds.h
index f210d5c1b78b..70e744c953f5 100644
--- a/include/asm-generic/module.lds.h
+++ b/include/asm-generic/module.lds.h
@@ -4,7 +4,14 @@
/*
* <asm/module.lds.h> can specify arch-specific sections for linking modules.
- * Empty for the asm-generic header.
+ *
+ * Normally, sections get thru tacitly, but for CONFIG_DYNAMIC_DEBUG,
+ * we need to insert the .gnu.linkonce sections into the output
+ * sections, so we have to say everything explicitly.
*/
+SECTIONS {
+__dyndbg_sites : ALIGN(8) { *(.gnu.linkonce.dyndbg_site) *(__dyndbg_sites) }
+__dyndbgs : ALIGN(8) { *(.gnu.linkonce.dyndbg) *(__dyndbgs) }
+}
#endif /* __ASM_GENERIC_MODULE_LDS_H */
--
2.31.1
Now that header records are in the __dyndbg* sections,
ddebug_add_module() sees them when they're present (when adding
loadable modules and the 1st builtin, but not 2nd..Nth). Teach
ddebug_add_module() to recognize and account for them.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index e4a22f7b153f..ad9971ded09a 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1036,7 +1036,18 @@ static int __ddebug_add_module(struct _ddebug *tab, struct _ddebug_site *sites,
int i;
v3pr_info("add-module: %s.%d sites\n", modname, numdbgs);
- if (!numdbgs) {
+
+ if (numdbgs && is_dyndbg_header_pair(tab, sites)) {
+
+ v3pr_info(" header: %d %s.%s.%d\n", tab[0]._index, modname,
+ tab[0].site->function, tab[0].lineno);
+ WARN_ON(tab[0].site != &sites[0]);
+ if (numdbgs <= 1) {
+ v3pr_info(" skip header %s\n", modname);
+ return 0;
+ }
+
+ } else if (!numdbgs) {
v3pr_info(" skip %s\n", modname);
return 0;
}
--
2.31.1
header record pairs are special in that the filename member no longer
points at string data, but back at the data structure.
Theres no reason to expose this in control file, and no reason to even
print a line for the header, since its not part of the interface.
Printing a "# header" line is a decent alternative, but separate.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index a7840584b402..9a089b8d2ccd 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -995,7 +995,9 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
dc = ddebug_site_get(dp);
- if (dc) {
+ if (dc && is_dyndbg_header_pair(dp, dc)) {
+ /* skip output on header */
+ } else if (dc) {
seq_printf(m, "%s:%u [%s]%s =%s \"",
trim_prefix(dc->filename), dp->lineno,
iter->table->mod_name, dc->function,
--
2.31.1
Hide declaration and uses of _ddebug.site pointer in SITE_* macros,
defined alternately for SITE_CHK true/undefined.
Mostly, this eliminates all the WARN_ONs in ddebug_site_get(),
which seem to be mostly proven (they were BUG_ONs).
NOTE:
This breaks when !defined SITE_CHK - the header, being just a struct
_ddebug, also loses the .site member. Need to specialize the header
struct to keep it, then unionize it back into the sites vector.
In DEFINE_DYNAMIC_DEBUG_TABLE, we keep the raw initialization;
we want this init when specialization & unionization is finished.
Signed-off-by: Jim Cromie <[email protected]>
---
include/linux/dynamic_debug.h | 18 +++++++++++++++---
lib/dynamic_debug.c | 13 ++++++++++---
2 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index a99973221f94..7acd4c88411d 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -6,6 +6,18 @@
#include <linux/jump_label.h>
#endif
+#define SITE_CHK /* include .site pointer and checks on it */
+
+#ifdef SITE_CHK
+#define SITE_CHK_(...) __VA_ARGS__
+#define SITE_COND(_i, _s) (_s == _i->site)
+#define SITE_INIT_(_name) .site = &_name,
+#else
+#define SITE_CHK_(...)
+#define SITE_COND(_i, _s) (true)
+#define SITE_INIT_(name)
+#endif
+
/*
* A pair of these structs are created in 2 special ELF sections
* (__dyndbg, __dyndbg_sites) for every dynamic debug callsite.
@@ -25,7 +37,7 @@ struct _ddebug_site {
} __aligned(8);
struct _ddebug {
- struct _ddebug_site *site;
+ SITE_CHK_(struct _ddebug_site *site;)
/* format is always needed, lineno shares word with flags */
const char *format;
const unsigned lineno:18;
@@ -106,7 +118,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
}; \
static struct _ddebug __aligned(8) \
__section("__dyndbgs") name = { \
- .site = &name##_site, \
+ SITE_INIT_(name##_site) \
.format = (fmt), \
.lineno = __LINE__, \
.flags = _DPRINTK_FLAGS_DEFAULT, \
@@ -144,7 +156,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
/* The header initializations as a distinguishing predicate */
#define is_dyndbg_header_pair(iter, sitep) \
- (sitep == iter->site \
+ (SITE_COND(iter, sitep) \
&& (!iter->lineno) \
&& (iter->format == sitep->modname) \
&& (sitep->modname == sitep->function) \
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 9a089b8d2ccd..f333fcb6cf1f 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -151,7 +151,7 @@ static struct _ddebug_site *ddebug_site_get(struct _ddebug *dp)
struct _ddebug *dh = dp - (dp->_index);
WARN_ON(!is_dyndbg_header_pair(dh, dh->site));
-
+#ifdef SITE_CHK
if (dp >= __start___dyndbg && dp < __stop___dyndbg) {
v5pr_info("get: %s is builtin: %d %s:%s:%d\n",
@@ -177,6 +177,9 @@ static struct _ddebug_site *ddebug_site_get(struct _ddebug *dp)
WARN_ON(&dh->site[dp->_index] != dp->site);
return dp->site;
+#else
+ return &dh->site[dp->_index];
+#endif
}
static inline void ddebug_site_put(struct _ddebug *dp)
{
@@ -241,6 +244,7 @@ static void ddebug_alter_site(struct _ddebug *dp,
} else if (modifiers->flags & _DPRINTK_FLAGS_PRINT)
static_branch_enable(&dp->key.dd_key_true);
#endif
+#ifdef SITE_CHK
/* delete site info for this callsite */
if (modifiers->flags & _DPRINTK_FLAGS_DELETE_SITE) {
if (dp->site) {
@@ -249,6 +253,7 @@ static void ddebug_alter_site(struct _ddebug *dp,
dp->site = NULL;
}
}
+#endif
}
/*
@@ -1073,7 +1078,9 @@ static int __ddebug_add_module(struct _ddebug *tab, struct _ddebug_site *sites,
v3pr_info(" header: %d %s.%s.%d\n", tab[0]._index, modname,
tab[0].site->function, tab[0].lineno);
- WARN_ON(tab[0].site != &sites[0]);
+
+ SITE_CHK_(BUG_ON(tab[0].site != &sites[0]));
+
if (numdbgs <= 1) {
v3pr_info(" skip header %s\n", modname);
return 0;
@@ -1251,7 +1258,7 @@ static int __init dynamic_debug_init(void)
for (i = 0; iter < __stop___dyndbg; iter++, site++, i++) {
- WARN_ON(site != iter->site);
+ SITE_CHK_(WARN_ON(site != iter->site));
if (strcmp(modname, site->modname)) {
modct++;
--
2.31.1
fix a null-ptr-deref when .site info is deleted.
$> echo +D > /proc/dynamic_debug/control
This protects the header.site pointer from zeroing; we need it for all
the SITE_CHK sanity checking. Probably should protect against
toggling the static_key too (in the same function), but this smaller
change fixes the crash.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 659e864837ad..f2f6f2b20d01 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -247,7 +247,7 @@ static void ddebug_alter_site(struct _ddebug *dp,
#ifdef SITE_CHK
/* delete site info for this callsite */
if (modifiers->flags & _DPRINTK_FLAGS_DELETE_SITE) {
- if (dp->site) {
+ if (dp->site && !is_dyndbg_header_pair(dp, dp->site)) {
vpr_info("dropping site info %s.%s.%d\n", dp->site->filename,
dp->site->function, dp->lineno);
dp->site = NULL;
--
2.31.1
Flip the switch on #define SITE_CHK. This finally removes the
_ddebug.site pointer, and almost all uses of it. Due to this, the
.site cross-link done by DEFINE_DYNAMIC_DEBUG_METADATA() is lost to
the compiler, so suppress the resulting warnings by adding a "__used"
to the _ddebug_site records initialized there.
In .c, use the union header types introduced in ~HEAD~1,
with some casts sprinkled in.
This seems to work, but should be cleaner. RFC
Signed-off-by: Jim Cromie <[email protected]>
---
include/linux/dynamic_debug.h | 4 ++--
lib/dynamic_debug.c | 16 ++++++++--------
2 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index c9c568a698a8..b2189ff94fda 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -6,7 +6,7 @@
#include <linux/jump_label.h>
#endif
-#define SITE_CHK /* include .site pointer and checks on it */
+// #define SITE_CHK /* include .site pointer and checks on it */
#ifdef SITE_CHK
#define SITE_CHK_(...) __VA_ARGS__
@@ -135,7 +135,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
const char *fmt, ...);
#define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt) \
- static struct _ddebug_site __aligned(8) \
+ static struct _ddebug_site __aligned(8) __used \
__section("__dyndbg_sites") name##_site = { \
.modname = KBUILD_MODNAME, \
.filename = __FILE__, \
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index f2f6f2b20d01..0e2e082b487b 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -148,9 +148,11 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
static struct _ddebug_site *ddebug_site_get(struct _ddebug *dp)
{
- struct _ddebug *dh = dp - (dp->_index);
+ union _ddebug_header *dh = (union _ddebug_header *) (dp - dp->_index);
+
+ if (!is_dyndbg_header_pair(dh, dh->site))
+ v3pr_info("get: header fail on %d-%d\n", dp->_index, dp->lineno);
- BUG_ON(!is_dyndbg_header_pair(dh, dh->site));
#ifdef SITE_CHK
if (dp >= __start___dyndbg && dp < __stop___dyndbg) {
@@ -178,7 +180,7 @@ static struct _ddebug_site *ddebug_site_get(struct _ddebug *dp)
return dp->site;
#else
- return &dh->site[dp->_index];
+ return (struct _ddebug_site *) &dh->site[dp->_index];
#endif
}
static inline void ddebug_site_put(struct _ddebug *dp)
@@ -1070,16 +1072,14 @@ static int __ddebug_add_module(struct _ddebug *tab, struct _ddebug_site *sites,
const char *modname)
{
struct ddebug_table *dt;
+ union _ddebug_header *dh = (union _ddebug_header *) &tab[0];
int i;
v3pr_info("add-module: %s.%d sites\n", modname, numdbgs);
- if (numdbgs && is_dyndbg_header_pair(tab, sites)) {
-
- v3pr_info(" header: %d %s.%s.%d\n", tab[0]._index, modname,
- tab[0].site->function, tab[0].lineno);
+ if (numdbgs && is_dyndbg_header_pair(dh, sites)) {
- SITE_CHK_(BUG_ON(tab[0].site != &sites[0]));
+ BUG_ON((void *) &dh->site[0] != (void *) &sites[0]);
if (numdbgs <= 1) {
v3pr_info(" skip header %s\n", modname);
--
2.31.1
Allow users & subsystems to selectively delete callsite info for
pr-debug callsites. Hopefully, this can lead to actual recovery of
memory.
DRM is a potential user which would drop the sites:
- has distinct categories for logging, and can map them over to a
format prefix, like: "drm:core:", "drm:kms:", etc.
- are happy with group control of all the callsites in a class/cateory.
individual control is still possible using queries including line numbers
- don't need dynamic "module:function:line:" prefixes in log messages
- don't care about loss of context in /proc/dynamic_debug/control
before:
init/initramfs.c:485 [initramfs]unpack_to_rootfs =_ "Detected %s compressed data\012"
init/main.c:1337 [main]run_init_process =pm " %s\012"
init/main.c:1335 [main]run_init_process =pm " with environment:\012"
init/main.c:1334 [main]run_init_process =pm " %s\012"
init/main.c:1332 [main]run_init_process =pm " with arguments:\012"
init/main.c:1121 [main]initcall_blacklisted =pm "initcall %s blacklisted\012"
init/main.c:1082 [main]initcall_blacklist =pm "blacklisting initcall %s\012"
then:
bash-5.0# echo file init/main.c +D > /proc/dynamic_debug/control
after:
init/initramfs.c:485 [initramfs]unpack_to_rootfs =_ "Detected %s compressed data\012"
[main]:1337 =pmD " %s\012"
[main]:1335 =pmD " with environment:\012"
[main]:1334 =pmD " %s\012"
[main]:1332 =pmD " with arguments:\012"
[main]:1121 =pmD "initcall %s blacklisted\012"
[main]:1082 =pmD "blacklisting initcall %s\012"
Notes:
If Drm adopted dyndbg, i915 + drm* would add ~1600 prdebugs, amdgpu +
drm* would add ~3200 callsites, so the additional memory costs are
substantial. In trade, drm and drivers would avoid lots of calls to
drm_debug_enabled(). This patch should reduce the costs.
Using this interface, drm could drop site info for all categories /
prefixes controlled by bits in drm.debug, while preserving site info
and individual selectivity for any uncategorized prdebugs, and for all
other modules.
Lastly, because lineno field was not moved into _ddebug_callsite, it
can be used to modify a single[*] callsite even if drm has dropped all
the callsite data:
echo module $mod format ^$prefix line $line +p >control
Dropping site info is a one-way, information losing operation, so
minor misuse is possible. Worst case is maybe (depending upon
previous settings) some loss of logging context/decorations.
echo +D > /proc/dynamic_debug/control
[*] amdgpu has some macros invoking clusters of pr_debugs; each use of
them creates a cluster of pr-debugs with the same line number.
Signed-off-by: Jim Cromie <[email protected]>
---
include/linux/dynamic_debug.h | 1 +
lib/dynamic_debug.c | 9 +++++++++
2 files changed, 10 insertions(+)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 40ea086853ff..f789608ab935 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -40,6 +40,7 @@ struct _ddebug {
#define _DPRINTK_FLAGS_INCL_FUNCNAME (1<<2)
#define _DPRINTK_FLAGS_INCL_LINENO (1<<3)
#define _DPRINTK_FLAGS_INCL_TID (1<<4)
+#define _DPRINTK_FLAGS_DELETE_SITE (1<<7) /* drop site info to save ram */
#define _DPRINTK_FLAGS_INCL_ANY \
(_DPRINTK_FLAGS_INCL_MODNAME | _DPRINTK_FLAGS_INCL_FUNCNAME |\
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index ad7cda840733..a4cb048357fb 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -93,6 +93,7 @@ static struct { unsigned flag:8; char opt_char; } opt_array[] = {
{ _DPRINTK_FLAGS_INCL_LINENO, 'l' },
{ _DPRINTK_FLAGS_INCL_TID, 't' },
{ _DPRINTK_FLAGS_NONE, '_' },
+ { _DPRINTK_FLAGS_DELETE_SITE, 'D' },
};
struct flagsbuf { char buf[ARRAY_SIZE(opt_array)+1]; };
@@ -202,6 +203,14 @@ static void ddebug_alter_site(struct _ddebug *dp,
} else if (modifiers->flags & _DPRINTK_FLAGS_PRINT)
static_branch_enable(&dp->key.dd_key_true);
#endif
+ /* delete site info for this callsite */
+ if (modifiers->flags & _DPRINTK_FLAGS_DELETE_SITE) {
+ if (dp->site) {
+ vpr_info("dropping site info %s.%s.%d\n", dp->site->filename,
+ dp->site->function, dp->lineno);
+ dp->site = NULL;
+ }
+ }
}
/*
--
2.31.1
HEAD~1 passed 0-day, Im not sure what it does with WARNs,
so make everything fatal, see what dies.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index f333fcb6cf1f..659e864837ad 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -150,7 +150,7 @@ static struct _ddebug_site *ddebug_site_get(struct _ddebug *dp)
{
struct _ddebug *dh = dp - (dp->_index);
- WARN_ON(!is_dyndbg_header_pair(dh, dh->site));
+ BUG_ON(!is_dyndbg_header_pair(dh, dh->site));
#ifdef SITE_CHK
if (dp >= __start___dyndbg && dp < __stop___dyndbg) {
@@ -159,14 +159,14 @@ static struct _ddebug_site *ddebug_site_get(struct _ddebug *dp)
dh->site[dp->_index].filename,
dh->site[dp->_index].function, dp->lineno);
- WARN_ON(dp != &__start___dyndbg[dp->_index]);
+ BUG_ON(dp != &__start___dyndbg[dp->_index]);
- WARN_ON(!(dp->_index == (dp - dh) &&
+ BUG_ON(!(dp->_index == (dp - dh) &&
dp->_index == (dp - __start___dyndbg) &&
dp->_index == (&__start___dyndbg_sites[dp->_index]
- &__start___dyndbg_sites[0])));
if (dp->site)
- WARN_ON(&__start___dyndbg_sites[dp->_index] != dp->site);
+ BUG_ON(&__start___dyndbg_sites[dp->_index] != dp->site);
} else {
v4pr_info("get: %s is loaded: %d %s:%s:%d\n",
dh->site[dp->_index].modname, dp->_index,
@@ -174,7 +174,7 @@ static struct _ddebug_site *ddebug_site_get(struct _ddebug *dp)
dh->site[dp->_index].function, dp->lineno);
}
if (dp->site)
- WARN_ON(&dh->site[dp->_index] != dp->site);
+ BUG_ON(&dh->site[dp->_index] != dp->site);
return dp->site;
#else
@@ -1258,7 +1258,7 @@ static int __init dynamic_debug_init(void)
for (i = 0; iter < __stop___dyndbg; iter++, site++, i++) {
- SITE_CHK_(WARN_ON(site != iter->site));
+ SITE_CHK_(BUG_ON(site != iter->site));
if (strcmp(modname, site->modname)) {
modct++;
--
2.31.1
This commit adds several BUG_ONs to assert all the invariants needed
to support the reliance on the "back-N-to-header-overto-site-out-N"
use of the 2 __dyndbg* vectors (with their included headers).
RFC: I don't think we want this permanently; BUG_ON/panic seems kinda
overkill, but its useful to know if it survives lkp auto-testing.
- dp is (struct _ddebug*) to the callsite, passed in.
- dh is (struct _ddebug*) to the header. derived from dp & _index.
known by BUG_ON(!is_dyndbg_header(dh))
this is the "up-N-to-header" from dp.
- dh has good site pointer, to __dyndbg_sites[]
by BUG_ON(!is_dyndbg_header_pair())
There are 2 main cases to validate: builtin and loadable modules.
For loadable modules, we will depend upon the headers presence, and
its site pointer to the vector of _ddebug_sites[], expressed as:
BUG_ON(&dh->site[dp->_index] != dp->site);
Builtin pr-debugs have the additional property:
!!(&__start___dyndbg <= dp < __stop___dyndbg),
We could use this property directly to return site, but since builtin
modules also have a header record, we can use that instead. So the
1st BUG_ON is hoisted out of the !builtin branch, and asserted just
before return. Also hoist dh derivation, making it a declaration +
initialization + BUG_ON instead.
NB: grep -- '->site' will confirm that site is now used just for
BUG_ON assertions, so we are close to the drop.
NEXT
To drop site pointer from struct _ddebug, we need:
- recast header as a different struct, unionized with _ddebug.
- preserve site pointer there.
- drop from struct _ddebug.
- fix and fold back any fallout from size reduction.
OR defer that, and proceed with compressing __dyndbg_sites[], then
replacing ddebug_site_get's guts (with all the BUG_ONs) with a
decompress and _index.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index ad9971ded09a..014e3a79d8e9 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -123,6 +123,8 @@ do { \
#define vpr_info(fmt, ...) vnpr_info(1, fmt, ##__VA_ARGS__)
#define v2pr_info(fmt, ...) vnpr_info(2, fmt, ##__VA_ARGS__)
#define v3pr_info(fmt, ...) vnpr_info(3, fmt, ##__VA_ARGS__)
+#define v4pr_info(fmt, ...) vnpr_info(4, fmt, ##__VA_ARGS__)
+#define v5pr_info(fmt, ...) vnpr_info(5, fmt, ##__VA_ARGS__)
static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
{
@@ -146,7 +148,34 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
static struct _ddebug_site *ddebug_site_get(struct _ddebug *dp)
{
- return dp->site; /* passthru abstraction */
+ struct _ddebug *dh = dp - (dp->_index);
+
+ WARN_ON(!is_dyndbg_header_pair(dh, dh->site));
+
+ if (dp >= __start___dyndbg && dp < __stop___dyndbg) {
+
+ v5pr_info("get: %s is builtin: %d %d %s:%s:%d\n",
+ dp->site->modname, dp->_index, (int)(dp - dh),
+ dh->site[dp->_index].filename,
+ dh->site[dp->_index].function, dp->lineno);
+
+ WARN_ON(dp != &__start___dyndbg[dp->_index]);
+
+ WARN_ON(!(dp->_index == (dp - dh) &&
+ dp->_index == (dp - __start___dyndbg) &&
+ dp->_index == (&__start___dyndbg_sites[dp->_index]
+ - &__start___dyndbg_sites[0])));
+
+ WARN_ON(&__start___dyndbg_sites[dp->_index] != dp->site);
+ } else {
+ v4pr_info("get: %s is loaded: %d %s:%s:%d\n",
+ dp->site->modname, dp->_index,
+ dh->site[dp->_index].filename,
+ dh->site[dp->_index].function, dp->lineno);
+ }
+ WARN_ON(&dh->site[dp->_index] != dp->site);
+
+ return dp->site;
}
static inline void ddebug_site_put(struct _ddebug *dp)
{
--
2.31.1
This extends HEAD~1 by packing not just into the each module's
sub-vector (slice of builtin dyndbg_sites[] vector), but into
the whole vector.
__ddebug_add_module() gets 2 new parameters to enable this; it
re-writes the packed_sites[] vector with each unique site record, and
tracks index of the last record written in packed_base.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 36 ++++++++++++++++++++++++------------
1 file changed, 24 insertions(+), 12 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 43f4c82d24c3..66b48f1cb2d0 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1069,13 +1069,16 @@ static const struct proc_ops proc_fops = {
*/
static int __ddebug_add_module(struct _ddebug *tab, struct _ddebug_site *sites,
unsigned int numdbgs, unsigned int base,
- const char *modname)
+ const char *modname,
+ struct _ddebug_site *packed_sites,
+ unsigned int *packed_base)
{
struct ddebug_table *dt;
union _ddebug_header *dh = (union _ddebug_header *) &tab[0];
int i, j;
- v3pr_info("add-module: %s.%d sites\n", modname, numdbgs);
+ v3pr_info("add-module: %s.%d sites, to %d->%d\n", modname, numdbgs,
+ base, *packed_base);
if (numdbgs && is_dyndbg_header_pair(dh, sites)) {
@@ -1116,12 +1119,13 @@ static int __ddebug_add_module(struct _ddebug *tab, struct _ddebug_site *sites,
for (i = j = 0; i < numdbgs; i++) {
tab[i]._back = base + i; /* index back to header */
- /* find 1st row with new fn, copy it to stack on j */
- if (sites[i].function != sites[j].function)
- memcpy((void *) &sites[++j], (void *) &sites[i],
- sizeof(struct _ddebug_site));
-
- tab[i]._map = base + j;
+ /* find 1st row with new fn, copy it to stack on packed_base */
+ if (sites[i].function != packed_sites[*packed_base].function) {
+ j++;
+ memcpy((void *) &packed_sites[++(*packed_base)],
+ (void *) &sites[i], sizeof(struct _ddebug_site));
+ }
+ tab[i]._map = *packed_base;
v3pr_info(" %d %d %d %d %s.%s.%d\n", i, j, tab[i]._back, tab[i]._map,
modname, sites[i].function, tab[i].lineno);
@@ -1140,7 +1144,10 @@ static int __ddebug_add_module(struct _ddebug *tab, struct _ddebug_site *sites,
int ddebug_add_module(struct _ddebug *tab, struct _ddebug_site *sites,
unsigned int numdbgs, const char *modname)
{
- return __ddebug_add_module(tab, sites, numdbgs, 0, modname);
+ unsigned int packed_base = 0; /* skip the header */
+
+ return __ddebug_add_module(tab, sites, numdbgs, 0, modname,
+ sites, &packed_base);
}
/* helper for ddebug_dyndbg_(boot|module)_param_cb */
@@ -1253,10 +1260,12 @@ static int __init dynamic_debug_init(void)
{
struct _ddebug *iter, *iter_mod_start;
struct _ddebug_site *site, *site_mod_start;
+
const char *modname = NULL;
char *cmdline;
int ret = 0;
int i, site_ct = 0, modct = 0, mod_index = 0;
+ unsigned int site_base;
if (&__start___dyndbg == &__stop___dyndbg) {
if (IS_ENABLED(CONFIG_DYNAMIC_DEBUG)) {
@@ -1272,7 +1281,7 @@ static int __init dynamic_debug_init(void)
site = site_mod_start = __start___dyndbg_sites;
modname = site->modname;
- for (i = 0; iter < __stop___dyndbg; iter++, site++, i++) {
+ for (site_base = i = 0; iter < __stop___dyndbg; iter++, site++, i++) {
SITE_CHK_(BUG_ON(site != iter->site));
@@ -1280,7 +1289,8 @@ static int __init dynamic_debug_init(void)
modct++;
ret = __ddebug_add_module(iter_mod_start, site_mod_start,
- site_ct, mod_index, modname);
+ site_ct, mod_index, modname,
+ __start___dyndbg_sites, &site_base);
if (ret)
goto out_err;
@@ -1292,7 +1302,9 @@ static int __init dynamic_debug_init(void)
}
site_ct++;
}
- ret = __ddebug_add_module(iter_mod_start, site_mod_start, site_ct, mod_index, modname);
+ ret = __ddebug_add_module(iter_mod_start, site_mod_start,
+ site_ct, mod_index, modname,
+ __start___dyndbg_sites, &site_base);
if (ret)
goto out_err;
--
2.31.1
In order to do:
__dyndbgs[-N].site->__dyndbg_sites[N]
._index had 2 jobs:
provide index back to __dyndbgs[0]
provide index out to __dyndbg_sites[N]
So rename ._index to ._back & ._map, adjust ddebug_add_module()
to init both fields identically, and replace other ._index's
with the "correct" replacement.
NOTES:
._map is a better name better than ._out. Since ._map is a
separate field, it can map arbitrarily. It now maps ddebugs->sites
1->1, but it can/will-soon map N->1; all a function's pr_debugs
will share a single _ddebug_site, and we can reclaim redundant rows.
The "correct" choice of ._map/._back cannot be tested properly now,
since ._map == ._back. Once N->1 is done, we can re-define SITE_CHK
and validate (or correct) the ._map/._back choices made here.
No functional changes.
Signed-off-by: Jim Cromie <[email protected]>
---
include/linux/dynamic_debug.h | 6 ++++--
lib/dynamic_debug.c | 35 ++++++++++++++++++-----------------
2 files changed, 22 insertions(+), 19 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index b2189ff94fda..fe70dda704d2 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -51,7 +51,8 @@ struct _ddebug {
/* format is always needed, lineno shares word with flags */
const char *format;
const unsigned lineno:18;
- unsigned _index:14;
+ unsigned _back:14;
+ unsigned _map:14;
/*
* The flags field controls the behaviour at the callsite.
* The bits here are changed dynamically when the user
@@ -95,7 +96,8 @@ union _ddebug_header {
union _ddebug_site_header *site;
const char *format;
const unsigned lineno:18;
- unsigned _index:14;
+ unsigned _back:14;
+ unsigned _map:14;
unsigned int flags:8;
};
};
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 0e2e082b487b..524303a04462 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -148,39 +148,39 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg)
static struct _ddebug_site *ddebug_site_get(struct _ddebug *dp)
{
- union _ddebug_header *dh = (union _ddebug_header *) (dp - dp->_index);
+ union _ddebug_header *dh = (union _ddebug_header *) (dp - dp->_back);
if (!is_dyndbg_header_pair(dh, dh->site))
- v3pr_info("get: header fail on %d-%d\n", dp->_index, dp->lineno);
+ v3pr_info("get: header fail on %d-%d\n", dp->_back, dp->lineno);
#ifdef SITE_CHK
if (dp >= __start___dyndbg && dp < __stop___dyndbg) {
v5pr_info("get: %s is builtin: %d %s:%s:%d\n",
- dh->site[dp->_index].modname, dp->_index,
- dh->site[dp->_index].filename,
- dh->site[dp->_index].function, dp->lineno);
+ dh->site[dp->_map].modname, dp->_back,
+ dh->site[dp->_map].filename,
+ dh->site[dp->_map].function, dp->lineno);
- BUG_ON(dp != &__start___dyndbg[dp->_index]);
+ BUG_ON(dp != &__start___dyndbg[dp->_back]);
- BUG_ON(!(dp->_index == (dp - dh) &&
- dp->_index == (dp - __start___dyndbg) &&
- dp->_index == (&__start___dyndbg_sites[dp->_index]
+ BUG_ON(!(dp->_back == (dp - dh) &&
+ dp->_back == (dp - __start___dyndbg) &&
+ dp->_back == (&__start___dyndbg_sites[dp->_back]
- &__start___dyndbg_sites[0])));
if (dp->site)
- BUG_ON(&__start___dyndbg_sites[dp->_index] != dp->site);
+ BUG_ON(&__start___dyndbg_sites[dp->_map] != dp->site);
} else {
v4pr_info("get: %s is loaded: %d %s:%s:%d\n",
- dh->site[dp->_index].modname, dp->_index,
- dh->site[dp->_index].filename,
- dh->site[dp->_index].function, dp->lineno);
+ dh->site[dp->_map].modname, dp->_back,
+ dh->site[dp->_map].filename,
+ dh->site[dp->_map].function, dp->lineno);
}
if (dp->site)
- BUG_ON(&dh->site[dp->_index] != dp->site);
+ BUG_ON(&dh->site[dp->_map] != dp->site);
return dp->site;
#else
- return (struct _ddebug_site *) &dh->site[dp->_index];
+ return (struct _ddebug_site *) &dh->site[dp->_map];
#endif
}
static inline void ddebug_site_put(struct _ddebug *dp)
@@ -1108,8 +1108,9 @@ static int __ddebug_add_module(struct _ddebug *tab, struct _ddebug_site *sites,
dt->sites = sites;
for (i = 0; i < numdbgs; i++) {
- tab[i]._index = base + i;
- v3pr_info(" %d %d %s.%s.%d\n", i, tab[i]._index,
+ tab[i]._back = base + i;
+ tab[i]._map = base + i;
+ v3pr_info(" %d %d %d %s.%s.%d\n", i, tab[i]._back, tab[i]._map,
modname, sites[i].function, tab[i].lineno);
}
--
2.31.1
After `echo module main +D > control` zeros the site pointer for
main's callsites, `cat control` causes a NULL deref in
ddebug_site_get(). Fix this with:
- in vpr_infos, avoid dp->site->module, use dh->sites[dp->_index]
- qualify WARN_ONs that test against dp->site.
Also return dp->site, which may be null. This restores the
abbreviated control output of deleted sites, rather than pretending it
wasnt deleted.
Deleting sites isn't an important feature, and its current form will
be obsolete when the site pointer gets dropped. Its also pointless if
the site data is in compressed blocks. But its still worthwhile to
maintain !site robustness for a bit.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 014e3a79d8e9..a7840584b402 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -154,8 +154,8 @@ static struct _ddebug_site *ddebug_site_get(struct _ddebug *dp)
if (dp >= __start___dyndbg && dp < __stop___dyndbg) {
- v5pr_info("get: %s is builtin: %d %d %s:%s:%d\n",
- dp->site->modname, dp->_index, (int)(dp - dh),
+ v5pr_info("get: %s is builtin: %d %s:%s:%d\n",
+ dh->site[dp->_index].modname, dp->_index,
dh->site[dp->_index].filename,
dh->site[dp->_index].function, dp->lineno);
@@ -165,15 +165,16 @@ static struct _ddebug_site *ddebug_site_get(struct _ddebug *dp)
dp->_index == (dp - __start___dyndbg) &&
dp->_index == (&__start___dyndbg_sites[dp->_index]
- &__start___dyndbg_sites[0])));
-
- WARN_ON(&__start___dyndbg_sites[dp->_index] != dp->site);
+ if (dp->site)
+ WARN_ON(&__start___dyndbg_sites[dp->_index] != dp->site);
} else {
v4pr_info("get: %s is loaded: %d %s:%s:%d\n",
- dp->site->modname, dp->_index,
+ dh->site[dp->_index].modname, dp->_index,
dh->site[dp->_index].filename,
dh->site[dp->_index].function, dp->lineno);
}
- WARN_ON(&dh->site[dp->_index] != dp->site);
+ if (dp->site)
+ WARN_ON(&dh->site[dp->_index] != dp->site);
return dp->site;
}
--
2.31.1
Up until now, to create our header record (pair), we reused struct
_ddebug(|_sites), initializing the pair with "impossible" field values
(for a pr_debug() site), which makes it recognizable as a header not a
pr-debug callsite.
Now we want to specialize the header; differentiate it from its model,
then we can drop _ddebug.site, and use header.site indirectly. This
is really just setup, next patch finishes conversion.
This commit has several elements combining:
Update DEFINE_DYNAMIC_DEBUG_TABLE by replacing "struct _ddebug*" with
"union _ddebug*_header". This change places the new object types into
our 2 elf sections.
our 2 new union types have 2 jobs:
a.fit properly in the section/array. We do this by putting the struct
inside the union, making the unions the same size as their contained
structs. This was sufficient to fix a data-misalignment crash on
2nd loop (ie 1st record after header) in _init.
TBD: u8[sizeof(struct _ddebug)] would also fit properly.
b.unions need the same field defns as their models, as refd in:
DEFINE_DYNAMIC_DEBUG_TABLE() - inits
is_dyndbg_header_pair(i,s) - validates that init
use anonymous struct.
RFC: My "anonymous union/struct fu" feels incomplete. The struct in b
must contain all the fields of its outer union's own contained struct,
maybe even in the same order (tho not obvious yet).
Signed-off-by: Jim Cromie <[email protected]>
---
include/linux/dynamic_debug.h | 31 ++++++++++++++++++++++++++++---
1 file changed, 28 insertions(+), 3 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 7acd4c88411d..c9c568a698a8 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -36,6 +36,16 @@ struct _ddebug_site {
const char *function;
} __aligned(8);
+/* verbatim copy of struct _ddebug_site. nothing to save here. */
+union _ddebug_site_header {
+ struct _ddebug_site __; /* inherit footprint */
+ struct { /* header validation */
+ const char *modname;
+ const char *filename;
+ const char *function;
+ };
+};
+
struct _ddebug {
SITE_CHK_(struct _ddebug_site *site;)
/* format is always needed, lineno shares word with flags */
@@ -74,6 +84,21 @@ struct _ddebug {
#endif
} __aligned(8);
+/*
+ * specialized version of struct _ddebug. The only field we NEED is
+ * .site, since keeping it here allows dropping from struct _ddebug
+ * itself. format, lineno are used to validate header records.
+ */
+union _ddebug_header {
+ struct _ddebug __; /* inherit footprint */
+ struct { /* header validation */
+ union _ddebug_site_header *site;
+ const char *format;
+ const unsigned lineno:18;
+ unsigned _index:14;
+ unsigned int flags:8;
+ };
+};
#if defined(CONFIG_DYNAMIC_DEBUG_CORE)
@@ -136,10 +161,10 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
*/
#define DEFINE_DYNAMIC_DEBUG_TABLE() \
/* forward decl, allowing loopback pointer */ \
- __weak struct _ddebug __used __aligned(8) \
+ __weak union _ddebug_header __used __aligned(8) \
__section(".gnu.linkonce.dyndbg") \
_LINKONCE_dyndbg_header; \
- __weak struct _ddebug_site __used __aligned(8) \
+ __weak union _ddebug_site_header __used __aligned(8) \
__section(".gnu.linkonce.dyndbg_site") \
_LINKONCE_dyndbg_site_header = { \
.modname = KBUILD_MODNAME, \
@@ -147,7 +172,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
/* forced pointer loopback, for distinction */ \
.filename = (void *) &_LINKONCE_dyndbg_header \
}; \
- __weak struct _ddebug __used __aligned(8) \
+ __weak union _ddebug_header __used __aligned(8) \
__section(".gnu.linkonce.dyndbg") \
_LINKONCE_dyndbg_header = { \
.site = &_LINKONCE_dyndbg_site_header, \
--
2.31.1
In ddebug_add_module, repack the module's sites[] sub-vector.
The previous commit detected 1st site for each function, and ._map'd
2nd-Nth repeats to the 1st. Instead, we now pack the sites[]
sub-vector, by treating it as a stack, and pushing each distinct
site-rec on top, then ._map's 2nd-Nth sites to that top-of-stack.
For loadable modules, this is enough to repack the __dyndbg_sites
elf section. If that section is separately kalloc'd, we could
probably just krealloc it to free the chunk past stack-top.
booting with dynamic_debug.verbose=3 will show:
- cols 3,4 row 1 - skip header
- cols 3,4 - full pack in module's slice of array
- fn count off by 1
rcu: Hierarchical SRCU implementation.
dyndbg: add-module: head64.1 sites
dyndbg: skip header head64
dyndbg: add-module: main.6 sites
Callback from call_rcu_tasks_trace() invoked.
dyndbg: 0 0 1 1 main.run_init_process.1358
dyndbg: 1 0 2 1 main.run_init_process.1356
dyndbg: 2 0 3 1 main.run_init_process.1355
dyndbg: 3 0 4 1 main.run_init_process.1353
dyndbg: 4 1 5 2 main.initcall_blacklisted.1142
dyndbg: 5 2 6 3 main.initcall_blacklist.1103
dyndbg: 6 debug prints in module main (in 2 functions)
dyndbg: add-module: initramfs.1 sites
dyndbg: 0 0 7 7 initramfs.unpack_to_rootfs.496
dyndbg: 1 debug prints in module initramfs (in 0 functions)
dyndbg: add-module: ibs.3 sites
dyndbg: 0 0 8 8 ibs.force_ibs_eilvt_setup.926
dyndbg: 1 1 9 9 ibs.setup_ibs_ctl.897
dyndbg: 2 1 10 9 ibs.setup_ibs_ctl.890
dyndbg: 3 debug prints in module ibs (in 1 functions)
for a loadable module:
- fn-ct looks good
- 1st-fn, packing looks good
dyndbg: add-module: intel_rapl_common.13 sites
dyndbg: 1 1 1 1 intel_rapl_common.rapl_remove_package.1279
dyndbg: 2 2 2 2 intel_rapl_common.rapl_detect_domains.1245
dyndbg: 3 2 3 2 intel_rapl_common.rapl_detect_domains.1242
dyndbg: 4 3 4 3 intel_rapl_common.rapl_package_register_powercap.1159
dyndbg: 5 3 5 3 intel_rapl_common.rapl_package_register_powercap.1145
dyndbg: 6 3 6 3 intel_rapl_common.rapl_package_register_powercap.1114
dyndbg: 7 3 7 3 intel_rapl_common.rapl_package_register_powercap.1108
dyndbg: 8 4 8 4 intel_rapl_common.rapl_update_domain_data.1083
dyndbg: 9 5 9 5 intel_rapl_common.rapl_check_unit_atom.824
dyndbg: 10 6 10 6 intel_rapl_common.rapl_check_unit_core.796
dyndbg: 11 7 11 7 intel_rapl_common.rapl_read_data_raw.722
dyndbg: 12 8 12 8 intel_rapl_common.contraint_to_pl.303
dyndbg: 13 debug prints in module intel_rapl_common (in 8 functions)
dyndbg: add-module: intel_rapl_msr.3 sites
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index e12538640b51..43f4c82d24c3 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1073,7 +1073,7 @@ static int __ddebug_add_module(struct _ddebug *tab, struct _ddebug_site *sites,
{
struct ddebug_table *dt;
union _ddebug_header *dh = (union _ddebug_header *) &tab[0];
- int i, j, ct;
+ int i, j;
v3pr_info("add-module: %s.%d sites\n", modname, numdbgs);
@@ -1113,13 +1113,14 @@ static int __ddebug_add_module(struct _ddebug *tab, struct _ddebug_site *sites,
* we watch for repeats on site.function, and reuse the index
* of the 1st site record in a repeating block.
*/
- for (ct = i = j = 0; i < numdbgs; i++) {
+ for (i = j = 0; i < numdbgs; i++) {
tab[i]._back = base + i; /* index back to header */
- if (sites[i].function != sites[j].function) {
- j = i; /* remember 1st row with new fn */
- ct++;
- }
+ /* find 1st row with new fn, copy it to stack on j */
+ if (sites[i].function != sites[j].function)
+ memcpy((void *) &sites[++j], (void *) &sites[i],
+ sizeof(struct _ddebug_site));
+
tab[i]._map = base + j;
v3pr_info(" %d %d %d %d %s.%s.%d\n", i, j, tab[i]._back, tab[i]._map,
@@ -1131,7 +1132,7 @@ static int __ddebug_add_module(struct _ddebug *tab, struct _ddebug_site *sites,
mutex_unlock(&ddebug_lock);
v2pr_info("%3u debug prints in module %s (in %d functions)\n",
- numdbgs, modname, ++ct);
+ numdbgs, modname, j);
return 0;
}
--
2.31.1
Since the 1st patch in this series, which partitioned into structs
_ddebug & _ddebug_site while leaving lineno in _ddebug, we have had
chunks of repeating site records; every pr_debug in a function
has identical site data.
To avoid reliance on the redundant data, we need:
0. ._map field (added HEAD~1)
1. detection of repeating sites.
In ddebug_add_module(), alter the index initialization loop to recognize
the 1st record in each block of repeats, remember it in "j", and use it
to ._map all the pr_debugs in that block. To find the block starts,
we can just look for a new .function in each row.
This gives us, for each function; 1st used row, 2nd-Nth unused rows.
To actually reclaim useful memory chunks, we will have to pack the
site records up towards the header.
Heres what booting with dynamic_debug.verbose=3 will show:
1st 3 builtin modules:
- cols 3,4 row 1 - shows +1 due to previous head64
- cols 2,4 - shows repeat detection working
rcu: Hierarchical SRCU implementation.
dyndbg: add-module: head64.1 sites
dyndbg: skip header head64
dyndbg: add-module: main.6 sites
Callback from call_rcu_tasks_trace() invoked.
dyndbg: 0 0 1 1 main.run_init_process.1358
dyndbg: 1 0 2 1 main.run_init_process.1356
dyndbg: 2 0 3 1 main.run_init_process.1355
dyndbg: 3 0 4 1 main.run_init_process.1353
dyndbg: 4 4 5 5 main.initcall_blacklisted.1142
dyndbg: 5 5 6 6 main.initcall_blacklist.1103
dyndbg: 6 debug prints in module main (in 3 functions)
dyndbg: add-module: initramfs.1 sites
dyndbg: 0 0 7 7 initramfs.unpack_to_rootfs.496
dyndbg: 1 debug prints in module initramfs (in 1 functions)
dyndbg: add-module: ibs.3 sites
dyndbg: 0 0 8 8 ibs.force_ibs_eilvt_setup.926
dyndbg: 1 1 9 9 ibs.setup_ibs_ctl.897
dyndbg: 2 1 10 9 ibs.setup_ibs_ctl.890
dyndbg: 3 debug prints in module ibs (in 2 functions)
for a loaded module:
- row 0000 - is the header (function==module) - lineno is garbage
- cols 2,4 - show repeat detect
- 9 functions - over by 1, due to counting the header
dyndbg: add-module: intel_rapl_common.13 sites
dyndbg: 0 0 0 0 intel_rapl_common.intel_rapl_common.16928
dyndbg: 1 1 1 1 intel_rapl_common.rapl_remove_package.1279
dyndbg: 2 2 2 2 intel_rapl_common.rapl_detect_domains.1245
dyndbg: 3 2 3 2 intel_rapl_common.rapl_detect_domains.1242
dyndbg: 4 4 4 4 intel_rapl_common.rapl_package_register_powercap.1159
dyndbg: 5 4 5 4 intel_rapl_common.rapl_package_register_powercap.1145
dyndbg: 6 4 6 4 intel_rapl_common.rapl_package_register_powercap.1114
dyndbg: 7 4 7 4 intel_rapl_common.rapl_package_register_powercap.1108
dyndbg: 8 8 8 8 intel_rapl_common.rapl_update_domain_data.1083
dyndbg: 9 9 9 9 intel_rapl_common.rapl_check_unit_atom.824
dyndbg: 10 10 10 10 intel_rapl_common.rapl_check_unit_core.796
dyndbg: 11 11 11 11 intel_rapl_common.rapl_read_data_raw.722
dyndbg: 12 12 12 12 intel_rapl_common.contraint_to_pl.303
dyndbg: 13 debug prints in module intel_rapl_common (in 9 functions)
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 26 ++++++++++++++++++++------
1 file changed, 20 insertions(+), 6 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 524303a04462..e12538640b51 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1073,7 +1073,7 @@ static int __ddebug_add_module(struct _ddebug *tab, struct _ddebug_site *sites,
{
struct ddebug_table *dt;
union _ddebug_header *dh = (union _ddebug_header *) &tab[0];
- int i;
+ int i, j, ct;
v3pr_info("add-module: %s.%d sites\n", modname, numdbgs);
@@ -1107,10 +1107,22 @@ static int __ddebug_add_module(struct _ddebug *tab, struct _ddebug_site *sites,
dt->ddebugs = tab;
dt->sites = sites;
- for (i = 0; i < numdbgs; i++) {
- tab[i]._back = base + i;
- tab[i]._map = base + i;
- v3pr_info(" %d %d %d %s.%s.%d\n", i, tab[i]._back, tab[i]._map,
+ /*
+ * Since sites don't have lineno (thats in struct _ddebug),
+ * all pr_debugs in a function have identical site data. So
+ * we watch for repeats on site.function, and reuse the index
+ * of the 1st site record in a repeating block.
+ */
+ for (ct = i = j = 0; i < numdbgs; i++) {
+ tab[i]._back = base + i; /* index back to header */
+
+ if (sites[i].function != sites[j].function) {
+ j = i; /* remember 1st row with new fn */
+ ct++;
+ }
+ tab[i]._map = base + j;
+
+ v3pr_info(" %d %d %d %d %s.%s.%d\n", i, j, tab[i]._back, tab[i]._map,
modname, sites[i].function, tab[i].lineno);
}
@@ -1118,7 +1130,9 @@ static int __ddebug_add_module(struct _ddebug *tab, struct _ddebug_site *sites,
list_add(&dt->link, &ddebug_tables);
mutex_unlock(&ddebug_lock);
- v2pr_info("%3u debug prints in module %s\n", numdbgs, modname);
+ v2pr_info("%3u debug prints in module %s (in %d functions)\n",
+ numdbgs, modname, ++ct);
+
return 0;
}
--
2.31.1
Expand ddebug.flags to 11 bits, and define new flags to support
pr_debug_once() and pr_debug_ratelimited() semantics:
echo module main +o > control # init/main runs once anyway
echo module foo +r > control # turn on ratelimiting
echo module foo +g > control # turn on group flag
Test these conditions in new is_onced_or_ratelimited(),
and call it from __dynamic_pr_debug and others.
print-once: can be done with just 2 bits in flags;
.o _DPRINTK_FLAGS_ONCE enables state test and set
.P _DPRINTK_FLAGS_PRINTED state bit
Just adding the flags lets the existing code operate them.
We will need new code to enforce constraints on flag combos;
'+ro' is nonsense, but this can wait, or can take a new meaning.
is_onced_or_ratelimited() should be correct for +o,
and should be testable now. tbd.
rate-limiting:
. for now, reserve the flag only !
.r _DPRINTK_FLAGS_RATELIMITED - track & limit prdbgs callrate
Intention is to wait til a prdebug is called, and if RATELIMITED is
set, THEN lookup a RateLimitState (RL) for it. If found, bump its
state and return true/false, otherwise create one, initialize it and
return false.
That lookup is basically a hash, with 2 part key:
. &builtin-vector-base OR &module
or the hash(s) could hang off the header struct
. ._back OR ._map
chosen by _DPRINTK_FLAGS_GROUPED
choice dictates per-site OR sharing across function
heres what happens:
- header fail seen before, time to dig more
dyndbg: get: header fail on 100-3231
dyndbg: changed drivers/gpu/drm/i915/gvt/mmio_context.c:3231 [i915]restore_context_mmio_for_inhibit =prg
dyndbg: get: header fail on 101-1412
dyndbg: changed drivers/gpu/drm/i915/gvt/cmd_parser.c:1412 [i915]init_cmd_table =prg
dyndbg: get: header fail on 102-1409
dyndbg: changed drivers/gpu/drm/i915/gvt/cmd_parser.c:1409 [i915]gen8_check_mi_display_flip =prg
dyndbg: get: header fail on 103-761
dyndbg: changed drivers/gpu/drm/i915/gvt/cmd_parser.c:761 [i915]gen8_check_mi_display_flip =prg
dyndbg: get: header fail on 104-760
dyndbg: changed drivers/gpu/drm/i915/gvt/cmd_parser.c:760 [i915]parser_exec_state_dump =prg
Signed-off-by: Jim Cromie <[email protected]>
---
include/linux/dynamic_debug.h | 10 ++++++---
lib/dynamic_debug.c | 42 ++++++++++++++++++++++++++++++++++-
2 files changed, 48 insertions(+), 4 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index fe70dda704d2..300fd0eed66f 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -64,18 +64,22 @@ struct _ddebug {
#define _DPRINTK_FLAGS_INCL_FUNCNAME (1<<2)
#define _DPRINTK_FLAGS_INCL_LINENO (1<<3)
#define _DPRINTK_FLAGS_INCL_TID (1<<4)
-#define _DPRINTK_FLAGS_DELETE_SITE (1<<7) /* drop site info to save ram */
-
#define _DPRINTK_FLAGS_INCL_ANY \
(_DPRINTK_FLAGS_INCL_MODNAME | _DPRINTK_FLAGS_INCL_FUNCNAME |\
_DPRINTK_FLAGS_INCL_LINENO | _DPRINTK_FLAGS_INCL_TID)
+#define _DPRINTK_FLAGS_ONCE (1<<5) /* print once flag */
+#define _DPRINTK_FLAGS_PRINTED (1<<6) /* print once state */
+#define _DPRINTK_FLAGS_RATELIMITED (1<<7)
+#define _DPRINTK_FLAGS_GROUPED (1<<8) /* manipulate as a group */
+#define _DPRINTK_FLAGS_DELETE_SITE (1<<9) /* drop site info to save ram */
+
#if defined DEBUG
#define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINT
#else
#define _DPRINTK_FLAGS_DEFAULT 0
#endif
- unsigned int flags:8;
+ unsigned int flags:11;
#ifdef CONFIG_JUMP_LABEL
union {
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 66b48f1cb2d0..a81461b58f6e 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -86,13 +86,17 @@ static inline const char *trim_prefix(const char *path)
return path + skip;
}
-static struct { unsigned flag:8; char opt_char; } opt_array[] = {
+static struct { unsigned flag:11; char opt_char; } opt_array[] = {
{ _DPRINTK_FLAGS_PRINT, 'p' },
{ _DPRINTK_FLAGS_INCL_MODNAME, 'm' },
{ _DPRINTK_FLAGS_INCL_FUNCNAME, 'f' },
{ _DPRINTK_FLAGS_INCL_LINENO, 'l' },
{ _DPRINTK_FLAGS_INCL_TID, 't' },
{ _DPRINTK_FLAGS_NONE, '_' },
+ { _DPRINTK_FLAGS_ONCE, 'o' },
+ { _DPRINTK_FLAGS_PRINTED, 'P' },
+ { _DPRINTK_FLAGS_RATELIMITED, 'r' },
+ { _DPRINTK_FLAGS_GROUPED, 'g' },
{ _DPRINTK_FLAGS_DELETE_SITE, 'D' },
};
@@ -728,6 +732,30 @@ static inline char *dynamic_emit_prefix(struct _ddebug *desc, char *buf)
return buf;
}
+/* test print-once or ratelimited conditions */
+static bool is_onced_or_limited(struct _ddebug *descriptor)
+{
+ if (descriptor->flags & _DPRINTK_FLAGS_ONCE &&
+ descriptor->flags & _DPRINTK_FLAGS_RATELIMITED)
+ pr_info(" ONCE & RATELIMITED together is nonsense\n");
+
+ if (descriptor->flags & _DPRINTK_FLAGS_ONCE) {
+ if (descriptor->flags & _DPRINTK_FLAGS_PRINTED) {
+ v3pr_info(" would suppress print once\n");
+ // return true;
+ }
+ descriptor->flags |= _DPRINTK_FLAGS_PRINTED;
+ // return false; // wanna see rate stuff
+ }
+ /* test rate-limits */
+ if (descriptor->flags & _DPRINTK_FLAGS_RATELIMITED) {
+ v3pr_info("todo: fetch RLstate{%s}\n",
+ descriptor->flags & _DPRINTK_FLAGS_GROUPED
+ ? "grouped" : "solo");
+ }
+ return false;
+}
+
void __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
{
va_list args;
@@ -737,6 +765,9 @@ void __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
BUG_ON(!descriptor);
BUG_ON(!fmt);
+ if (is_onced_or_limited(descriptor))
+ return;
+
va_start(args, fmt);
vaf.fmt = fmt;
@@ -757,6 +788,9 @@ void __dynamic_dev_dbg(struct _ddebug *descriptor,
BUG_ON(!descriptor);
BUG_ON(!fmt);
+ if (is_onced_or_limited(descriptor))
+ return;
+
va_start(args, fmt);
vaf.fmt = fmt;
@@ -788,6 +822,9 @@ void __dynamic_netdev_dbg(struct _ddebug *descriptor,
BUG_ON(!descriptor);
BUG_ON(!fmt);
+ if (is_onced_or_limited(descriptor))
+ return;
+
va_start(args, fmt);
vaf.fmt = fmt;
@@ -824,6 +861,9 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
struct va_format vaf;
va_list args;
+ if (is_onced_or_limited(descriptor))
+ return;
+
va_start(args, fmt);
vaf.fmt = fmt;
--
2.31.1