lkml.org 
[lkml]   [2021]   [Sep]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Subject[PATCH v8 08/16] dyndbg: add DEFINE_DYNAMIC_DEBUG_CATEGORIES bitmap and callbacks
Date
DEFINE_DYNAMIC_DEBUG_CATEGORIES(fsname, var, bitmap_desc, @bit_descs)
allows users to create a drm.debug style (bitmap) sysfs interface,
mapping each bit to a pr_debug "category".

Unlike drm, dyndbg has no coding of "category", but it can select a
set of pr_debugs with a substr match on their formats. Since many
pr_debugs use a system of format prefixes, dyndbg can en/disable those
existing groupings as sets/categories.

the new macro's categories are used like so:

echo format '^$category ' +/-p >control

DEFINE_DYNAMIC_DEBUG_CATEGORIES(debug_gvt, __gvt_debug,
"i915/gvt bitmap desc",
/*
* map each bit to a category
* helper macro adds ^ prefix and ' ' suffix seen above
*/
_DD_cat_(0, "gvt:cmd:"),
_DD_cat_(1, "gvt:core:"),
_DD_cat_(2, "gvt:dpy:"),
_DD_cat_(3, "gvt:el:"),
_DD_cat_(4, "gvt:irq:"),
_DD_cat_(5, "gvt:mm:"),
_DD_cat_(6, "gvt:mmio:"),
_DD_cat_(7, "gvt:render:"),
_DD_cat_(8, "gvt:sched:"));

- int param_set_dyndbg()
- int param_get_dyndbg()
- struct kernel_param_ops param_ops_dyndbg

Following the model of kernel/params.c STANDARD_PARAM_DEFS, these are
non-static and exported.

get/set use an augmented kernel_param; the arg refs a new struct
dyndbg_bitmap_param containing:

A- the map (array, indexed by bitpos) of "categories" (strings to find
in prdbg.formats). These define the set/category of callsites per bit.

B- a pointer to the user's ulong holding the bits/state.
by sharing state, we coordinate with code that still uses it directly

B allows drm-debug api to be converted incrementally, while still
using __drm_debug & drm_debug_enabled() in other parts.

param_set_dyndbg() compares new vs old bits, and only updates prdbgs
on changes. This maximally preserves the underlying state, which may
have been customized via later `echo $cmd >control`. So if a user
really wants to know that all prdbgs are set precisely, they must
pre-clear then set.

dynamic_debug.h:

Add DEFINE_DYNAMIC_DEBUG_CATEGORIES() described above, and a stub
throwing a BUILD_BUG (RFC) when used without DYNAMIC_DEBUG support.

Add structs dyndbg_bitdesc, dyndbg_bitmap_param to support the main
macro, and several helper macros wrapping the given categories with
^prefix and ' ' suffix. This way the callback can be more broadly
used, by using the right helper macro.

Also externs the struct kernel_param param_ops_dyndbg symbol, as is
done in moduleparams.h for all the STANDARD params.

USAGE NOTES:

Using dyndbg to query on "format ^$prefix" requires that $prefix must be
present in the compiled-in format string. Searching on "%s" does not
define a useful set of callsites.

Using DEFINE_DYNAMIC_DEBUG_CATEGORIES wo support gets a BUILD_BUG.
ISTM there is already action at a declarative distance, nobody needs
mystery as to why the /sysfs thingy didn't appear.

Dyndbg is completely agnostic wrt the categorization scheme used, in
order to play well with any prefix convention already in use in the
codebase. Ad-hoc categories and sub-categories are implicitly
allowed, author discipline and review is expected.

In fact, the ^anchor is not required; "format $floating_substr" is the
default meaning. The _DD_cat_() helper macro adds the ^anchor, and a
trailing space (see below for why).

Here are some examples: (these all need ^anchor to be practical)

"^1","^2","^3" 2 doesn't imply 1.
otherwize, sorta like printk levels
"^1:","^2:","^3:" are better, avoiding [1-9]\d+ ambiguity
"^hi:","^mid:","^low:" are reasonable, and imply independence
"^todo:","^rfc:" might be handy
"^A:".."^Z:" uhm, yeah

Hierarchical classes/categories are natural:

"^drm:<CAT>:" is used in a later commit
"^drm:<CAT>:<SUB>:" is a natural extension.
"^drm:atomic:fail:" has been proposed, sounds directly useful

High density "punctuation" tld classification is possible:
"^.","^:","^!","^#","^@", etc.

NB: in a real sense we abandon enum strictures here, and lose some
compiler help, on spelling errs for example. Obviously "drm:" != "DRM:".

Some properties of a hierarchical category deserve explication:

Trailing spaces matter !

With 1..3-space ("drm: ", "drm:atomic: ", "drm:atomic:fail: "), the
":" doesn't terminate the search-space, the trailing space does. So a
"drm:" search spec will match all DRM categories & subcategories, and
will not be useful in an interface where all categories are already
controlled together. That said, "drm:atomic:" & "drm:atomic: " are
different, and both are useful in cases.

Ad-Hoc categories & sub-categories:

