[PATCH 08/10] dyndbg: cache the dynamic prefixes per callsite
Jim Cromie
jim.cromie at gmail.com
Tue Feb 3 13:19:32 PST 2026
Currently __dynamic_emit_lookup() re-writes a callsites's dynamic
prefix each time it is needed/invoked. For busy pr_debugs with +mfslt
prefix-flags, this can get expensive.
Eliminate that repeated work:
- for +pmfsl sites only: others never need a lookup
- kmalloc the prefix-str, and
- sprintf to it, then to printk
- save ref in a maple-tree, keyed by callsite-addy
Delete a cached prefix when a site's +-mfsl flags are changed, so they
will be rewritten fresh if needed again. Note that this doesn't
delete on -p, preserving entries for reuse after a +p on that site.
TODO:
1 measure this vs HEAD~1
2 flush modules descriptor-range from cache on unload.
3 memory-pressure-dump-cache-on-demand
NOTES
On __dynamic_emit_lookup(), drop the const on arg1: ddebug descriptor,
so the _CACHED flag can be set on the record.
If we replace direct field refs with accessor macros doing lookups
from compressed/de-duplicated module,func,file info-tables, then the
dynamic-prefix costs will increase, so the cache's value rises too.
NB-ASIDE: the memory freeable in-principle by de-duplication into new
storage is significant: the mod_name column can be dropped entirely
once loaded, since its copied into ddebug_table._ddebug_info.mod_name.
The file & function columns reduce by approx ~90%.
using MAPLE_TREE
The generated prefixes are cached in a maple-tree (pr_prefixes), keyed
on the site descriptor. This simple key ignores the possible sharing
of prefixes as too complicated to figure out just yet.
But since this seems to work, lets speculate !
#:> echo function foo +pmfs > /proc/dynamic_debug/control
If any of foo's pr_debugs is called (most likely the 1st in foo), the
generated prefix could be reused for every other callsite.
This range is why I chose a maple tree here; each mt_store(key) is the
"start" of a region, and/or the end of the previous interval.
mt_store_range is probably what I need, to figure out and use.
TLDR:
Invariants:
istm, key = desc insures key is always >4096, any LSBs we can use to
modify mt_store behavior are still available.
the struct _ddebug[] organization means that callsites are
modified/enabled in order. IIUC, this has enough impact that
static-key toggling is batched under the hood - RFC.
That static-key batching could work naturally on function-ranges
of pr-debugs, since they're all after the fn's entry-point.
While flag changes other than +-p don't toggle the static-key, the
"locality" of grouped changes could still be advantageous ?
A dynamic-prefix is created when foo()s 1st +p armed +mfsl? prefixed
pr_debug() is called, its almost always 1st in fn lexically, and thus
induced by the 1st _ddebug_desc in foo()s sub-range.
So in the common case where user avoids +l flag, a single prefix entry
per function seems possible, and would be a clear win over the naive
per-callsite cache. ISTM this reduces to the key/range used to store
& load an entry.
Does this work ?
key = desc_uniq_mask(desc) | essence_of_flags(flags);
desc_uniq_mask(desc): rounddown_pow_of_two(sizeof(struct _ddebug)
gives us uniqueness, with 1:x gaps that istm dont matter.
essence_of_flags(flags):
What about the +l flag ? With the current/naive key=desc it doesnt
matter, but a +mfs-l gen'd prefix could span all of foo(), and shrink
the cache ~9:1 based on the builtin average of pr_debugs:func.
ISTM real users dont use the +l flag (or perhaps any of them, but I
digress), so the cache size reduction will mostly be in play.
So easy path: 1st gen'd prefix is +fms-l, it will span foo().
A previous patch defined the *_CACHED flag, to mark a callsite as
having been called, and a prefix gen'd and cached, so theres a need to
erase and kfree when its invalidated.
We could change it to _CACHE, to mark that the sites prefix should be
cached when generated, and it still needs to be mt_erase'd and
kfree'd, but its ok if not found. Now it could signal programmer
intent. What exactly I dont know yet.
Could we have a "pin-prick" entry for +l callsites ? Such an entry
could be added to an existing range, and it would not prevent an
enveloping range added later. This would allow a few of foo()s
callsites to add the +l without spoiling the cache-hit on the rest of
the +mfs callsites. store_type === wr_exact_fit feels close.
WAG: if any LSBs in maple-tree keys can be exploited by user, then
maybe 3 levels of nested intervals are possible: mod_name, filename,
function. Consider:
# single prefix per module
:#> echo +pm > /proc/dyanamic_debug/control
# single prefix per function in drm
:#> echo module drm +pfm > /proc/dyanamic_debug/control
cc: maple-tree at lists.infradead.org
Signed-off-by: Jim Cromie <jim.cromie at gmail.com>
Signed-off-by: Jim Cromie <jim.cromie at gmail.com>
---
lib/dynamic_debug.c | 82 ++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 74 insertions(+), 8 deletions(-)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index ccc448e4a1ff..146bfe0cd6da 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -31,6 +31,7 @@
#include <linux/dynamic_debug.h>
#include <linux/debugfs.h>
+#include <linux/maple_tree.h>
#include <linux/slab.h>
#include <linux/jump_label.h>
#include <linux/hardirq.h>
@@ -78,6 +79,11 @@ module_param(verbose, int, 0644);
MODULE_PARM_DESC(verbose, " dynamic_debug/control processing "
"( 0 = off (default), 1 = module add/rm, 2 = >control summary, 3 = parsing, 4 = per-site changes)");
+/* cache of composed prefixes for enabled and invoked pr_debugs */
+static DEFINE_MTREE(pr_prefixes);
+static void ddebug_drop_cached_prefix(struct _ddebug *dp);
+#define prefix_flags(flags) (flags & _DPRINTK_FLAGS_INCL_LOOKUP)
+
/* Return the path relative to source root */
static inline const char *trim_prefix(const char *path)
{
@@ -366,6 +372,10 @@ static int ddebug_change(const struct ddebug_query *query, struct flag_settings
newflags = (dp->flags & modifiers->mask) | modifiers->flags;
if (newflags == dp->flags)
continue;
+
+ if (prefix_flags(dp->flags) != prefix_flags(newflags))
+ ddebug_drop_cached_prefix(dp);
+
#ifdef CONFIG_JUMP_LABEL
if (dp->flags & _DPRINTK_FLAGS_PRINT) {
if (!(newflags & _DPRINTK_FLAGS_PRINT))
@@ -887,24 +897,59 @@ static int remaining(int wrote)
return 0;
}
-static int __dynamic_emit_lookup(const struct _ddebug *desc, char *buf, int pos)
+static int __dynamic_emit_lookup(struct _ddebug *desc, char *buf, int pos)
{
+ char *prefix;
+ int len;
+
+ if (desc->flags & _DPRINTK_FLAGS_PREFIX_CACHED) {
+ prefix = (char *) mtree_load(&pr_prefixes, (unsigned long)desc);
+ if (prefix) {
+ pos += snprintf(buf + pos, remaining(pos), "%s", prefix);
+ v4pr_info("using prefix cache:%px %s\n", buf, prefix);
+ return pos;
+ }
+ }
+ if (!(desc->flags & _DPRINTK_FLAGS_INCL_LOOKUP))
+ return pos;
+
+ prefix = kmalloc(PREFIX_SIZE, GFP_KERNEL);
+ if (!prefix)
+ return pos;
+
+ len = 0;
if (desc->flags & _DPRINTK_FLAGS_INCL_MODNAME)
- pos += snprintf(buf + pos, remaining(pos), "%s:",
+ len += snprintf(prefix + len, PREFIX_SIZE - len, "%s:",
desc->modname);
if (desc->flags & _DPRINTK_FLAGS_INCL_FUNCNAME)
- pos += snprintf(buf + pos, remaining(pos), "%s:",
+ len += snprintf(prefix + len, PREFIX_SIZE - len, "%s:",
desc->function);
if (desc->flags & _DPRINTK_FLAGS_INCL_SOURCENAME)
- pos += snprintf(buf + pos, remaining(pos), "%s:",
+ len += snprintf(prefix + len, PREFIX_SIZE - len, "%s:",
trim_prefix(desc->filename));
if (desc->flags & _DPRINTK_FLAGS_INCL_LINENO)
- pos += snprintf(buf + pos, remaining(pos), "%d:",
+ len += snprintf(prefix + len, PREFIX_SIZE - len, "%d:",
desc->lineno);
+ if (PREFIX_SIZE - len > 0) {
+ prefix[len++] = ' ';
+ prefix[len] = '\0';
+ }
+
+ /* shrink the prefix to smaller slab? */
+ char *cpy = krealloc(prefix, len + 1, GFP_KERNEL);
+ if (!cpy) {
+ /* krealloc failed, original `prefix` is still valid */
+ pos += snprintf(buf + pos, remaining(pos), "%s", prefix);
+ kfree(prefix);
+ return pos;
+ }
+ /* save the dynamic prefix to cache */
+ mtree_store(&pr_prefixes, (unsigned long)desc, (void *)cpy, GFP_KERNEL);
+ desc->flags |= _DPRINTK_FLAGS_PREFIX_CACHED;
+ v3pr_info("filling prefix cache:%px %s", desc, cpy);
- /* we have a non-empty prefix, add trailing space */
- if (remaining(pos))
- buf[pos++] = ' ';
+ /* copy the newly created prefix to the original stack buffer */
+ pos += snprintf(buf + pos, remaining(pos), "%s", cpy);
return pos;
}
@@ -1532,6 +1577,21 @@ static void ddebug_table_free(struct ddebug_table *dt)
kfree(dt);
}
+#define prefix_is_cached(dp) (dp->flags & _DPRINTK_FLAGS_PREFIX_CACHED)
+
+static void ddebug_drop_cached_prefix(struct _ddebug *dp)
+{
+ char *prefix;
+
+ if (!prefix_is_cached(dp))
+ return;
+ prefix = mtree_erase(&pr_prefixes, (unsigned long)dp);
+ if (prefix) {
+ kfree(prefix);
+ dp->flags &= ~_DPRINTK_FLAGS_PREFIX_CACHED;
+ }
+}
+
#ifdef CONFIG_MODULES
/*
@@ -1546,6 +1606,12 @@ static int ddebug_remove_module(const char *mod_name)
mutex_lock(&ddebug_lock);
list_for_each_entry_safe(dt, nextdt, &ddebug_tables, link) {
if (dt->info.mod_name == mod_name) {
+ int i;
+ struct _ddebug *dp;
+
+ for_subvec(i, dp, &dt->info, descs)
+ ddebug_drop_cached_prefix(dp);
+
ddebug_table_free(dt);
ret = 0;
break;
--
2.52.0
More information about the maple-tree
mailing list