[RFC PATCH 00/10] how to reclaim unneeded vmlinux memory ?

[RFC PATCH 00/10] how to reclaim unneeded vmlinux memory ?

[Jim Cromie]

For builtin modules, dynamic-debug allocates blocks of memory into
DATA, via vmlinux.lds.h.

dyndbg's struct _ddebug has fields: modname, filename, function, which
keeps the the code's structural/organizational info used to enable &
prefix prdbg callsites.

The linker packs the callsites in-order, which means the repetition in
those 3 columns can be compactly encoded in non-overlapping intervals.

So this saves each unique column-val and its interval into a
maple-tree per column, and retrieves them as needed with accessors.

It also splits out _ddebug_site and __dyndbg_sites section, and no
longer needs the section, so that block is ready to reclaim.

Somethings wrong with patch-9, but it seems worth showing around.


Jim Cromie (10):
  dyndbg: prep to isolate 3 repetetive fields
  dyndbg: split __dyndbg_sites section out from __dyndbg
  dyndbg: add 2nd cursor pair to init-fn
  dyndbg: save _ddebug_site mod,file,func fields into maple-trees
  dyndbg: avoid _ddebug.site in ddebug_condense_sites
  dyndbg: add site_*() macros to avoid using _ddebug.site
  dyndbg: wire in __desc_*() functions
  dyndbg: drop _ddebug.site member
  dyndbg: add dd_clear_range to prune mtrees
  dyndbg: cache the dynamically generated prefixes per callsite

 include/asm-generic/vmlinux.lds.h |   1 +
 include/linux/dynamic_debug.h     |  40 +++--
 kernel/module/main.c              |   3 +
 lib/dynamic_debug.c               | 238 +++++++++++++++++++++++++++---
 4 files changed, 252 insertions(+), 30 deletions(-)

-- 
2.41.0

[RFC PATCH 01/10] dyndbg: prep to isolate 3 repetetive fields

[Jim Cromie]

Move 3 fields: modname, filename, function into an anonymous struct,
and rename with '_' prefix to catch stale uses.

Add 3 desc_*() macros to abstract the field refs.

Add DYNAMIC_DEBUG_SITE_INIT() to initialize the fields.

no functional change.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 include/linux/dynamic_debug.h | 22 ++++++++++++++++------
 lib/dynamic_debug.c           | 28 ++++++++++++++--------------
 2 files changed, 30 insertions(+), 20 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 2237d454bc19..aacfafc466c0 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -13,14 +13,17 @@
  * ELF section at every dynamic debug callsite.  At runtime,
  * the special section is treated as an array of these.
  */
+
 struct _ddebug {
 	/*
 	 * These fields are used to drive the user interface
 	 * for selecting and displaying debug callsites.
 	 */
-	const char *modname;
-	const char *function;
-	const char *filename;
+	struct /* _ddebug_site */ {
+		const char *_modname;
+		const char *_function;
+		const char *_filename;
+	};
 	const char *format;
 	unsigned int lineno:18;
 #define CLS_BITS 6
@@ -61,6 +64,10 @@ struct _ddebug {
 #endif
 } __attribute__((aligned(8)));
 
+#define desc_modname(d)		(d)->modname
+#define desc_filename(d)	(d)->filename
+#define desc_function(d)	(d)->function
+
 enum ddebug_class_map_type {
 	DD_CLASS_TYPE_DISJOINT_BITS,
 	/**
@@ -213,12 +220,15 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
 			 const struct ib_device *ibdev,
 			 const char *fmt, ...);
 
+#define DYNAMIC_DEBUG_SITE_INIT()			\
+	._modname = KBUILD_MODNAME,			\
+	._function = __func__,				\
+	._filename = __FILE__
+
 #define DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, cls, fmt)	\
 	static struct _ddebug  __aligned(8)			\
 	__section("__dyndbg") name = {				\
-		.modname = KBUILD_MODNAME,			\
-		.function = __func__,				\
-		.filename = __FILE__,				\
+		DYNAMIC_DEBUG_SITE_INIT(),			\
 		.format = (fmt),				\
 		.lineno = __LINE__,				\
 		.flags = _DPRINTK_FLAGS_DEFAULT,		\
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 3dc512fb1d66..c0e595483cb9 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -235,16 +235,16 @@ static int ddebug_change(const struct ddebug_query *query,
 
 			/* match against the source filename */
 			if (query->filename &&
-			    !match_wildcard(query->filename, dp->filename) &&
+			    !match_wildcard(query->filename, desc_filename(dp)) &&
 			    !match_wildcard(query->filename,
-					   kbasename(dp->filename)) &&
+					    kbasename(desc_filename(dp))) &&
 			    !match_wildcard(query->filename,
-					   trim_prefix(dp->filename)))
+					    trim_prefix(desc_filename(dp))))
 				continue;
 
 			/* match against the function */
 			if (query->function &&
-			    !match_wildcard(query->function, dp->function))
+			    !match_wildcard(query->function, desc_function(dp)))
 				continue;
 
 			/* match against the format */
@@ -281,8 +281,8 @@ static int ddebug_change(const struct ddebug_query *query,
 			}
 #endif
 			v4pr_info("changed %s:%d [%s]%s %s => %s\n",
-				  trim_prefix(dp->filename), dp->lineno,
-				  dt->mod_name, dp->function,
+				  trim_prefix(desc_filename(dp)), dp->lineno,
+				  dt->mod_name, desc_function(dp),
 				  ddebug_describe_flags(dp->flags, &fbuf),
 				  ddebug_describe_flags(newflags, &nbuf));
 			dp->flags = newflags;
@@ -781,13 +781,13 @@ static int __dynamic_emit_prefix(const struct _ddebug *desc, char *buf, int pos)
 {
 	if (desc->flags & _DPRINTK_FLAGS_INCL_MODNAME)
 		pos += snprintf(buf + pos, remaining(pos), "%s:",
-				desc->modname);
+				desc_modname(desc));
 	if (desc->flags & _DPRINTK_FLAGS_INCL_FUNCNAME)
 		pos += snprintf(buf + pos, remaining(pos), "%s:",
-				desc->function);
+				desc_function(desc));
 	if (desc->flags & _DPRINTK_FLAGS_INCL_SOURCENAME)
 		pos += snprintf(buf + pos, remaining(pos), "%s:",
-				trim_prefix(desc->filename));
+				trim_prefix(desc_filename(desc)));
 	return pos;
 }
 
@@ -1110,8 +1110,8 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
 	}
 
 	seq_printf(m, "%s:%u [%s]%s =%s \"",
-		   trim_prefix(dp->filename), dp->lineno,
-		   iter->table->mod_name, dp->function,
+		   trim_prefix(desc_filename(dp)), dp->lineno,
+		   iter->table->mod_name, desc_function(dp),
 		   ddebug_describe_flags(dp->flags, &flags));
 	seq_escape_str(m, dp->format, ESCAPE_SPACE, "\t\r\n\"");
 	seq_puts(m, "\"");
