2021-11-05 22:30:56

by Jim Cromie

[permalink] [raw]
Subject: [PATCH v10 00/10] use DYNAMIC_DEBUG to implement DRM.debug & DRM.trace

Hi Jason, Greg, DRM-everyone,

This patchset has 3 separate but related parts:

1. DEFINE_DYNAMIC_DEBUG_BITGRPS [patch 1/10]

Declares DRM.debug style bitmap, bits control pr_debugs by matching formats
Adds callback to translate bits to $cmd > dynamic_debug/control
This could obsolete EXPORT(dynamic_debug_exec_queries) not included.

/* anticipated_usage */
static struct dyndbg_desc drm_categories_map[] = {
[0] = { DRM_DBG_CAT_CORE },
[1] = { DRM_DBG_CAT_DRIVER },
[2] = { DRM_DBG_CAT_KMS },
[3] = { DRM_DBG_CAT_PRIME }, ... };

DEFINE_DYNAMIC_DEBUG_BITGRPS(debug, __drm_debug,
" bits control drm.debug categories ",
drm_categories_map);

Please consider this patch for -next:
- new interface, new code, no users to break
- allows DRM folks to consider in earnest.
- api bikeshedding to do ?
struct dyndbg_desc isnt that great a name, others too probably.

2. use (1) to reimplement drm.debug [patches 3-7]:

1st in amdgpu & i915 to control existing pr_debugs by their formats
then in drm-print, for all drm.debug API users
POC for (1)
avoids drm_debug_enabled(), gives NOOP savings & new flexibility.
changes drm.debug categories from enum to format-prefix-string
alters in-log format to include the format-prefix-string
Daniel Vetter liked this at -v3
https://lore.kernel.org/lkml/YPbPvm%[email protected]/
Im sure Ive missed stuff.

3. separately, Sean Paul proposed drm.trace to mirror drm.debug to tracefs
https://patchwork.freedesktop.org/series/78133/

He argues:
tracefs is fast/lightweight compared to syslog
independent selection (of drm categories) to tracefs
gives tailored traffic w.o flooding syslog

ISTM he's correct. So it follows that write-to-tracefs is also a good
feature for dyndbg, where its then available for all pr_debug users,
on a per-site basis, via echo +T >control. (iff CONFIG_TRACING).

So basically, I borg'd his:
[patch 14/14] drm/print: Add tracefs support to the drm logging helpers

Then I added a T flag, so it can be toggled from shell:

# turn on all drm's pr_debug --> tracefs
echo module drm +T > /proc/dynamic_debug/control

It appears to just work: (RFC)

The instance name is a placeholder, per-module subdirs kinda fits the
tracefs pattern, full mod/file-basename/function/line feels like
overkill, mod/basename-func.line would flatten it nicely. RFC.

[root@gandalf dyndbg-tracefs]# pwd
/sys/kernel/tracing/instances/dyndbg-tracefs
[root@gandalf dyndbg-tracefs]# echo 1 > /sys/module/drm/parameters/trace
[root@gandalf dyndbg-tracefs]# head -n16 trace | sed -e 's/^#//'
tracer: nop

entries-in-buffer/entries-written: 405/405 #P:24

_-----=> irqs-off
/ _----=> need-resched
| / _---=> hardirq/softirq
|| / _--=> preempt-depth
||| / _-=> migrate-disable
|||| / delay
TASK-PID CPU# ||||| TIMESTAMP FUNCTION
| | | ||||| | |
<...>-2254 [000] ..... 7040.894352: __dynamic_pr_debug: drm:core: comm="gnome-shel:cs0" pid=2254, dev=0xe200, auth=1, AMDGPU_CS
<...>-2207 [015] ..... 7040.894654: __dynamic_pr_debug: drm:core: comm="gnome-shell" pid=2207, dev=0xe200, auth=1, DRM_IOCTL_MODE_ADDFB2
<...>-2207 [015] ..... 7040.995403: __dynamic_pr_debug: drm:core: comm="gnome-shell" pid=2207, dev=0xe200, auth=1, DRM_IOCTL_MODE_RMFB
<...>-2207 [015] ..... 7040.995413: __dynamic_pr_debug: drm:core: OBJ ID: 121 (2)

This is the pr-debug doing most of that logging: (from dynamic_debug/control)

drivers/gpu/drm/drm_ioctl.c:866 [drm]drm_ioctl =T "drm:core: comm=\042%s\042 pid=%d, dev=0x%lx, auth=%d, %s\012"

Turning on decoration flags changes the trace:

echo module drm format drm:core: +mflt > /proc/dynamic_debug/control

TASK-PID CPU# ||||| TIMESTAMP FUNCTION
| | | ||||| | |
<...>-2254 [003] ..... 15980.936660: __dynamic_pr_debug: [2254] drm:drm_ioctl:866: drm:core: comm="gnome-shel:cs0" pid=2254, dev=0xe200, auth=1, AMDGPU_CS
<...>-2207 [015] ..... 15980.936966: __dynamic_pr_debug: [2207] drm:drm_ioctl:866: drm:core: comm="gnome-shell" pid=2207, dev=0xe200, auth=1, DRM_IOCTL_MODE_ADDFB2
<...>-2207 [015] ..... 15981.037727: __dynamic_pr_debug: [2207] drm:drm_ioctl:866: drm:core: comm="gnome-shell" pid=2207, dev=0xe200, auth=1, DRM_IOCTL_MODE_RMFB
<...>-2207 [015] ..... 15981.037739: __dynamic_pr_debug: [2207] drm:drm_mode_object_put:195: drm:core: OBJ ID: 124 (2)
<...>-2207 [015] ..... 15981.037742: __dynamic_pr_debug: [2207] drm:drm_mode_object_put:195: drm:core: OBJ ID: 124 (1)

The FUNCTION could stand tweaking (to match the callsite in the
control file, cited above), or perhaps replaced by the 'mfl'
decorations; the 't' flag is redundant for trace. Meh.

SELFTEST

A previous version of this patchset added test_dynamic_debug.ko, but
it relied upon code I ripped out when I made tracefs available by
default (without modules registering 1st). So it fails 10/29 tests,
which counted +T sites executed, via side effect.

TODO: userspace selftest

# to set expected tracing activity
echo module test_dynamic_debug function do_debugging +T > control

# run do_debugging function (todo: add sysfs knob)
echo 2 > /sys/module/test-dynamic-debug/parameters/run_me

If thats wrapped in the right trace_on, trace_pipe, etc incantations,
the +T enabled pr_debugs in do_debugging() can be counted, compared
against expectations, and passed or failed.

change since v9:
. patch confict against drm-misc resolved
. s/CATEGORIES/BITGRPS/ in 1/10 - keep name generic: bitmap + groups
. CATEGORIES is drm term, use it there only
. fix api friction wrt drm.trace knob
2 separate macros: DEFINE_DYNAMIC_DEBUG_{LOG,TRACE}_GROUPS - JBaron

v9: https://patchwork.freedesktop.org/series/96327/
v8: https://patchwork.freedesktop.org/series/93914/
https://lore.kernel.org/lkml/[email protected]/

The major change since v8 is that +T now works for all users, if
CONFIG_TRACING=y, otherwise it complains/errors.


Jim Cromie (10):
dyndbg: add DEFINE_DYNAMIC_DEBUG_BITGRPS macro and callbacks
drm: fix doc grammar
amdgpu: use dyndbg.BITGRPS to control existing pr_debugs
i915/gvt: trim spaces from pr_debug "gvt: core:" prefixes
i915/gvt: use dyndbg.BITGRPS for existing pr_debugs
drm_print: add choice to use dynamic debug in drm-debug
drm_print: instrument drm_debug_enabled
dyndbg: add print-to-tracefs, selftest with it - RFC
dyndbg: create DEFINE_DYNAMIC_DEBUG_LOG|TRACE_GROUPS
drm: use DEFINE_DYNAMIC_DEBUG_TRACE_GROUPS in 3 places

.../admin-guide/dynamic-debug-howto.rst | 7 +-
MAINTAINERS | 1 +
drivers/gpu/drm/Kconfig | 26 ++
drivers/gpu/drm/Makefile | 2 +
drivers/gpu/drm/amd/amdgpu/Makefile | 2 +
.../gpu/drm/amd/display/dc/core/dc_debug.c | 55 ++++-
drivers/gpu/drm/drm_print.c | 62 +++--
drivers/gpu/drm/i915/Makefile | 2 +
drivers/gpu/drm/i915/gvt/debug.h | 18 +-
drivers/gpu/drm/i915/intel_gvt.c | 47 ++++
include/drm/drm_drv.h | 2 +-
include/drm/drm_print.h | 182 +++++++++++---
include/linux/dynamic_debug.h | 88 ++++++-
lib/Kconfig.debug | 11 +
lib/Makefile | 1 +
lib/dynamic_debug.c | 203 ++++++++++++++--
lib/test_dynamic_debug.c | 222 ++++++++++++++++++
17 files changed, 843 insertions(+), 88 deletions(-)
create mode 100644 lib/test_dynamic_debug.c

--
2.31.1


2021-11-05 22:31:28

by Jim Cromie

[permalink] [raw]
Subject: [PATCH v10 10/10] drm: use DEFINE_DYNAMIC_DEBUG_TRACE_GROUPS in 3 places

add sysfs knobs to enable modules' pr_debug()s ---> tracefs

