v4 fixes 'series grooming errors',
Reported-by: kernel test robot <[email protected]>
CONFIG_DYNAMIC_DEBUG creates a struct _ddebug (56 bytes) for each
callsite, which includes 3 pointers to: module, filename, function.
These are repetetive, and compressible, this patch series goes about
doing that, it:
- splits struct _ddebug and __dyndbg[] section/array into 2
creating struct _ddebug_site and __dyndbg_sites[]
temporary _ddebug.site connects them.
- makes _ddebug_site data optional
- minor optimizations
- makes _ddebug_site data deleteable
not necessary, proof of optionality
The RFC stuff comes at the end:
- attach __dyndbg_sites[] to module-info, like __dyndbg[]
- add index to struct _ddebug, use it for builtins
- add ddebug_site(_get|_put) abstraction to hide _ddebug.site
At this point, its practical to: compress __dyndbg_sites[] & replace
the section, then expand it on-demand to serve ddebug_site_get()
calls. For enabled callsites (with decorations flags), the retrieved
records can be cached in a hash after theyre 1st needed/used (when
callsite is actually executed).
Whats my (ideal?) decompressing interface ?
And whats the name of the api call ? I couldnt suss it out yet.
For any control read, a simple block decompress and cache is close to
ideal; all site data is then available to iterate over, and each
ddebug_site_get() just indexes into it. A stream of decompressed site
records would also work well, with less lumpy memory allocs and frees.
Actually dropping _ddebug.site is not yet possible. While we could
drop it for builtin modules, thats cuz we know __start___dyndbg_sites.
For loaded modules, I need the elf section: __dyndbg_sites. This is
in load-info, but I dont have a path to it. In:
- add _ddebug_header/table
That adds a single header entry pair (structs _ddebug*s with special
initialization) into the arrays, and it links to the front of the
array, where its useful. But creating this header entry only works
for vmlinux itself (because of vmlinux.ld.h patch), not for loadable
modules.
- add linker rules to module.lds.h
I tried to re-use the DYMAMIC_DEBUG_DATA macro added above for
modules, this failed, commit msg guesses the cause. This breaks with
a linker script syntax error
TODO
Presuming the fixed header can be linked reliably in front doing
something like I tried, it can be recast as a ddebug_header and
unionized with struct _ddebug, and the _ddebugs[] will fit nicely as a
flex-array:
struct ddebug_table2 {
struct ddebug_header foo;
struct _ddebug ddebugs[];
}
A header would have 40 bytes, room to contain most/all of struct
ddebug_table's fields, a pointer to the __dyndbg_sites[] table, and a
list-head too, meaning it supports essential and nice-to-have
properties:
- the mapping: __dyndbg[N] --> __dyndbg_sites[N] # NEEDed to drop .site
using container_of_flex() for flex-arrays
- we can add them directly to ddebug_tables list # freebie
ie avoid the kzalloc in ddebug_add_module()
If not all fields fit in the space available in __dyndbg[0], there is
space available in __dyndbg_sites[0].
Additionally, at the end of __init, ddebug_tables list is composed of
memory entirely in __dyndbg[], which could then be make readonly (by
means I dont know). If this breaks insertions of loadable modules, we
can easily a 2nd list for that.
Jim Cromie (19):
dyndbg: split struct _ddebug, move display fields to new _ddebug_site
dyndbg: __init iterate over __dyndbg & __dyndbg_site in parallel
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: optimize ddebug_emit_prefix
dyndbg: avoid calling dyndbg_emit_prefix when it has no work
dyndbg: refactor ddebug_alter_site out of ddebug_change
dyndbg: allow deleting site info via control interface
dyndbg+module: expose ddebug_sites to modules
dyndbg: add ddebug_site(_get|_put) abstraction
dyndbg: add _index to struct _ddebug
dyndbg: prevent build bugs via -DNO_DYNAMIC_DEBUG_TABLE
dyndbg: RFC - DECLARE/DEFINE_DYNAMIC_DEBUG_TABLE
dyndbg: shuffle ddebug_table fields
dyndbg: RFC add linker rules to module.lds.h
arch/x86/boot/compressed/Makefile | 1 +
arch/x86/entry/vdso/Makefile | 3 +
arch/x86/purgatory/Makefile | 1 +
drivers/firmware/efi/libstub/Makefile | 3 +-
drivers/gpu/drm/i915/i915_drv.c | 2 +
include/asm-generic/module.lds.h | 21 ++
include/asm-generic/vmlinux.lds.h | 24 +-
include/linux/dynamic_debug.h | 180 +++++++++++++--
kernel/module-internal.h | 1 +
kernel/module.c | 9 +-
lib/dynamic_debug.c | 313 +++++++++++++++++++-------
scripts/Makefile.lib | 2 +
12 files changed, 449 insertions(+), 111 deletions(-)
--
2.29.2
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 151e55ab6bb5..0c535f3c2ba9 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -631,15 +631,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.29.2
Add early return if no callsite info is specified in site-flags.
This avoids fetching site info that isn't going to be printed.
Signed-off-by: Jim Cromie <[email protected]>
---
include/linux/dynamic_debug.h | 9 +++++++++
lib/dynamic_debug.c | 3 +++
2 files changed, 12 insertions(+)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index bc8027292c02..aa4a3f5d8d35 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -40,6 +40,15 @@ 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_ANYSITE \
+ (_DPRINTK_FLAGS_INCL_MODNAME \
+ | _DPRINTK_FLAGS_INCL_FUNCNAME \
+ | _DPRINTK_FLAGS_INCL_LINENO)
+#define _DPRINTK_FLAGS_INCL_ANY \
+ (_DPRINTK_FLAGS_INCL_ANYSITE \
+ | _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 1f0cac4a66af..af9cf97f869b 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -632,6 +632,9 @@ static char *dynamic_emit_prefix(const struct _ddebug *dp, char *buf)
}
pos_after_tid = pos;
+ if (!(dp->flags & _DPRINTK_FLAGS_INCL_ANYSITE))
+ return buf;
+
if (desc) {
if (dp->flags & _DPRINTK_FLAGS_INCL_MODNAME)
pos += snprintf(buf + pos, remaining(pos), "%s:",
--
2.29.2
Replace direct ->site refs with _get(),_put() internal API. Right
now, _get() just returns ->site and _put() does nothing. Later we can
replace that implementation with one using ->module_index to fetch
then forget site data dynamically.
Several approaches are possible:
A: !site -> fill from backing store
1st try at this is/was using zram. At init, it copied each callsite
into a zs-allocation, and all site-> refs afterward went thru
_get/_put to zs-map on demand, and zs-unmap the site info. This
worked until I tried to keep callsites mapped while they're enabled,
when it gave lockdep warns/panics. IIRC theres a zram patchset doing
something with locking; I need to retry this approach, even if other
options are better, this might be a validating use case.
B: block store
Another approach is to compress the new linker section, using some
algorithm thats good at indexed decompression. I probed this
approach, using objcopy, unsuccessfully:
objcopy --dump-section __dyndbg=dd \
--dump-section __dyndbg_sites=ddsites $IMG
From vmlinux.o dumps were mostly empty (pre-link/reloc data?)
and vmlinux didnt have the section.
C: callsite composed from __dyndbg[N] & __dyndbg_site[N]
We know _ddebug records are in a vector, either in the builtin
__dyndbg linker section, or the same from a modprobed one. The
builtin section has all builtin module sub-sections catenated
dogether.
At init, we iterate over the section, and "parse it" by creating a
ddebug_table for each module with prdebugs. ddebug_table.num_debugs
remembers the size of each modules' vector of prdebugs.
We need a few things:
- _ddebug.index field, which knows offset to start of this sub-vector.
this new field will be "free" because the struct has padding.
it can be initialized during init, then RO.
- a back-pointer at the beginning of the sub-vector, to the
ddebug_table "owning" (but not containing) this sub-vector of
prdebugs.
If we had both, we could get from the ddebug element to its vector
root, back up to the owning ddebug_table, then down to the _callsite
vector, and index to the right element. While slower than a pointer
deref, this is a cold path, and it allows elimination of the
per-callsite pointer member, thus greater density of the sections, and
still can support sparse site info.
That back-pointer feels tricky. It needs to be 1st in the sub-vector
D: (C1?) add a header record to each sub-vector
If we can insert a header record into each modules' __dyndbg* section
sub-vectors, we can simplify the cold path above; a single sites*
pointer in the header can give us access to __dyndbg_sites[N]
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 be2e97ae2da9..1f59407b6a83 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;
*buf = '\0';
@@ -653,6 +666,7 @@ static char *__dynamic_emit_prefix(const struct _ddebug *dp, char *buf)
if (!(dp->flags & _DPRINTK_FLAGS_INCL_ANYSITE))
return buf;
+ desc = ddebug_site_get(dp);
if (desc) {
if (dp->flags & _DPRINTK_FLAGS_INCL_MODNAME)
pos += snprintf(buf + pos, remaining(pos), "%s:",
@@ -670,6 +684,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;
}
@@ -952,7 +968,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,
@@ -968,6 +985,8 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
seq_puts(m, "\"\n");
}
+ ddebug_site_put(dp);
+
return 0;
}
--
2.29.2
We currently use dp->site to map: &__dyndbg[N] -> &__dyndbg_sites[N].
We want to drop site, new _ddebug._index provides the N.
The mapping is done in ddebug_site_get():
For builtin modules, a _ddebug *ptr is between __start___dyndbg and
__stop___dyndbg, and we can use &__start___dyndbg_sites[N] directly.
For loadable modules, we still need work, so we print rubbish, and
just return site pointer (which is correct).
ddebug_add_module() handles _index initialization:
Its new task is to number each module consecutively, so it gets new
base arg to pass the next starting index.
To actually drop site, We need both the module's __dyndbg* section
addys, and we need their relative placement to have a base-to-base
offset.
PLAN - a table header connecting 2 tables.
- ddebug_table points to both __dyndbgs & __dyndbg_sites.
but *ddebugs & *sites are independent.
no path from ddebugs[n] -> ddebug_sites[n]
If we have a header record in-situ, which keeps the site pointer we
seek to eliminate from _ddebug, and its in element[0] of both vectors,
we can go:
ddebugs[n] -> ddebugs[0] -> containerof -> site[n]
union ddebug_table_header {
struct ddebug_table *owner;
struct _ddebug item;
}
and
struct ddebug_table_vector {
struct ddebug_table *owner;
struct _ddebug vector[];
}
Signed-off-by: Jim Cromie <[email protected]>
---
include/linux/dynamic_debug.h | 2 ++
lib/dynamic_debug.c | 40 +++++++++++++++++++++++++++++------
2 files changed, 36 insertions(+), 6 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 7d33475d226a..18689db0e2c0 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
@@ -56,6 +57,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 1f59407b6a83..54737647556c 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,17 @@ 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 */
+ v4pr_info("get %d: %s.%s.%d\n", dp->_index, dp->site->modname,
+ dp->site->function, dp->lineno);
+
+ if (dp >= __start___dyndbg && dp < __stop___dyndbg) {
+ v4pr_info(" is builtin: %d %ld\n", dp->_index, dp - __start___dyndbg);
+ return &__start___dyndbg_sites[ dp - __start___dyndbg ];
+ } else {
+ v3pr_info(" is loaded: %d %ld\n", dp->_index, dp - __start___dyndbg);
+ return dp->site;
+ }
+ return dp->site;
}
static inline void ddebug_site_put(struct _ddebug *dp)
{
@@ -1034,10 +1046,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 numdbgs, unsigned base,
+ const char *modname)
{
struct ddebug_table *dt;
+ int i;
dt = kzalloc(sizeof(*dt), GFP_KERNEL);
if (dt == NULL) {
@@ -1055,6 +1069,13 @@ int ddebug_add_module(struct _ddebug *tab, struct _ddebug_site *sites,
dt->ddebugs = tab;
dt->sites = sites;
+ v3pr_info("add-module: %s\n", modname);
+ for (i = 0; i < numdbgs; i++) {
+ tab[i]._index = base++;
+ v3pr_info(" %d %d %s.%s.%d\n", i, tab[i]._index, modname,
+ tab[i].site->function, tab[i].lineno);
+ }
+
mutex_lock(&ddebug_lock);
list_add(&dt->link, &ddebug_tables);
mutex_unlock(&ddebug_lock);
@@ -1063,6 +1084,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 +1204,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 +1228,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 +1239,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.29.2
In order to drop the pointer connecting _ddebug's to _ddebug_sites,
we need to elevate the latter; we need to track it in (internal)
ddebug_tables, and set it in ddebug_add_module. That last part
exposes it by interface to module.c, so we add a field to load_info,
and adjust load_module to initialize it from the elf section.
Its possible that this closes a "hole" created when __dyndbg_sites
section was added, in that the section wasn't "managed" directly, and
could conceivably get lost later. I never saw any misbehavior loading
i915.ko into a vm, but still..
TBD/RFC:
these 2 vectors should be in a single struct. if this struct can have
ddebugs[], ie a flex-array, then container_of can get us to the
sites*, yielding &ddebug_sites[N], and allowing to drop _ddebug.site
The trouble with this is that ddebugs* now points to someone elses
memory, and we cant just steal it and stomp on the memory just in
front of it (for the sites ptr).
rename n to numdbgs
Signed-off-by: Jim Cromie <[email protected]>
---
include/linux/dynamic_debug.h | 4 ++--
kernel/module-internal.h | 1 +
kernel/module.c | 9 ++++++---
lib/dynamic_debug.c | 20 ++++++++++++--------
4 files changed, 21 insertions(+), 13 deletions(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index d811273c8484..7d33475d226a 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -70,8 +70,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, ...);
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 30479355ab85..792903230acd 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, "__dyndbg",
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 fdc2d0e15731..be2e97ae2da9 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 {
@@ -1014,14 +1015,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;
}
/*
@@ -1030,15 +1031,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;
}
@@ -1178,7 +1180,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;
@@ -1188,7 +1192,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.29.2
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 da732e0c56e3..abe3382aabd5 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -143,10 +143,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 == '^') {
@@ -167,7 +166,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)
@@ -219,9 +217,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.29.2
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 abe3382aabd5..151e55ab6bb5 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -235,10 +235,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.29.2
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 0c535f3c2ba9..1f0cac4a66af 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -918,18 +918,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.29.2
Copy the DYNAMIC_DEBUG_DATA macro, that works in vmlinux.lds.h,
into module.lds.h This does not work here
The point of the KEEPS is to pack section pairs into consecutive
memory, a property we need in order to drop the _ddebug.site pointer.
The problem (ISTM) is that for linking vmlinux, the data is linked
into .data (and the sections are subsumed), and it is is accessed by
iterating beteween __(fstart|stop)___dyndbg(_sites)? symbols. That
breaks down here cuz kernel/module.c specifically grabs the __dyndb*
sections by name.
I lack the linker-fu to sort this, so I left my commented out
attempts, to show my errors.
no-bisect: expect linker error
Signed-off-by: Jim Cromie <[email protected]>
---
include/asm-generic/module.lds.h | 21 +++++++++++++++++++++
1 file changed, 21 insertions(+)
diff --git a/include/asm-generic/module.lds.h b/include/asm-generic/module.lds.h
index f210d5c1b78b..0074c5f2421b 100644
--- a/include/asm-generic/module.lds.h
+++ b/include/asm-generic/module.lds.h
@@ -7,4 +7,25 @@
* Empty for the asm-generic header.
*/
+/* implement dynamic printk debug section packing */
+#if defined(CONFIG_DYNAMIC_DEBUG) || \
+ (defined(CONFIG_DYNAMIC_DEBUG_CORE) \
+ && defined(DYNAMIC_DEBUG_MODULE))
+#define DYNAMIC_DEBUG_DATA() \
+ . = ALIGN(8); \
+ KEEP(*(__dyndbg_sites .gnu.linkonce.dyndbg_site)) \
+ KEEP(*(__dyndbg .gnu.linkonce.dyndbg))
+#else
+#define DYNAMIC_DEBUG_DATA()
+#endif
+
+SECTIONS {
+__dyndbg : { (*(__dyndbg .gnu.linkonce.dyndbg)) }
+__dyndbg_sites : { (*(__dyndbg_sites .gnu.linkonce.dyndbg_site)) }
+
+ //.data.dyndbg : { DYNAMIC_DEBUG_DATA() } // syntax ok
+ //: { DYNAMIC_DEBUG_DATA() }
+ //DYNAMIC_DEBUG_DATA()
+}
+
#endif /* __ASM_GENERIC_MODULE_LDS_H */
--
2.29.2
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 2d011ac3308d..6fbd9099c2fa 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -191,6 +191,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
@@ -227,13 +239,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.29.2
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 c3c35dcc6a59..9cff9db15937 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -142,6 +142,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
@@ -170,38 +212,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.29.2
Wrap function in a static-inline one, which checks flags to avoid
calling the function unnecessarily.
Signed-off-by: Jim Cromie <[email protected]>
---
lib/dynamic_debug.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index af9cf97f869b..2d011ac3308d 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -615,7 +615,7 @@ static int remaining(int wrote)
return 0;
}
-static char *dynamic_emit_prefix(const struct _ddebug *dp, char *buf)
+static char *__dynamic_emit_prefix(const struct _ddebug *dp, char *buf)
{
int pos_after_tid;
int pos = 0;
@@ -655,6 +655,13 @@ static char *dynamic_emit_prefix(const struct _ddebug *dp, char *buf)
return buf;
}
+static inline char *dynamic_emit_prefix(struct _ddebug *dp, char *buf)
+{
+ if (unlikely(dp->flags & _DPRINTK_FLAGS_INCL_ANY))
+ return __dynamic_emit_prefix(dp, buf);
+ return buf;
+}
+
void __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
{
va_list args;
--
2.29.2
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 aa4a3f5d8d35..d811273c8484 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_ANYSITE \
(_DPRINTK_FLAGS_INCL_MODNAME \
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 6fbd9099c2fa..fdc2d0e15731 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -92,6 +92,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]; };
@@ -201,6 +202,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.29.2
V4-proto - now currently diet-3i
Simplify:
.gnu.linkonce._mod_.dyndbg -> .gnu.linkonce.dyndbg
ie drop _mod_
This puts a single header record at front of the vectors, solving 2
problems (discussed below) simultaneously:
- header in front, allowing flex-array rep of layout.
- single header, not per-module. adequate for needs, no wasted space.
this now appears to work
- get: dp range-check good on builtin, !builtin.
todo:
- _sym_##_site symbols in init/main.i still busted
- _index init in __ddebug_add_module
- cleanup commentary below.
DEFINE_DYNAMIC_DEBUG_TABLE is based on DECLARE_DYNAMIC_DEBUG_METADATA.
Like its model, it creates/allocates a pair of structs: _ddebug &
_ddebug_site, and inits them distinctively.
Its purpose is to create an in-situ module-header-pair for each
module's sub-vectors of _ddebug[], _ddebug_sites[]. Once the
header-pair is reliably linked into the vectors, code can get from
_ddebug[N] to the header-pair, then to _ddebug_sites[N] at runtime by
saving N into _ddebug._index during init. With this, we can drop the
site pointer in struct _ddebug.
Eventually, this module-header-pair can be adapted to be an in-situ
replacement for the separately allocated struct ddebug_tables, and be
linked into the ddebug_tables list.
RFC, NOTES:
DYNAMIC_DEBUG is a 'transparent' facility, in that uses of pr_debug
get extra features without additional api. Because of this,
DEFINE_DYNAMIC_DEBUG_TABLE is 'transparently' invoked by
dynamic_debug.h on behalf of all its (indirect) users, including
printk.h.
IOW this has wide effects; it results in multiple redundant
declarations of header records, even single object files may get
multiple copies. Using .gnu.linkonce._mod_.dyndbg(|_site) section
names and "__used __weak " seems to resolve the redundancies. I
havent tried with clang.
In vmlinux-lds.h, the 2 KEEPs are modified to append those 2 new
header sections to their respective existing __dyndbg* sections, in an
interleaved manner. This places the header records immediately after
the modules' block of _ddebug*s, in a knowable offset from &__dyndbg[0].
scripts/Makefile.lib gets a new -DKBUILD_MODSYM defn, with a value
like KBUILD_MODNAME, except that its not __stringified. It is used to
create a pair of module-ish named variables: _sym_##_dyndbg_base.
For some non-obvious reason, the substitution doesnt work, resulting
in per-module symbol names like KBUILD_MODSYM_dyndbg_base. This
subtly alters the header initialization and is_dyndbg_header_pair(),
which is used in __init to find the headers adjacent to each modules'
block of _ddebug records. This isnt fatal to the plan; we just need
the storage reserved where its accessible by known offset from the
_ddebug[N] record of an enabled pr-debug. But it would be nice to
have the symbol names consistent with the intent. I looked for a
MODULE_SINGLETON(name_suffix) to use, similarly to how UNIQUE_ID is
used to construct names, but found nothing.
The .gnu.linkonce._mod_. works to eliminate all the extra headers in
each module, but a problem remains; it adds unneeded headers for
modules with zero pr-debugs. So Im seeing ~1500 extra headers.
I tried several flavors of conditional linking, now think I want/need
a linker-script language extension:
KEEP ( *(__dyndbg) AND *(.gnu.linkonce.dyndbg) ) # my need
KEEP ( *(foo_tbl) OR *(.gnu.linkonce.foo_alt) ) # for completeness
I've managed to alter ld's grammar, but its only compile tested, and
is missing the conditional linking pieces. Since this patchset's
value proposition is a memory shrink, ~1500 extra headers is fatal,
and this patchset must be a slow-cook.
V4 todo:
Time to simplify, drop _mod_, and change _ddebug.module_index to
._index, indicating that its no longer keyed to module, but to the
whole compilation unit, which for the kernel includes all the builtin
modules. loadable modules should get their own. tbt.
this could obsolete all the above problems.
Signed-off-by: Jim Cromie <[email protected]>
---
drivers/gpu/drm/i915/i915_drv.c | 2 +
include/asm-generic/vmlinux.lds.h | 27 +++++---
include/linux/dynamic_debug.h | 100 +++++++++++++++++++++++++++++-
lib/dynamic_debug.c | 27 +++++---
scripts/Makefile.lib | 2 +
5 files changed, 139 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 8e9cb44e66e5..51163e0d40cf 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -91,6 +91,8 @@
static const struct drm_driver driver;
+//DEFINE_DYNAMIC_DEBUG_TABLE();
+
static int i915_get_bridge_dev(struct drm_i915_private *dev_priv)
{
int domain = pci_domain_nr(dev_priv->drm.pdev->bus);
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 4f2af9de2f03..482d94078785 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -335,6 +335,22 @@
KEEP(*(.dtb.init.rodata)) \
__dtb_end = .;
+/* implement dynamic printk debug */
+#if defined(CONFIG_DYNAMIC_DEBUG) || \
+ (defined(CONFIG_DYNAMIC_DEBUG_CORE) \
+ && defined(DYNAMIC_DEBUG_MODULE))
+#define DYNAMIC_DEBUG_DATA() \
+ . = ALIGN(8); \
+ __start___dyndbg_sites = .; \
+ KEEP(*(__dyndbg_sites .gnu.linkonce.dyndbg_site)) \
+ __stop___dyndbg_sites = .; \
+ __start___dyndbg = .; \
+ KEEP(*(__dyndbg .gnu.linkonce.dyndbg)) \
+ __stop___dyndbg = .;
+#else
+#define DYNAMIC_DEBUG_DATA()
+#endif
+
/*
* .data section
*/
@@ -351,15 +367,8 @@
__end_once = .; \
STRUCT_ALIGN(); \
*(__tracepoints) \
- /* implement dynamic printk debug */ \
- . = ALIGN(8); \
- __start___dyndbg_sites = .; \
- KEEP(*(__dyndbg_sites)) \
- __stop___dyndbg_sites = .; \
- __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 18689db0e2c0..229cfd81ffb3 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -101,10 +101,92 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
const struct ib_device *ibdev,
const char *fmt, ...);
-#define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt) \
+/**
+ * DEFINE_DYNAMIC_DEBUG_TABLE(), DECLARE_DYNAMIC_DEBUG_TABLE()
+ *
+ * These are special versions of DEFINE_DYNAMIC_DEBUG_METADATA(). The
+ * job is to create/reserve a module-header struct-pair as the last
+ * element of the module's sub-vectors of __dyndbg & __dyndbg_sites,
+ * ie at a fixed offset from them. I expect to settle on 1 of these
+ * 2; DEFINE_ has seen most testing recently, and is favored.
+ *
+ * With this record reliably in-situ at a fixed offset for each
+ * callsite, we can use ._index to remember this offset, find the
+ * header, find the parallel vector, then index the corresponding site
+ * data.
+ *
+ * This macro is invoked at the bottom of this header. It is
+ * typically invoked multiple times for a module, generally at least
+ * once per object file. The combination of .gnu.linkonce._ and
+ * __weak appear to resolve earlier troubles with compile errors or
+ * multiple copies of headers.
+ */
+
+#define DECLARE_DYNAMIC_DEBUG_TABLE_(_sym_, _mod_) \
+ static struct _ddebug_site \
+ __section(".gnu.linkonce.dyndbg_site") \
+ __aligned(8) \
+ __maybe_unused \
+ _sym_##_dyndbg_site; \
+ static struct _ddebug \
+ __section(".gnu.linkonce.dyndbg") \
+ __aligned(8) \
+ __maybe_unused \
+ _sym_##_dyndbg_base
+
+#define DYNAMIC_DEBUG_TABLE_refer(_sym_) \
+ static void __used _sym_##_take_internal_refs(void) \
+{ \
+ struct _ddebug_site * dc = &_sym_##_dyndbg_site; \
+ struct _ddebug * dp = &_sym_##_dyndbg_base; \
+ printk(KERN_INFO "%s %d\n", dc->function, dp->lineno); \
+}
+
+#define DECLARE_DYNAMIC_DEBUG_TABLE() \
+ DECLARE_DYNAMIC_DEBUG_TABLE_(KBUILD_MODSYM, KBUILD_MODNAME); \
+ DYNAMIC_DEBUG_TABLE_refer(KBUILD_MODSYM)
+
+//#define DEFN_SC static // clashes with extern forward decl
+//#define DEFN_SC extern // warning: ‘KBUILD_MODNAME_dyndbg_site’ initialized and declared ‘extern’
+//#define DEFN_SC // no section allowd on locals
+#define DEFN_SC __weak
+
+#define DEFINE_DYNAMIC_DEBUG_TABLE_(_sym_,_mod_) \
+ DEFN_SC struct _ddebug_site \
+ __section(".gnu.linkonce.dyndbg_site") \
+ __used __aligned(8) \
+ _sym_ ##_dyndbg_site = { \
+ .modname = _mod_, \
+ .filename = __FILE__, \
+ .function = (void*) _mod_ \
+ }; \
+ DEFN_SC struct _ddebug \
+ __section(".gnu.linkonce.dyndbg") \
+ __used __aligned(8) \
+ _sym_ ##_dyndbg_base = { \
+ .site = & _sym_ ##_dyndbg_site, \
+ .format = _mod_, \
+ .lineno = 0 \
+ }
+
+/* above init conditions as distinguishing predicate.
+ * (site == iter->site) should work but doesnt, possibly cuz MODSYM
+ * expansion problem
+ */
+#define is_dyndbg_header_pair(iter, site) \
+ ((iter->format == site->modname) \
+ && (site->modname == site->function))
+
+// build-time expensive, shows repetitive includes
+// #pragma message "<" __stringify(KBUILD_MODSYM) "> adding DYNDBG_TABLE"
+
+#define DEFINE_DYNAMIC_DEBUG_TABLE() \
+ DEFINE_DYNAMIC_DEBUG_TABLE_(KBUILD_MODSYM, KBUILD_MODNAME);
+
+#define DEFINE_DYNAMIC_DEBUG_METADATA_(_mod_, name, fmt) \
static struct _ddebug_site __aligned(8) \
__section("__dyndbg_sites") name##_site = { \
- .modname = KBUILD_MODNAME, \
+ .modname = _mod_, \
.filename = __FILE__, \
.function = __func__, \
}; \
@@ -114,9 +196,13 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
.format = (fmt), \
.lineno = __LINE__, \
.flags = _DPRINTK_FLAGS_DEFAULT, \
+ ._index = 0, \
_DPRINTK_KEY_INIT \
}
+#define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt) \
+ DEFINE_DYNAMIC_DEBUG_METADATA_(KBUILD_MODNAME, name, fmt)
+
#ifdef CONFIG_JUMP_LABEL
#ifdef DEBUG
@@ -247,4 +333,14 @@ 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 for -DNO_DYNAMIC_DEBUG_TABLE */
+DEFINE_DYNAMIC_DEBUG_TABLE()
+
+#endif
+
#endif
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 54737647556c..c1a7345277eb 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1203,8 +1203,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)) {
@@ -1220,10 +1219,20 @@ 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++) {
+ /* we should have 1 header for kernel, at beginning of blocks
+ * but lets be safe for a while
+ */
+ if (is_dyndbg_header_pair(iter, site)) {
+ v3pr_info("header.%d: %s\n", i, site->modname);
+ iter->site = site; /* fixup bad init */
+ continue;
+ }
BUG_ON(site != iter->site);
- entries++;
+
+ v3pr_info("site: %s.%s.%d\n", site->modname,
+ site->function, iter->lineno);
if (strcmp(modname, site->modname)) {
modct++;
@@ -1232,10 +1241,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++;
}
@@ -1245,9 +1256,9 @@ static int __init dynamic_debug_init(void)
ddebug_init_success = 1;
vpr_info("%d modules, %d entries and %d bytes in ddebug tables, %d bytes in __dyndbg section, %d bytes in __dyndbg_sites section\n",
- modct, entries, (int)(modct * sizeof(struct ddebug_table)),
- (int)(entries * sizeof(struct _ddebug)),
- (int)(entries * sizeof(struct _ddebug_site)));
+ modct, i, (int)(modct * sizeof(struct ddebug_table)),
+ (int)(i * sizeof(struct _ddebug)),
+ (int)(i * sizeof(struct _ddebug_site)));
/* apply ddebug_query boot param, dont unload tables on err */
if (ddebug_setup_string[0] != '\0') {
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 8cd67b1b6d15..051993ef9b92 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -111,10 +111,12 @@ target-stem = $(basename $(patsubst $(obj)/%,%,$@))
# These flags are needed for modversions and compiling, so we define them here
# $(modname_flags) defines KBUILD_MODNAME as the name of the module it will
# end up in (or would, if it gets compiled in)
+
name-fix-token = $(subst $(comma),_,$(subst -,_,$1))
name-fix = $(call stringify,$(call name-fix-token,$1))
basename_flags = -DKBUILD_BASENAME=$(call name-fix,$(basetarget))
modname_flags = -DKBUILD_MODNAME=$(call name-fix,$(modname)) \
+ -DKBUILD_MODSYM=$(call name-fix-token,$(modname)) \
-D__KBUILD_MODNAME=kmod_$(call name-fix-token,$(modname))
modfile_flags = -DKBUILD_MODFILE=$(call stringify,$(modfile))
--
2.29.2
The next patch adds DEFINE_DYNAMIC_DEBUG_TABLE(), which breaks some
subtrees with special compile constraints (efi etc).
Avoid this by adding a define to suppress the *remote declaration*
done by DEFINE_DYNAMIC_DEBUG_TABLE(), automatically, on behalf of all
possible users of pr_debug.
Signed-off-by: Jim Cromie <[email protected]>
---
arch/x86/boot/compressed/Makefile | 1 +
arch/x86/entry/vdso/Makefile | 3 +++
arch/x86/purgatory/Makefile | 1 +
drivers/firmware/efi/libstub/Makefile | 3 ++-
4 files changed, 7 insertions(+), 1 deletion(-)
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.29.2
In preparation to unionize structs _ddebug & ddebug_table, shuffle
fields in latter so they match the layout of the former. This MAY
simplify initialization of the header field, in particular by
preserving *sites.
It also sets up a later conversion to a flex-array ddebugs[].
This step is mostly to isolate/prove no breakage before HEAD++
Signed-off-by: Jim Cromie <[email protected]>
---
include/linux/dynamic_debug.h | 29 +++++++++++++++++++++++++++++
lib/dynamic_debug.c | 2 +-
2 files changed, 30 insertions(+), 1 deletion(-)
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 229cfd81ffb3..22f11218047f 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -66,6 +66,35 @@ struct _ddebug {
#endif
} __aligned(8);
+/* this pair of header structs correspond quite deeply to struct
+ * _ddebug(|_site)s above; they are also created into __dyndbg*
+ * sections (by DEFINE_DYNAMIC_DEBUG_TABLE), and should be unionized
+ * with them to reinforce this.
+ *
+ * struct _ddebug_header is the important one, it has enough space to
+ * take struct ddebug_table's job, ie: link together into a list, and
+ * keep track of the modname & _ddebug(|_sites) vectors.
+ *
+ * Its other job is handled by its placement in the front of a
+ * module's _ddebug[N] entries. Each _ddebug knows its N, so the
+ * header's address is computable, and its site pointer is available
+ * to get _ddebug_sites[N]. Then we can drop _ddebug.sites, regaining
+ * parity with original _ddebug footprint.
+ *
+ * Eventually, N will index a fetch from a compressed block, or for
+ * enabled callsites, a hash. A global hash is probably adequate, if
+ * ~5k elements doesnt degrade access time.
+ */
+struct _ddebug_site_header {
+ /* we have 24 bytes total here, all currently unused */
+} __aligned(8);
+
+struct _ddebug_header {
+ struct _ddebug_site* sites;
+ struct list_head link;
+ const char* mod_name;
+ unsigned num_ddebugs;
+} __aligned(8);
#if defined(CONFIG_DYNAMIC_DEBUG_CORE)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index c1a7345277eb..5d1ce7f21c30 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -45,11 +45,11 @@ extern struct _ddebug_site __start___dyndbg_sites[];
extern struct _ddebug_site __stop___dyndbg_sites[];
struct ddebug_table {
+ struct _ddebug_site *sites;
struct list_head link;
const char *mod_name;
unsigned int num_ddebugs;
struct _ddebug *ddebugs;
- struct _ddebug_site *sites;
};
struct ddebug_query {
--
2.29.2