* [PATCH 0/5] dyndbg add exclusive class support @ 2022-03-11 4:47 Jim Cromie 2022-03-11 4:47 ` [PATCH 1/5] dyndbg: fix static_branch manipulation Jim Cromie ` (4 more replies) 0 siblings, 5 replies; 11+ messages in thread From: Jim Cromie @ 2022-03-11 4:47 UTC (permalink / raw) To: jbaron, gregkh, linux-kernel Cc: daniel.vetter, seanpaul, robdclark, linux, joe, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx, Jim Cromie Hi Greg, Jason, Please consider these for char/misc or linux-next/soon/mumble. This patchset adds exclusive class support to dyndbg, allowing it to directly represent drm's debug_category. It is the dyndbg half of: https://lore.kernel.org/lkml/20220217034829.64395-1-jim.cromie@gmail.com/ The DRM half of that patchset uses this support to reimplement drm.debug on dyndbg, and uses its callsite patching to avoid the runtime checking done by drm_debug_enabled(). Background: In the past, various extensions to dyndbg/pr_debug have been proposed, none seemed to gain any consensus. This list is certainly incomplete. https://lore.kernel.org/lkml/20200609104604.1594-1-stanimir.varbanov@linaro.org/ pr_levels was discussed around this patchset, in revs 3,4,5 This search helps: s:venus: s:dynamic f:stanimir.varbanov@linaro.org IMO, pr_levels suffers from implied meaning between the levels: 2 > 1. In contrast, DRM has logically disjoint categories, and is implemented in an enum (despite its flag/mask values, a micro-optimization). https://lore.kernel.org/lkml/20210813151734.1236324-1-jim.cromie@gmail.com/ That patchset used pr_debug in DRM, and prepended "drm:kms:" etc to the format strings so each category was selectable; "format drm:kms:" in the query. This worked, but it made the format config-dependent, and was hard to explain without undue "artifact". So this patchset adds .class_id field (4-bits) to dynamic-debug callsites, and 'class N' query/command support to select upon it. Existing callsites and queries get .class_id=15, so 0-14 are available for use by the client (DRM wants 0-10). The DRM patchset then: . renumbers drm_debug_category to fit into the 4-bit .class_id the new enumerations *are* the bit-positions in drm/parameters/debug. . adapts the category-macro layer to use _CLS macros, mapping categories. . adds macro layer under the category-macro layer which wraps drm_*dbg inside a dyndbg Factory macro . uses DEFINE_DYNAMIC_DEBUG_CLASSBITS to tie to __drm_debug callbacks ref the var, so drm_debug_enabled(cat) just works. Jim Cromie (5): dyndbg: fix static_branch manipulation fixes a latent bug, before a 2nd "enable" flag exposes it. dyndbg: add class_id field and query support will allow (with that drm patchset): #> # turn on DRM_ATOMIC in amdgpu #> echo module amdgpu class 4 +p > /proc/dynamic_debug/control #> # turn on DRM_CORE in drm #> echo module drm class 0 +p > /proc/dynamic_debug/control dyndbg: add DEFINE_DYNAMIC_DEBUG_CLASSBITS macro adds macro & callbacks to support drm.debug bitmap #> echo 4 > /sys/module/drm/parameters/debug dyndbg: drop EXPORTed dynamic_debug_exec_queries unused yet, obsoleted by 2,3 dyndbg: show both old and new in change-info minor debug improvement .../admin-guide/dynamic-debug-howto.rst | 7 + include/linux/dynamic_debug.h | 111 ++++++++++--- lib/dynamic_debug.c | 150 ++++++++++++++---- 3 files changed, 213 insertions(+), 55 deletions(-) -- 2.35.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/5] dyndbg: fix static_branch manipulation 2022-03-11 4:47 [PATCH 0/5] dyndbg add exclusive class support Jim Cromie @ 2022-03-11 4:47 ` Jim Cromie 2022-03-11 18:03 ` Jason Baron 2022-03-11 4:47 ` [PATCH 2/5] dyndbg: add class_id field and query support Jim Cromie ` (3 subsequent siblings) 4 siblings, 1 reply; 11+ messages in thread From: Jim Cromie @ 2022-03-11 4:47 UTC (permalink / raw) To: jbaron, gregkh, linux-kernel Cc: daniel.vetter, seanpaul, robdclark, linux, joe, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx, Jim Cromie, vincent.whitchurch In https://lore.kernel.org/lkml/20211209150910.GA23668@axis.com/ Vincent's patch commented on, and worked around, a bug toggling static_branch's, when a 2nd PRINTK-ish flag was added. The bug results in a premature static_branch_disable when the 1st of 2 flags was disabled. The cited commit computed newflags, but then in the JUMP_LABEL block, failed to use that result, instead using just one of the terms in it. Using newflags instead made the code work properly. This is Vincents test-case, reduced. It needs the 2nd flag to work properly, but it's explanatory here. pt_test() { echo 5 > /sys/module/dynamic_debug/verbose site="module tcp" # just one callsite echo " $site =_ " > /proc/dynamic_debug/control # clear it # A B ~A ~B for flg in +T +p "-T #broke here" -p; do echo " $site $flg " > /proc/dynamic_debug/control done; # A B ~B ~A for flg in +T +p "-p #broke here" -T; do echo " $site $flg " > /proc/dynamic_debug/control done } pt_test Fixes: 84da83a6ffc0 dyndbg: combine flags & mask into a struct, simplify with it CC: vincent.whitchurch@axis.com Signed-off-by: Jim Cromie <jim.cromie@gmail.com> -- .drop @stable, no exposed bug. --- lib/dynamic_debug.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index dd7f56af9aed..a56c1286ffa4 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c @@ -211,10 +211,11 @@ static int ddebug_change(const struct ddebug_query *query, continue; #ifdef CONFIG_JUMP_LABEL if (dp->flags & _DPRINTK_FLAGS_PRINT) { - if (!(modifiers->flags & _DPRINTK_FLAGS_PRINT)) + if (!(newflags & _DPRINTK_FLAGS_PRINT)) static_branch_disable(&dp->key.dd_key_true); - } else if (modifiers->flags & _DPRINTK_FLAGS_PRINT) + } else if (newflags & _DPRINTK_FLAGS_PRINT) { static_branch_enable(&dp->key.dd_key_true); + } #endif dp->flags = newflags; v4pr_info("changed %s:%d [%s]%s =%s\n", -- 2.35.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 1/5] dyndbg: fix static_branch manipulation 2022-03-11 4:47 ` [PATCH 1/5] dyndbg: fix static_branch manipulation Jim Cromie @ 2022-03-11 18:03 ` Jason Baron 0 siblings, 0 replies; 11+ messages in thread From: Jason Baron @ 2022-03-11 18:03 UTC (permalink / raw) To: Jim Cromie, gregkh, linux-kernel Cc: daniel.vetter, seanpaul, robdclark, linux, joe, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx, vincent.whitchurch On 3/10/22 23:47, Jim Cromie wrote: > In https://urldefense.com/v3/__https://lore.kernel.org/lkml/20211209150910.GA23668@axis.com/__;!!GjvTz_vk!HGKKoni4RVdEBgv_V0zPSNSX428bpf02zkCy2WbeQkBdVtp1QJqGX-lJYlRDGg$ > > Vincent's patch commented on, and worked around, a bug toggling > static_branch's, when a 2nd PRINTK-ish flag was added. The bug > results in a premature static_branch_disable when the 1st of 2 flags > was disabled. > > The cited commit computed newflags, but then in the JUMP_LABEL block, > failed to use that result, instead using just one of the terms in it. > Using newflags instead made the code work properly. > > This is Vincents test-case, reduced. It needs the 2nd flag to work > properly, but it's explanatory here. > > pt_test() { > echo 5 > /sys/module/dynamic_debug/verbose > > site="module tcp" # just one callsite > echo " $site =_ " > /proc/dynamic_debug/control # clear it > > # A B ~A ~B > for flg in +T +p "-T #broke here" -p; do > echo " $site $flg " > /proc/dynamic_debug/control > done; > > # A B ~B ~A > for flg in +T +p "-p #broke here" -T; do > echo " $site $flg " > /proc/dynamic_debug/control > done > } > pt_test > > Fixes: 84da83a6ffc0 dyndbg: combine flags & mask into a struct, simplify with it > CC: vincent.whitchurch@axis.com > Signed-off-by: Jim Cromie <jim.cromie@gmail.com> > > -- > .drop @stable, no exposed bug. > --- > lib/dynamic_debug.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c > index dd7f56af9aed..a56c1286ffa4 100644 > --- a/lib/dynamic_debug.c > +++ b/lib/dynamic_debug.c > @@ -211,10 +211,11 @@ static int ddebug_change(const struct ddebug_query *query, > continue; > #ifdef CONFIG_JUMP_LABEL > if (dp->flags & _DPRINTK_FLAGS_PRINT) { > - if (!(modifiers->flags & _DPRINTK_FLAGS_PRINT)) > + if (!(newflags & _DPRINTK_FLAGS_PRINT)) > static_branch_disable(&dp->key.dd_key_true); > - } else if (modifiers->flags & _DPRINTK_FLAGS_PRINT) > + } else if (newflags & _DPRINTK_FLAGS_PRINT) { > static_branch_enable(&dp->key.dd_key_true); > + } > #endif > dp->flags = newflags; > v4pr_info("changed %s:%d [%s]%s =%s\n", Hi Jim, If iiuc this is currently a bug but could be if we add a second 'print' bit such as for printing to the tracing logs. That said I agree that using 'newflags' here makes the code more straightforward/readable. So this one is fine with me. Acked-by: Jason Baron <jbaron@akamai.com> Thanks, -Jason ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/5] dyndbg: add class_id field and query support 2022-03-11 4:47 [PATCH 0/5] dyndbg add exclusive class support Jim Cromie 2022-03-11 4:47 ` [PATCH 1/5] dyndbg: fix static_branch manipulation Jim Cromie @ 2022-03-11 4:47 ` Jim Cromie 2022-03-11 19:06 ` Jason Baron 2022-03-11 4:47 ` [PATCH 3/5] dyndbg: add DEFINE_DYNAMIC_DEBUG_CLASSBITS macro Jim Cromie ` (2 subsequent siblings) 4 siblings, 1 reply; 11+ messages in thread From: Jim Cromie @ 2022-03-11 4:47 UTC (permalink / raw) To: jbaron, gregkh, linux-kernel Cc: daniel.vetter, seanpaul, robdclark, linux, joe, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx, Jim Cromie DRM defines/uses 10 enum drm_debug_category's to create exclusive classes of debug messages. To support this directly in dynamic-debug, add the following: - struct _ddebug.class_id:4 - 4 bits is enough - define _DPRINTK_SITE_UNCLASSED 15 - see below and the query support: - struct _ddebug_query.class_id - ddebug_parse_query - looks for "class" keyword, then calls.. - parse_class - accepts uint: 0-15, sets query.class_id and marker - vpr_info_dq - displays new field - ddebug_proc_show - append column with "cls:%d" if !UNCLASSED With the patch, one can enable current/unclassed callsites by: #> echo drm class 15 +p > /proc/dynamic_debug/control parse_class() accepts 0 .. _DPRINTK_SITE_UNCLASSED, which allows the >control interface to explicitly manipulate unclassed callsites. After parsing keywords, ddebug_parse_query() sets .class_id=15, iff it wasnt explicitly set. This allows future classed/categorized callsites to be untouched by legacy (class unaware) queries. DEFINE_DYNAMIC_DEBUG_METADATA gets _CLS(cls,) suffix and arg, and initializes the new .class_id=cls field. The old name gets the default. Then, these _CLS(cls,...) modifications are repeated up through the stack of *dynamic_func_call* macros that use the METADATA initializer, so as to actually supply the category into it. NOTES: _DPRINTK_SITE_UNCLASSED: this symbol is used to initialize all existing/unclassed pr-debug callsites. Normally, the default would be zero, but DRM_UT_CORE "uses" that value, in the sense that 0 is exposed as a bit position in drm.debug. Using 15 allows identity mapping from category to class, avoiding fiddly offsets. Default .class_id = 15 means that ``echo +p > control`` no longer toggles ALL the callsites, only the unclassed ones. This was only useful for static-branch toggle load testing anyway. RFC: The new _CLS macro flavor gets a warning from DRM/dri-devel's CI, but not from checkpatch (on this subject). a8f6c71f283e dyndbg: add class_id field and query support -:141: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'id' - possible side-effects? +#define __dynamic_func_call_cls(id, cls, fmt, func, ...) do { \ + DEFINE_DYNAMIC_DEBUG_METADATA_CLS(id, cls, fmt); \ + if (DYNAMIC_DEBUG_BRANCH(id)) \ + func(&id, ##__VA_ARGS__); \ } while (0) I couldn't fix it with a ``typeof(id) _id = id`` construct. I haven't seen the warning myself, on the _CLS extended macro, nor the original. CC: Rasmus Villemoes <linux@rasmusvillemoes.dk> Signed-off-by: Jim Cromie <jim.cromie@gmail.com> --- .../admin-guide/dynamic-debug-howto.rst | 7 +++ include/linux/dynamic_debug.h | 54 ++++++++++++++----- lib/dynamic_debug.c | 48 ++++++++++++++--- 3 files changed, 88 insertions(+), 21 deletions(-) diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst index a89cfa083155..8ef8d7dcd140 100644 --- a/Documentation/admin-guide/dynamic-debug-howto.rst +++ b/Documentation/admin-guide/dynamic-debug-howto.rst @@ -35,6 +35,7 @@ Dynamic debug has even more useful features: - line number (including ranges of line numbers) - module name - format string + - class number:0-15 * Provides a debugfs control file: ``<debugfs>/dynamic_debug/control`` which can be read to display the complete list of known debug @@ -143,6 +144,7 @@ against. Possible keywords are::: 'module' string | 'format' string | 'line' line-range + 'class' integer:[0-15] line-range ::= lineno | '-'lineno | @@ -217,6 +219,11 @@ line line -1605 // the 1605 lines from line 1 to line 1605 line 1600- // all lines from line 1600 to the end of the file +class + This expects a single integer in range: 0-15. + 15 is used/reserved for existing/unclassed callsites, + and is defaulted in unless specified to >control + The flags specification comprises a change operation followed by one or more flag characters. The change operation is one of the characters:: diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h index dce631e678dd..d4b48f3cc6e8 100644 --- a/include/linux/dynamic_debug.h +++ b/include/linux/dynamic_debug.h @@ -6,6 +6,8 @@ #include <linux/jump_label.h> #endif +#include <linux/build_bug.h> + /* * An instance of this structure is created in a special * ELF section at every dynamic debug callsite. At runtime, @@ -21,6 +23,9 @@ struct _ddebug { const char *filename; const char *format; unsigned int lineno:18; +#define CLS_BITS 4 + unsigned int class_id:CLS_BITS; +#define _DPRINTK_SITE_UNCLASSED ((1 << CLS_BITS) - 1) /* * The flags field controls the behaviour at the callsite. * The bits here are changed dynamically when the user @@ -87,7 +92,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor, const struct ib_device *ibdev, const char *fmt, ...); -#define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt) \ +#define DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, cls, fmt) \ static struct _ddebug __aligned(8) \ __section("__dyndbg") name = { \ .modname = KBUILD_MODNAME, \ @@ -96,8 +101,14 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor, .format = (fmt), \ .lineno = __LINE__, \ .flags = _DPRINTK_FLAGS_DEFAULT, \ + .class_id = cls, \ _DPRINTK_KEY_INIT \ - } + }; \ + BUILD_BUG_ON_MSG(cls > _DPRINTK_SITE_UNCLASSED, \ + "classid value overflow") + +#define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt) \ + DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, _DPRINTK_SITE_UNCLASSED, fmt) #ifdef CONFIG_JUMP_LABEL @@ -128,18 +139,26 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor, #endif /* CONFIG_JUMP_LABEL */ -#define __dynamic_func_call(id, fmt, func, ...) do { \ - DEFINE_DYNAMIC_DEBUG_METADATA(id, fmt); \ - if (DYNAMIC_DEBUG_BRANCH(id)) \ - func(&id, ##__VA_ARGS__); \ +#define __dynamic_func_call_cls(id, cls, fmt, func, ...) do { \ + DEFINE_DYNAMIC_DEBUG_METADATA_CLS(id, cls, fmt); \ + if (DYNAMIC_DEBUG_BRANCH(id)) \ + func(&id, ##__VA_ARGS__); \ } while (0) -#define __dynamic_func_call_no_desc(id, fmt, func, ...) do { \ - DEFINE_DYNAMIC_DEBUG_METADATA(id, fmt); \ - if (DYNAMIC_DEBUG_BRANCH(id)) \ - func(__VA_ARGS__); \ +#define __dynamic_func_call_no_desc_cls(id, cls, fmt, func, ...) do { \ + DEFINE_DYNAMIC_DEBUG_METADATA_CLS(id, cls, fmt); \ + if (DYNAMIC_DEBUG_BRANCH(id)) \ + func(__VA_ARGS__); \ } while (0) +#define __dynamic_func_call(id, fmt, func, ...) \ + __dynamic_func_call_cls(id, _DPRINTK_SITE_UNCLASSED, \ + fmt, func, ##__VA_ARGS__) + +#define __dynamic_func_call_no_desc(id, fmt, func, ...) \ + __dynamic_func_call_no_desc_cls(id, _DPRINTK_SITE_UNCLASSED, \ + fmt, func, ##__VA_ARGS__) + /* * "Factory macro" for generating a call to func, guarded by a * DYNAMIC_DEBUG_BRANCH. The dynamic debug descriptor will be @@ -148,15 +167,24 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor, * the varargs. Note that fmt is repeated in invocations of this * macro. */ +#define _dynamic_func_call_cls(cls, fmt, func, ...) \ + __dynamic_func_call_cls(__UNIQUE_ID(ddebug), cls, fmt, func, ##__VA_ARGS__) #define _dynamic_func_call(fmt, func, ...) \ - __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__) + _dynamic_func_call_cls(_DPRINTK_SITE_UNCLASSED, fmt, func, ##__VA_ARGS__) + /* * A variant that does the same, except that the descriptor is not * passed as the first argument to the function; it is only called * with precisely the macro's varargs. */ -#define _dynamic_func_call_no_desc(fmt, func, ...) \ - __dynamic_func_call_no_desc(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__) +#define _dynamic_func_call_no_desc_cls(fmt, cat, func, ...) \ + __dynamic_func_call_no_desc_cls(__UNIQUE_ID(ddebug), cat, \ + fmt, func, ##__VA_ARGS__) + +#define _dynamic_func_call_no_desc(fmt, func, ...) \ + __dynamic_func_call_no_desc_cls(__UNIQUE_ID(ddebug), \ + _DPRINTK_SITE_UNCLASSED, \ + fmt, func, ##__VA_ARGS__) #define dynamic_pr_debug(fmt, ...) \ _dynamic_func_call(fmt, __dynamic_pr_debug, \ diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index a56c1286ffa4..ee2129becacc 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c @@ -55,6 +55,8 @@ struct ddebug_query { const char *function; const char *format; unsigned int first_lineno, last_lineno; + unsigned int class_id; + unsigned int class_marked:1; }; struct ddebug_iter { @@ -134,13 +136,13 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg) fmtlen--; } - v3pr_info("%s: func=\"%s\" file=\"%s\" module=\"%s\" format=\"%.*s\" lineno=%u-%u\n", - msg, - query->function ?: "", - query->filename ?: "", - query->module ?: "", - fmtlen, query->format ?: "", - query->first_lineno, query->last_lineno); + v3pr_info("%s: func=\"%s\" file=\"%s\" module=\"%s\" format=\"%.*s\" lineno=%u-%u class=%u\n", + msg, + query->function ?: "", + query->filename ?: "", + query->module ?: "", + fmtlen, query->format ?: "", + query->first_lineno, query->last_lineno, query->class_id); } /* @@ -170,6 +172,10 @@ static int ddebug_change(const struct ddebug_query *query, for (i = 0; i < dt->num_ddebugs; i++) { struct _ddebug *dp = &dt->ddebugs[i]; + /* match against the class_id, either given or default */ + if (query->class_id != dp->class_id) + continue; + /* match against the source filename */ if (query->filename && !match_wildcard(query->filename, dp->filename) && @@ -308,6 +314,21 @@ static inline int parse_lineno(const char *str, unsigned int *val) return 0; } +static inline int parse_class(struct ddebug_query *query, const char *str) +{ + int rc; + unsigned int val; + + rc = kstrtouint(str, 10, &val); + if (rc < 0 || val > _DPRINTK_SITE_UNCLASSED) { + pr_err("expecting class:[0-%d], not %s\n", _DPRINTK_SITE_UNCLASSED, str); + return -EINVAL; + } + query->class_id = val; + query->class_marked = 1; + return 0; +} + static int parse_linerange(struct ddebug_query *query, const char *first) { char *last = strchr(first, '-'); @@ -421,6 +442,9 @@ static int ddebug_parse_query(char *words[], int nwords, } else if (!strcmp(keyword, "line")) { if (parse_linerange(query, arg)) return -EINVAL; + } else if (!strcmp(keyword, "class")) { + if (parse_class(query, arg)) + return -EINVAL; } else { pr_err("unknown keyword \"%s\"\n", keyword); return -EINVAL; @@ -428,6 +452,10 @@ static int ddebug_parse_query(char *words[], int nwords, if (rc) return rc; } + /* post-validate the query, set default */ + if (!query->class_marked) + query->class_id = _DPRINTK_SITE_UNCLASSED; + vpr_info_dq(query, "parsed"); return 0; } @@ -900,7 +928,11 @@ static int ddebug_proc_show(struct seq_file *m, void *p) iter->table->mod_name, dp->function, ddebug_describe_flags(dp->flags, &flags)); seq_escape(m, dp->format, "\t\r\n\""); - seq_puts(m, "\"\n"); + seq_puts(m, "\""); + + if (dp->class_id != _DPRINTK_SITE_UNCLASSED) + seq_printf(m, " cls:%u", dp->class_id); + seq_puts(m, "\n"); return 0; } -- 2.35.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/5] dyndbg: add class_id field and query support 2022-03-11 4:47 ` [PATCH 2/5] dyndbg: add class_id field and query support Jim Cromie @ 2022-03-11 19:06 ` Jason Baron 2022-03-12 1:06 ` jim.cromie 0 siblings, 1 reply; 11+ messages in thread From: Jason Baron @ 2022-03-11 19:06 UTC (permalink / raw) To: Jim Cromie, gregkh, linux-kernel Cc: daniel.vetter, seanpaul, robdclark, linux, joe, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx On 3/10/22 23:47, Jim Cromie wrote: > DRM defines/uses 10 enum drm_debug_category's to create exclusive > classes of debug messages. To support this directly in dynamic-debug, > add the following: > > - struct _ddebug.class_id:4 - 4 bits is enough > - define _DPRINTK_SITE_UNCLASSED 15 - see below > > and the query support: > - struct _ddebug_query.class_id > - ddebug_parse_query - looks for "class" keyword, then calls.. > - parse_class - accepts uint: 0-15, sets query.class_id and marker > - vpr_info_dq - displays new field > - ddebug_proc_show - append column with "cls:%d" if !UNCLASSED > > With the patch, one can enable current/unclassed callsites by: > > #> echo drm class 15 +p > /proc/dynamic_debug/control > To me, this is hard to read, what the heck is '15'? I have to go look it up in the control file and it's not descriptive. I think that using classes/categories makes sense but I'm wondering if it can be a bit more user friendly? Perhaps, we can pass an array of strings that is indexed by the class id to each pr_debug() site that wants to use class. So something like: enum levels { LOW, MEDIUM, HIGH }; static const char * const level_to_strings[] = { [LOW] = "low", [MEDIUM] = "medium", [HIGH] = "high", }; And then you'd have a wrapper macros in your driver: #define module_foo_pr_debug_class(level, fmt, args...) pr_debug_class(level, level_to_strings, fmt, args); Such that call sites look like: module_foo_pr_debug_class(LOW, fmt, args...); Such that you're not always passing the strings array around. Now, this does mean another pointer for struct _ddebug and most wouldn't have it. Maybe we could just add another linker section for these so as to save space. > parse_class() accepts 0 .. _DPRINTK_SITE_UNCLASSED, which allows the >> control interface to explicitly manipulate unclassed callsites. > > After parsing keywords, ddebug_parse_query() sets .class_id=15, iff it > wasnt explicitly set. This allows future classed/categorized > callsites to be untouched by legacy (class unaware) queries. > > DEFINE_DYNAMIC_DEBUG_METADATA gets _CLS(cls,) suffix and arg, and > initializes the new .class_id=cls field. The old name gets the default. > > Then, these _CLS(cls,...) modifications are repeated up through the > stack of *dynamic_func_call* macros that use the METADATA initializer, > so as to actually supply the category into it. > > NOTES: > > _DPRINTK_SITE_UNCLASSED: this symbol is used to initialize all > existing/unclassed pr-debug callsites. Normally, the default would be > zero, but DRM_UT_CORE "uses" that value, in the sense that 0 is > exposed as a bit position in drm.debug. Using 15 allows identity > mapping from category to class, avoiding fiddly offsets. > > Default .class_id = 15 means that ``echo +p > control`` no longer > toggles ALL the callsites, only the unclassed ones. This was only > useful for static-branch toggle load testing anyway. > I think that # echo +p > control should continue to work as is, why should the introduction of classes change that ? > RFC: > > The new _CLS macro flavor gets a warning from DRM/dri-devel's CI, > but not from checkpatch (on this subject). > > a8f6c71f283e dyndbg: add class_id field and query support > -:141: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'id' - possible side-effects? > +#define __dynamic_func_call_cls(id, cls, fmt, func, ...) do { \ > + DEFINE_DYNAMIC_DEBUG_METADATA_CLS(id, cls, fmt); \ > + if (DYNAMIC_DEBUG_BRANCH(id)) \ > + func(&id, ##__VA_ARGS__); \ > } while (0) > > I couldn't fix it with a ``typeof(id) _id = id`` construct. I haven't > seen the warning myself, on the _CLS extended macro, nor the original. > > CC: Rasmus Villemoes <linux@rasmusvillemoes.dk> > Signed-off-by: Jim Cromie <jim.cromie@gmail.com> > --- > .../admin-guide/dynamic-debug-howto.rst | 7 +++ > include/linux/dynamic_debug.h | 54 ++++++++++++++----- > lib/dynamic_debug.c | 48 ++++++++++++++--- > 3 files changed, 88 insertions(+), 21 deletions(-) > > diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst > index a89cfa083155..8ef8d7dcd140 100644 > --- a/Documentation/admin-guide/dynamic-debug-howto.rst > +++ b/Documentation/admin-guide/dynamic-debug-howto.rst > @@ -35,6 +35,7 @@ Dynamic debug has even more useful features: > - line number (including ranges of line numbers) > - module name > - format string > + - class number:0-15 > > * Provides a debugfs control file: ``<debugfs>/dynamic_debug/control`` > which can be read to display the complete list of known debug > @@ -143,6 +144,7 @@ against. Possible keywords are::: > 'module' string | > 'format' string | > 'line' line-range > + 'class' integer:[0-15] > > line-range ::= lineno | > '-'lineno | > @@ -217,6 +219,11 @@ line > line -1605 // the 1605 lines from line 1 to line 1605 > line 1600- // all lines from line 1600 to the end of the file > > +class > + This expects a single integer in range: 0-15. > + 15 is used/reserved for existing/unclassed callsites, > + and is defaulted in unless specified to >control > + > The flags specification comprises a change operation followed > by one or more flag characters. The change operation is one > of the characters:: > diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h > index dce631e678dd..d4b48f3cc6e8 100644 > --- a/include/linux/dynamic_debug.h > +++ b/include/linux/dynamic_debug.h > @@ -6,6 +6,8 @@ > #include <linux/jump_label.h> > #endif > > +#include <linux/build_bug.h> > + > /* > * An instance of this structure is created in a special > * ELF section at every dynamic debug callsite. At runtime, > @@ -21,6 +23,9 @@ struct _ddebug { > const char *filename; > const char *format; > unsigned int lineno:18; > +#define CLS_BITS 4 > + unsigned int class_id:CLS_BITS; > +#define _DPRINTK_SITE_UNCLASSED ((1 << CLS_BITS) - 1) > /* > * The flags field controls the behaviour at the callsite. > * The bits here are changed dynamically when the user > @@ -87,7 +92,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor, > const struct ib_device *ibdev, > const char *fmt, ...); > > -#define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt) \ > +#define DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, cls, fmt) \ > static struct _ddebug __aligned(8) \ > __section("__dyndbg") name = { \ > .modname = KBUILD_MODNAME, \ > @@ -96,8 +101,14 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor, > .format = (fmt), \ > .lineno = __LINE__, \ > .flags = _DPRINTK_FLAGS_DEFAULT, \ > + .class_id = cls, \ > _DPRINTK_KEY_INIT \ > - } > + }; \ > + BUILD_BUG_ON_MSG(cls > _DPRINTK_SITE_UNCLASSED, \ > + "classid value overflow") > + > +#define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt) \ > + DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, _DPRINTK_SITE_UNCLASSED, fmt) > > #ifdef CONFIG_JUMP_LABEL > > @@ -128,18 +139,26 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor, > > #endif /* CONFIG_JUMP_LABEL */ > > -#define __dynamic_func_call(id, fmt, func, ...) do { \ > - DEFINE_DYNAMIC_DEBUG_METADATA(id, fmt); \ > - if (DYNAMIC_DEBUG_BRANCH(id)) \ > - func(&id, ##__VA_ARGS__); \ > +#define __dynamic_func_call_cls(id, cls, fmt, func, ...) do { \ > + DEFINE_DYNAMIC_DEBUG_METADATA_CLS(id, cls, fmt); \ > + if (DYNAMIC_DEBUG_BRANCH(id)) \ > + func(&id, ##__VA_ARGS__); \ > } while (0) > > -#define __dynamic_func_call_no_desc(id, fmt, func, ...) do { \ > - DEFINE_DYNAMIC_DEBUG_METADATA(id, fmt); \ > - if (DYNAMIC_DEBUG_BRANCH(id)) \ > - func(__VA_ARGS__); \ > +#define __dynamic_func_call_no_desc_cls(id, cls, fmt, func, ...) do { \ > + DEFINE_DYNAMIC_DEBUG_METADATA_CLS(id, cls, fmt); \ > + if (DYNAMIC_DEBUG_BRANCH(id)) \ > + func(__VA_ARGS__); \ > } while (0) > > +#define __dynamic_func_call(id, fmt, func, ...) \ > + __dynamic_func_call_cls(id, _DPRINTK_SITE_UNCLASSED, \ > + fmt, func, ##__VA_ARGS__) > + > +#define __dynamic_func_call_no_desc(id, fmt, func, ...) \ > + __dynamic_func_call_no_desc_cls(id, _DPRINTK_SITE_UNCLASSED, \ > + fmt, func, ##__VA_ARGS__) > + > /* > * "Factory macro" for generating a call to func, guarded by a > * DYNAMIC_DEBUG_BRANCH. The dynamic debug descriptor will be > @@ -148,15 +167,24 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor, > * the varargs. Note that fmt is repeated in invocations of this > * macro. > */ > +#define _dynamic_func_call_cls(cls, fmt, func, ...) \ > + __dynamic_func_call_cls(__UNIQUE_ID(ddebug), cls, fmt, func, ##__VA_ARGS__) > #define _dynamic_func_call(fmt, func, ...) \ > - __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__) > + _dynamic_func_call_cls(_DPRINTK_SITE_UNCLASSED, fmt, func, ##__VA_ARGS__) > + > /* > * A variant that does the same, except that the descriptor is not > * passed as the first argument to the function; it is only called > * with precisely the macro's varargs. > */ > -#define _dynamic_func_call_no_desc(fmt, func, ...) \ > - __dynamic_func_call_no_desc(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__) > +#define _dynamic_func_call_no_desc_cls(fmt, cat, func, ...) \ > + __dynamic_func_call_no_desc_cls(__UNIQUE_ID(ddebug), cat, \ > + fmt, func, ##__VA_ARGS__) > + > +#define _dynamic_func_call_no_desc(fmt, func, ...) \ > + __dynamic_func_call_no_desc_cls(__UNIQUE_ID(ddebug), \ > + _DPRINTK_SITE_UNCLASSED, \ > + fmt, func, ##__VA_ARGS__) > > #define dynamic_pr_debug(fmt, ...) \ > _dynamic_func_call(fmt, __dynamic_pr_debug, \ > diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c > index a56c1286ffa4..ee2129becacc 100644 > --- a/lib/dynamic_debug.c > +++ b/lib/dynamic_debug.c > @@ -55,6 +55,8 @@ struct ddebug_query { > const char *function; > const char *format; > unsigned int first_lineno, last_lineno; > + unsigned int class_id; > + unsigned int class_marked:1; > }; > > struct ddebug_iter { > @@ -134,13 +136,13 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg) > fmtlen--; > } > > - v3pr_info("%s: func=\"%s\" file=\"%s\" module=\"%s\" format=\"%.*s\" lineno=%u-%u\n", > - msg, > - query->function ?: "", > - query->filename ?: "", > - query->module ?: "", > - fmtlen, query->format ?: "", > - query->first_lineno, query->last_lineno); > + v3pr_info("%s: func=\"%s\" file=\"%s\" module=\"%s\" format=\"%.*s\" lineno=%u-%u class=%u\n", > + msg, > + query->function ?: "", > + query->filename ?: "", > + query->module ?: "", > + fmtlen, query->format ?: "", > + query->first_lineno, query->last_lineno, query->class_id); > } > > /* > @@ -170,6 +172,10 @@ static int ddebug_change(const struct ddebug_query *query, > for (i = 0; i < dt->num_ddebugs; i++) { > struct _ddebug *dp = &dt->ddebugs[i]; > > + /* match against the class_id, either given or default */ > + if (query->class_id != dp->class_id) > + continue; > + Could that instead be: if (query->class_id != _DPRINTK_SITE_UNCLASSED && query->class_id != dp->class_id) continue; That way by default we fall through and enable callsites for queries that do not have the 'class' keyboard specified. I think I would also disallow setting _DPRINTK_SITE_UNCLASSED explicitly since it would now mean 'class *', and what if we expand class_id size to greater than 15 at some point? > /* match against the source filename */ > if (query->filename && > !match_wildcard(query->filename, dp->filename) && > @@ -308,6 +314,21 @@ static inline int parse_lineno(const char *str, unsigned int *val) > return 0; > } > > +static inline int parse_class(struct ddebug_query *query, const char *str) > +{ > + int rc; > + unsigned int val; > + > + rc = kstrtouint(str, 10, &val); > + if (rc < 0 || val > _DPRINTK_SITE_UNCLASSED) { > + pr_err("expecting class:[0-%d], not %s\n", _DPRINTK_SITE_UNCLASSED, str); > + return -EINVAL; > + } > + query->class_id = val; > + query->class_marked = 1; > + return 0; > +} > + > static int parse_linerange(struct ddebug_query *query, const char *first) > { > char *last = strchr(first, '-'); > @@ -421,6 +442,9 @@ static int ddebug_parse_query(char *words[], int nwords, > } else if (!strcmp(keyword, "line")) { > if (parse_linerange(query, arg)) > return -EINVAL; > + } else if (!strcmp(keyword, "class")) { > + if (parse_class(query, arg)) > + return -EINVAL; > } else { > pr_err("unknown keyword \"%s\"\n", keyword); > return -EINVAL; > @@ -428,6 +452,10 @@ static int ddebug_parse_query(char *words[], int nwords, > if (rc) > return rc; > } > + /* post-validate the query, set default */ > + if (!query->class_marked) > + query->class_id = _DPRINTK_SITE_UNCLASSED; > + > vpr_info_dq(query, "parsed"); > return 0; > } > @@ -900,7 +928,11 @@ static int ddebug_proc_show(struct seq_file *m, void *p) > iter->table->mod_name, dp->function, > ddebug_describe_flags(dp->flags, &flags)); > seq_escape(m, dp->format, "\t\r\n\""); > - seq_puts(m, "\"\n"); > + seq_puts(m, "\""); > + > + if (dp->class_id != _DPRINTK_SITE_UNCLASSED) > + seq_printf(m, " cls:%u", dp->class_id); > + seq_puts(m, "\n"); > > return 0; > } ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/5] dyndbg: add class_id field and query support 2022-03-11 19:06 ` Jason Baron @ 2022-03-12 1:06 ` jim.cromie 2022-03-14 21:29 ` Jason Baron 0 siblings, 1 reply; 11+ messages in thread From: jim.cromie @ 2022-03-12 1:06 UTC (permalink / raw) To: Jason Baron Cc: Greg KH, LKML, Daniel Vetter, Sean Paul, robdclark, Rasmus Villemoes, Joe Perches, dri-devel, amd-gfx mailing list, intel-gvt-dev, Intel Graphics Development On Fri, Mar 11, 2022 at 12:06 PM Jason Baron <jbaron@akamai.com> wrote: > > > > On 3/10/22 23:47, Jim Cromie wrote: > > DRM defines/uses 10 enum drm_debug_category's to create exclusive > > classes of debug messages. To support this directly in dynamic-debug, > > add the following: > > > > - struct _ddebug.class_id:4 - 4 bits is enough > > - define _DPRINTK_SITE_UNCLASSED 15 - see below > > > > and the query support: > > - struct _ddebug_query.class_id > > - ddebug_parse_query - looks for "class" keyword, then calls.. > > - parse_class - accepts uint: 0-15, sets query.class_id and marker > > - vpr_info_dq - displays new field > > - ddebug_proc_show - append column with "cls:%d" if !UNCLASSED > > > > With the patch, one can enable current/unclassed callsites by: > > > > #> echo drm class 15 +p > /proc/dynamic_debug/control > > > > To me, this is hard to read, what the heck is '15'? I have to go look it > up in the control file and it's not descriptive. I think that using > classes/categories makes sense but I'm wondering if it can be a bit more > user friendly? Perhaps, we can pass an array of strings that is indexed > by the class id to each pr_debug() site that wants to use class. So > something like: > Im not at all averse to nice names, but as something added on. 1st, the interface to make friendlier is DRM's echo 0x04 > /sys/module/drm/parameters/debug # which category ? parm: debug:Enable debug output, where each bit enables a debug category. Bit 0 (0x01) will enable CORE messages (drm core code) Bit 1 (0x02) will enable DRIVER messages (drm controller code) Bit 2 (0x04) will enable KMS messages (modesetting code) echo DRM_UT_DRIVER,DRM_UT_KMS > /sys/module/drm/parameters/debug # now its pretty clear If that works, its cuz DEFINE_DYNAMIC_DEBUG_CLASSBITS() plucks class symbols out of its __VA_ARGS__, and #stringifes them. So that macro could then build the 1-per-module static constant string array and (only) the callbacks would be able to use it. from there, it shouldnt be hard to ref that from the module's ddebug_table, so parse_query could validate class args against the module given (or fail if none given) Speaking strictly, theres a gap: echo module * class DRM_UT_KMS +p > control is nonsense for * other than drm + drivers, but its fair/safe to just disallow wildcards on modname for this purpose. it does however imply that module foo must exist for FOO_CAT_1 to be usable. thats not currently the case: bash-5.1# echo module foo +p > /proc/dynamic_debug/control [ 15.403749] dyndbg: read 14 bytes from userspace [ 15.405413] dyndbg: query 0: "module foo +p" mod:* [ 15.406486] dyndbg: split into words: "module" "foo" "+p" [ 15.407070] dyndbg: op='+' [ 15.407388] dyndbg: flags=0x1 [ 15.407809] dyndbg: *flagsp=0x1 *maskp=0xffffffff [ 15.408300] dyndbg: parsed: func="" file="" module="foo" format="" lineno=0-0 class=15 [ 15.409151] dyndbg: no matches for query [ 15.409591] dyndbg: no-match: func="" file="" module="foo" format="" lineno=0-0 class=15 [ 15.410524] dyndbg: processed 1 queries, with 0 matches, 0 errs bash-5.1# ISTM we can keep that "0 errs" response for that case, but still reject this: echo module foo class FOO_NOT_HERE +p > /proc/dynamic_debug/control > enum levels { > LOW, > MEDIUM, > HIGH > }; I want to steer clear of "level" anything, since 2>1 implies non independence of the categories > > static const char * const level_to_strings[] = { > [LOW] = "low", > [MEDIUM] = "medium", > [HIGH] = "high", > }; > > And then you'd have a wrapper macros in your driver: > > #define module_foo_pr_debug_class(level, fmt, args...) > pr_debug_class(level, level_to_strings, fmt, args); > > Such that call sites look like: > > module_foo_pr_debug_class(LOW, fmt, args...); > That macro, minus the "module_foo_" prefix, could go into dynamic_debug.h I didnt do that, for 2 reasons: DRM didnt need it - it had an enum var, and a set of macros to encapsulate the categories. - the "prototype" looks like this might be ok: define LOW "low" pr_debug_class(LOW, "mumble about something %p %p\n", foo, bar) ok thats a stretch, but... Basically, I didnt want to deal with creating a new interface. KIS > Such that you're not always passing the strings array around. Now, this > does mean another pointer for struct _ddebug and most wouldn't have it. > Maybe we could just add another linker section for these so as to save > space. From a space consideration, adding DRM users means adding thousands of callsites ~2k for i915, ~4k for amdgpu Also increasing per/callsite memory seems counterproductive. see DEFINE_DYNAMIC_DEBUG_CLASSBITS for the way forward. > > parse_class() accepts 0 .. _DPRINTK_SITE_UNCLASSED, which allows the > >> control interface to explicitly manipulate unclassed callsites. > > > > After parsing keywords, ddebug_parse_query() sets .class_id=15, iff it > > wasnt explicitly set. This allows future classed/categorized > > callsites to be untouched by legacy (class unaware) queries. > > > > DEFINE_DYNAMIC_DEBUG_METADATA gets _CLS(cls,) suffix and arg, and > > initializes the new .class_id=cls field. The old name gets the default. > > > > Then, these _CLS(cls,...) modifications are repeated up through the > > stack of *dynamic_func_call* macros that use the METADATA initializer, > > so as to actually supply the category into it. > > > > NOTES: > > > > _DPRINTK_SITE_UNCLASSED: this symbol is used to initialize all > > existing/unclassed pr-debug callsites. Normally, the default would be > > zero, but DRM_UT_CORE "uses" that value, in the sense that 0 is > > exposed as a bit position in drm.debug. Using 15 allows identity > > mapping from category to class, avoiding fiddly offsets. > > > > Default .class_id = 15 means that ``echo +p > control`` no longer > > toggles ALL the callsites, only the unclassed ones. This was only > > useful for static-branch toggle load testing anyway. > > > > I think that # echo +p > control should continue to work as is, why > should the introduction of classes change that ? 1st, its a good way to flood your syslog, forex if theres a serial driver built. And this might get dramatically worse if DRM_VBLANK got lumped in. but basically, it still affects the same universe of callsites as before DRM popped/s into existence (which could double the callsites) ISTM ``echo "+p;-p" >control`` is primarily useful for work-load generation. whether 1 cycle is 4k or 8k toggles is unimportant. > > > RFC: > > > > The new _CLS macro flavor gets a warning from DRM/dri-devel's CI, > > but not from checkpatch (on this subject). > > > > a8f6c71f283e dyndbg: add class_id field and query support > > -:141: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'id' - possible side-effects? > > +#define __dynamic_func_call_cls(id, cls, fmt, func, ...) do { \ > > + DEFINE_DYNAMIC_DEBUG_METADATA_CLS(id, cls, fmt); \ > > + if (DYNAMIC_DEBUG_BRANCH(id)) \ > > + func(&id, ##__VA_ARGS__); \ > > } while (0) > > > > I couldn't fix it with a ``typeof(id) _id = id`` construct. I haven't > > seen the warning myself, on the _CLS extended macro, nor the original. > > > > CC: Rasmus Villemoes <linux@rasmusvillemoes.dk> > > Signed-off-by: Jim Cromie <jim.cromie@gmail.com> > > --- > > .../admin-guide/dynamic-debug-howto.rst | 7 +++ > > include/linux/dynamic_debug.h | 54 ++++++++++++++----- > > lib/dynamic_debug.c | 48 ++++++++++++++--- > > 3 files changed, 88 insertions(+), 21 deletions(-) > > > > diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst > > index a89cfa083155..8ef8d7dcd140 100644 > > --- a/Documentation/admin-guide/dynamic-debug-howto.rst > > +++ b/Documentation/admin-guide/dynamic-debug-howto.rst > > @@ -35,6 +35,7 @@ Dynamic debug has even more useful features: > > - line number (including ranges of line numbers) > > - module name > > - format string > > + - class number:0-15 > > > > * Provides a debugfs control file: ``<debugfs>/dynamic_debug/control`` > > which can be read to display the complete list of known debug > > @@ -143,6 +144,7 @@ against. Possible keywords are::: > > 'module' string | > > 'format' string | > > 'line' line-range > > + 'class' integer:[0-15] > > > > line-range ::= lineno | > > '-'lineno | > > @@ -217,6 +219,11 @@ line > > line -1605 // the 1605 lines from line 1 to line 1605 > > line 1600- // all lines from line 1600 to the end of the file > > > > +class > > + This expects a single integer in range: 0-15. > > + 15 is used/reserved for existing/unclassed callsites, > > + and is defaulted in unless specified to >control > > + > > The flags specification comprises a change operation followed > > by one or more flag characters. The change operation is one > > of the characters:: > > diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h > > index dce631e678dd..d4b48f3cc6e8 100644 > > --- a/include/linux/dynamic_debug.h > > +++ b/include/linux/dynamic_debug.h > > @@ -6,6 +6,8 @@ > > #include <linux/jump_label.h> > > #endif > > > > +#include <linux/build_bug.h> > > + > > /* > > * An instance of this structure is created in a special > > * ELF section at every dynamic debug callsite. At runtime, > > @@ -21,6 +23,9 @@ struct _ddebug { > > const char *filename; > > const char *format; > > unsigned int lineno:18; > > +#define CLS_BITS 4 > > + unsigned int class_id:CLS_BITS; > > +#define _DPRINTK_SITE_UNCLASSED ((1 << CLS_BITS) - 1) > > /* > > * The flags field controls the behaviour at the callsite. > > * The bits here are changed dynamically when the user > > @@ -87,7 +92,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor, > > const struct ib_device *ibdev, > > const char *fmt, ...); > > > > -#define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt) \ > > +#define DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, cls, fmt) \ > > static struct _ddebug __aligned(8) \ > > __section("__dyndbg") name = { \ > > .modname = KBUILD_MODNAME, \ > > @@ -96,8 +101,14 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor, > > .format = (fmt), \ > > .lineno = __LINE__, \ > > .flags = _DPRINTK_FLAGS_DEFAULT, \ > > + .class_id = cls, \ > > _DPRINTK_KEY_INIT \ > > - } > > + }; \ > > + BUILD_BUG_ON_MSG(cls > _DPRINTK_SITE_UNCLASSED, \ > > + "classid value overflow") > > + > > +#define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt) \ > > + DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, _DPRINTK_SITE_UNCLASSED, fmt) > > > > #ifdef CONFIG_JUMP_LABEL > > > > @@ -128,18 +139,26 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor, > > > > #endif /* CONFIG_JUMP_LABEL */ > > > > -#define __dynamic_func_call(id, fmt, func, ...) do { \ > > - DEFINE_DYNAMIC_DEBUG_METADATA(id, fmt); \ > > - if (DYNAMIC_DEBUG_BRANCH(id)) \ > > - func(&id, ##__VA_ARGS__); \ > > +#define __dynamic_func_call_cls(id, cls, fmt, func, ...) do { \ > > + DEFINE_DYNAMIC_DEBUG_METADATA_CLS(id, cls, fmt); \ > > + if (DYNAMIC_DEBUG_BRANCH(id)) \ > > + func(&id, ##__VA_ARGS__); \ > > } while (0) > > > > -#define __dynamic_func_call_no_desc(id, fmt, func, ...) do { \ > > - DEFINE_DYNAMIC_DEBUG_METADATA(id, fmt); \ > > - if (DYNAMIC_DEBUG_BRANCH(id)) \ > > - func(__VA_ARGS__); \ > > +#define __dynamic_func_call_no_desc_cls(id, cls, fmt, func, ...) do { \ > > + DEFINE_DYNAMIC_DEBUG_METADATA_CLS(id, cls, fmt); \ > > + if (DYNAMIC_DEBUG_BRANCH(id)) \ > > + func(__VA_ARGS__); \ > > } while (0) > > > > +#define __dynamic_func_call(id, fmt, func, ...) \ > > + __dynamic_func_call_cls(id, _DPRINTK_SITE_UNCLASSED, \ > > + fmt, func, ##__VA_ARGS__) > > + > > +#define __dynamic_func_call_no_desc(id, fmt, func, ...) \ > > + __dynamic_func_call_no_desc_cls(id, _DPRINTK_SITE_UNCLASSED, \ > > + fmt, func, ##__VA_ARGS__) > > + > > /* > > * "Factory macro" for generating a call to func, guarded by a > > * DYNAMIC_DEBUG_BRANCH. The dynamic debug descriptor will be > > @@ -148,15 +167,24 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor, > > * the varargs. Note that fmt is repeated in invocations of this > > * macro. > > */ > > +#define _dynamic_func_call_cls(cls, fmt, func, ...) \ > > + __dynamic_func_call_cls(__UNIQUE_ID(ddebug), cls, fmt, func, ##__VA_ARGS__) > > #define _dynamic_func_call(fmt, func, ...) \ > > - __dynamic_func_call(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__) > > + _dynamic_func_call_cls(_DPRINTK_SITE_UNCLASSED, fmt, func, ##__VA_ARGS__) > > + > > /* > > * A variant that does the same, except that the descriptor is not > > * passed as the first argument to the function; it is only called > > * with precisely the macro's varargs. > > */ > > -#define _dynamic_func_call_no_desc(fmt, func, ...) \ > > - __dynamic_func_call_no_desc(__UNIQUE_ID(ddebug), fmt, func, ##__VA_ARGS__) > > +#define _dynamic_func_call_no_desc_cls(fmt, cat, func, ...) \ > > + __dynamic_func_call_no_desc_cls(__UNIQUE_ID(ddebug), cat, \ > > + fmt, func, ##__VA_ARGS__) > > + > > +#define _dynamic_func_call_no_desc(fmt, func, ...) \ > > + __dynamic_func_call_no_desc_cls(__UNIQUE_ID(ddebug), \ > > + _DPRINTK_SITE_UNCLASSED, \ > > + fmt, func, ##__VA_ARGS__) > > > > #define dynamic_pr_debug(fmt, ...) \ > > _dynamic_func_call(fmt, __dynamic_pr_debug, \ > > diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c > > index a56c1286ffa4..ee2129becacc 100644 > > --- a/lib/dynamic_debug.c > > +++ b/lib/dynamic_debug.c > > @@ -55,6 +55,8 @@ struct ddebug_query { > > const char *function; > > const char *format; > > unsigned int first_lineno, last_lineno; > > + unsigned int class_id; > > + unsigned int class_marked:1; > > }; > > > > > > /* > > @@ -170,6 +172,10 @@ static int ddebug_change(const struct ddebug_query *query, > > for (i = 0; i < dt->num_ddebugs; i++) { > > struct _ddebug *dp = &dt->ddebugs[i]; > > > > + /* match against the class_id, either given or default */ > > + if (query->class_id != dp->class_id) > > + continue; > > + > > Could that instead be: > > if (query->class_id != _DPRINTK_SITE_UNCLASSED && query->class_id != dp->class_id) > continue; > I think that is the effect currently. query construction adds in the default if the term wasnt specified. > That way by default we fall through and enable callsites for queries that do not have > the 'class' keyboard specified. thats what I was avoiding - I suppose its a matter of perspective all existing callsites are unclassified. - ie 15 to preserve the universe affected by old queries, adding 15 by default comports with this. then when class !=15 callsites are added, they are immune from effects by existing queries. classed queries are the alternate universe. and need "class N" in >control to get there. Id expect DRM would like the isolation, at least against inadvertent changes. that said, ``echo +p >control`` can only affect 1 universe at a time. ISTM this is fine, the only thing lost is 1/2 the workload for while true do echo "+p ; -p" >control; done > I think I would also disallow setting > _DPRINTK_SITE_UNCLASSED explicitly since it would now mean 'class *', and what if > we expand class_id size to greater than 15 at some point? it doesnt mean class *, it means class OTHER than 0-14 earlier version of parse_class disallowed 15 explicitly, I changed it to simplify testing. re-adding that constraint could be argued, but if class 15 is the default when unspecified, its hard to explain why "class 15" could draw an errror. we'd just increase bit-field size, class_id:5 and bump 15 to 31. is there an appropriate *_MAX macro to use, to add the right implications / connotations ? theres something vaguely overflowy / rollover-y here. What choice words would convey it succinctly ? > > > /* match against the source filename */ > > if (query->filename && > > !match_wildcard(query->filename, dp->filename) && > > @@ -308,6 +314,21 @@ static inline int parse_lineno(const char *str, unsigned int *val) > > return 0; > > } > > > > +static inline int parse_class(struct ddebug_query *query, const char *str) > > +{ > > + int rc; > > + unsigned int val; > > + > > + rc = kstrtouint(str, 10, &val); > > + if (rc < 0 || val > _DPRINTK_SITE_UNCLASSED) { > > + pr_err("expecting class:[0-%d], not %s\n", _DPRINTK_SITE_UNCLASSED, str); > > + return -EINVAL; > > + } > > + query->class_id = val; > > + query->class_marked = 1; > > + return 0; > > +} > > + > > static int parse_linerange(struct ddebug_query *query, const char *first) > > { > > char *last = strchr(first, '-'); > > @@ -421,6 +442,9 @@ static int ddebug_parse_query(char *words[], int nwords, > > } else if (!strcmp(keyword, "line")) { > > if (parse_linerange(query, arg)) > > return -EINVAL; > > + } else if (!strcmp(keyword, "class")) { > > + if (parse_class(query, arg)) > > + return -EINVAL; > > } else { > > pr_err("unknown keyword \"%s\"\n", keyword); > > return -EINVAL; > > @@ -428,6 +452,10 @@ static int ddebug_parse_query(char *words[], int nwords, > > if (rc) > > return rc; > > } > > + /* post-validate the query, set default */ > > + if (!query->class_marked) > > + query->class_id = _DPRINTK_SITE_UNCLASSED; > > + > > vpr_info_dq(query, "parsed"); > > return 0; > > } > > @@ -900,7 +928,11 @@ static int ddebug_proc_show(struct seq_file *m, void *p) > > iter->table->mod_name, dp->function, > > ddebug_describe_flags(dp->flags, &flags)); > > seq_escape(m, dp->format, "\t\r\n\""); > > - seq_puts(m, "\"\n"); > > + seq_puts(m, "\""); > > + > > + if (dp->class_id != _DPRINTK_SITE_UNCLASSED) > > + seq_printf(m, " cls:%u", dp->class_id); > > + seq_puts(m, "\n"); > > > > return 0; > > } ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/5] dyndbg: add class_id field and query support 2022-03-12 1:06 ` jim.cromie @ 2022-03-14 21:29 ` Jason Baron 2022-03-28 19:07 ` jim.cromie 0 siblings, 1 reply; 11+ messages in thread From: Jason Baron @ 2022-03-14 21:29 UTC (permalink / raw) To: jim.cromie Cc: Greg KH, LKML, Daniel Vetter, Sean Paul, robdclark, Rasmus Villemoes, Joe Perches, dri-devel, amd-gfx mailing list, intel-gvt-dev, Intel Graphics Development On 3/11/22 20:06, jim.cromie@gmail.com wrote: > On Fri, Mar 11, 2022 at 12:06 PM Jason Baron <jbaron@akamai.com> wrote: >> >> >> >> On 3/10/22 23:47, Jim Cromie wrote: >>> DRM defines/uses 10 enum drm_debug_category's to create exclusive >>> classes of debug messages. To support this directly in dynamic-debug, >>> add the following: >>> >>> - struct _ddebug.class_id:4 - 4 bits is enough >>> - define _DPRINTK_SITE_UNCLASSED 15 - see below >>> >>> and the query support: >>> - struct _ddebug_query.class_id >>> - ddebug_parse_query - looks for "class" keyword, then calls.. >>> - parse_class - accepts uint: 0-15, sets query.class_id and marker >>> - vpr_info_dq - displays new field >>> - ddebug_proc_show - append column with "cls:%d" if !UNCLASSED >>> >>> With the patch, one can enable current/unclassed callsites by: >>> >>> #> echo drm class 15 +p > /proc/dynamic_debug/control >>> >> >> To me, this is hard to read, what the heck is '15'? I have to go look it >> up in the control file and it's not descriptive. I think that using >> classes/categories makes sense but I'm wondering if it can be a bit more >> user friendly? Perhaps, we can pass an array of strings that is indexed >> by the class id to each pr_debug() site that wants to use class. So >> something like: >> > > Im not at all averse to nice names, but as something added on. > > 1st, the interface to make friendlier is DRM's > > echo 0x04 > /sys/module/drm/parameters/debug # which category ? > > parm: debug:Enable debug output, where each bit enables a > debug category. > Bit 0 (0x01) will enable CORE messages (drm core code) > Bit 1 (0x02) will enable DRIVER messages (drm controller code) > Bit 2 (0x04) will enable KMS messages (modesetting code) > > echo DRM_UT_DRIVER,DRM_UT_KMS > /sys/module/drm/parameters/debug # > now its pretty clear > > If that works, its cuz DEFINE_DYNAMIC_DEBUG_CLASSBITS() > plucks class symbols out of its __VA_ARGS__, and #stringifes them. > So that macro could then build the 1-per-module static constant string array > and (only) the callbacks would be able to use it. > > from there, it shouldnt be hard to ref that from the module's ddebug_table, > so parse_query could validate class args against the module given (or > fail if none given) > > Speaking strictly, theres a gap: > echo module * class DRM_UT_KMS +p > control > is nonsense for * other than drm + drivers, > but its fair/safe to just disallow wildcards on modname for this purpose. > > it does however imply that module foo must exist for FOO_CAT_1 to be usable. > thats not currently the case: > bash-5.1# echo module foo +p > /proc/dynamic_debug/control > [ 15.403749] dyndbg: read 14 bytes from userspace > [ 15.405413] dyndbg: query 0: "module foo +p" mod:* > [ 15.406486] dyndbg: split into words: "module" "foo" "+p" > [ 15.407070] dyndbg: op='+' > [ 15.407388] dyndbg: flags=0x1 > [ 15.407809] dyndbg: *flagsp=0x1 *maskp=0xffffffff > [ 15.408300] dyndbg: parsed: func="" file="" module="foo" format="" > lineno=0-0 class=15 > [ 15.409151] dyndbg: no matches for query > [ 15.409591] dyndbg: no-match: func="" file="" module="foo" > format="" lineno=0-0 class=15 > [ 15.410524] dyndbg: processed 1 queries, with 0 matches, 0 errs > bash-5.1# > > ISTM we can keep that "0 errs" response for that case, but still reject this: > > echo module foo class FOO_NOT_HERE +p > /proc/dynamic_debug/control > > Ok, yeah so I guess I'm thinking about the 'class' more as global namespace, so that for example, it could span modules, or be specific to certain modules. I'm also thinking of it as a 'string' which is maybe hierarchical, so that it's clear what sub-system, or sub-sub-system it belongs to. So for DRM for example, it could be a string like "DRM:CORE". The index num I think is still helpful for implementation so we don't have to store a pointer size, but I don't think it's really exposed (except perhaps in module params b/c drm is doing that already?). >> enum levels { >> LOW, >> MEDIUM, >> HIGH >> }; > > I want to steer clear of "level" anything, > since 2>1 implies non independence of the categories > > Agreed, that was a bad example on my part. I've put together a rough patch on top of your series, to make my thinking hopefully clear. I also think that the module param callback thing could be implemented in this 'global' space with the 0-14 low numbers, by adding some sort of offset into the table. IE you would add the low number + the offset to get the 'string' to add to the ddebug query. I commented it out in my patch below b/c I didn't implement that part. Thoughts? diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h index 664bb83778d2..b0bc1b536d54 100644 --- a/include/linux/dynamic_debug.h +++ b/include/linux/dynamic_debug.h @@ -8,6 +8,14 @@ #include <linux/build_bug.h> +enum ddebug_categories { + FOO_BAR = 0, + DRM_A = 1, + DRM_B = 2, + DRM_C = 3, + //_DPRINTK_SITE_DEFAULT 63 is max +}; + /* * An instance of this structure is created in a special * ELF section at every dynamic debug callsite. At runtime, @@ -23,9 +31,9 @@ struct _ddebug { const char *filename; const char *format; unsigned int lineno:18; -#define CLS_BITS 4 +#define CLS_BITS 6 unsigned int class_id:CLS_BITS; -#define _DPRINTK_SITE_UNCLASSED ((1 << CLS_BITS) - 1) +#define _DPRINTK_SITE_DEFAULT ((1 << CLS_BITS) - 1) /* * The flags field controls the behaviour at the callsite. * The bits here are changed dynamically when the user @@ -101,11 +109,11 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor, .class_id = cls, \ _DPRINTK_KEY_INIT \ }; \ - BUILD_BUG_ON_MSG(cls > _DPRINTK_SITE_UNCLASSED, \ + BUILD_BUG_ON_MSG(cls > _DPRINTK_SITE_DEFAULT, \ "classid value overflow") #define DEFINE_DYNAMIC_DEBUG_METADATA(name, fmt) \ - DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, _DPRINTK_SITE_UNCLASSED, fmt) + DEFINE_DYNAMIC_DEBUG_METADATA_CLS(name, _DPRINTK_SITE_DEFAULT, fmt) #ifdef CONFIG_JUMP_LABEL @@ -149,11 +157,11 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor, } while (0) #define __dynamic_func_call(id, fmt, func, ...) \ - __dynamic_func_call_cls(id, _DPRINTK_SITE_UNCLASSED, \ + __dynamic_func_call_cls(id, _DPRINTK_SITE_DEFAULT, \ fmt, func, ##__VA_ARGS__) #define __dynamic_func_call_no_desc(id, fmt, func, ...) \ - __dynamic_func_call_no_desc_cls(id, _DPRINTK_SITE_UNCLASSED, \ + __dynamic_func_call_no_desc_cls(id, _DPRINTK_SITE_DEFAULT, \ fmt, func, ##__VA_ARGS__) /* @@ -167,7 +175,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor, #define _dynamic_func_call_cls(cls, fmt, func, ...) \ __dynamic_func_call_cls(__UNIQUE_ID(ddebug), cls, fmt, func, ##__VA_ARGS__) #define _dynamic_func_call(fmt, func, ...) \ - _dynamic_func_call_cls(_DPRINTK_SITE_UNCLASSED, fmt, func, ##__VA_ARGS__) + _dynamic_func_call_cls(_DPRINTK_SITE_DEFAULT, fmt, func, ##__VA_ARGS__) /* * A variant that does the same, except that the descriptor is not @@ -180,7 +188,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor, #define _dynamic_func_call_no_desc(fmt, func, ...) \ __dynamic_func_call_no_desc_cls(__UNIQUE_ID(ddebug), \ - _DPRINTK_SITE_UNCLASSED, \ + _DPRINTK_SITE_DEFAULT, \ fmt, func, ##__VA_ARGS__) #define dynamic_pr_debug(fmt, ...) \ @@ -284,7 +292,7 @@ struct dyndbg_classbits_param { static struct dyndbg_classbits_param ddcats_##_var = { \ .bits = &(_var), \ .flags = _flgs, \ - .classes = { __VA_ARGS__, _DPRINTK_SITE_UNCLASSED } \ + .classes = { __VA_ARGS__, _DPRINTK_SITE_DEFAULT } \ }; \ module_param_cb(fsname, ¶m_ops_dyndbg_classbits, \ &ddcats_##_var, 0644) diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index 11c8d0771cd2..0f390638e46c 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c @@ -39,6 +39,14 @@ #include <rdma/ib_verbs.h> +static const char * const cat_to_strings[] = { + [FOO_BAR] = "foo:bar", + [DRM_A] = "drm:A", + [DRM_B] = "drm:B", + [DRM_C] = "drm:C", + [_DPRINTK_SITE_DEFAULT] = "default", +}; + extern struct _ddebug __start___dyndbg[]; extern struct _ddebug __stop___dyndbg[]; @@ -55,8 +63,7 @@ struct ddebug_query { const char *function; const char *format; unsigned int first_lineno, last_lineno; - unsigned int class_id; - unsigned int class_marked:1; + const char *class_string; }; struct ddebug_iter { @@ -136,13 +143,13 @@ static void vpr_info_dq(const struct ddebug_query *query, const char *msg) fmtlen--; } - v3pr_info("%s: func=\"%s\" file=\"%s\" module=\"%s\" format=\"%.*s\" lineno=%u-%u class=%u\n", + v3pr_info("%s: func=\"%s\" file=\"%s\" module=\"%s\" format=\"%.*s\" lineno=%u-%u class=%s\n", msg, query->function ?: "", query->filename ?: "", query->module ?: "", fmtlen, query->format ?: "", - query->first_lineno, query->last_lineno, query->class_id); + query->first_lineno, query->last_lineno, query->class_string); } /* @@ -158,7 +165,7 @@ static int ddebug_change(const struct ddebug_query *query, struct ddebug_table *dt; unsigned int newflags; unsigned int nfound = 0; - struct flagsbuf fbuf; + struct flagsbuf fbuf, nbuf; /* search for matching ddebugs */ mutex_lock(&ddebug_lock); @@ -172,8 +179,8 @@ static int ddebug_change(const struct ddebug_query *query, for (i = 0; i < dt->num_ddebugs; i++) { struct _ddebug *dp = &dt->ddebugs[i]; - /* match against the class_id, either given or default */ - if (query->class_id != dp->class_id) + /* if class is not set fall through, ot check if it matches */ + if (query->class_string && strcmp(query->class_string, cat_to_strings[dp->class_id])) continue; /* match against the source filename */ @@ -223,11 +230,12 @@ 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, + ddebug_describe_flags(dp->flags, &fbuf), + ddebug_describe_flags(newflags, &nbuf)); dp->flags = newflags; - v4pr_info("changed %s:%d [%s]%s =%s\n", - trim_prefix(dp->filename), dp->lineno, - dt->mod_name, dp->function, - ddebug_describe_flags(dp->flags, &fbuf)); } } mutex_unlock(&ddebug_lock); @@ -314,21 +322,6 @@ static inline int parse_lineno(const char *str, unsigned int *val) return 0; } -static inline int parse_class(struct ddebug_query *query, const char *str) -{ - int rc; - unsigned int val; - - rc = kstrtouint(str, 10, &val); - if (rc < 0 || val > _DPRINTK_SITE_UNCLASSED) { - pr_err("expecting class:[0-%d], not %s\n", _DPRINTK_SITE_UNCLASSED, str); - return -EINVAL; - } - query->class_id = val; - query->class_marked = 1; - return 0; -} - static int parse_linerange(struct ddebug_query *query, const char *first) { char *last = strchr(first, '-'); @@ -443,8 +436,7 @@ static int ddebug_parse_query(char *words[], int nwords, if (parse_linerange(query, arg)) return -EINVAL; } else if (!strcmp(keyword, "class")) { - if (parse_class(query, arg)) - return -EINVAL; + rc = check_set(&query->class_string, arg, "class"); } else { pr_err("unknown keyword \"%s\"\n", keyword); return -EINVAL; @@ -452,9 +444,6 @@ static int ddebug_parse_query(char *words[], int nwords, if (rc) return rc; } - /* post-validate the query, set default */ - if (!query->class_marked) - query->class_id = _DPRINTK_SITE_UNCLASSED; vpr_info_dq(query, "parsed"); return 0; @@ -616,6 +605,8 @@ int param_set_dyndbg_classbits(const char *instr, const struct kernel_param *kp) } vpr_info("set_dyndbg_classbits: new 0x%lx old 0x%lx\n", inbits, *dcp->bits); + +/* for (i = 0; (i < _DPRINTK_SITE_UNCLASSED && dcp->classes[i] < _DPRINTK_SITE_UNCLASSED); i++) { @@ -630,6 +621,7 @@ int param_set_dyndbg_classbits(const char *instr, const struct kernel_param *kp) matches, dcp->classes[i]); totct += matches; } +*/ *dcp->bits = inbits; vpr_info("total matches: %d\n", totct); return 0; @@ -978,8 +970,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p) seq_escape(m, dp->format, "\t\r\n\""); seq_puts(m, "\""); - if (dp->class_id != _DPRINTK_SITE_UNCLASSED) - seq_printf(m, " cls:%u", dp->class_id); + seq_printf(m, " cat:%s", cat_to_strings[dp->class_id]); seq_puts(m, "\n"); return 0; ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/5] dyndbg: add class_id field and query support 2022-03-14 21:29 ` Jason Baron @ 2022-03-28 19:07 ` jim.cromie 0 siblings, 0 replies; 11+ messages in thread From: jim.cromie @ 2022-03-28 19:07 UTC (permalink / raw) To: Jason Baron Cc: Greg KH, LKML, Daniel Vetter, Sean Paul, robdclark, Rasmus Villemoes, Joe Perches, dri-devel, amd-gfx mailing list, intel-gvt-dev, Intel Graphics Development On Mon, Mar 14, 2022 at 3:30 PM Jason Baron <jbaron@akamai.com> wrote: > > > > On 3/11/22 20:06, jim.cromie@gmail.com wrote: > > On Fri, Mar 11, 2022 at 12:06 PM Jason Baron <jbaron@akamai.com> wrote: > >> > >> > >> > >> On 3/10/22 23:47, Jim Cromie wrote: > >>> > >>> With the patch, one can enable current/unclassed callsites by: > >>> > >>> #> echo drm class 15 +p > /proc/dynamic_debug/control > >>> > >> > >> To me, this is hard to read, what the heck is '15'? I have to go look it > >> up in the control file and it's not descriptive. I think that using > >> classes/categories makes sense but I'm wondering if it can be a bit more > >> user friendly? Perhaps, we can pass an array of strings that is indexed > >> by the class id to each pr_debug() site that wants to use class. So > >> something like: hi Jason, Im now in basically full agreement with you 1. .class_id is a "global" space, in that all callsites have one. 2. 0-15 is an exceedingly small range for a global space Fix that by 3. make it private (by removing "class N" parsing) now nobody can do echo module * class N +p >control 4. add private/per-module "string" -> class_id map each module will have to declare the class-strings they use/accept opt-in - want coordinated / shared names for DRM_UT_KMS etc. 5. validate input using the known class_string -> class_id then, this is knowably right or wrong, depending on DRM_FOO: echo module drm class DRM_FOO +p > control it also means that echo class DRM_UT_KMS +p >control is both wellformed and minimal; any module that has DRM_UT_KMS defined will know which class_id *they* have mapped it to. theres no global "DRM_UT_KMS" to be abused. So Ive been working towards that.. Heres my current biggest roadblock DEFINE_DYNAMIC_DEBUG_CLASSBITS creates the class-strings[] array declaratively, at compile-time. This array is attached to the kernel-param.args, so it can be used by the callbacks (to construct the command) But its not obviously available from outside the sysfs knob that its attached to, as is needed to apply command >control generally. If I can attach the class-strings[] to struct ddebug_table, then ddebug_change() can use it to validate >control input. So, how to attach ? its got to work for both builtin & loadable modules. (which precludes something in struct module ?) I looked for a kernel_param_register callback, came up empty. Trying to add it feels invasive / imposing. > > > > If that works, its cuz DEFINE_DYNAMIC_DEBUG_CLASSBITS() > > plucks class symbols out of its __VA_ARGS__, and #stringifes them. > > So that macro could then build the 1-per-module static constant string array > > and (only) the callbacks would be able to use it. > > So Ive been tinkering hard on this macro, since its pretty central to the interface defn. heres some choices: this is what Ive been working towards. using enum symbols directly like this associates them by grep, in contrast, class-strings are mealy-mouthed, milquetoast. DEFINE_DYNAMIC_DEBUG_CLASSBITS(debug, __drm_debug, "p", "enable drm.debug categories - 1 bit per category", DRM_UT_CORE, DRM_UT_DRIVER, DRM_UT_KMS, DRM_UT_PRIME, DRM_UT_ATOMIC, DRM_UT_VBL, DRM_UT_STATE, DRM_UT_LEASE, DRM_UT_DP, DRM_UT_DRMRES); I found a slick MAP ( ) macro to do this: #define DEFINE_DYNAMIC_DEBUG_CLASSBITS(fsname, _var, _flgs, desc, ...) \ MODULE_PARM_DESC(fsname, desc); \ static struct dyndbg_classbits_param ddcats_##_var = { \ .bits = &(_var), \ .flags = _flgs, \ .classes = { __VA_ARGS__, _DPRINTK_CLASS_DFLT }, \ .class_names = { mgMAP(__stringify, sCOMMA, \ __VA_ARGS__, _DPRINTK_CLASS_DFLT) } \ }; \ module_param_cb(fsname, ¶m_ops_dyndbg_classbits, \ &ddcats_##_var, 0644) __VA_ARGS__ is used 2x .class_names is available for validating command >control As much as I like the above, the MAP macro has a longer, more risky path to the kernel so a more modest alternative: module user defines class-strings in interface, but they *must* align manually with the enum values they correspond to; the order determines the bit-pos in the sysfs node, since the interface no longer provides the enum values themselves. DEFINE_DYNAMIC_DEBUG_CLASS_STRINGS(debug, __drm_debug, "p", "enable drm.debug categories - 1 bit per category", "DRM_UT_CORE", "DRM_UT_DRIVER", "DRM_UT_KMS", different name allows CLASSBITs or similar in future, if MAP works out. class-strings are completely defined by users, drm can drop UT naming TLDR: FWIW iSTM that the same macro will support the coordinated use of class-strings across multiple modules. drm_print.c - natural home for exposed sysfs node amdgpu/, drm_helper/ i915/ nouveau/ will all need a DEFINE_DD added, so that ddebug_change() can allow those .class_ids to be controlled. sysfs perm inits can disable their nodes, since theyre coordinated. > > > > Ok, yeah so I guess I'm thinking about the 'class' more as global namespace, > so that for example, it could span modules, or be specific to certain modules. > I'm also thinking of it as a 'string' which is maybe hierarchical, so that it's > clear what sub-system, or sub-sub-system it belongs to. So for DRM for example, > it could be a string like "DRM:CORE". The index num I think is still helpful for > implementation so we don't have to store a pointer size, but I don't think it's > really exposed (except perhaps in module params b/c drm is doing that already?). > So what Ive got here is as described above, I just need a few bright ideas, then I can bring it together. got a spare tuit? Jim ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/5] dyndbg: add DEFINE_DYNAMIC_DEBUG_CLASSBITS macro 2022-03-11 4:47 [PATCH 0/5] dyndbg add exclusive class support Jim Cromie 2022-03-11 4:47 ` [PATCH 1/5] dyndbg: fix static_branch manipulation Jim Cromie 2022-03-11 4:47 ` [PATCH 2/5] dyndbg: add class_id field and query support Jim Cromie @ 2022-03-11 4:47 ` Jim Cromie 2022-03-11 4:47 ` [PATCH 4/5] dyndbg: drop EXPORTed dynamic_debug_exec_queries Jim Cromie 2022-03-11 4:47 ` [PATCH 5/5] dyndbg: show both old and new in change-info Jim Cromie 4 siblings, 0 replies; 11+ messages in thread From: Jim Cromie @ 2022-03-11 4:47 UTC (permalink / raw) To: jbaron, gregkh, linux-kernel Cc: daniel.vetter, seanpaul, robdclark, linux, joe, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx, Jim Cromie DEFINE_DYNAMIC_DEBUG_CLASSBITS(fsname, var, bitmap_desc, classes..) allows users to create a drm.debug style (bitmap) sysfs interface, to control sets of pr_debug's according to their .class_id's This wraps existing "class" keyword and behavior: bash-5.1# echo <<END > /proc/dynamic_debug/control > module drm class 0 +p > module drm class 2 +p > END With the macro used (in a client), this is basically equivalent: # but this also clears other flags, if theyre set echo 0x05 > /sys/module/drm/parameters/debug To use: DEFINE_DYNAMIC_DEBUG_CLASSBITS(debug, __drm_debug, "pmfl", "drm.debug - bits => categories:", /* vector of uint:4 symbols, ala enum drm_debug_category, 15 is EOL */ DRM_UT_CORE, DRM_UT_DRIVER, DRM_UT_KMS ... ); The 3rd arg is a string with any of the dyndbg.flags [pmflt_]+ Full exposure of the flags here lets the module author: - fully customize/take-over the decorations of enabled sites. generally leaving decorations to user is preferred. - aim the debug-stream: now printk, later tracefs. using both together means more work (p or T, in practice) iface doesn't care about new flags added later - declare 2 separate sysfs-knobs, one each for p, T, if desired. - decorations are per callsite, shared across sysfs-knobs for any controlled classes To support the macro, the patch adds: - int param_set_dyndbg_classbits() - int param_get_dyndbg_classbits() - struct kernel_param_ops param_ops_dyndbg_classbits 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 vector of classes (drm.debug "categories") being controlled This in-line vector of constants (uint [0-14]) specifies a sequence of controlling bits (by position, starting at 0) with the values naming the class_id's mapped to that bit. A value of _DPRINTK_SITE_UNCLASSED terminates the vector processing by param_set_dyndbg_classbits(), and is appended by the macro to insure a defined termination after max 15 classes are applied. Technically, the vector is a flex-array, but its size is practically limited to max 15 in length (repeats are pointless). B- a pointer to the user module's ulong holding the bits/state. By accessing client's bit-state, we coordinate with existing code that still uses drm_debug_enabled(), so they work unchanged. The change to ulong allows use of BIT() etc. NOTES: _DPRINTK_SITE_UNCLASSED = 15, ie 2**CLS_BITS-1, deserves special mention; it already marks all existing pr-debug callsites, only the new drm.debug callsites are initialized to other (DRM_UT_*) values. _DPRINTK_SITE_UNCLASSED is used in param_set_dyndbg_classbits() to limit the range of bits that are processed to what fits in uint:4. It also terminates the class-id list passed into the macro, so dont use it halfway through your list of classes-to-control. param_set_dyndbg_classbits() compares new vs old bits, and only updates each class on changes. This maximally preserves the underlying state, which may have been customized at some point via `echo $cmd >control`. So if users really want to know that all prdbgs are set precisely, they must pre-clear then set. Identity mapping in (A) is encouraged. Your vector should exclude 15, if used, it terminates the list prematurely; any following bit mappings will be ignored (it is the default/non category). The whole (A) vector/list passed to the macro is: - not strictly needed: 0-N bits are scanned, only N is needed in the macro interface to do this, not the whole list. - 0-N list allows juggling the bit->class map Identity map is preferred. 15 terminates list if used. (macro impl does this) That said, (A) is self-documenting; the explicit list is greppable, 'DRM_UT_*' provides lots of clues. Further, it could be upgraded, something like: _pick_sym_(DRM_UT_CORE, "mumble something useful about CORE debug") _pick_sym_(a,b) a // gives us what we need here _pick_help_(a,b) #a " : " b // mod-info fodder Signed-off-by: Jim Cromie <jim.cromie@gmail.com> --- include/linux/dynamic_debug.h | 50 +++++++++++++++++++++++ lib/dynamic_debug.c | 77 +++++++++++++++++++++++++++++++++++ 2 files changed, 127 insertions(+) diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h index d4b48f3cc6e8..e83c4e36ad29 100644 --- a/include/linux/dynamic_debug.h +++ b/include/linux/dynamic_debug.h @@ -209,6 +209,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_classbits(const char *instr, const struct kernel_param *kp); +int param_get_dyndbg_classbits(char *buffer, const struct kernel_param *kp); + #else /* !CONFIG_DYNAMIC_DEBUG_CORE */ #include <linux/string.h> @@ -255,6 +259,52 @@ static inline int dynamic_debug_exec_queries(const char *query, const char *modn return 0; } +struct kernel_param; +static inline int param_set_dyndbg_classbits(const char *instr, const struct kernel_param *kp) +{ return 0; } +static inline int param_get_dyndbg_classbits(char *buffer, const struct kernel_param *kp) +{ return 0; } + #endif /* !CONFIG_DYNAMIC_DEBUG_CORE */ +#define FLAGS_LEN 8 +struct dyndbg_classbits_param { + unsigned long *bits; /* ref to shared state */ + const char flags[FLAGS_LEN]; /* toggle these flags on bit-changes */ + const int classes[]; /* indexed by bitpos */ +}; + +#if defined(CONFIG_DYNAMIC_DEBUG) || defined(CONFIG_DYNAMIC_DEBUG_CORE) +/** + * DEFINE_DYNAMIC_DEBUG_CLASSBITS() - bitmap control of classed pr_debugs + * @sysname: sysfs-node name + * @_var: C-identifier holding bit-vector (Bits 0-14 are usable) + * @_flgs: string with dyndbg flags: 'p' and/or 'T', and maybe "fmlt" also. + * @desc: string summarizing the controls provided + * @classes: vector of callsite.class_id's (uint:4, 15 is reserved) + * + * This macro implements a DRM.debug API style bitmap, mapping bits + * 0-14 to classes of prdbg's, as initialized in their .class_id fields. + * @_flgs chooses the debug recipient; p - syslog, T - tracefs, and + * can include log decorations; m - module, f - function, l - line_num + */ +#define DEFINE_DYNAMIC_DEBUG_CLASSBITS(fsname, _var, _flgs, desc, ...) \ + MODULE_PARM_DESC(fsname, desc); \ + static struct dyndbg_classbits_param ddcats_##_var = { \ + .bits = &(_var), \ + .flags = _flgs, \ + .classes = { __VA_ARGS__, _DPRINTK_SITE_UNCLASSED } \ + }; \ + module_param_cb(fsname, ¶m_ops_dyndbg_classbits, \ + &ddcats_##_var, 0644) + +extern const struct kernel_param_ops param_ops_dyndbg_classbits; + +#else /* no dyndbg configured, throw error on macro use */ + +#define DEFINE_DYNAMIC_DEBUG_CLASSBITS(fsname, var, bitmap_desc, ...) \ + BUILD_BUG_ON_MSG(1, "CONFIG_DYNAMIC_DEBUG* needed to use this macro: " #fsname) + +#endif + #endif diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index ee2129becacc..704361af5b23 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c @@ -611,6 +611,83 @@ 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_classbits - 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 the + * DEFINE_DYNAMIC_DEBUG_BITGRPS.classbits argument. + * + * Returns: 0 or <0 if error. + */ +int param_set_dyndbg_classbits(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_classbits_param *dcp = kp->arg; + + if (!dcp) { + pr_err("set_dyndbg_classbits: no bits=>queries map\n"); + return -EINVAL; + } + rc = kstrtoul(instr, 0, &inbits); + if (rc) { + pr_err("set_dyndbg_classbits: expecting unsigned int\n"); + return rc; + } + vpr_info("set_dyndbg_classbits: new 0x%lx old 0x%lx\n", inbits, *dcp->bits); + + for (i = 0; (i < _DPRINTK_SITE_UNCLASSED && + dcp->classes[i] < _DPRINTK_SITE_UNCLASSED); i++) { + + if (test_bit(i, &inbits) == test_bit(i, dcp->bits)) + continue; + snprintf(query, FMT_QUERY_SIZE, "class %d %cT", + dcp->classes[i], test_bit(i, &inbits) ? '+' : '-'); + + matches = ddebug_exec_queries(query, KP_MOD_NAME); + + v2pr_info("bit-%d: %d matches on class:%u\n", i, + matches, dcp->classes[i]); + totct += matches; + } + *dcp->bits = inbits; + vpr_info("total matches: %d\n", totct); + return 0; +} +EXPORT_SYMBOL(param_set_dyndbg_classbits); + +/** + * param_get_dyndbg_classbits - classbits reader + * @buffer: string description of controlled bits -> classes + * @kp: kp->arg has state: bits, map + * + * Reads last written bits, underlying prdbg state may have changed since. + * Returns: #chars written or <0 on error + */ +int param_get_dyndbg_classbits(char *buffer, const struct kernel_param *kp) +{ + const struct dyndbg_classbits_param *p = kp->arg; + unsigned long val = *p->bits; + + return scnprintf(buffer, PAGE_SIZE, "0x%lx\n", val); +} +EXPORT_SYMBOL(param_get_dyndbg_classbits); + +const struct kernel_param_ops param_ops_dyndbg_classbits = { + .set = param_set_dyndbg_classbits, + .get = param_get_dyndbg_classbits, +}; +EXPORT_SYMBOL(param_ops_dyndbg_classbits); + #define PREFIX_SIZE 64 static int remaining(int wrote) -- 2.35.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 4/5] dyndbg: drop EXPORTed dynamic_debug_exec_queries 2022-03-11 4:47 [PATCH 0/5] dyndbg add exclusive class support Jim Cromie ` (2 preceding siblings ...) 2022-03-11 4:47 ` [PATCH 3/5] dyndbg: add DEFINE_DYNAMIC_DEBUG_CLASSBITS macro Jim Cromie @ 2022-03-11 4:47 ` Jim Cromie 2022-03-11 4:47 ` [PATCH 5/5] dyndbg: show both old and new in change-info Jim Cromie 4 siblings, 0 replies; 11+ messages in thread From: Jim Cromie @ 2022-03-11 4:47 UTC (permalink / raw) To: jbaron, gregkh, linux-kernel Cc: daniel.vetter, seanpaul, robdclark, linux, joe, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx, Jim Cromie This exported fn is effectively obsoleted by Commit:HEAD~2, so remove it. Its intent was to allow drm.debug to use the exported function to implement its drm.debug bitmap api using dynamic_debug. Instead, HEAD~2 implements the bitmap inside dyndbg, using the internal fn that the export wraps. Since there are no other expected users, and any prospects would likely reuse the bitmap or a straightforward extension of it, we can drop the exported function until its really needed. This also drops the CONFIG_DYNAMIC_DEBUG=N stub-func, and its pr_warn(), which I avoided in 2012, then added in 2020 :-/ Fixes: a2d375eda771 ("dyndbg: refine export, rename to dynamic_debug_exec_queries()") Fixes: 4c0d77828d4f ("dyndbg: export ddebug_exec_queries") Signed-off-by: Jim Cromie <jim.cromie@gmail.com> --- include/linux/dynamic_debug.h | 9 --------- lib/dynamic_debug.c | 29 ----------------------------- 2 files changed, 38 deletions(-) diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h index e83c4e36ad29..664bb83778d2 100644 --- a/include/linux/dynamic_debug.h +++ b/include/linux/dynamic_debug.h @@ -60,9 +60,6 @@ struct _ddebug { #if defined(CONFIG_DYNAMIC_DEBUG_CORE) -/* 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); extern int ddebug_remove_module(const char *mod_name); @@ -253,12 +250,6 @@ static inline int ddebug_dyndbg_module_param_cb(char *param, char *val, rowsize, groupsize, buf, len, ascii); \ } while (0) -static inline int dynamic_debug_exec_queries(const char *query, const char *modname) -{ - pr_warn("kernel not built with CONFIG_DYNAMIC_DEBUG_CORE\n"); - return 0; -} - struct kernel_param; static inline int param_set_dyndbg_classbits(const char *instr, const struct kernel_param *kp) { return 0; } diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index 704361af5b23..b15a9c715e5b 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c @@ -582,35 +582,6 @@ static int ddebug_exec_queries(char *query, const char *modname) return nfound; } -/** - * dynamic_debug_exec_queries - select and change dynamic-debug prints - * @query: query-string described in admin-guide/dynamic-debug-howto - * @modname: string containing module name, usually &module.mod_name - * - * This uses the >/proc/dynamic_debug/control reader, allowing module - * authors to modify their dynamic-debug callsites. The modname is - * canonically struct module.mod_name, but can also be null or a - * module-wildcard, for example: "drm*". - */ -int dynamic_debug_exec_queries(const char *query, const char *modname) -{ - int rc; - char *qry; /* writable copy of query */ - - if (!query) { - pr_err("non-null query/command string expected\n"); - return -EINVAL; - } - qry = kstrndup(query, PAGE_SIZE, GFP_KERNEL); - if (!qry) - return -ENOMEM; - - rc = ddebug_exec_queries(qry, modname); - kfree(qry); - return rc; -} -EXPORT_SYMBOL_GPL(dynamic_debug_exec_queries); - #ifdef CONFIG_MODULES #define KP_MOD_NAME kp->mod->name #else -- 2.35.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 5/5] dyndbg: show both old and new in change-info 2022-03-11 4:47 [PATCH 0/5] dyndbg add exclusive class support Jim Cromie ` (3 preceding siblings ...) 2022-03-11 4:47 ` [PATCH 4/5] dyndbg: drop EXPORTed dynamic_debug_exec_queries Jim Cromie @ 2022-03-11 4:47 ` Jim Cromie 4 siblings, 0 replies; 11+ messages in thread From: Jim Cromie @ 2022-03-11 4:47 UTC (permalink / raw) To: jbaron, gregkh, linux-kernel Cc: daniel.vetter, seanpaul, robdclark, linux, joe, dri-devel, amd-gfx, intel-gvt-dev, intel-gfx, Jim Cromie print old -> new flag values in the info("change") message. Signed-off-by: Jim Cromie <jim.cromie@gmail.com> --- lib/dynamic_debug.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c index b15a9c715e5b..cceac8ebbacd 100644 --- a/lib/dynamic_debug.c +++ b/lib/dynamic_debug.c @@ -158,7 +158,7 @@ static int ddebug_change(const struct ddebug_query *query, struct ddebug_table *dt; unsigned int newflags; unsigned int nfound = 0; - struct flagsbuf fbuf; + struct flagsbuf fbuf, nbuf; /* search for matching ddebugs */ mutex_lock(&ddebug_lock); @@ -223,11 +223,12 @@ 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, + ddebug_describe_flags(dp->flags, &fbuf), + ddebug_describe_flags(newflags, &nbuf)); dp->flags = newflags; - v4pr_info("changed %s:%d [%s]%s =%s\n", - trim_prefix(dp->filename), dp->lineno, - dt->mod_name, dp->function, - ddebug_describe_flags(dp->flags, &fbuf)); } } mutex_unlock(&ddebug_lock); -- 2.35.1 ^ permalink raw reply related [flat|nested] 11+ messages in thread
end of thread, other threads:[~2022-03-28 19:08 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2022-03-11 4:47 [PATCH 0/5] dyndbg add exclusive class support Jim Cromie 2022-03-11 4:47 ` [PATCH 1/5] dyndbg: fix static_branch manipulation Jim Cromie 2022-03-11 18:03 ` Jason Baron 2022-03-11 4:47 ` [PATCH 2/5] dyndbg: add class_id field and query support Jim Cromie 2022-03-11 19:06 ` Jason Baron 2022-03-12 1:06 ` jim.cromie 2022-03-14 21:29 ` Jason Baron 2022-03-28 19:07 ` jim.cromie 2022-03-11 4:47 ` [PATCH 3/5] dyndbg: add DEFINE_DYNAMIC_DEBUG_CLASSBITS macro Jim Cromie 2022-03-11 4:47 ` [PATCH 4/5] dyndbg: drop EXPORTed dynamic_debug_exec_queries Jim Cromie 2022-03-11 4:47 ` [PATCH 5/5] dyndbg: show both old and new in change-info Jim Cromie
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).