[Intel-gfx] [PATCH v6 40/57] dyndbg: abstraction macros for modname, function, filename fields

Jim Cromie
2022-09-04T21:48:03Z

The .data table built of struct _ddebug[]s has about 40% repetition in
the "decorator" fields: modname, function, filename. The real fix is
to "normalize the db", but 1st we need a layer of accessor macros,
giving us just one field deref to alter later.

So:
define 3 field-abstraction macros: desc_{modname,function,filename}(desc).
replace all raw field refs in lib/dynamic_debug.c with them.

The macros protect against NULL pointer derefs, substituting "_na_"
otherwise. This provides a generic guard, opening options to drop the
contents of __dyndbg_sites[] opportunistically, and trim kernel/module
data. DRM, which could use only .class_id to control drm.debug, could
drop them.

The 3 macros use a common foundation: _ddebug_map_site(), which will
adapt to follow the coming table-split.

Also change field names; adding '_' prefix to insure that bare
field-refs are found and fixed. Most field uses get the macros,
except for dynamic_debug_init(), which will need to follow the
rearrangements.

NOTE: macros are private, not currently for general use.

trace/events/dyndbg.h was a candidate to use these macros, for which I
included the header. On rethink, these macros are wrong abstraction
for tracing; better to expose a dynamic_prefix_emit(1) flavor, and use
it in TP_printk to "decorate" enabled trace-logs like it does for
sys-logs.

So this patch removes the raw field-refs rather than use the macros.
I left the include to mark the intention to use privatish interfaces,
see if it draws objections or compile errs.

[1] fills caller provided char-buffer, perhaps not ideal for tracefs.
A fixed max-size-possible per-callsite (or globally) is practical, but
the struct _ddebug_site * val is probably best; it refs a prdbg's
site-rec of a builtin or loadable module, which is unique over the
pertinent lifetime, and has all the info.

WAG,TLDR: "decorating" could be defered until `cat trace`, modulo
loadable sites[] data being needed to render after module is unloaded.

no functional changes.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>

TP-print-dont-use-desc-foo
---
include/linux/dynamic_debug.h | 12 ++++++------
include/trace/events/dyndbg.h | 7 +++----
lib/dynamic_debug.c | 37 ++++++++++++++++++++++-------------
3 files changed, 32 insertions(+), 24 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 04f49df308a7..e04f5b0a31e2 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -18,9 +18,9 @@ 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;
+ const char *_modname;
+ const char *_function;
+ const char *_filename;
const char *format;
unsigned int lineno:18;
#define CLS_BITS 6
@@ -166,9 +166,9 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
#define DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, cls, fmt) \
static struct _ddebug __aligned(8) \
__section("__dyndbg") name = { \
- .modname = KBUILD_MODNAME, \
- .function = __func__, \
- .filename = __FILE__, \
+ ._modname = KBUILD_MODNAME, \
+ ._function = __func__, \
+ ._filename = __FILE__, \
.format = (fmt), \
.lineno = __LINE__, \
.flags = _DPRINTK_FLAGS_DEFAULT, \
diff --git a/include/trace/events/dyndbg.h b/include/trace/events/dyndbg.h
index e19fcb56566c..2997289c0e3f 100644
--- a/include/trace/events/dyndbg.h
+++ b/include/trace/events/dyndbg.h
@@ -6,6 +6,7 @@
#define _TRACE_DYNDBG_H

#include <linux/tracepoint.h>
+#include <linux/dynamic_debug.h>

/* capture pr_debug() callsite descriptor and message */
TRACE_EVENT(prdbg,
@@ -32,8 +33,7 @@ TRACE_EVENT(prdbg,
__get_str(msg)[len] = 0;
),

- TP_printk("%s.%s %s", __entry->desc->modname,
- __entry->desc->function, __get_str(msg))
+ TP_printk("%s", __get_str(msg))
);

/* capture dev_dbg() callsite descriptor, device, and message */
@@ -64,8 +64,7 @@ TRACE_EVENT(devdbg,
__get_str(msg)[len] = 0;
),

- TP_printk("%s.%s %s", __entry->desc->modname,
- __entry->desc->function, __get_str(msg))
+ TP_printk("%s", __get_str(msg))
);

#endif /* _TRACE_DYNDBG_H */
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 0870e939f255..5a22708679a7 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -167,6 +167,15 @@ static struct ddebug_class_map *ddebug_find_valid_class(struct ddebug_table cons
return NULL;
}

+static inline struct _ddebug_site * _ddebug_map_site(const struct _ddebug *desc)
+{
+ return desc->site;
+}
+#define _desc_field(desc, _fld) (desc ? (_ddebug_map_site(desc)->_fld) : "_na_")
+#define desc_modname(desc) _desc_field(desc, _modname)
+#define desc_function(desc) _desc_field(desc, _function)
+#define desc_filename(desc) _desc_field(desc, _filename)
+
#define __outvar /* filled by callee */
/*
* Search the tables for _ddebug's which match the given `query' and
@@ -212,16 +221,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 */
@@ -257,9 +266,9 @@ static int ddebug_change(const struct ddebug_query *query,
static_branch_enable(&dp->key.dd_key_true);
}
#endif
- v4pr_info("changed %s:%d [%s]%s %s => %s\n",
- trim_prefix(dp->filename), dp->lineno,
- dt->mod_name, dp->function,
+ v4pr_info("changed %s:%d [%s]%s %s to %s\n",
+ 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;
@@ -836,10 +845,10 @@ static char *__dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
pos_after_tid = 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_LINENO)
pos += snprintf(buf + pos, remaining(pos), "%d:",
desc->lineno);
@@ -1241,8 +1250,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, "\"");
@@ -1504,12 +1513,12 @@ static int __init dynamic_debug_init(void)
}

iter = iter_mod_start = __start___dyndbg;
- modname = iter->modname;
+ modname = iter->_modname;
i = mod_sites = mod_ct = 0;

for (; iter < __stop___dyndbg; iter++, i++, mod_sites++) {

- if (strcmp(modname, iter->modname)) {
+ if (strcmp(modname, iter->_modname)) {
mod_ct++;
di.num_descs = mod_sites;
di.descs = iter_mod_start;
@@ -1518,7 +1527,7 @@ static int __init dynamic_debug_init(void)
goto out_err;

mod_sites = 0;
- modname = iter->modname;
+ modname = iter->_modname;
iter_mod_start = iter;
}
}
--
2.37.2