Signed-off-by: Jim Cromie <[email protected]>
---
drivers/gpu/drm/amd/display/dc/core/dc_debug.c | 8 ++++++++
drivers/gpu/drm/drm_print.c | 13 ++++++++++---
drivers/gpu/drm/i915/intel_gvt.c | 15 ++++++++++++---
3 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_debug.c b/drivers/gpu/drm/amd/display/dc/core/dc_debug.c
index e49a755c6a69..58c56c1708e7 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc_debug.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc_debug.c
@@ -80,6 +80,14 @@ DEFINE_DYNAMIC_DEBUG_LOG_GROUPS(debug_dc, __debug_dc,
DC_DYNDBG_BITMAP_DESC(debug_dc),
amdgpu_bitmap);

+#if defined(CONFIG_TRACING)
+
+unsigned long __trace_dc;
+EXPORT_SYMBOL(__trace_dc);
+DEFINE_DYNAMIC_DEBUG_LOG_GROUPS(trace_dc, __trace_dc,
+ DC_DYNDBG_BITMAP_DESC(trace_dc),
+ amdgpu_bitmap);
+#endif
#endif

#define DC_LOGGER_INIT(logger)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index d5e0ffad467b..ee20e9c14ce9 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -72,9 +72,16 @@ static struct dyndbg_bitdesc drm_dyndbg_bitmap[] = {
[8] = { DRM_DBG_CAT_DP },
[9] = { DRM_DBG_CAT_DRMRES }
};
-DEFINE_DYNAMIC_DEBUG_BITGRPS(debug, __drm_debug, DRM_DEBUG_DESC,
- drm_dyndbg_bitmap);
-
+DEFINE_DYNAMIC_DEBUG_LOG_GROUPS(debug, __drm_debug, DRM_DEBUG_DESC,
+ drm_dyndbg_bitmap);
+
+#ifdef CONFIG_TRACING
+struct trace_array *trace_arr;
+unsigned long __drm_trace;
+EXPORT_SYMBOL(__drm_trace);
+DEFINE_DYNAMIC_DEBUG_TRACE_GROUPS(trace, __drm_trace, DRM_DEBUG_DESC,
+ drm_dyndbg_bitmap);
+#endif
#endif

void __drm_puts_coredump(struct drm_printer *p, const char *str)
diff --git a/drivers/gpu/drm/i915/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c
index efaac5777873..84348d4aedf6 100644
--- a/drivers/gpu/drm/i915/intel_gvt.c
+++ b/drivers/gpu/drm/i915/intel_gvt.c
@@ -195,8 +195,17 @@ static struct dyndbg_bitdesc i915_dyndbg_bitmap[] = {
help_(7, "gvt:render:") \
help_(8, "gvt:sched:")

-DEFINE_DYNAMIC_DEBUG_BITGRPS(debug_gvt, __gvt_debug,
- I915_GVT_CATEGORIES(debug_gvt),
- i915_dyndbg_bitmap);
+DEFINE_DYNAMIC_DEBUG_LOG_GROUPS(debug_gvt, __gvt_debug,
+ I915_GVT_CATEGORIES(debug_gvt),
+ i915_dyndbg_bitmap);

+#if defined(CONFIG_TRACING)
+
+unsigned long __gvt_trace;
+EXPORT_SYMBOL(__gvt_trace);
+DEFINE_DYNAMIC_DEBUG_TRACE_GROUPS(trace_gvt, __gvt_trace,
+ I915_GVT_CATEGORIES(trace_gvt),
+ i915_dyndbg_bitmap);
+
+#endif
#endif
--
2.31.1

2021-11-05 22:42:33

by Jim Cromie

[permalink] [raw]
Subject: [PATCH v10 06/10] drm_print: add choice to use dynamic debug in drm-debug

drm's debug system writes 10 distinct categories of messages to syslog
using a small API[1]: drm_dbg*(10 names), DRM_DEV_DEBUG*(3 names),
DRM_DEBUG*(8 names). There are thousands of these callsites, each
categorized in this systematized way.

These callsites can be enabled at runtime by their category, each
controlled by a bit in drm.debug (/sys/modules/drm/parameter/debug).
In the current "basic" implementation, drm_debug_enabled() tests these
bits in __drm_debug each time an API[1] call is executed; while cheap
individually, the costs accumulate with uptime.

This patch uses dynamic-debug with (required) jump-label to patch
enabled callsites onto their respective NOOP slots, avoiding all
runtime bit-checks of __drm_debug by drm_debug_enabled().

Dynamic debug has no concept of category, but we can emulate one by
replacing enum categories with a set of prefix-strings; "drm:core:",
"drm:kms:" "drm:driver:" etc, and prepend them (at compile time) to
the given formats.

Then we can use:

# echo module drm format "^drm:core: " +p > control`

to enable the whole category with one query.

This conversion yields many new prdbg callsites:

dyndbg: 207 debug prints in module drm_kms_helper
dyndbg: 376 debug prints in module drm
dyndbg: 1811 debug prints in module i915
dyndbg: 3917 debug prints in module amdgpu

Each site costs 56 bytes of .data, which is a big increase for
drm modules, so CONFIG_DRM_USE_DYNAMIC_DEBUG makes it optional.

CONFIG_JUMP_LABEL is also required, to get the promised optimizations.

The "basic" -> "dyndbg" switchover is layered into the macro scheme

A. A "prefix" version of DRM_UT_<CATs> map, named DRM_DBG_CAT_<CATs>

"basic": DRM_DBG_CAT_<CATs> <=== DRM_UT_<CATs>. Identity map.
"dyndbg":
#define DRM_DBG_CAT_KMS "drm:kms: "
#define DRM_DBG_CAT_PRIME "drm:prime: "
#define DRM_DBG_CAT_ATOMIC "drm:atomic: "

DRM_UT_* are preserved, since theyre used elsewhere. Since the
callback maintains its state in __drm_debug, drm_debug_enabled() will
stay synchronized, and continue to work. We can address them
separately if they are called enough to be worth fixing.

B. drm_dev_dbg() & drm_debug() are interposed with macros

basic: forward to renamed fn, with args preserved
enabled: redirect to pr_debug, dev_dbg, with CATEGORY format catenated

This is where drm_debug_enabled() is avoided. The prefix is prepended
at compile-time, no category at runtime.

C. API[1] uses DRM_DBG_CAT_<CAT>s

The API already uses B, now it uses A too, instead of DRM_UT_<CAT>, to
get the correct token type for "basic" and "dyndbg" configs.

D. use DEFINE_DYNAMIC_DEBUG_CATEGORIES()

This defines the map using DRM_CAT_<CAT>s, and creates the /sysfs
bitmap to control those categories.

CONFIG_DRM_USE_DYNAMIC_DEBUG is also used to adjust amdgpu, i915
makefiles to add -DDYNAMIC_DEBUG_MODULE; it includes the current
CONFIG_DYNAMIC_DEBUG_CORE and is enabled by the user.

LIMITATIONS:

dev_dbg(etal) effectively prepends twice, category then driver-name,
yielding format strings like so:

bash-5.1# grep amdgpu: /proc/dynamic_debug/control | grep drm: | cut -d= -f2-
_ "amdgpu: drm:core: fence driver on ring %s use gpu addr 0x%016llx\012"
_ "amdgpu: drm:kms: Cannot create framebuffer from imported dma_buf\012"

This means we cannot use anchored "^drm:kms: " to specify the
category, a small loss of precision.

Note that searching on "format ^amdgpu: " works, but this is less
valuable, because the same can be done with "module amdgpu".

NOTES:

Because the dyndbg callback is keeping state in __drm_debug, it
synchronizes with drm_debug_enabled() and its remaining users; the
switchover should be transparent.

Code Review is expected to catch the lack of correspondence between
bit=>prefix definitions (the selector) and the prefixes used in the
API[1] layer above pr_debug()

I've coded the categories with trailing spaces. This excludes any
sub-categories which might get added later. This convention protects
any "drm:atomic:fail:" callsites from getting stomped on by `echo 0 >
debug`. Other categories could differ, but we need some default.

Dyndbg requires that the prefix be in the compiled-in format string;
run-time prefixing evades callsite selection by category.

pr_debug("%s: ...", __func__, ...) // not ideal

Unfortunately __func__ is not a macro, and cannot be catenated at
preprocess/compile time.

If you want that, you might consider +mfl flags instead;)

Signed-off-by: Jim Cromie <[email protected]>
---
v5:
. use DEFINE_DYNAMIC_DEBUG_CATEGORIES in drm_print.c
. s/DRM_DBG_CLASS_/DRM_DBG_CAT_/ - dont need another term
. default=y in Kconfig entry - per @DanVet
. move some commit-log prose to dyndbg commit
. add-prototyes to (param_get/set)_dyndbg
. more wrinkles found by <[email protected]>
. relocate ratelimit chunk from elsewhere
v6:
. add kernel doc
. fix cpp paste, drop '#'
v7:
. change __drm_debug to long, to fit with DEFINE_DYNAMIC_DEBUG_CATEGORIES
. add -DDYNAMIC_DEBUG_MODULE to ccflags if DRM_USE_DYNAMIC_DEBUG
v8:
. adapt to altered ^ insertion
. add mem cost numbers to kconfig
. kdoc improvements (I hope)
---
drivers/gpu/drm/Kconfig | 26 +++++
drivers/gpu/drm/Makefile | 2 +
drivers/gpu/drm/amd/amdgpu/Makefile | 2 +-
drivers/gpu/drm/drm_print.c | 55 ++++++---
drivers/gpu/drm/i915/Makefile | 2 +-
include/drm/drm_print.h | 175 ++++++++++++++++++++++------
6 files changed, 210 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 2a926d0de423..6223d853907d 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -62,6 +62,32 @@ config DRM_DEBUG_MM