Ad-hoc categories are those format-prefixes already in use; both
amdgpu and i915 have numerous (120,~1800) pr_debugs, most of these use
a system, a small set (9,13) of prefixes, to categorize the output.
Dyndbg already works on these, this patch just adds a bitmap knob.

ad-hoc sub-categories are slightly trickier.
If drm_dbg_atomic("fail: ...") is a macro:
pr_debug("drm:atomic:" " " format,...) // cpp-paste in a trailing space

We get "drm:atomic: fail:", with that undesirable embedded space;
obviously not ideal wrt clear and simple prefixes.

a fix: drm_dbg_atomic_("fail: ..."); // trailing _ for ad-hoc subcat

Summarizing:

- "drm:kms: " & "drm:kms:" are different
- "drm:kms" also different - includes drm:kms2:
- "drm:kms:\t" also different - could be troublesome
- "drm:kms:*" doesn't work, no wildcard on format atm.

Order matters in DEFINE_DYNAMIC_DEBUG_CATEGORIES(... @bit_descs)

Since bits are/will-stay applied 0-N, the later bits can countermand
the earlier ones, but it is tricky - consider;

DD_CATs(... "drm:atomic:", "drm:atomic:fail:" ) // misleading

The 1st search-term is misleading, because it includes (modifies)
subcategories, but then 2nd overrides it. So don't do that.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
v5:
. rename to DEFINE_DYNAMIC_DEBUG_CATEGORIES from DEFINE_DYNDBG_BITMAP
. in set_dyndbg, replace hardcoded "i915" w kp->mod->name
. static inline the stubs
. const *str in structs, const array. - Emil
. dyndbg: add do-nothing DEFINE_DYNAMIC_DEBUG_CATEGORIES if !DD_CORE
. call MOD_PARM_DESC(name, "$desc") for users
. simplify callback, remove bit-change detection
. config errs reported by <lkp@intel.com>

v6:
. return rc, bitmap->, snprintf, ws - Andy Shevchenko
. s/chgct/matches/ - old varname is misleading
. move code off file bottom to a "better" place
. change ##fsname to ##var for less generic varname (ie: not "debug")
. add KP_MOD_NAME workaround for !CONFIG_MODULES
. move forward decl down to where its needed

v7:
. use kp->arg, dont need kp->data or previous patch-1 - jbaron
. use client's ulong for bits, share state
. throw BUILD_BUG if DEFINE_DYNAMIC_DEBUG_CATEGORIES used wo support

v8:
. drop help field from dyndbg_bitdesc
. move ^ constraint from callback to helper macro: _DD_cat_
. add other helper macros
---
include/linux/dynamic_debug.h | 62 ++++++++++++++++++++++++++++
lib/dynamic_debug.c | 76 +++++++++++++++++++++++++++++++++++
2 files changed, 138 insertions(+)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index dce631e678dd..233e3cddf6de 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -181,6 +181,10 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
KERN_DEBUG, prefix_str, prefix_type, \
rowsize, groupsize, buf, len, ascii)

+struct kernel_param;
+int param_set_dyndbg(const char *instr, const struct kernel_param *kp);
+int param_get_dyndbg(char *buffer, const struct kernel_param *kp);
+
#else /* !CONFIG_DYNAMIC_DEBUG_CORE */

#include <linux/string.h>
@@ -227,6 +231,64 @@ static inline int dynamic_debug_exec_queries(const char *query, const char *modn
return 0;
}

+struct kernel_param;
+static inline int param_set_dyndbg(const char *instr, const struct kernel_param *kp)
+{ return 0; }
+static inline int param_get_dyndbg(char *buffer, const struct kernel_param *kp)
+{ return 0; }
+
#endif /* !CONFIG_DYNAMIC_DEBUG_CORE */