@@ -1528,12 +1528,12 @@ static int __init dynamic_debug_init(void)
 	}
 
 	iter = iter_mod_start = __start___dyndbg;
-	modname = iter->modname;
+	modname = desc_modname(iter);
 	i = mod_sites = mod_ct = 0;
 
 	for (; iter < __stop___dyndbg; iter++, i++, mod_sites++) {
 
-		if (strcmp(modname, iter->modname)) {
+		if (strcmp(modname, desc_modname(iter))) {
 			mod_ct++;
 			di.num_descs = mod_sites;
 			di.descs = iter_mod_start;
@@ -1542,7 +1542,7 @@ static int __init dynamic_debug_init(void)
 				goto out_err;
 
 			mod_sites = 0;
-			modname = iter->modname;
+			modname = desc_modname(iter);
 			iter_mod_start = iter;
 		}
 	}
-- 
2.41.0

[RFC PATCH 02/10] dyndbg: split __dyndbg_sites section out from __dyndbg

[Jim Cromie]

split struct _ddebug_site out from struct _ddebug (adding a site ptr),
and add new __dyndbg_sites section placement to vmlinux.lds.h

This is an implementation detail to isolate the redundant columns into
a separate section, so it specifically excludes lineno.

This allows (later) to copy and compress the info into a better (more
compact) representation thats still fast enough.  Then we can just
reclaim the whole __dyndbg_sites section.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 include/asm-generic/vmlinux.lds.h |  1 +
 include/linux/dynamic_debug.h     | 40 ++++++++++++++++++-------------
 kernel/module/main.c              |  3 +++
 lib/dynamic_debug.c               |  6 +++++
 4 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 5451f926a753..1d128259e373 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -368,6 +368,7 @@
 	BOUNDED_SECTION_BY(__dyndbg_classes, ___dyndbg_classes)		\
 	BOUNDED_SECTION_BY(__dyndbg_class_users, ___dyndbg_class_users)	\
 	BOUNDED_SECTION_BY(__dyndbg, ___dyndbg)				\
+	BOUNDED_SECTION_BY(__dyndbg_sites, ___dyndbg_sites)		\
 	LIKELY_PROFILE()		       				\
 	BRANCH_PROFILE()						\
 	TRACE_PRINTKS()							\
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index aacfafc466c0..5206a2cfdb37 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -9,21 +9,25 @@
 #include <linux/build_bug.h>
 
 /*
- * 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 into 2 special ELF sections for
+ * each pr_debug callsite.  At runtime, the special 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 are used to:
+	 * - display callsites in the control file
+	 * - query/select callsites by the code's organization
+	 * - prefix/decorate pr_debug messages per user choices
 	 */
-	struct /* _ddebug_site */ {
-		const char *_modname;
-		const char *_function;
-		const char *_filename;
-	};
+	const char *_modname;
+	const char *_function;
+	const char *_filename;
+};
+
+struct _ddebug {
+	struct _ddebug_site *site;
 	const char *format;
 	unsigned int lineno:18;
 #define CLS_BITS 6
@@ -64,10 +68,6 @@ struct _ddebug {
 #endif
 } __attribute__((aligned(8)));
 
-#define desc_modname(d)		(d)->modname
-#define desc_filename(d)	(d)->filename
-#define desc_function(d)	(d)->function
-
 enum ddebug_class_map_type {
 	DD_CLASS_TYPE_DISJOINT_BITS,
 	/**
@@ -139,9 +139,11 @@ struct ddebug_class_user {
 /* encapsulate linker provided built-in (or module) dyndbg data */
 struct _ddebug_info {
 	struct _ddebug *descs;
+	struct _ddebug_site *sites;
 	struct ddebug_class_map *classes;
 	struct ddebug_class_user *class_users;
 	unsigned int num_descs;
+	unsigned int num_sites;
 	unsigned int num_classes;
 	unsigned int num_class_users;
 };
@@ -226,9 +228,13 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
 	._filename = __FILE__
 
 #define DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, cls, fmt)	\
+	static struct _ddebug_site  __aligned(8)		\
+	__section("__dyndbg_sites") name ##_site = {		\
+		DYNAMIC_DEBUG_SITE_INIT(),			\
+	};							\
 	static struct _ddebug  __aligned(8)			\
 	__section("__dyndbg") name = {				\
-		DYNAMIC_DEBUG_SITE_INIT(),			\
+		.site = &(name ##_site),			\
 		.format = (fmt),				\
 		.lineno = __LINE__,				\
 		.flags = _DPRINTK_FLAGS_DEFAULT,		\
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 6b0b0d82b5ab..43458184744d 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -2211,6 +2211,9 @@ static int find_module_sections(struct module *mod, struct load_info *info)
 	mod->dyndbg_info.descs = section_objs(info, "__dyndbg",
 					      sizeof(*mod->dyndbg_info.descs),
 					      &mod->dyndbg_info.num_descs);
+	mod->dyndbg_info.sites = section_objs(info, "__dyndbg_sites",
+					      sizeof(*mod->dyndbg_info.sites),
+					      &mod->dyndbg_info.num_sites);
 	mod->dyndbg_info.classes = section_objs(info, "__dyndbg_classes",
 						sizeof(*mod->dyndbg_info.classes),
 						&mod->dyndbg_info.num_classes);
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index c0e595483cb9..0ad9f1bc00f0 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -41,6 +41,8 @@
 
 extern struct _ddebug __start___dyndbg[];
 extern struct _ddebug __stop___dyndbg[];
+extern struct _ddebug_site __start___dyndbg_sites[];
+extern struct _ddebug_site __stop___dyndbg_sites[];
 extern struct ddebug_class_map __start___dyndbg_classes[];
 extern struct ddebug_class_map __stop___dyndbg_classes[];
 extern struct ddebug_class_user __start___dyndbg_class_users[];
@@ -191,6 +193,10 @@ static struct ddebug_class_map *ddebug_find_valid_class(struct ddebug_table cons
 	return NULL;
 }
 
+#define desc_modname(d)		(d)->site->_modname
+#define desc_filename(d)	(d)->site->_filename
+#define desc_function(d)	(d)->site->_function
+
 /*
  * Search the tables for _ddebug's which match the given `query' and
  * apply the `flags' and `mask' to them.  Returns number of matching
-- 
2.41.0

[RFC PATCH 03/10] dyndbg: add 2nd cursor pair to init-fn

[Jim Cromie]

In dynamic_debug_init(), add 2nd cursor pair to walk the
__dyndbg_sites section in parallel with the __dyndbg section.

This avoids using the _ddebug.site pointer during initialization,
which is a 1st step towards dropping the member entirely, and reducing
the struct size and section footprint.

no functional change

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 lib/dynamic_debug.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 0ad9f1bc00f0..51af6a75ae92 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1501,6 +1501,7 @@ static int __init dynamic_debug_init_control(void)
 
 static int __init dynamic_debug_init(void)
 {
+	struct _ddebug_site *site, *site_mod_start;
 	struct _ddebug *iter, *iter_mod_start;
 	int ret, i, mod_sites, mod_ct;
 	const char *modname;
@@ -1508,9 +1509,11 @@ static int __init dynamic_debug_init(void)
 
 	struct _ddebug_info di = {
 		.descs = __start___dyndbg,
+		.sites = __start___dyndbg_sites,
 		.classes = __start___dyndbg_classes,
 		.class_users = __start___dyndbg_class_users,
 		.num_descs = __stop___dyndbg - __start___dyndbg,
+		.num_sites = __stop___dyndbg_sites - __start___dyndbg_sites,
 		.num_classes = __stop___dyndbg_classes - __start___dyndbg_classes,
 		.num_class_users = __stop___dyndbg_class_users - __start___dyndbg_class_users,
 	};
@@ -1533,16 +1536,19 @@ static int __init dynamic_debug_init(void)
 		return 0;
 	}
 
+	site = site_mod_start = di.sites;
 	iter = iter_mod_start = __start___dyndbg;
 	modname = desc_modname(iter);
 	i = mod_sites = mod_ct = 0;
 
-	for (; iter < __stop___dyndbg; iter++, i++, mod_sites++) {
+	for (; iter < __stop___dyndbg; iter++, site++, i++, mod_sites++) {
 
 		if (strcmp(modname, desc_modname(iter))) {
 			mod_ct++;
 			di.num_descs = mod_sites;
+			di.num_sites = mod_sites;
 			di.descs = iter_mod_start;
+			di.sites = site_mod_start;
 			ret = ddebug_add_module(&di, modname);
 			if (ret)
 				goto out_err;
@@ -1550,10 +1556,13 @@ static int __init dynamic_debug_init(void)
 			mod_sites = 0;
 			modname = desc_modname(iter);
 			iter_mod_start = iter;
+			site_mod_start = site;
 		}
 	}
 	di.num_descs = mod_sites;
+	di.num_sites = mod_sites;
 	di.descs = iter_mod_start;
+	di.sites = site_mod_start;
 	ret = ddebug_add_module(&di, modname);
 	if (ret)
 		goto out_err;
-- 
2.41.0

[RFC PATCH 04/10] dyndbg: save _ddebug_site mod,file,func fields into maple-trees

[Jim Cromie]

The __dyndbg_sites section contains the "code organization"
hierarchical callsite info used to select & alter pr_debugs.

The table's 3 columns: modname, filename, function have substantial
repetition; this can be efficiently represented with intervals/ranges.

So this add 3 private maple trees, and ddebug_add_module() now calls
new ddebug_condense_sites() to scan the module's ddebug_sites, find
the 3 intervals, and call dd_store_range() to save them into their
respective trees.

dd_mt_scan() iterates thru these 3 trees and prints to syslog, it is
exposed via /sys/module/dynamic_debug/parameters/do_scan to give some
observability/testability.

3 accessors are coded, but unused atm.

Some Invariants (not all are relevant):

the pr_debugs are strictly "sorted" by linkage order; that is by
module, filename, function, line.  The 1st 3 are pointers into some
string table(s)? somewhere.

repeating column values are identical pointers, not just strcmp equal.

the descriptors (for builtin modules' or a loadable module) are all in
narrow/high memory ranges.  They wont conflict with maple-tree's
0-4096 value constraints.

the string table also has a knowable limited address range, or there
could be 3 tables, one for each column.  The table(s) contents may
also be sorted by linkage order.  Again, this might be useful.

the lineno is not repeating, except for some rare nested macro
expansions (amdgpu iirc, its not important).

NOTES:

I tried, but didnt succeed, to combine all 3 nested intervals into one
tree.  This is what I thought:

ISTM (1/2 baked WAG):

- the 3 columns could be compactly rep'd with run-length-encoding.
- the repeating blocks are intervals.
- the linker guarantees full ordering.
- maple-tree is good at these kind of intervals.

- using &descriptor (align-8) as insertion index provides gaps.
- inserting markers at those gaps allows "nesting" intervals.

The nested intervals of the columns' repetitions could be carefully
packed into a maple-tree, and efficiently retrieved.

So this tries:

adds a maple-tree field: _ddebug_info.sites_mt, to hold a
compressed copy of the .sites table.

ddebug_add_module() calls new ddebug_condense_sites(*_ddebug_info), which

scans the descriptors, finds the spans for each function, and stores
it as a range into the maple-tree, with 1st descriptor as index.

Some of those intervals also start larger filename and module
intervals, we mark these by inserting the file/module value
just "before" the function-range just stored.

This depends upon the
align(8) of all descriptor pointers to afford the -1 and -2 offsets for the modname and filename en

[    2.340903] dyndbg: add-module: i2c_designware_core 10 sites 0.0 classes.refs
[    2.341161] dyndbg:  mod name: i2c_designware_core ffffffff8311a43d
[    2.342045] dyndbg:  file name: drivers/i2c/busses/i2c-designware-common.c ffffffff8311a43e
[    2.342155] dyndbg:  function: i2c_dw_handle_tx_abort, 1 debugs ffffffff8311a440-ffffffff8311a468
[    2.343151] dyndbg:  function: i2c_dw_set_sda_hold, 1 debugs ffffffff8311a468-ffffffff8311a490
[    2.344155] dyndbg:  file name: drivers/i2c/busses/i2c-designware-master.c ffffffff8311a48e
[    2.345150] dyndbg:  function: i2c_dw_init_recovery_info, 1 debugs ffffffff8311a490-ffffffff8311a4b8
[    2.346151] dyndbg:  function: i2c_dw_isr, 1 debugs ffffffff8311a4b8-ffffffff8311a4e0
[    2.347151] dyndbg:  function: i2c_dw_xfer, 1 debugs ffffffff8311a4e0-ffffffff8311a508
[    2.348151] dyndbg:  function: i2c_dw_set_timings_master, 4 debugs ffffffff8311a508-ffffffff8311a5a8
[    2.349165] dyndbg:  file name: drivers/i2c/busses/i2c-designware-slave.c ffffffff8311a5a6
[    2.350152] dyndbg:  function: i2c_dw_isr_slave, 1 debugs ffffffff8311a5a8-ffffffff8311a5d0
[    2.351154] dyndbg:  10 debug prints in module i2c_designware_core

intervals of those called by a

, to
scan the .sites table, and do mtree_insert() to save each new module,
filename, or function found.  The insert index is offset by -N=3,2,1
for the respective columns.

TODO: use mtree_store_range(), with 0 offset, for function blocks.

This does a few things:

- it encodes the kind of interval (function, filename, modname).
- it yields a pre-order, top-down traversal: modname, filename, function

this is ddebug_condense_sites() in action:

[    1.506878] dyndbg: add-module: kobject 10 sites 0.0
[    1.507383] dyndbg:  modname: kobject
[    1.507763] dyndbg:  filename: lib/kobject.c
[    1.507877] dyndbg:  function: kset_release
[    1.508305] dyndbg:  function: dynamic_kobj_release
[    1.508803] dyndbg:  function: kobject_cleanup
[    1.508880] dyndbg:  function: __kobject_del
[    1.509313] dyndbg:  function: kobject_add_internal
[    1.509817] dyndbg:  function: fill_kobj_path
[    1.509882] dyndbg:  10 debug prints in module kobject

[    1.529907] dyndbg: 3424 prdebugs in 307 modules, 19 KiB ....

And heres the tail of the test-mod's do_print traversal of the tree.
kobject is the last builtin, i2c-piix4 is 1st loaded module.
2864 is #intervals, deduplicated from 3424 * 3 fields.
Looks like a 62% shrink, if you squint.

[  142.226760] test_dd:   2854: kobject
[  142.227013] test_dd:   2855: lib/kobject.c
[  142.227285] test_dd:   2856: kset_release
[  142.227568] test_dd:   2857: dynamic_kobj_release
[  142.227885] test_dd:   2858: kobject_cleanup
[  142.228180] test_dd:   2859: __kobject_del
[  142.228456] test_dd:   2860: kobject_add_internal
[  142.228780] test_dd:   2861: fill_kobj_path
[  142.229069] test_dd:   2862: kobject_uevent
[  142.229356] test_dd:   2863: lib/kobject_uevent.c
[  142.229687] test_dd:   2864: kobject_uevent_env
[  142.229998] test_dd:   2865: i2c_piix4
[  142.230253] test_dd:   2866: drivers/i2c/busses/i2c-piix4.c
[  142.230632] test_dd:   2867: piix4_transaction
[  142.230936] test_dd:   2868: piix4_setup_aux
[  142.231227] test_dd:   2869: piix4_setup_sb800
[  142.231527] test_dd:   2870: piix4_setup
[  142.231806] test_dd:   2871: serio_raw
[  142.232066] test_dd:   2872: drivers/input/serio/serio_raw.c
[  142.232447] test_dd:   2873: serio_raw_reconnect
[  142.232769] test_dd:   2874: serio_raw_connect
[  142.233079] test_dd:   2875: intel_rapl_common
[  142.233375] test_dd:   2876: drivers/powercap/intel_rapl_common.c
[  142.233788] test_dd:   2877: rapl_remove_package
[  142.234107] test_dd:   2878: rapl_detect_domains
[  142.234417] test_dd:   2879: rapl_package_register_powercap
[  142.234798] test_dd:   2880: rapl_update_domain_data
[  142.235145] test_dd:   2881: rapl_check_unit_tpmi
[  142.235469] test_dd:   2882: rapl_check_unit_atom
[  142.235804] test_dd:   2883: rapl_check_unit_core
[  142.236131] test_dd:   2884: rapl_read_data_raw
[  142.236445] test_dd:   2885: contraint_to_pl
[  142.236756] test_dd:   2886: intel_rapl_msr
[  142.237053] test_dd:   2887: drivers/powercap/intel_rapl_msr.c
[  142.237446] test_dd:   2888: rapl_msr_probe
[  142.237742] test_dd:   2889: rapl_msr_read_raw
[  142.238048] test_dd:   2890: test_dynamic_debug
[  142.238355] test_dd:   2891: lib/test_dynamic_debug.c
[  142.238720] test_dd:   2892: test_dynamic_debug_exit
[  142.239058] test_dd:   2893: test_dynamic_debug_init
[  142.239395] test_dd:   2894: do_prints
[  142.239660] test_dd:   2895: do_levels
[  142.239924] test_dd:   2896: do_cats
[  142.240168] test_dd:   2897: test_dynamic_debug_submod
[  142.240521] test_dd:   2898: lib/test_dynamic_debug.c
[  142.240871] test_dd:   2899: test_dynamic_debug_exit
[  142.241183] test_dd:   2900: test_dynamic_debug_init
[  142.241484] test_dd:   2901: do_prints
[  142.241725] test_dd:   2902: do_levels
[  142.241962] test_dd:   2903: do_cats

If this proves practical for accessors to traverse well (seems
likely), it will be simple to:

- move the 3 columns to _debug_site, linked from _ddebug
- scan them at boot, copy to the tree
- reclaim the whole memory block,
  which is all ptrs into a (one, or more/segmented) string.symbol table
- store the &tree into struct _ddebug_info,
  for both builtins and modules

Its slightly inconvenient that the fn-ptr is a string, with align(1),
but the "encoding" done in ddebug_condense_sites() could force the
insertion of an end-of-function-array token (marked by N, in &desc -
N).  With N=4, we'd get a marker ahead of module, yielding a proper
"framing".

The tree would be basically read-only for its respective lifetime,
its presumed virtue for this app is primarily size, speed is bonus.

(I probably need to look at advanced api...)

I added the MT_FLAGS_ALLOC_RANGE flag to the tree, thinking maybe I
could stuff the nested interval starters (each distinct function,
filename, modname) into the internal RANGE containing nodes.  Thats
probably nonsense, but it helped me figure out the N option.

(this needs further thinking)

Then magically, an augmented _find would return a vector of modname,
filename, function when needed.  Or maybe a sequence of calls to
mtree_find_upper_range() could collect the internal node values of the
nested intervals {module, filename, function} a descriptor is part of.

And thats about where the souffle collapses.

Any suggestions ?

And how to get the maple-tree size ?

cc: Liam R. Howlett <Liam.Howlett@Oracle.com>
cc: Matthew Wilcox (Oracle) <willy@infradead.org>
cc: linux-mm@kvack.org
Cc: jbaron@akamai.com
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 lib/dynamic_debug.c | 142 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 142 insertions(+)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 51af6a75ae92..3dc17922a1d1 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -25,6 +25,9 @@
 #include <linux/sysctl.h>
 #include <linux/ctype.h>
 #include <linux/string.h>
+
+#include <linux/maple_tree.h>
+
 #include <linux/parser.h>
 #include <linux/string_helpers.h>
 #include <linux/uaccess.h>
@@ -83,6 +86,32 @@ 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)");
 
+/* fill from __dyndbg_sites */
+static DEFINE_MTREE(mt_funcs);
+static DEFINE_MTREE(mt_files);
+static DEFINE_MTREE(mt_mods);
+
+static void dd_mt_scan(struct maple_tree *mt, const char *kind);
+static int param_set_do_scan(const char *instr, const struct kernel_param *kp)
+{
+	dd_mt_scan(&mt_funcs, "funcs");
+	dd_mt_scan(&mt_files, "files");
+	dd_mt_scan(&mt_mods, "mods");
+	return 0;
+}
+static int param_get_do_scan(char *buffer, const struct kernel_param *kp)
+{
+	dd_mt_scan(&mt_funcs, "funcs");
+	dd_mt_scan(&mt_files, "files");
+	dd_mt_scan(&mt_mods, "mods");
+	return scnprintf(buffer, PAGE_SIZE, "did do_prints\n");
+}
+static const struct kernel_param_ops param_ops_do_scan = {
+	.set = param_set_do_scan,
+	.get = param_get_do_scan,
+};
+module_param_cb(do_scan, &param_ops_do_scan, NULL, 0600);
+
 /* Return the path relative to source root */
 static inline const char *trim_prefix(const char *path)
 {
@@ -132,6 +161,7 @@ do {								\
 #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)
 {
@@ -196,6 +226,48 @@ static struct ddebug_class_map *ddebug_find_valid_class(struct ddebug_table cons
 #define desc_modname(d)		(d)->site->_modname
 #define desc_filename(d)	(d)->site->_filename
 #define desc_function(d)	(d)->site->_function
+/*
+ * tmp accessors, they cheat and seek a match in builtins.  Obviously
+ * this wont work for loaded modules, but doesnt work at all yet.
+ */
+static const char * __desc_function(struct _ddebug const *dp)
+{
+	struct maple_tree *mt = &mt_funcs;
+
+	void *ret = mtree_load(mt, (unsigned long)dp);
+
+	if (ret != desc_function(dp))
+		pr_err("mt-load func %lx got %s want %s\n",
+		       (unsigned long)dp, (char*)ret, desc_function(dp));
+
+	return ret;
+}
+
+static const char * __desc_filename(struct _ddebug const *dp)
+{
+	struct maple_tree *mt = &mt_files;
+
+	void *ret = mtree_load(mt, (unsigned long)dp);
+
+	if (ret != desc_filename(dp))
+		pr_err("mt-load file %lx got %s want %s\n",
+		       (unsigned long)dp, (char*)ret, desc_filename(dp));
+
+	return ret;
+}
+
+static const char * __desc_modname(struct _ddebug const *dp)
+{
+	struct maple_tree *mt = &mt_mods;
+
+	void *ret = mtree_load(mt, (unsigned long)dp);
+
+	if (ret != desc_modname(dp))
+		pr_err("mt-load mod %lx got %s want %s\n",
+		       (unsigned long)dp, (char*)ret, desc_modname(dp));
+
+	return ret;
+}
 
 /*
  * Search the tables for _ddebug's which match the given `query' and
@@ -1314,6 +1386,74 @@ static void ddebug_attach_user_module_classes(struct ddebug_table *dt,
 	vpr_dt_info(dt, "attach-client-module: ");
 }
 
+/*
+ * use a maple-tree to hold the intervals in all 3 columns of the
+ * __dyndbg_sites section (struct _ddebug_sites table[])
+ */
+
+static void dd_mt_scan(struct maple_tree *mt, const char *kind)
+{
+	long unsigned int idx = 0;
+	void * ent;
+	int ct = 0;
+
+	mt_for_each(mt, ent, idx, ULONG_MAX) {
+		v3pr_info("  %d: %lx %s\n", ct, idx, (char*)ent);
+		ct++;
+	}
+	v2pr_info("mt-%s has %d entries\n", kind, ct);
+}
+
+static void dd_store_range(struct maple_tree *mt, const struct _ddebug *start,
+			   const struct _ddebug *next, const char *kind, const char *name)
+{
+	unsigned long first = (unsigned long)start;
+	unsigned long last = (unsigned long)(next - 1); /* cast after decrement */
+	int rc, reps = next - start;
+	char *val;
+
+	v3pr_info("  %s: %s,\t%d debugs %lx-%lx\n", kind, name, reps, first, last);
+	rc = mtree_insert_range(mt, first, last, (void*)name, GFP_KERNEL);
+	if (rc)
+		pr_err("%s:%s range store failed: %d\n", kind, name, rc);
+	else
+		v4pr_info("  OK %s: %s, %d debugs %lx-%lx\n", kind, name, reps, first, last);
+
+	val = (char*) mtree_load(mt, first);
+	if (!val)
+		pr_err("%s:%s find on range store failed\n", kind, name);
+	else
+		v4pr_info("  ok %s at %lx\n", val, first);
+}
+
+static void ddebug_condense_sites(struct _ddebug_info *di)
+{
+	struct _ddebug *cur, *funcp, *filep, *modp;
+	int i;
+
+	funcp = filep = modp = di->descs;
+	for_each_boxed_vector(di, descs, num_descs, i, cur) {
+
+		if (!strcmp(desc_function(cur), desc_function(funcp)))
+			continue;
+		dd_store_range(&mt_funcs, funcp, cur, "func", desc_function(funcp));
+		funcp = cur;
+
+		if (!strcmp(desc_filename(cur), desc_filename(filep)))
+			continue;
+		dd_store_range(&mt_files, filep, cur, "file", desc_filename(filep));
+		filep = cur;
+
+		if (!strcmp(desc_modname(cur), desc_modname(modp)))
+			continue;
+		dd_store_range(&mt_mods, modp, cur, "mod", desc_modname(modp));
+		modp = cur;
+	}
+	dd_store_range(&mt_funcs, funcp, cur, "func:", desc_function(funcp));
+	dd_store_range(&mt_files, filep, cur, "file:", desc_filename(filep));
+	dd_store_range(&mt_mods, modp, cur, "mod:", desc_modname(modp));
+}
+
 /*
  * Allocate a new ddebug_table for the given module
  * and add it to the global list.
@@ -1346,6 +1486,8 @@ static int ddebug_add_module(struct _ddebug_info *di, const char *modname)
 
 	INIT_LIST_HEAD(&dt->link);
 
+	ddebug_condense_sites(di);
+
 	for_each_boxed_vector(di, descs, num_descs, i, iter)
 		if (iter->class_id != _DPRINTK_CLASS_DFLT)
 			class_ct++;
-- 
2.41.0

[RFC PATCH 05/10] dyndbg: avoid _ddebug.site in ddebug_condense_sites

[Jim Cromie]

Modify the for-loop in ddebug_condense_sites() to walk thru both
vectors: descs, sites in parallel.

This requires a 2nd set of cursor variables (*_ds) that mark the
start-of-range in the sites vector for the intervals to be
dd_store_range()d.  So also rename the old cursors (*_dd) for better
consistency and readability.

This is a partial step.  It still uses the desc_*() macros to provide
the values, and the macros use the site pointer.  Next, we replace the
macros with site_*(), passing the *_ds vars.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 lib/dynamic_debug.c | 39 ++++++++++++++++++++++++---------------
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 3dc17922a1d1..563d373224ba 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1426,32 +1426,41 @@ static void dd_store_range(struct maple_tree *mt, const struct _ddebug *start,
 		v4pr_info("  ok %s at %lx\n", val, first);
 }
 
+
 static void ddebug_condense_sites(struct _ddebug_info *di)
 {
-	struct _ddebug *cur, *funcp, *filep, *modp;
+	struct _ddebug_site *cur_ds, *func_ds, *file_ds, *mod_ds;
+	struct _ddebug      *cur_dd, *func_dd, *file_dd, *mod_dd;
 	int i;
 
-	funcp = filep = modp = di->descs;
-	for_each_boxed_vector(di, descs, num_descs, i, cur) {
+	cur_dd = func_dd = file_dd = mod_dd = di->descs;
+	cur_ds = func_ds = file_ds = mod_ds = di->sites;
+	i = 0;
+	for (; i < di->num_descs; i++, cur_dd++, cur_ds++) {
+
+		BUG_ON(site_function(cur_ds) != desc_function(cur_dd));
 
-		if (!strcmp(desc_function(cur), desc_function(funcp)))
+		if (!strcmp(desc_function(cur_dd), desc_function(func_dd)))
 			continue;
-		dd_store_range(&mt_funcs, funcp, cur, "func", desc_function(funcp));
-		funcp = cur;
+		dd_store_range(&mt_funcs, func_dd, cur_dd, "func", desc_function(func_dd));
+		func_dd = cur_dd;
+		func_ds = cur_ds;
 
-		if (!strcmp(desc_filename(cur), desc_filename(filep)))
+		if (!strcmp(desc_filename(cur_dd), desc_filename(file_dd)))
 			continue;
-		dd_store_range(&mt_files, filep, cur, "file", desc_filename(filep));
-		filep = cur;
+		dd_store_range(&mt_files, file_dd, cur_dd, "file", desc_filename(file_dd));
+		file_dd = cur_dd;
+		file_ds = cur_ds;
 
-		if (!strcmp(desc_modname(cur), desc_modname(modp)))
+		if (!strcmp(desc_modname(cur_dd), desc_modname(mod_dd)))
 			continue;
-		dd_store_range(&mt_mods, modp, cur, "mod", desc_modname(modp));
-		modp = cur;
+		dd_store_range(&mt_mods, mod_dd, cur_dd, "mod", desc_modname(mod_dd));
+		mod_dd = cur_dd;
+		mod_ds = cur_ds;
 	}
-	dd_store_range(&mt_funcs, funcp, cur, "func:", desc_function(funcp));
-	dd_store_range(&mt_files, filep, cur, "file:", desc_filename(filep));
-	dd_store_range(&mt_mods, modp, cur, "mod:", desc_modname(modp));
+	dd_store_range(&mt_funcs, func_dd, cur_dd, "func:", desc_function(func_dd));
+	dd_store_range(&mt_files, file_dd, cur_dd, "file:", desc_filename(file_dd));
+	dd_store_range(&mt_mods, mod_dd, cur_dd, "mod:", desc_modname(mod_dd));
 }
 
 /*
-- 
2.41.0

[RFC PATCH 06/10] dyndbg: add site_*() macros to avoid using _ddebug.site

[Jim Cromie]

adjust ddebug_condense_sites() and dynamic_debug_init() to replace
desc_*() uses with new site_*() macros which avoid the _ddebug.site
pointers by relying upon site* cursors instead.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 lib/dynamic_debug.c | 29 ++++++++++++++++-------------
 1 file changed, 16 insertions(+), 13 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 563d373224ba..48564625a37e 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1426,6 +1426,9 @@ static void dd_store_range(struct maple_tree *mt, const struct _ddebug *start,
 		v4pr_info("  ok %s at %lx\n", val, first);
 }
 
+#define site_function(s)	(s)->_function
+#define site_filename(s)	(s)->_filename
+#define site_modname(s)		(s)->_modname
 
 static void ddebug_condense_sites(struct _ddebug_info *di)
 {
@@ -1440,27 +1443,27 @@ static void ddebug_condense_sites(struct _ddebug_info *di)
 
 		BUG_ON(site_function(cur_ds) != desc_function(cur_dd));
 
-		if (!strcmp(desc_function(cur_dd), desc_function(func_dd)))
+		if (!strcmp(site_function(cur_ds), site_function(func_ds)))
 			continue;
-		dd_store_range(&mt_funcs, func_dd, cur_dd, "func", desc_function(func_dd));
+		dd_store_range(&mt_funcs, func_dd, cur_dd, "func", site_function(func_ds));
 		func_dd = cur_dd;
 		func_ds = cur_ds;
 
-		if (!strcmp(desc_filename(cur_dd), desc_filename(file_dd)))
+		if (!strcmp(site_filename(cur_ds), site_filename(file_ds)))
 			continue;
-		dd_store_range(&mt_files, file_dd, cur_dd, "file", desc_filename(file_dd));
+		dd_store_range(&mt_files, file_dd, cur_dd, "file", site_filename(file_ds));
 		file_dd = cur_dd;
 		file_ds = cur_ds;
 
-		if (!strcmp(desc_modname(cur_dd), desc_modname(mod_dd)))
+		if (!strcmp(site_modname(cur_ds), site_modname(mod_ds)))
 			continue;
-		dd_store_range(&mt_mods, mod_dd, cur_dd, "mod", desc_modname(mod_dd));
+		dd_store_range(&mt_mods, mod_dd, cur_dd, "mod", site_modname(mod_ds));
 		mod_dd = cur_dd;
 		mod_ds = cur_ds;
 	}
-	dd_store_range(&mt_funcs, func_dd, cur_dd, "func:", desc_function(func_dd));
-	dd_store_range(&mt_files, file_dd, cur_dd, "file:", desc_filename(file_dd));
-	dd_store_range(&mt_mods, mod_dd, cur_dd, "mod:", desc_modname(mod_dd));
+	dd_store_range(&mt_funcs, func_dd, cur_dd, "func:", site_function(func_ds));
+	dd_store_range(&mt_files, file_dd, cur_dd, "file:", site_filename(file_ds));
+	dd_store_range(&mt_mods, mod_dd, cur_dd, "mod:", site_modname(mod_ds));
 }
 
 /*
@@ -1688,13 +1691,13 @@ static int __init dynamic_debug_init(void)
 	}
 
 	site = site_mod_start = di.sites;
-	iter = iter_mod_start = __start___dyndbg;
-	modname = desc_modname(iter);
+	iter = iter_mod_start = di.descs;
+	modname = site_modname(site);
 	i = mod_sites = mod_ct = 0;
 
 	for (; iter < __stop___dyndbg; iter++, site++, i++, mod_sites++) {
 
-		if (strcmp(modname, desc_modname(iter))) {
+		if (strcmp(modname, site_modname(site))) {
 			mod_ct++;
 			di.num_descs = mod_sites;
 			di.num_sites = mod_sites;
@@ -1705,7 +1708,7 @@ static int __init dynamic_debug_init(void)
 				goto out_err;
 
 			mod_sites = 0;
-			modname = desc_modname(iter);
+			modname = site_modname(site);
 			iter_mod_start = iter;
 			site_mod_start = site;
 		}
-- 
2.41.0

[RFC PATCH 07/10] dyndbg: wire in __desc_*() functions

[Jim Cromie]

change desc_*() macros to call __desc_*() functions, and #if 0 the
comparisons to the site->_* ref.  This makes the _ddebug.site pointer
unneccesary.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 lib/dynamic_debug.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 48564625a37e..fb72a7b05b01 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -223,9 +223,9 @@ static struct ddebug_class_map *ddebug_find_valid_class(struct ddebug_table cons
 	return NULL;
 }
 
-#define desc_modname(d)		(d)->site->_modname
-#define desc_filename(d)	(d)->site->_filename
-#define desc_function(d)	(d)->site->_function
+#define desc_modname(d)		__desc_modname(d)
+#define desc_filename(d)	__desc_filename(d)
+#define desc_function(d)	__desc_function(d)
 /*
  * tmp accessors, they cheat and seek a match in builtins.  Obviously
  * this wont work for loaded modules, but doesnt work at all yet.
@@ -235,11 +235,11 @@ static const char * __desc_function(struct _ddebug const *dp)
 	struct maple_tree *mt = &mt_funcs;
 
 	void *ret = mtree_load(mt, (unsigned long)dp);
-
+#if 0
 	if (ret != desc_function(dp))
 		pr_err("mt-load func %lx got %s want %s\n",
 		       (unsigned long)dp, (char*)ret, desc_function(dp));
-
+#endif
 	return ret;
 }
 
@@ -248,11 +248,11 @@ static const char * __desc_filename(struct _ddebug const *dp)
 	struct maple_tree *mt = &mt_files;
 
 	void *ret = mtree_load(mt, (unsigned long)dp);
-
+#if 0
 	if (ret != desc_filename(dp))
 		pr_err("mt-load file %lx got %s want %s\n",
 		       (unsigned long)dp, (char*)ret, desc_filename(dp));
-
+#endif
 	return ret;
 }
 
@@ -261,11 +261,11 @@ static const char * __desc_modname(struct _ddebug const *dp)
 	struct maple_tree *mt = &mt_mods;
 
 	void *ret = mtree_load(mt, (unsigned long)dp);
-
+#if 0
 	if (ret != desc_modname(dp))
 		pr_err("mt-load mod %lx got %s want %s\n",
 		       (unsigned long)dp, (char*)ret, desc_modname(dp));
-
+#endif
 	return ret;
 }
 
@@ -1441,7 +1441,7 @@ static void ddebug_condense_sites(struct _ddebug_info *di)
 	i = 0;
 	for (; i < di->num_descs; i++, cur_dd++, cur_ds++) {
 
-		BUG_ON(site_function(cur_ds) != desc_function(cur_dd));
+		//BUG_ON(site_function(cur_ds) != desc_function(cur_dd));
 
 		if (!strcmp(site_function(cur_ds), site_function(func_ds)))
 			continue;
-- 
2.41.0

[RFC PATCH 08/10] dyndbg: drop _ddebug.site member

[Jim Cromie]

drop site ptr, and rely upon the maple-tree intervals loaded by
ddebug_condense_sites().

In DEFINE_DYNAMIC_DEBUG_METADATA_CLS(), which creates a pair of
_ddebug & _ddebug_site structs, add '__used' to the latter, because it
is no longer linked from the former, and then also to the former, so
they both have the same number of entries.  Replace the BUG_ON with a
pr_warn that shows the size of the mismatch.

TODO: sort out where the extra records are coming from.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 include/linux/dynamic_debug.h | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 5206a2cfdb37..406b30d8eb98 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -27,7 +27,6 @@ struct _ddebug_site {
 };
 
 struct _ddebug {
-	struct _ddebug_site *site;
 	const char *format;
 	unsigned int lineno:18;
 #define CLS_BITS 6
@@ -228,13 +227,12 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
 	._filename = __FILE__
 
 #define DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, cls, fmt)	\
-	static struct _ddebug_site  __aligned(8)		\
+	static struct _ddebug_site  __used __aligned(8) 	\
 	__section("__dyndbg_sites") name ##_site = {		\
 		DYNAMIC_DEBUG_SITE_INIT(),			\
 	};							\
-	static struct _ddebug  __aligned(8)			\
+	static struct _ddebug  __used __aligned(8)		\
 	__section("__dyndbg") name = {				\
-		.site = &(name ##_site),			\
 		.format = (fmt),				\
 		.lineno = __LINE__,				\
 		.flags = _DPRINTK_FLAGS_DEFAULT,		\
-- 
2.41.0

[RFC PATCH 09/10] dyndbg: add dd_clear_range to prune mtrees

[Jim Cromie]

Call new dd_clear_range() from ddebug_remove_module().  It calls
mtree_erase() on the trees storing the function, filename, modname
intervals, and passing the 1st descriptor of the interval (ie the
index used on the insert).

dd_clear_range() should properly undo the 3 mtree_insert_ranges done
by dd_store_range.

RFC: it doesnt work as I expected. What am I missing ?

The following log shows that 'rmmod amdgpu' only removes 1 entry from
each maple-tree, not the whole interval.  My index is the 1st
descriptor in each interval.

ISTM (naive reader) this contradicts the documented behavior.

  void *mtree_erase(struct maple_tree *mt, unsigned long index)
  Find an index and erase the entire range.

what is my "entire range" ?

bash-5.2# modprobe amdgpu
....
[   74.256006] dyndbg: attach-client-module:  module:amdgpu nd:4652 nc:0 nu:1
[   74.256968] dyndbg: 4652 debug prints in module amdgpu

bash-5.2# echo 2 > /sys/module/dynamic_debug/parameters/do_scan
[   81.370509] dyndbg: cache: funcs has 3741 entries
[   81.371233] dyndbg: cache: files has 911 entries
[   81.371819] dyndbg: cache: mods has 323 entries

bash-5.2# rmmod amdgpu
[  102.325851] dyndbg: removed module "amdgpu"

bash-5.2# echo 2 > /sys/module/dynamic_debug/parameters/do_scan
[  105.277439] dyndbg: cache: funcs has 3740 entries
[  105.278163] dyndbg: cache: files has 910 entries
[  105.278756] dyndbg: cache: mods has 322 entries

cc: Liam R. Howlett <Liam.Howlett@Oracle.com>
cc: linux-mm@kvack.org
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 lib/dynamic_debug.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index fb72a7b05b01..92ffd70a07de 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1426,6 +1426,14 @@ static void dd_store_range(struct maple_tree *mt, const struct _ddebug *start,
 		v4pr_info("  ok %s at %lx\n", val, first);
 }
 
+static void dd_clear_range(const struct _ddebug *start)
+{
+	v3pr_info("clearing %px\n", start);
+	mtree_erase(&mt_funcs, (unsigned long)start);
+	mtree_erase(&mt_files, (unsigned long)start);
+	mtree_erase(&mt_mods, (unsigned long)start);
+}
+
 #define site_function(s)	(s)->_function
 #define site_filename(s)	(s)->_filename
 #define site_modname(s)		(s)->_modname
@@ -1578,6 +1586,8 @@ static int ddebug_remove_module(const char *mod_name)
 	mutex_lock(&ddebug_lock);
 	list_for_each_entry_safe(dt, nextdt, &ddebug_tables, link) {
 		if (dt->mod_name == mod_name) {
+			/* free mtree entries on descs */
+			dd_clear_range(dt->ddebugs);
 			ddebug_table_free(dt);
 			ret = 0;
 			break;
-- 
2.41.0

[RFC PATCH 10/10] dyndbg: cache the dynamically generated prefixes per callsite

[Jim Cromie]

Add a private maple-tree to cache any prefix strings required (and
then generated) for any enabled pr_debug callsites.  And delete any
cache entries if any flags are changed afterwards.

This cache is strictly per-callsite, so if a function has 20
pr_debugs, all enabled with the same flags:

  echo function foo +pmfs > /proc/dynamic_debug/control

there will be 20 separate, identical cache items created.
Maybe this can be trivially improved later.
Or lineno could be folded in too, so the %d is rendered once.

NB: +tl flags are added outside the cache; the thread-id doesnt belong
in the cache, the lineno could be added, esp if the 20:1 sharing isnt
trivial enough.

NBB: lineno:18 is enormously "sparse".  At this point its not so far
to pack it into something else.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 lib/dynamic_debug.c | 27 ++++++++++++++++++++++++++-
 1 file changed, 26 insertions(+), 1 deletion(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 92ffd70a07de..02df61ab6403 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -90,6 +90,8 @@ MODULE_PARM_DESC(verbose, " dynamic_debug/control processing "
 static DEFINE_MTREE(mt_funcs);
 static DEFINE_MTREE(mt_files);
 static DEFINE_MTREE(mt_mods);
+/* cache of composed prefixes for enabled pr_debugs */
+static DEFINE_MTREE(pr_prefixes);
 
 static void dd_mt_scan(struct maple_tree *mt, const char *kind);
 static int param_set_do_scan(const char *instr, const struct kernel_param *kp)
@@ -112,6 +114,11 @@ static const struct kernel_param_ops param_ops_do_scan = {
 };
 module_param_cb(do_scan, &param_ops_do_scan, NULL, 0600);
 
+void ddebug_clear_prefix_cache(const struct _ddebug *dp)
+{
+	mtree_erase(&pr_prefixes, (unsigned long)dp);
+}
+
 /* Return the path relative to source root */
 static inline const char *trim_prefix(const char *path)
 {
@@ -350,6 +357,7 @@ static int ddebug_change(const struct ddebug_query *query,
 			newflags = (dp->flags & modifiers->mask) | modifiers->flags;
 			if (newflags == dp->flags)
 				continue;
+			ddebug_clear_prefix_cache(dp);
 #ifdef CONFIG_JUMP_LABEL
 			if (dp->flags & _DPRINTK_FLAGS_PRINT) {
 				if (!(newflags & _DPRINTK_FLAGS_PRINT))
@@ -855,8 +863,18 @@ static int remaining(int wrote)
 	return 0;
 }
 
-static int __dynamic_emit_prefix(const struct _ddebug *desc, char *buf, int pos)
+static int __dynamic_emit_prefix(struct _ddebug *desc, char *buf, int pos)
 {
+	char *prefix, *cpy;
+
+	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", buf, buf + pos);
+			return pos;
+		}
+	}
 	if (desc->flags & _DPRINTK_FLAGS_INCL_MODNAME)
 		pos += snprintf(buf + pos, remaining(pos), "%s:",
 				desc_modname(desc));
@@ -866,6 +884,13 @@ static int __dynamic_emit_prefix(const struct _ddebug *desc, char *buf, int pos)
 	if (desc->flags & _DPRINTK_FLAGS_INCL_SOURCENAME)
 		pos += snprintf(buf + pos, remaining(pos), "%s:",
 				trim_prefix(desc_filename(desc)));
+
+	/* save dup of buf to cache */
+	cpy = kstrdup(buf + pos, GFP_KERNEL);
+	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);
+
 	return pos;
 }
 
-- 
2.41.0

[RFC PATCH 07/10] dyndbg: wire in __desc_*() functions

[Jim Cromie]

change desc_*() macros to call __desc_*() functions, and #if 0 the
comparisons to the site->_* ref.  This makes the _ddebug.site pointer
unneccesary.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 lib/dynamic_debug.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 48564625a37e..fb72a7b05b01 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -223,9 +223,9 @@ static struct ddebug_class_map *ddebug_find_valid_class(struct ddebug_table cons
 	return NULL;
 }
 
-#define desc_modname(d)		(d)->site->_modname
-#define desc_filename(d)	(d)->site->_filename
-#define desc_function(d)	(d)->site->_function
+#define desc_modname(d)		__desc_modname(d)
+#define desc_filename(d)	__desc_filename(d)
+#define desc_function(d)	__desc_function(d)
 /*
  * tmp accessors, they cheat and seek a match in builtins.  Obviously
  * this wont work for loaded modules, but doesnt work at all yet.
@@ -235,11 +235,11 @@ static const char * __desc_function(struct _ddebug const *dp)
 	struct maple_tree *mt = &mt_funcs;
 
 	void *ret = mtree_load(mt, (unsigned long)dp);
-
+#if 0
 	if (ret != desc_function(dp))
 		pr_err("mt-load func %lx got %s want %s\n",
 		       (unsigned long)dp, (char*)ret, desc_function(dp));
-
+#endif
 	return ret;
 }
 
@@ -248,11 +248,11 @@ static const char * __desc_filename(struct _ddebug const *dp)
 	struct maple_tree *mt = &mt_files;
 
 	void *ret = mtree_load(mt, (unsigned long)dp);
-
+#if 0
 	if (ret != desc_filename(dp))
 		pr_err("mt-load file %lx got %s want %s\n",
 		       (unsigned long)dp, (char*)ret, desc_filename(dp));
-
+#endif
 	return ret;
 }
 
@@ -261,11 +261,11 @@ static const char * __desc_modname(struct _ddebug const *dp)
 	struct maple_tree *mt = &mt_mods;
 
 	void *ret = mtree_load(mt, (unsigned long)dp);
-
+#if 0
 	if (ret != desc_modname(dp))
 		pr_err("mt-load mod %lx got %s want %s\n",
 		       (unsigned long)dp, (char*)ret, desc_modname(dp));
-
+#endif
 	return ret;
 }
 
@@ -1441,7 +1441,7 @@ static void ddebug_condense_sites(struct _ddebug_info *di)
 	i = 0;
 	for (; i < di->num_descs; i++, cur_dd++, cur_ds++) {
 
-		BUG_ON(site_function(cur_ds) != desc_function(cur_dd));
+		//BUG_ON(site_function(cur_ds) != desc_function(cur_dd));
 
 		if (!strcmp(site_function(cur_ds), site_function(func_ds)))
 			continue;
-- 
2.41.0