If in doubt, say "N".

+config DRM_USE_DYNAMIC_DEBUG
+ bool "use dynamic debug to implement drm.debug"
+ default y
+ depends on DRM
+ depends on DYNAMIC_DEBUG || DYNAMIC_DEBUG_CORE
+ depends on JUMP_LABEL
+ help
+ The "basic" drm.debug facility does a lot of unlikely
+ bit-field tests at runtime; while cheap individually, the
+ cost accumulates. DYNAMIC_DEBUG can patch pr_debug()s into
+ NOOP slots, in a running kernel, so avoids those bit-test
+ overheads, and is therefore recommended by default.
+
+ DRM_USE_DYNAMIC_DEBUG converts "basic" to "dyndbg", this
+ creates many new dyndbg callsites (56 bytes each), which
+ significantly increases drm* module .data, so is optional.
+ On an x86-64 kernel, with a config derived from fedora, that
+ price is:
+ #prdbgs KiB #with DRM_USE_DYNAMIC_DEBUG=y
+ kernel 3079 166k
+ drm 1 .06k 376 21k
+ drm_kms_helper 207 12k
+ i915 167 9.3k 1811 101k
+ amdgpu 2339 130k 3917 220k
+ nouveau 3 .17k 105 ~60k
+
config DRM_DEBUG_SELFTEST
tristate "kselftests for DRM"
depends on DRM
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 0dff40bb863c..786d3256a163 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -33,6 +33,8 @@ drm-$(CONFIG_PCI) += drm_pci.o
drm-$(CONFIG_DEBUG_FS) += drm_debugfs.o drm_debugfs_crc.o
drm-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o

+ccflags-$(CONFIG_DRM_USE_DYNAMIC_DEBUG) += -DDYNAMIC_DEBUG_MODULE
+
obj-$(CONFIG_DRM_DP_AUX_BUS) += drm_dp_aux_bus.o

drm_vram_helper-y := drm_gem_vram_helper.o
diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
index 077342ca803f..d840319d29a5 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -38,7 +38,7 @@ ccflags-y := -I$(FULL_AMD_PATH)/include/asic_reg \
-I$(FULL_AMD_DISPLAY_PATH)/amdgpu_dm \
-I$(FULL_AMD_PATH)/amdkfd

-ccflags-$(CONFIG_DYNAMIC_DEBUG_CORE) += -DYNAMIC_DEBUG_MODULE
+ccflags-$(CONFIG_DRM_USE_DYNAMIC_DEBUG) += -DYNAMIC_DEBUG_MODULE

amdgpu-y := amdgpu_drv.o

diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index f783d4963d4b..d5e0ffad467b 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -28,9 +28,11 @@
#include <linux/stdarg.h>

#include <linux/io.h>
+#include <linux/module.h>
#include <linux/moduleparam.h>
#include <linux/seq_file.h>
#include <linux/slab.h>
+#include <linux/dynamic_debug.h>

#include <drm/drm.h>
#include <drm/drm_drv.h>
@@ -40,19 +42,40 @@
* __drm_debug: Enable debug output.
* Bitmask of DRM_UT_x. See include/drm/drm_print.h for details.
*/
-unsigned int __drm_debug;
+unsigned long __drm_debug;
EXPORT_SYMBOL(__drm_debug);

-MODULE_PARM_DESC(debug, "Enable debug output, where each bit enables a debug category.\n"
-"\t\tBit 0 (0x01) will enable CORE messages (drm core code)\n"
-"\t\tBit 1 (0x02) will enable DRIVER messages (drm controller code)\n"
-"\t\tBit 2 (0x04) will enable KMS messages (modesetting code)\n"
-"\t\tBit 3 (0x08) will enable PRIME messages (prime code)\n"
-"\t\tBit 4 (0x10) will enable ATOMIC messages (atomic code)\n"
-"\t\tBit 5 (0x20) will enable VBL messages (vblank code)\n"
-"\t\tBit 7 (0x80) will enable LEASE messages (leasing code)\n"
-"\t\tBit 8 (0x100) will enable DP messages (displayport code)");
-module_param_named(debug, __drm_debug, int, 0600);
+#define DRM_DEBUG_DESC \
+"Enable debug output, where each bit enables a debug category.\n" \
+"\t\tBit 0 (0x01) will enable CORE messages (drm core code)\n" \
+"\t\tBit 1 (0x02) will enable DRIVER messages (drm controller code)\n" \
+"\t\tBit 2 (0x04) will enable KMS messages (modesetting code)\n" \
+"\t\tBit 3 (0x08) will enable PRIME messages (prime code)\n" \
+"\t\tBit 4 (0x10) will enable ATOMIC messages (atomic code)\n" \
+"\t\tBit 5 (0x20) will enable VBL messages (vblank code)\n" \
+"\t\tBit 7 (0x80) will enable LEASE messages (leasing code)\n" \
+"\t\tBit 8 (0x100) will enable DP messages (displayport code)."
+
+#if !defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
+MODULE_PARM_DESC(debug, DRM_DEBUG_DESC);
+module_param_named(debug, __drm_debug, ulong, 0600);
+#else
+static struct dyndbg_bitdesc drm_dyndbg_bitmap[] = {
+ [0] = { DRM_DBG_CAT_CORE },
+ [1] = { DRM_DBG_CAT_DRIVER },
+ [2] = { DRM_DBG_CAT_KMS },
+ [3] = { DRM_DBG_CAT_PRIME },
+ [4] = { DRM_DBG_CAT_ATOMIC },
+ [5] = { DRM_DBG_CAT_VBL },
+ [6] = { DRM_DBG_CAT_STATE },
+ [7] = { DRM_DBG_CAT_LEASE },
+ [8] = { DRM_DBG_CAT_DP },
+ [9] = { DRM_DBG_CAT_DRMRES }
+};
+DEFINE_DYNAMIC_DEBUG_BITGRPS(debug, __drm_debug, DRM_DEBUG_DESC,
+ drm_dyndbg_bitmap);
+
+#endif

void __drm_puts_coredump(struct drm_printer *p, const char *str)
{
@@ -256,8 +279,8 @@ void drm_dev_printk(const struct device *dev, const char *level,
}
EXPORT_SYMBOL(drm_dev_printk);

-void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
- const char *format, ...)
+void __drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
+ const char *format, ...)
{
struct va_format vaf;
va_list args;
@@ -278,9 +301,9 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,

va_end(args);
}
-EXPORT_SYMBOL(drm_dev_dbg);
+EXPORT_SYMBOL(__drm_dev_dbg);

-void __drm_dbg(enum drm_debug_category category, const char *format, ...)
+void ___drm_dbg(enum drm_debug_category category, const char *format, ...)
{
struct va_format vaf;
va_list args;
@@ -297,7 +320,7 @@ void __drm_dbg(enum drm_debug_category category, const char *format, ...)

va_end(args);
}
-EXPORT_SYMBOL(__drm_dbg);
+EXPORT_SYMBOL(___drm_dbg);