+struct dyndbg_bitdesc {
+ const char *match; /* search format for this substr */
+};
+
+struct dyndbg_bitmap_param {
+ unsigned long *bits; /* ref to shared state */
+ struct dyndbg_bitdesc map[]; /* indexed by bitpos */
+};
+
+#if defined(CONFIG_DYNAMIC_DEBUG) || \
+ (defined(CONFIG_DYNAMIC_DEBUG_CORE) && defined(DYNAMIC_DEBUG_MODULE))
+/**
+ * DEFINE_DYNAMIC_DEBUG_CATEGORIES() - bitmap control of categorized prdbgs
+ * @fsname: parameter basename under /sys
+ * @_var: C-identifier holding bitmap
+ * @desc: string summarizing the controls provided
+ * @...: list of struct dyndbg_bitdesc initializations
+ *
+ * Intended for modules with substantial use of "categorized" prdbgs
+ * (those with some systematic prefix in the format string), this lets
+ * modules using pr_debug to control them in groups according to their
+ * format prefixes, and map them to bits 0-N of a sysfs control point.
+ * Each @... gives the index and prefix map.
+ */
+#define DEFINE_DYNAMIC_DEBUG_CATEGORIES(fsname, _var, desc, ...) \
+ MODULE_PARM_DESC(fsname, desc); \
+ static struct dyndbg_bitmap_param ddcats_##_var = \
+ { .bits = &(_var), .map = { __VA_ARGS__, { .match = NULL }}}; \
+ module_param_cb(fsname, &param_ops_dyndbg, &ddcats_##_var, 0644)
+
+/* helper macros provide combos of ^prefix and ' ' postfix */
+#define _DD_cat_(N, pfx) [N] = { .match = "^" pfx " " }
+#define _DD_cats_(N, pfx) [N] = { .match = "^" pfx }
+#define _DD_subtr_(N, flt) [N] = { .match = flt }
+
+extern const struct kernel_param_ops param_ops_dyndbg;
+
+#elif (defined(CONFIG_DYNAMIC_DEBUG_CORE) && !defined(DYNAMIC_DEBUG_MODULE))
+
+#define DEFINE_DYNAMIC_DEBUG_CATEGORIES(fsname, var, bitmap_desc, ...) \
+ BUILD_BUG_ON_MSG(1, "you need -DDYNAMIC_DEBUG_MODULE in compile")
+#define _DD_cat_(pfx)
+#define _DD_cats_(N, pfx)
+#define _DD_subtr_(N, flt)
+#else
+#define DEFINE_DYNAMIC_DEBUG_CATEGORIES(fsname, var, bitmap_desc, ...) \
+ BUILD_BUG_ON_MSG(1, "CONFIG_DYNAMIC_DEBUG needed to use this macro: " #var)
+#define _DD_cat_(pfx)
+#define _DD_cats_(pfx)
+#define _DD_subtr_(N, flt)
+#endif
+
#endif
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 356c8231175f..03ef9865fd5d 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -580,6 +580,82 @@ int dynamic_debug_exec_queries(const char *query, const char *modname)
}
EXPORT_SYMBOL_GPL(dynamic_debug_exec_queries);

+#ifdef CONFIG_MODULES
+#define KP_MOD_NAME kp->mod->name
+#else
+#define KP_MOD_NAME NULL /* wildcard */
+#endif
+#define FMT_QUERY_SIZE 128 /* typically need <40 */
+/**
+ * param_set_dyndbg - bits => categories >control setter
+ * @instr: string echo>d to sysfs
+ * @kp: kp->arg has state: bits, map
+ *
+ * Enable/disable prdbgs by their "category", as specified in
+ * DEFINE_DYNAMIC_DEBUG_CATEGORIES arguments.
+ *
+ * Returns: 0 or <0 if error.
+ */
+int param_set_dyndbg(const char *instr, const struct kernel_param *kp)
+{
+ unsigned long inbits;
+ int rc, i, matches = 0, totct = 0;
+ char query[FMT_QUERY_SIZE];
+ const struct dyndbg_bitmap_param *p = kp->arg;
+ const struct dyndbg_bitdesc *map = p->map;
+
+ if (!map) {
+ pr_err("set_dyndbg: no bits=>queries map\n");
+ return -EINVAL;
+ }
+ rc = kstrtoul(instr, 0, &inbits);
+ if (rc) {
+ pr_err("set_dyndbg: expecting unsigned int\n");
+ return rc;
+ }
+ vpr_info("set_dyndbg: new 0x%lx old 0x%lx\n", inbits, *p->bits);
+
+ for (i = 0; map->match && i < BITS_PER_LONG; map++, i++) {
+ if (test_bit(i, &inbits) == test_bit(i, p->bits))
+ continue;
+ snprintf(query, FMT_QUERY_SIZE, "format '%s' %cp", map->match,
+ test_bit(i, &inbits) ? '+' : '-');
+
+ matches = ddebug_exec_queries(query, KP_MOD_NAME);
+
+ v2pr_info("bit-%d: %d matches on format '%s'\n", i,
+ matches, map->match);
+ totct += matches;
+ }
+ *p->bits = inbits;
+ vpr_info("total matches: %d\n", totct);
+ return 0;
+}
+EXPORT_SYMBOL(param_set_dyndbg);
+
+/**
+ * param_get_dyndbg - bitmap reader
+ * @buffer: receives string rep of bitmap
+ * @kp: kp->arg has state: bits, map
+ *
+ * Reads last written bits, underlying prdbg state may have changed since.
+ * Returns: #chars written
+ */
+int param_get_dyndbg(char *buffer, const struct kernel_param *kp)
+{
+ const struct dyndbg_bitmap_param *p = kp->arg;
+ unsigned long val = *p->bits;
+
+ return scnprintf(buffer, PAGE_SIZE, "0x%lx\n", val);
+}
+EXPORT_SYMBOL(param_get_dyndbg);
+
+const struct kernel_param_ops param_ops_dyndbg = {
+ .set = param_set_dyndbg,
+ .get = param_get_dyndbg,
+};
+EXPORT_SYMBOL(param_ops_dyndbg);
+
#define PREFIX_SIZE 64

static int remaining(int wrote)
--
2.31.1
\
 
 \ /
  Last update: 2021-09-15 18:42    [W:0.102 / U:0.208 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site