void __drm_err(const char *format, ...)
{
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 0fa5f53312a8..9801ac245b5d 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -317,7 +317,7 @@ i915-y += intel_gvt.o
include $(src)/gvt/Makefile
endif

-ccflags-$(CONFIG_DYNAMIC_DEBUG_CORE) += -DDYNAMIC_DEBUG_MODULE
+ccflags-$(CONFIG_DRM_USE_DYNAMIC_DEBUG) += -DDYNAMIC_DEBUG_MODULE

obj-$(CONFIG_DRM_I915) += i915.o
obj-$(CONFIG_DRM_I915_GVT_KVMGT) += gvt/kvmgt.o
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 22fabdeed297..392cff7cb95c 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -35,7 +35,7 @@
#include <drm/drm.h>

/* Do *not* use outside of drm_print.[ch]! */
-extern unsigned int __drm_debug;
+extern unsigned long __drm_debug;

/**
* DOC: print
@@ -252,15 +252,15 @@ static inline struct drm_printer drm_err_printer(const char *prefix)
/**
* enum drm_debug_category - The DRM debug categories
*
- * Each of the DRM debug logging macros use a specific category, and the logging
- * is filtered by the drm.debug module parameter. This enum specifies the values
- * for the interface.
+ * The drm.debug logging API[1] has 10 enumerated categories of
+ * messages, issued by 3 families of macros: 10 drm_dbg_<CATs>, 8
+ * DRM_DEBUG_<CATs>, and 3 DRM_DEV_DEBUG_<CATs>.
*
* Each DRM_DEBUG_<CATEGORY> macro logs to DRM_UT_<CATEGORY> category, except
* DRM_DEBUG() logs to DRM_UT_CORE.
*
- * Enabling verbose debug messages is done through the drm.debug parameter, each
- * category being enabled by a bit:
+ * Enabling categories of debug messages is done through the drm.debug
+ * parameter, each category being enabled by a bit:
*
* - drm.debug=0x1 will enable CORE messages
* - drm.debug=0x2 will enable DRIVER messages
@@ -268,8 +268,8 @@ static inline struct drm_printer drm_err_printer(const char *prefix)
* - ...
* - drm.debug=0x1ff will enable all messages
*
- * An interesting feature is that it's possible to enable verbose logging at
- * run-time by echoing the debug value in its sysfs node::
+ * It's possible to enable drm.debug logging at run-time by echoing
+ * the debug value in its sysfs node::
*
* # echo 0xf > /sys/module/drm/parameters/debug
*
@@ -319,6 +319,103 @@ enum drm_debug_category {
DRM_UT_DRMRES = 0x200,
};

+/**
+ * DOC: CONFIG_DRM_USE_DYNAMIC_DEBUG - using dyndbg in drm.debug
+ *
+ * In the "basic" drm.debug implementation outlined above, each time a
+ * drm-debug API[1] call is executed, drm_debug_enabled(cat) tests
+ * __drm_debug vs cat before printing.
+ *
+ * DYNAMIC_DEBUG (aka: dyndbg) patches pr_debug()s in^out of the
+ * running kernel, so it can avoid drm_debug_enabled() and skip lots
+ * of unlikely bit tests.
+ *
+ * dyndbg has no concept of category, but we can prepend a
+ * class-prefix string: "drm:core: ", "drm:kms: ", "drm:driver: " etc,
+ * to pr_debug's format (at compile time).
+ *
+ * Then control the category::
+ *
+ * # echo module drm format "^drm:core: " +p > control
+ * c: dynamic_debug_exec_queries("format '^drm:core: ' +p", "drm");
+ *
+ * To do this for "basic" | "dyndbg", adaptation adds some macro indirection:
+ *
+ * 0. use dyndbg support to define the bits => prefixes map, and creates sysfs node
+ *
+ * DEFINE_DYNAMIC_DEBUG_CATEGORIES(debug, __drm_debug,
+ * "drm.debug - overview",
+ * { [0] = "drm:core: " },
+ * { [1] = "drm:kms: " }, ...);
+ *
+ * 1. DRM_DBG_CAT_<CAT>
+ *
+ * This set of symbols replaces DRM_UT_<CAT> inside the drm-debug API;
+ * for "basic" it is a copy of DRM_UT_<CAT>, otherwise they are the set
+ * of class prefix strings used in pr_debugs (either directly or by
+ * macro wrappers).
+ *
+ * 2. drm_dev_dbg & drm_debug are called by drm.debug API
+ *
+ * These are now macros, either forwarding to renamed functions, or
+ * prepending the class string to the format, and invoking pr_debug
+ * directly. Since the API is all macros, dyndbg's pr_debug sees the
+ * actual (broad population of) callsites, and they're all
+ * individually controllable.
+ */
+#if !defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
+
+#define __drm_dbg(cls, fmt, ...) \
+ ___drm_dbg(cls, fmt, ##__VA_ARGS__)
+#define drm_dev_dbg(dev, cls, fmt, ...) \
+ __drm_dev_dbg(dev, cls, fmt, ##__VA_ARGS__)
+
+#define DRM_DBG_CAT_CORE DRM_UT_CORE
+#define DRM_DBG_CAT_DRIVER DRM_UT_DRIVER
+#define DRM_DBG_CAT_KMS DRM_UT_KMS
+#define DRM_DBG_CAT_PRIME DRM_UT_PRIME
+#define DRM_DBG_CAT_ATOMIC DRM_UT_ATOMIC
+#define DRM_DBG_CAT_VBL DRM_UT_VBL
+#define DRM_DBG_CAT_STATE DRM_UT_STATE
+#define DRM_DBG_CAT_LEASE DRM_UT_LEASE
+#define DRM_DBG_CAT_DP DRM_UT_DP
+#define DRM_DBG_CAT_DRMRES DRM_UT_DRMRES
+
+#else /* CONFIG_DRM_USE_DYNAMIC_DEBUG */
+
+/* join prefix + format in cpp so dyndbg can see it */
+#define __drm_dbg(pfx, fmt, ...) \
+ pr_debug(pfx fmt, ##__VA_ARGS__)
+#define drm_dev_dbg(dev, pfx, fmt, ...) \
+ dev_dbg(dev, pfx fmt, ##__VA_ARGS__)
+
+/**
+ * enum-ish DRM_DBG_CAT_<categories>::
+ *
+ * - DRM_DBG_CAT_CORE "drm:core: "
+ * - DRM_DBG_CAT_DRIVER "drm:drvr: "
+ * - DRM_DBG_CAT_KMS "drm:kms: "
+ * - DRM_DBG_CAT_PRIME "drm:prime: "
+ * - DRM_DBG_CAT_ATOMIC "drm:atomic: "
+ * - DRM_DBG_CAT_VBL "drm:vbl: "
+ * - DRM_DBG_CAT_STATE "drm:state: "
+ * - DRM_DBG_CAT_LEASE "drm:lease: "
+ * - DRM_DBG_CAT_DP "drm:dp: "
+ * - DRM_DBG_CAT_DRMRES "drm:res: "
+ */
+#define DRM_DBG_CAT_CORE "drm:core: "
+#define DRM_DBG_CAT_DRIVER "drm:drvr: "
+#define DRM_DBG_CAT_KMS "drm:kms: "
+#define DRM_DBG_CAT_PRIME "drm:prime: "
+#define DRM_DBG_CAT_ATOMIC "drm:atomic: "
+#define DRM_DBG_CAT_VBL "drm:vbl: "
+#define DRM_DBG_CAT_STATE "drm:state: "
+#define DRM_DBG_CAT_LEASE "drm:lease: "
+#define DRM_DBG_CAT_DP "drm:dp: "
+#define DRM_DBG_CAT_DRMRES "drm:res: "
+
+#endif /* CONFIG_DRM_USE_DYNAMIC_DEBUG */
+
static inline bool drm_debug_enabled(enum drm_debug_category category)
{
return unlikely(__drm_debug & category);
@@ -334,8 +431,8 @@ __printf(3, 4)
void drm_dev_printk(const struct device *dev, const char *level,
const char *format, ...);
__printf(3, 4)
-void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
- const char *format, ...);
+void __drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
+ const char *format, ...);

/**
* DRM_DEV_ERROR() - Error output.
@@ -383,6 +480,16 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
} \
})

+/**
+ * DRM debug API - library of macros to issue categorized syslog messages
+ *
+ * multiple flavors::
+ * - DRM_DEV_DEBUG<,_DRIVER,_KMS>
+ * - drm_dbg_<core,kms,prime,atomic,vbl,state,lease,dp,drmres> (prefer these)
+ * - DRM_DEBUG<,_DRIVER,_KMS,_PRIME,_ATOMIC,_VBL,_LEASE,_DP> (over these)
+ * - DRM_DEBUG_KMS_RATELIMITED
+ */
+
/**
* DRM_DEV_DEBUG() - Debug output for generic drm code
*
@@ -392,7 +499,7 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
* @fmt: printf() like format string.
*/
#define DRM_DEV_DEBUG(dev, fmt, ...) \
- drm_dev_dbg(dev, DRM_UT_CORE, fmt, ##__VA_ARGS__)
+ drm_dev_dbg(dev, DRM_DBG_CAT_CORE, fmt, ##__VA_ARGS__)
/**
* DRM_DEV_DEBUG_DRIVER() - Debug output for vendor specific part of the driver
*
@@ -402,7 +509,7 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
* @fmt: printf() like format string.
*/
#define DRM_DEV_DEBUG_DRIVER(dev, fmt, ...) \
- drm_dev_dbg(dev, DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
+ drm_dev_dbg(dev, DRM_DBG_CAT_DRIVER, fmt, ##__VA_ARGS__)
/**
* DRM_DEV_DEBUG_KMS() - Debug output for modesetting code
*
@@ -412,7 +519,7 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
* @fmt: printf() like format string.
*/
#define DRM_DEV_DEBUG_KMS(dev, fmt, ...) \
- drm_dev_dbg(dev, DRM_UT_KMS, fmt, ##__VA_ARGS__)
+ drm_dev_dbg(dev, DRM_DBG_CAT_KMS, fmt, ##__VA_ARGS__)

/*
* struct drm_device based logging
@@ -454,27 +561,26 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
#define drm_err_ratelimited(drm, fmt, ...) \
__drm_printk((drm), err, _ratelimited, "*ERROR* " fmt, ##__VA_ARGS__)

-
#define drm_dbg_core(drm, fmt, ...) \
- drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_CORE, fmt, ##__VA_ARGS__)
+ drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_DBG_CAT_CORE, fmt, ##__VA_ARGS__)
#define drm_dbg(drm, fmt, ...) \
- drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
+ drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_DBG_CAT_DRIVER, fmt, ##__VA_ARGS__)
#define drm_dbg_kms(drm, fmt, ...) \
- drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_KMS, fmt, ##__VA_ARGS__)
+ drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_DBG_CAT_KMS, fmt, ##__VA_ARGS__)
#define drm_dbg_prime(drm, fmt, ...) \
- drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_PRIME, fmt, ##__VA_ARGS__)
+ drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_DBG_CAT_PRIME, fmt, ##__VA_ARGS__)
#define drm_dbg_atomic(drm, fmt, ...) \
- drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_ATOMIC, fmt, ##__VA_ARGS__)
+ drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_DBG_CAT_ATOMIC, fmt, ##__VA_ARGS__)
#define drm_dbg_vbl(drm, fmt, ...) \
- drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_VBL, fmt, ##__VA_ARGS__)
+ drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_DBG_CAT_VBL, fmt, ##__VA_ARGS__)
#define drm_dbg_state(drm, fmt, ...) \
- drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_STATE, fmt, ##__VA_ARGS__)
+ drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_DBG_CAT_STATE, fmt, ##__VA_ARGS__)
#define drm_dbg_lease(drm, fmt, ...) \
- drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_LEASE, fmt, ##__VA_ARGS__)
+ drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_DBG_CAT_LEASE, fmt, ##__VA_ARGS__)
#define drm_dbg_dp(drm, fmt, ...) \
- drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_DP, fmt, ##__VA_ARGS__)
+ drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_DBG_CAT_DP, fmt, ##__VA_ARGS__)
#define drm_dbg_drmres(drm, fmt, ...) \
- drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_UT_DRMRES, fmt, ##__VA_ARGS__)
+ drm_dev_dbg((drm) ? (drm)->dev : NULL, DRM_DBG_CAT_DRMRES, fmt, ##__VA_ARGS__)


/*
@@ -484,7 +590,7 @@ void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
*/

__printf(2, 3)
-void __drm_dbg(enum drm_debug_category category, const char *format, ...);
+void ___drm_dbg(enum drm_debug_category category, const char *format, ...);
__printf(1, 2)
void __drm_err(const char *format, ...);

@@ -523,35 +629,35 @@ void __drm_err(const char *format, ...);

/* NOTE: this is deprecated in favor of drm_dbg_core(NULL, ...). */
#define DRM_DEBUG(fmt, ...) \
- __drm_dbg(DRM_UT_CORE, fmt, ##__VA_ARGS__)
+ __drm_dbg(DRM_DBG_CAT_CORE, fmt, ##__VA_ARGS__)

/* NOTE: this is deprecated in favor of drm_dbg(NULL, ...). */
#define DRM_DEBUG_DRIVER(fmt, ...) \
- __drm_dbg(DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
+ __drm_dbg(DRM_DBG_CAT_DRIVER, fmt, ##__VA_ARGS__)

/* NOTE: this is deprecated in favor of drm_dbg_kms(NULL, ...). */
#define DRM_DEBUG_KMS(fmt, ...) \
- __drm_dbg(DRM_UT_KMS, fmt, ##__VA_ARGS__)
+ __drm_dbg(DRM_DBG_CAT_KMS, fmt, ##__VA_ARGS__)

/* NOTE: this is deprecated in favor of drm_dbg_prime(NULL, ...). */
#define DRM_DEBUG_PRIME(fmt, ...) \
- __drm_dbg(DRM_UT_PRIME, fmt, ##__VA_ARGS__)
+ __drm_dbg(DRM_DBG_CAT_PRIME, fmt, ##__VA_ARGS__)

/* NOTE: this is deprecated in favor of drm_dbg_atomic(NULL, ...). */
#define DRM_DEBUG_ATOMIC(fmt, ...) \
- __drm_dbg(DRM_UT_ATOMIC, fmt, ##__VA_ARGS__)
+ __drm_dbg(DRM_DBG_CAT_ATOMIC, fmt, ##__VA_ARGS__)

/* NOTE: this is deprecated in favor of drm_dbg_vbl(NULL, ...). */
#define DRM_DEBUG_VBL(fmt, ...) \
- __drm_dbg(DRM_UT_VBL, fmt, ##__VA_ARGS__)
+ __drm_dbg(DRM_DBG_CAT_VBL, fmt, ##__VA_ARGS__)

/* NOTE: this is deprecated in favor of drm_dbg_lease(NULL, ...). */
#define DRM_DEBUG_LEASE(fmt, ...) \
- __drm_dbg(DRM_UT_LEASE, fmt, ##__VA_ARGS__)
+ __drm_dbg(DRM_DBG_CAT_LEASE, fmt, ##__VA_ARGS__)

/* NOTE: this is deprecated in favor of drm_dbg_dp(NULL, ...). */
#define DRM_DEBUG_DP(fmt, ...) \
- __drm_dbg(DRM_UT_DP, fmt, ## __VA_ARGS__)
+ __drm_dbg(DRM_DBG_CAT_DP, fmt, ## __VA_ARGS__)

#define __DRM_DEFINE_DBG_RATELIMITED(category, drm, fmt, ...) \
({ \
@@ -559,7 +665,8 @@ void __drm_err(const char *format, ...);
const struct drm_device *drm_ = (drm); \
\
if (drm_debug_enabled(DRM_UT_ ## category) && __ratelimit(&rs_)) \
- drm_dev_printk(drm_ ? drm_->dev : NULL, KERN_DEBUG, fmt, ## __VA_ARGS__); \
+ drm_dev_dbg((drm_) ? (drm_)->dev : NULL, \
+ DRM_DBG_CAT_ ## category, fmt, ##__VA_ARGS__); \
})

#define drm_dbg_kms_ratelimited(drm, fmt, ...) \
--
2.31.1

2021-11-05 22:42:54

by Jim Cromie

[permalink] [raw]
Subject: [PATCH v10 08/10] dyndbg: add print-to-tracefs, selftest with it - RFC

Sean Paul proposed, in:
https://patchwork.freedesktop.org/series/78133/
drm/trace: Mirror DRM debug logs to tracefs

His patchset's objective is to be able to independently steer some of
the drm.debug stream to an alternate tracing destination, by splitting
drm_debug_enabled() into syslog & trace flavors, and enabling them
separately. 2 advantages were identified:

1- syslog is heavyweight, tracefs is much lighter
2- separate selection of enabled categories means less traffic

Dynamic-Debug can do 2nd exceedingly well:

A- all work is behind jump-label's NOOP, zero off cost.
B- exact site selectivity, precisely the useful traffic.
can tailor enabled set interactively, at shell.

Since the tracefs interface is effective for drm (the threads suggest
so), adding that interface to dynamic-debug has real potential for
everyone including drm.

if CONFIG_TRACING:

Grab Sean's trace_init/cleanup code, use it to provide tracefs
available by default to all pr_debugs. This will likely need some
further per-module treatment; perhaps something reflecting hierarchy
of module,file,function,line, maybe with a tuned flattening.

endif CONFIG_TRACING

Add a new +T flag to enable tracing, independent of +p, and add and
use 3 macros: dyndbg_site_is_enabled/logging/tracing(), to encapsulate
the flag checks. Existing code treats T like other flags.

Add ddebug_validate_flags() as last step in ddebug_parse_flags(). Its
only job is to fail on +T for non-CONFIG_TRACING builds. It only sees
the new flags, and cannot validate specific state transitions. This
is fine, since we have no need for that; such a test would have to be
done in ddebug_change(), which actually updates the callsites.

ddebug_change() adjusts the static-key-enable/disable condition to use
_DPRINTK_ENABLED / abstraction macros.

dynamic_emit_prefix() now gates on _DPRINTK_ENABLED too, as an
optimization but mostly to allow decluttering of its users.

__dynamic_pr_debug() etal get minor changes:

- call dynamic_emit_prefix() 1st, _enabled() optimizes.
- if (T) call trace_array_printk
- if (!p) go around original printk code.
done to minimize diff,
goto-ectomy + reindent later/separately
- share vaf across p|T

WRT <net|ib>_dev, I skipped all the <T,!!dev> specific dev_emit_prefix
additions for now. tracefs is a fast customer with different needs,
its not clear that pretty device-ID-ish strings is useful tracefs
content (on ingest), or that couldn't be done more efficiently while
analysing or postprocesing the tracefs buffer.

SELFTEST: test_dynamic_debug.ko:

Uses the tracer facility to implement a kernel module selftest.

TODO:

Earlier core code had (tracerfn)() indirection, allowing a plugin
side-effector we could test the results of.

ATM all the tests which count +T'd callsite executions (and which
expect >0) are failing.

Now it needs a rethink to test from userspace, rather than the current
test-once at module-load. It needs a parameters/testme button.

So remainder of this is a bit stale ....

- A custom tracer counts the number of calls (of T-enabled pr_debugs),
- do_debugging(x) calls a set of categorized pr_debugs x times

- test registers the tracer on the module
then iteratively:
manipulates dyndbg states via query-cmds, mostly format ^prefix
runs do_debugging()
counts enabled callsite executions
reports mismatches

- modprobe test_dynamic_debug use_bad_tracer=1
attaches a bad/recursive tracer
Bad Things (did) Happen.
has thrown me interesting panics.
cannot replicate atm.

RFC: (DONE)

The "tracer" interface probably needs work and a new name. It is only
1/2 way towards a real tracefs interface; and the code I lifted from
Sean Paul in the next patch could be implemented in dynamic_debug.c
instead, and made available for all pr_debug users.

This would also eliminate need for dynamic_debug_(un)register_tracer(),
since dyndbg could just provide it when TRACING is on.

NOTES:

$> modprobe test_dynamic_debug dyndbg=+p

it fails 3/29 tests. havent looked at why.

$> modprobe test_dynamic_debug use_bad_tracer=1

Earlier in dev, bad_tracer() exploded in recursion, I havent been able
to replicate that lately.

Signed-off-by: Jim Cromie <[email protected]>
---
.../admin-guide/dynamic-debug-howto.rst | 7 +-
MAINTAINERS | 1 +
include/linux/dynamic_debug.h | 12 +-
lib/Kconfig.debug | 11 +
lib/Makefile | 1 +
lib/dynamic_debug.c | 127 ++++++++--
lib/test_dynamic_debug.c | 222 ++++++++++++++++++
7 files changed, 355 insertions(+), 26 deletions(-)
create mode 100644 lib/test_dynamic_debug.c

diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
index b119b8277b3e..48d32782bb11 100644
--- a/Documentation/admin-guide/dynamic-debug-howto.rst
+++ b/Documentation/admin-guide/dynamic-debug-howto.rst
@@ -227,7 +227,8 @@ of the characters::

The flags are::

- p enables the pr_debug() callsite.
+ p enables the pr_debug() callsite to syslog
+ T enables the pr_debug() callsite to tracefs
f Include the function name in the printed message
l Include line number in the printed message
m Include module name in the printed message
@@ -240,8 +241,8 @@ have meaning, other flags ignored.
For display, the flags are preceded by ``=``
(mnemonic: what the flags are currently equal to).

-Note the regexp ``^[-+=][flmpt_]+$`` matches a flags specification.
-To clear all flags at once, use ``=_`` or ``-flmpt``.
+Note the regexp ``^[-+=][flmptT_]+$`` matches a flags specification.
+To clear all flags at once, use ``=_`` or ``-flmptT``.


Debug messages during Boot Process
diff --git a/MAINTAINERS b/MAINTAINERS
index 2c9070aeba2a..ead3332db862 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6615,6 +6615,7 @@ M: Jason Baron <[email protected]>
S: Maintained
F: include/linux/dynamic_debug.h
F: lib/dynamic_debug.c
+F: lib/test_dynamic_debug.c

DYNAMIC INTERRUPT MODERATION
M: Tal Gilboa <[email protected]>
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index a9430168b072..792bcff0297e 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -28,15 +28,25 @@ struct _ddebug {
*/
#define _DPRINTK_FLAGS_NONE 0
#define _DPRINTK_FLAGS_PRINT (1<<0) /* printk() a message using the format */
+#define _DPRINTK_FLAGS_TRACE (1<<5) /* trace_printk() the message */
+#define _DPRINTK_ENABLED (_DPRINTK_FLAGS_PRINT | _DPRINTK_FLAGS_TRACE)
+
+/* internal, need type protection for external use */
+#define __dyndbg_site_is_enabled(desc) (!!(desc->flags & _DPRINTK_ENABLED))
+#define __dyndbg_site_is_logging(desc) (!!(desc->flags & _DPRINTK_FLAGS_PRINT))
+#define __dyndbg_site_is_tracing(desc) (!!(desc->flags & _DPRINTK_FLAGS_TRACE))
+
#define _DPRINTK_FLAGS_INCL_MODNAME (1<<1)
#define _DPRINTK_FLAGS_INCL_FUNCNAME (1<<2)
#define _DPRINTK_FLAGS_INCL_LINENO (1<<3)
#define _DPRINTK_FLAGS_INCL_TID (1<<4)

-#define _DPRINTK_FLAGS_INCL_ANY \
+#define _DPRINTK_FLAGS_INCL_ANY \
(_DPRINTK_FLAGS_INCL_MODNAME | _DPRINTK_FLAGS_INCL_FUNCNAME |\
_DPRINTK_FLAGS_INCL_LINENO | _DPRINTK_FLAGS_INCL_TID)

+#define __dyndbg_site_is_decorated(desc) (!!(desc->flags & _DPRINTK_FLAGS_INCL_ANY))
+
#if defined DEBUG
#define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINT
#else
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 6fdbf9613aec..cf48a242b3a2 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2480,6 +2480,17 @@ config TEST_STATIC_KEYS

If unsure, say N.

+config TEST_DYNAMIC_DEBUG
+ tristate "Test DYNAMIC_DEBUG"
+ depends on m
+ depends on DYNAMIC_DEBUG
+ help
+ This module registers a tracer callback to count enabled
+ pr_debugs in a 'do_debugging' function, then alters their
+ enablements, calls the function, and compares counts.
+
+ If unsure, say N.
+
config TEST_KMOD
tristate "kmod stress tester"
depends on m
diff --git a/lib/Makefile b/lib/Makefile
index 364c23f15578..5dd4cb7c02c6 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -82,6 +82,7 @@ obj-$(CONFIG_TEST_SORT) += test_sort.o
obj-$(CONFIG_TEST_USER_COPY) += test_user_copy.o
obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_keys.o
obj-$(CONFIG_TEST_STATIC_KEYS) += test_static_key_base.o
+obj-$(CONFIG_TEST_DYNAMIC_DEBUG) += test_dynamic_debug.o
obj-$(CONFIG_TEST_PRINTF) += test_printf.o
obj-$(CONFIG_TEST_SCANF) += test_scanf.o
obj-$(CONFIG_TEST_BITMAP) += test_bitmap.o
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 71ffa19ae9a5..d6e9c833f5d4 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -36,6 +36,7 @@
#include <linux/sched.h>
#include <linux/device.h>
#include <linux/netdevice.h>
+#include <linux/trace.h>

#include <rdma/ib_verbs.h>

@@ -72,6 +73,7 @@ static LIST_HEAD(ddebug_tables);
static int verbose;
module_param(verbose, int, 0644);

+
/* Return the path relative to source root */
static inline const char *trim_prefix(const char *path)
{
@@ -85,6 +87,7 @@ static inline const char *trim_prefix(const char *path)

static struct { unsigned flag:8; char opt_char; } opt_array[] = {
{ _DPRINTK_FLAGS_PRINT, 'p' },
+ { _DPRINTK_FLAGS_TRACE, 'T' },
{ _DPRINTK_FLAGS_INCL_MODNAME, 'm' },
{ _DPRINTK_FLAGS_INCL_FUNCNAME, 'f' },
{ _DPRINTK_FLAGS_INCL_LINENO, 'l' },
@@ -205,11 +208,12 @@ static int ddebug_change(const struct ddebug_query *query,
newflags = (dp->flags & modifiers->mask) | modifiers->flags;
if (newflags == dp->flags)
continue;
+
#ifdef CONFIG_JUMP_LABEL
- if (dp->flags & _DPRINTK_FLAGS_PRINT) {
- if (!(modifiers->flags & _DPRINTK_FLAGS_PRINT))
+ if (__dyndbg_site_is_enabled(dp)) {
+ if (!__dyndbg_site_is_enabled(modifiers))
static_branch_disable(&dp->key.dd_key_true);
- } else if (modifiers->flags & _DPRINTK_FLAGS_PRINT)
+ } else if (__dyndbg_site_is_enabled(modifiers))
static_branch_enable(&dp->key.dd_key_true);
#endif
dp->flags = newflags;
@@ -427,6 +431,16 @@ static int ddebug_parse_query(char *words[], int nwords,
return 0;
}

+static int ddebug_validate_flags(struct flag_settings *modifiers)
+{
+#if !defined(CONFIG_TRACING)
+ if (__dyndbg_site_is_tracing(modifiers)) {
+ WARN_ONCE(1, "cannot enable T, CONFIG_TRACE=n\n");
+ return -EINVAL;
+ }
+#endif
+ return 0;
+}
/*
* Parse `str' as a flags specification, format [-+=][p]+.
* Sets up *maskp and *flagsp to be used when changing the
@@ -479,7 +493,7 @@ static int ddebug_parse_flags(const char *str, struct flag_settings *modifiers)
}
vpr_info("*flagsp=0x%x *maskp=0x%x\n", modifiers->flags, modifiers->mask);

- return 0;
+ return ddebug_validate_flags(modifiers);
}

static int ddebug_exec_query(char *query_string, const char *modname)
@@ -694,11 +708,24 @@ static char *__dynamic_emit_prefix(const struct _ddebug *desc, char *buf)

static inline char *dynamic_emit_prefix(struct _ddebug *desc, char *buf)
{
- if (unlikely(desc->flags & _DPRINTK_FLAGS_INCL_ANY))
+ if (unlikely(__dyndbg_site_is_enabled(desc) &&
+ __dyndbg_site_is_decorated(desc)))
return __dynamic_emit_prefix(desc, buf);
return buf;
}

+static struct trace_array *trace_arr;
+
+#if !defined(CONFIG_TRACING)
+/* private stub for 4 users below */
+static inline int __printf(3, 0)
+ trace_array_printk(struct trace_array *tr, unsigned long ip,
+ const char *fmt, ...)
+{
+ return 0;
+}
+#endif
+
void __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
{
va_list args;
@@ -713,7 +740,13 @@ void __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
vaf.fmt = fmt;
vaf.va = &args;

- printk(KERN_DEBUG "%s%pV", dynamic_emit_prefix(descriptor, buf), &vaf);
+ dynamic_emit_prefix(descriptor, buf);
+
+ if (__dyndbg_site_is_tracing(descriptor))
+ trace_array_printk(trace_arr, _THIS_IP_, "%s%pV", buf, &vaf);
+
+ if (__dyndbg_site_is_logging(descriptor))
+ printk(KERN_DEBUG "%s%pV", buf, &vaf);

va_end(args);
}
@@ -724,6 +757,7 @@ void __dynamic_dev_dbg(struct _ddebug *descriptor,
{
struct va_format vaf;
va_list args;
+ char buf[PREFIX_SIZE] = "";

BUG_ON(!descriptor);
BUG_ON(!fmt);
@@ -733,17 +767,21 @@ void __dynamic_dev_dbg(struct _ddebug *descriptor,
vaf.fmt = fmt;
vaf.va = &args;

+ dynamic_emit_prefix(descriptor, buf);
+
+ if (__dyndbg_site_is_tracing(descriptor))
+ trace_array_printk(trace_arr, _THIS_IP_, "%s%pV", buf, &vaf);
+
+ if (!__dyndbg_site_is_logging(descriptor))
+ goto out;
+
if (!dev) {
printk(KERN_DEBUG "(NULL device *): %pV", &vaf);
} else {
- char buf[PREFIX_SIZE] = "";
-
dev_printk_emit(LOGLEVEL_DEBUG, dev, "%s%s %s: %pV",
- dynamic_emit_prefix(descriptor, buf),
- dev_driver_string(dev), dev_name(dev),
- &vaf);
+ buf, dev_driver_string(dev), dev_name(dev), &vaf);
}
-
+out:
va_end(args);
}
EXPORT_SYMBOL(__dynamic_dev_dbg);
@@ -755,6 +793,7 @@ void __dynamic_netdev_dbg(struct _ddebug *descriptor,
{
struct va_format vaf;
va_list args;
+ char buf[PREFIX_SIZE] = "";

BUG_ON(!descriptor);
BUG_ON(!fmt);
@@ -764,12 +803,17 @@ void __dynamic_netdev_dbg(struct _ddebug *descriptor,
vaf.fmt = fmt;
vaf.va = &args;

- if (dev && dev->dev.parent) {
- char buf[PREFIX_SIZE] = "";
+ dynamic_emit_prefix(descriptor, buf);
+
+ if (__dyndbg_site_is_tracing(descriptor))
+ trace_array_printk(trace_arr, _THIS_IP_, "%s%pV", buf, &vaf);

+ if (!__dyndbg_site_is_logging(descriptor))
+ goto out;
+
+ if (dev && dev->dev.parent) {
dev_printk_emit(LOGLEVEL_DEBUG, dev->dev.parent,
- "%s%s %s %s%s: %pV",
- dynamic_emit_prefix(descriptor, buf),
+ "%s%s %s %s%s: %pV", buf,
dev_driver_string(dev->dev.parent),
dev_name(dev->dev.parent),
netdev_name(dev), netdev_reg_state(dev),
@@ -780,7 +824,7 @@ void __dynamic_netdev_dbg(struct _ddebug *descriptor,
} else {
printk(KERN_DEBUG "(NULL net_device): %pV", &vaf);
}
-
+out:
va_end(args);
}
EXPORT_SYMBOL(__dynamic_netdev_dbg);
@@ -794,18 +838,24 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
{
struct va_format vaf;
va_list args;
+ char buf[PREFIX_SIZE] = "";

va_start(args, fmt);

vaf.fmt = fmt;
vaf.va = &args;

- if (ibdev && ibdev->dev.parent) {
- char buf[PREFIX_SIZE] = "";
+ dynamic_emit_prefix(descriptor, buf);
+
+ if (__dyndbg_site_is_tracing(descriptor))
+ trace_array_printk(trace_arr, _THIS_IP_, "%s%pV", buf, &vaf);
+
+ if (!__dyndbg_site_is_logging(descriptor))
+ goto out;

+ if (ibdev && ibdev->dev.parent) {
dev_printk_emit(LOGLEVEL_DEBUG, ibdev->dev.parent,
- "%s%s %s %s: %pV",
- dynamic_emit_prefix(descriptor, buf),
+ "%s%s %s %s: %pV", buf,
dev_driver_string(ibdev->dev.parent),
dev_name(ibdev->dev.parent),
dev_name(&ibdev->dev),
@@ -815,7 +865,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
} else {
printk(KERN_DEBUG "(NULL ib_device): %pV", &vaf);
}
-
+out:
va_end(args);
}
EXPORT_SYMBOL(__dynamic_ibdev_dbg);
@@ -1130,6 +1180,35 @@ static void ddebug_remove_all_tables(void)
mutex_unlock(&ddebug_lock);
}

+#if defined(CONFIG_TRACING)
+
+static void ddebug_trace_cleanup(void)
+{
+ if (trace_arr) {
+ trace_array_put(trace_arr);
+ trace_array_destroy(trace_arr);
+ trace_arr = NULL;
+ }
+}
+
+static void ddebug_trace_init(void)
+{
+ int ret;
+
+ trace_arr = trace_array_get_by_name("dyndbg-tracefs");
+ if (!trace_arr)
+ return;
+
+ ret = trace_array_init_printk(trace_arr);
+ if (ret)
+ ddebug_trace_cleanup();
+}
+
+#else
+static inline void ddebug_trace_init(void) {}
+static inline void ddebug_trace_cleanup(void) {}
+#endif
+
static __initdata int ddebug_init_success;

static int __init dynamic_debug_init_control(void)
@@ -1172,6 +1251,9 @@ static int __init dynamic_debug_init(void)
ddebug_init_success = 1;
return 0;
}
+
+ ddebug_trace_init();
+
iter = __start___dyndbg;
modname = iter->modname;
iter_start = iter;
@@ -1223,6 +1305,7 @@ static int __init dynamic_debug_init(void)

out_err:
ddebug_remove_all_tables();
+ ddebug_trace_cleanup();
return 0;
}
/* Allow early initialization for boot messages via boot param */
diff --git a/lib/test_dynamic_debug.c b/lib/test_dynamic_debug.c
new file mode 100644
index 000000000000..dd41a09fd9c8
--- /dev/null
+++ b/lib/test_dynamic_debug.c
@@ -0,0 +1,222 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Kernel module for testing dynamic_debug
+ *
+ * Authors:
+ * Jim Cromie <[email protected]>
+ */
+
+/*
+ * test-setup: use trace_print attachment interface as a test harness,
+ * define a custom trace_printer which counts invocations, and a
+ * pr_debug event generator function which calls a set of categorized
+ * pr_debugs.
+ * test-run: manipulate the pr_debug's enablement, run the event
+ * generator, and check for the expected side effects.
+ */
+
+#define pr_fmt(fmt) "test_dd: " fmt
+
+#include <linux/module.h>
+
+static int hit_ct;
+static int test_ct;
+static int errors;
+
+static int __verbose;
+module_param_named(verbose, __verbose, int, 0444);
+MODULE_PARM_DESC(verbose, "enable print from trace (output verify)");
+
+static int __bad_tracer;
+module_param_named(use_bad_tracer, __bad_tracer, int, 0444);
+MODULE_PARM_DESC(use_bad_tracer,
+ "use broken tracer, recursing with pr_debug\n"
+ "\tonly works at modprobe time\n");
+
+static void (*my_tracer)(const char *lbl, struct va_format *vaf);
+
+static void good_tracer(const char *lbl, struct va_format *vaf)
+{
+ hit_ct++;
+
+ if (__verbose)
+ pr_notice("%s%pV", lbl, vaf);
+}
+
+static void bad_tracer(const char *lbl, struct va_format *vaf)
+{
+ hit_ct++;
+ if (__verbose)
+ pr_notice("%s%pV", lbl, vaf);
+
+ pr_debug("%s.%pV", lbl, vaf);
+}
+
+static void pick_tracer(void)
+{
+ if (__bad_tracer) {
+ pr_notice("using bad tracer - fails hit count tests\n");
+ my_tracer = bad_tracer;
+ } else
+ my_tracer = good_tracer;
+}
+
+static int expect_count(int want, const char *story)
+{
+ test_ct++;
+ if (want != hit_ct) {
+ pr_err("nok %d: want %d, got %d: %s\n", test_ct, want, hit_ct, story);
+ errors++;
+ hit_ct = 0;
+ return 1;
+ }
+ pr_info("ok %d: hits %d, on <%s>\n", test_ct, want, story);
+ hit_ct = 0;
+ return 0;
+}
+
+/* call pr_debug (4 * reps) + 2 times, for tracer side-effects */
+static void do_debugging(int reps)
+{
+ int i;
+
+ pr_debug("Entry:\n");
+ pr_info("%s %d time(s)\n", __func__, reps);
+ for (i = 0; i < reps; i++) {
+ pr_debug("hi: %d\n", i);
+ pr_debug("mid: %d\n", i);
+ pr_debug("low: %d\n", i);
+ pr_debug("low:lower: %d subcategory test\n", i);
+ }
+ pr_debug("Exit:\n");
+}
+
+static void expect_matches(int want, int got, const char *story)
+{
+ /* todo: got <0 are errors, bubbled up. no test for that */
+ test_ct++;
+ if (got != want) {
+ pr_warn("nok %d: want %d matches, got %d on <%s>\n", test_ct, want, got, story);
+ errors++;
+ } else
+ pr_info("ok %d: %d matches on <%s>\n", test_ct, want, story);
+}
+
+static int report(char *who)
+{
+ if (errors)
+ pr_err("%s failed %d of %d tests\n", who, errors, test_ct);
+ else
+ pr_info("%s passed %d tests\n", who, test_ct);
+ return errors;
+}
+
+struct exec_test {
+ int matches;
+ int loops;
+ int hits;
+ const char *mod;
+ const char *qry;
+};
+
+static void do_exec_test(struct exec_test *tst)
+{
+ int match_count;
+
+ match_count = dynamic_debug_exec_queries(tst->qry, tst->mod);
+ expect_matches(tst->matches, match_count, tst->qry);
+ do_debugging(tst->loops);
+ expect_count(tst->hits, tst->qry);
+}
+
+/* these tests rely on register stuff having been done ?? */
+struct exec_test exec_tests[] = {
+ /*
+ * use original single string query style once, to test it.
+ * standard use is with separate module param, like:
+ * dynamic_debug_exec_queries("func do_debugging +_", "test_dynamic_debug");
+ */
+ { 6, 1, 0, NULL, "module test_dynamic_debug func do_debugging -T" },
+
+ /* no modification probe */
+ { 6, 3, 0, KBUILD_MODNAME, "func do_debugging +_" },
+
+ /* enable all prdbgs in DUT */
+ { 6, 4, 18, KBUILD_MODNAME, "func do_debugging +T" },
+
+ /* disable hi call */
+ { 1, 4, 14, KBUILD_MODNAME, "format '^hi:' -T" },
+
+ /* disable mid call */
+ { 1, 4, 10, KBUILD_MODNAME, "format '^mid:' -T" },
+
+ /* repeat same disable */
+ { 1, 4, 10, KBUILD_MODNAME, "format '^mid:' -T" },
+
+ /* repeat same disable, diff run ct */
+ { 1, 5, 12, KBUILD_MODNAME, "format '^mid:' -T" },
+
+ /* include subclass */
+ { 2, 4, 2, KBUILD_MODNAME, "format '^low:' -T" },
+
+ /* re-disable, exclude subclass */
+ { 1, 4, 2, KBUILD_MODNAME, "format '^low: ' -T" },
+
+ /* enable, exclude subclass */
+ { 1, 4, 6, KBUILD_MODNAME, "format '^low: ' +T" },
+
+ /* enable the subclass */
+ { 1, 4, 10, KBUILD_MODNAME, "format '^low:lower:' +T" },
+
+ /* enable the subclass */
+ { 1, 6, 14, KBUILD_MODNAME, "format '^low:lower:' +T" },
+};
+
+static int __init test_dynamic_debug_init(void)
+{
+ int i;
+
+ pick_tracer();
+
+ pr_debug("Entry:\n");
+ do_debugging(1);
+ expect_count(0, "nothing on");
+
+ //dynamic_debug_register_tracer(THIS_MODULE, my_tracer);
+ /* 2nd time gets a complaint */
+ //dynamic_debug_register_tracer(THIS_MODULE, my_tracer);
+
+ for (i = 0; i < ARRAY_SIZE(exec_tests); i++)
+ do_exec_test(&exec_tests[i]);
+
+ //dynamic_debug_unregister_tracer(THIS_MODULE, my_tracer);
+
+ /* this gets missing tracer warnings, cuz +T is still on */
+ do_debugging(1);
+ expect_count(0, "unregistered, but +T still on");
+
+ /* reuse test 0 to turn off T */
+ do_exec_test(&exec_tests[0]);
+
+ /* this draws warning about failed deregistration */
+ //dynamic_debug_unregister_tracer(THIS_MODULE, my_tracer);
+
+ do_debugging(1);
+ expect_count(0, "all off");
+
+ report("init");
+ pr_debug("Exit:\n");
+ return 0;
+}
+
+static void __exit test_dynamic_debug_exit(void)
+{
+ report("exit");
+ pr_debug("Exit:");
+}
+
+module_init(test_dynamic_debug_init);
+module_exit(test_dynamic_debug_exit);
+
+MODULE_AUTHOR("Jim Cromie <[email protected]>");
+MODULE_LICENSE("GPL");
--
2.31.1

2021-11-05 22:47:50

by Jim Cromie

[permalink] [raw]
Subject: [PATCH v10 09/10] dyndbg: create DEFINE_DYNAMIC_DEBUG_LOG|TRACE_GROUPS

With the recent addition of pr_debug to tracefs via +T flag, we now
want to add drm.trace; like its model: drm.debug, it maps bits to
pr_debug categories, but this one enables/disables writing to tracefs
(iff CONFIG_TRACING).

Do this by:

1. add flags to dyndbg_bitmap_param, holds "p" or "T" to work for either.

add DEFINE_DYNAMIC_DEBUG_BITGRPS_FLAGS to init .flags
DEFINE_DYNAMIC_DEBUG_BITGRPS gets "p" for compat.
use it from...

2. DEFINE_DYNAMIC_DEBUG_LOG_GROUPS as (1) with "p" flags - print to syslog
DEFINE_DYNAMIC_DEBUG_TRACE_GROUPS as (1) with "T" flags - trace to tracefs
add kdoc to these

NOTES

The flags args (1) is a string, not just a 'p' or 'T'. This allows
use of decorator flags ("mflt") too, in case the module author wants
to insure those decorations are in the trace & log.

The LOG|TRACE (2) macros don't use any decorator flags, (and therefore
don't toggle them), allowing users to control those themselves.

Decorator flags are shared for both LOG and TRACE consumers,
coordination between users is expected. ATM, theres no declarative
way to preset decorator flags, but DEFINE_DYNAMIC_DEBUG_BITGRPS_FLAGS
can be used to explicitly toggle them.

Signed-off-by: Jim Cromie <[email protected]>
---
include/linux/dynamic_debug.h | 44 ++++++++++++++++++++++++++---------
lib/dynamic_debug.c | 4 ++--
2 files changed, 35 insertions(+), 13 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 792bcff0297e..918ac1a92358 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -255,30 +255,52 @@ struct dyndbg_bitdesc {

struct dyndbg_bitmap_param {
unsigned long *bits; /* ref to shared state */
+ const char *flags;
unsigned int maplen;
struct dyndbg_bitdesc *map; /* indexed by bitpos */
};

#if defined(CONFIG_DYNAMIC_DEBUG) || \
(defined(CONFIG_DYNAMIC_DEBUG_CORE) && defined(DYNAMIC_DEBUG_MODULE))
+
+#define DEFINE_DYNAMIC_DEBUG_BITGRPS_FLAGS(fsname, _var, _flags, desc, data) \
+ MODULE_PARM_DESC(fsname, desc); \
+ static struct dyndbg_bitmap_param ddcats_##_var = \
+ { .bits = &(_var), .flags = (_flags), \
+ .map = data, .maplen = ARRAY_SIZE(data) }; \
+ module_param_cb(fsname, &param_ops_dyndbg, &ddcats_##_var, 0644)
+
+#define DEFINE_DYNAMIC_DEBUG_BITGRPS(fsname, _var, desc, data) \
+ DEFINE_DYNAMIC_DEBUG_BITGRPS_FLAGS(fsname, _var, "p", desc, data)
+
/**
- * DEFINE_DYNAMIC_DEBUG_BITGRPS() - bitmap control of pr_debugs, by format match
+ * DEFINE_DYNAMIC_DEBUG_LOG_GROUPS() - bitmap control of grouped pr_debugs --> syslog
+ *
* @fsname: parameter basename under /sys
* @_var: C-identifier holding bitmap
* @desc: string summarizing the controls provided
* @bitmap: C array of struct dyndbg_bitdescs
*
- * Intended for modules with a systematic use of pr_debug prefixes in
- * the format strings, this allows modules calling pr_debugs to
- * control them in groups by matching against their formats, and map
- * them to bits 0-N of a sysfs control point.
+ * Intended for modules having pr_debugs with prefixed/categorized
+ * formats; this lets you group them by substring match, map groups to
+ * bits, and enable per group to write to syslog, via @fsname.
*/
-#define DEFINE_DYNAMIC_DEBUG_BITGRPS(fsname, _var, desc, data) \
- MODULE_PARM_DESC(fsname, desc); \
- static struct dyndbg_bitmap_param ddcats_##_var = \
- { .bits = &(_var), .map = data, \
- .maplen = ARRAY_SIZE(data) }; \
- module_param_cb(fsname, &param_ops_dyndbg, &ddcats_##_var, 0644)
+#define DEFINE_DYNAMIC_DEBUG_LOG_GROUPS(fsname, _var, desc, data) \
+ DEFINE_DYNAMIC_DEBUG_BITGRPS_FLAGS(fsname, _var, "p", desc, data)
+
+/**
+ * DEFINE_DYNAMIC_DEBUG_TRACE_GROUPS() - bitmap control of pr_debugs --> tracefs
+ * @fsname: parameter basename under /sys
+ * @_var: C-identifier holding bitmap
+ * @desc: string summarizing the controls provided
+ * @bitmap: C array of struct dyndbg_bitdescs
+ *
+ * Intended for modules having pr_debugs with prefixed/categorized
+ * formats; this lets you group them by substring match, map groups to
+ * bits, and enable per group to write to tracebuf, via @fsname.
+ */
+#define DEFINE_DYNAMIC_DEBUG_TRACE_GROUPS(fsname, _var, desc, data) \
+ DEFINE_DYNAMIC_DEBUG_BITGRPS_FLAGS(fsname, _var, "T", desc, data)

extern const struct kernel_param_ops param_ops_dyndbg;

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index d6e9c833f5d4..f55861dd76b2 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -629,8 +629,8 @@ int param_set_dyndbg(const char *instr, const struct kernel_param *kp)
for (i = 0; i < p->maplen && 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) ? '+' : '-');
+ snprintf(query, FMT_QUERY_SIZE, "format '%s' %c%s", map->match,
+ test_bit(i, &inbits) ? '+' : '-', p->flags);

matches = ddebug_exec_queries(query, KP_MOD_NAME);

--
2.31.1