[PATCH v1 00/12] dyndbg: add support for writing debug logs to trace

[PATCH v1 00/12] dyndbg: add support for writing debug logs to trace

[Łukasz Bartosik]

Add support for writing debug logs to trace events and trace instances.
The rationale behing this feature is to be able to redirect debug logs
(per each callsite indivdually) to trace to aid in debugging. The debug
logs output to trace can be enabled with T flag. Additionally trace
destination can be provided to the T flag after ":". The trace destination
field is used to determine where debug logs will be written. Setting trace
destination value to 0 (default) enables output to prdbg and devdbg trace
events. Setting trace destination value to a value in range of [1..255]
enables output to trace instance identified by trace destination value.
For example when trace destination value is 2 then debug logs will
be written to <debugfs>/tracing/instances/dyndbg_inst_2 instance.

Usage examples:

localhost ~ # echo "module thunderbolt =pT:7" > 
				<debugfs>/dynamic_debug/control

This will enable output of debug logs to trace instance 
<debugfs>/tracing/instances/dyndbg_inst_7 and debug logs will
be written to the syslog also because p flag is set.

localhost ~ # echo "module thunderbolt =pT:7,l" > 
                                <debugfs>/dynamic_debug/control

When trace destination is followed by another flag then trace
destination has to be followed by ",".

localhost ~ # echo "module thunderbolt =pTl" > 
                                <debugfs>/dynamic_debug/control

When trace destination is not provided explicitly then its value
defaults to 0. In this case debug logs will be written to the prdbg
and devdbg trace events.

localhost ~ # echo "module thunderbolt =T:25" > 
                                <debugfs>/dynamic_debug/control

This will enable output of debug logs to trace instance
<debugfs>/tracing/instances/dyndbg_inst_25 with debug logs output
to syslog disabled.

Given trace instance will not be initialized until debug logs are
requested to be written to it and afer init it will persist until
reboot.

Please note that output of debug logs to syslog (p flag) and trace
(T flag) can be independently enabled/disabled for each callsite.



Jim I took the liberty and based my work on your patches you pointed me
to https://github.com/jimc/linux/tree/dd-kitchen-sink. I picked up
the commits relevant to trace from the dd-kitchen-sink branch.
The only changes I introduced in your commits were related to checkpatch
complains. There are two errors still left:

1)
ERROR: need consistent spacing around '*' (ctx:WxV)
140: FILE: lib/dynamic_debug.c:1070:
+				  va_list *args)

Which seems to be a false positive to me.

2)
ERROR: Macros with complex values should be enclosed in parentheses
62: FILE: include/trace/stages/stage3_trace_output.h:12:
+#define TP_printk_no_nl(fmt, args...) fmt, args

I have not figured out how to fix it.

Changes:
V1) Major rework after receiving feedback in 
https://lore.kernel.org/all/20230915154856.1896062-1-lb@semihalf.com/

Jim Cromie (7):
  dyndbg: add _DPRINTK_FLAGS_ENABLED
  dyndbg: add _DPRINTK_FLAGS_TRACE
  dyndbg: add write-events-to-tracefs code
  dyndbg: add 2 trace-events: pr_debug, dev_dbg
  tracefs: add TP_printk_no_nl - RFC
  trace: use TP_printk_no_nl in dyndbg:prdbg,devdbg
  dyndbg: repack struct _ddebug

Łukasz Bartosik (5):
  dyndbg: move flags field to a new structure
  dyndbg: add trace destination field to _ddebug
  dyndbg: add processing of T(race) flag argument
  dyndbg: write debug logs to trace instance
  dyndbg: add trace support for hexdump

 .../admin-guide/dynamic-debug-howto.rst       |   5 +-
 MAINTAINERS                                   |   1 +
 include/linux/dynamic_debug.h                 |  57 ++-
 include/trace/events/dyndbg.h                 |  54 +++
 include/trace/stages/stage3_trace_output.h    |   3 +
 include/trace/stages/stage7_class_define.h    |   3 +
 lib/Kconfig.debug                             |   1 +
 lib/dynamic_debug.c                           | 414 +++++++++++++++---
 8 files changed, 465 insertions(+), 73 deletions(-)
 create mode 100644 include/trace/events/dyndbg.h

-- 
2.42.0.869.gea05f2083d-goog

[PATCH v1 01/12] dyndbg: add _DPRINTK_FLAGS_ENABLED

[Łukasz Bartosik]

From: Jim Cromie <jim.cromie@gmail.com>

Distinguish the condition: _DPRINTK_FLAGS_ENABLED from the bit:
_DPRINTK_FLAGS_PRINT, and re-define former in terms of latter, in
preparation to add a 2nd bit: _DPRINTK_FLAGS_TRACE

Update JUMP_LABEL code block to check _DPRINTK_FLAGS_ENABLED symbol.
Also add a 'K' to get new symbol _DPRINTK_FLAGS_PRINTK, in order to
break any stale uses.

CC: vincent.whitchurch@axis.com
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 include/linux/dynamic_debug.h | 10 ++++++----
 lib/dynamic_debug.c           |  8 ++++----
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 4fcbf4d4fd0a..7be791af7cf1 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -32,7 +32,7 @@ struct _ddebug {
 	 * writes commands to <debugfs>/dynamic_debug/control
 	 */
 #define _DPRINTK_FLAGS_NONE	0
-#define _DPRINTK_FLAGS_PRINT	(1<<0) /* printk() a message using the format */
+#define _DPRINTK_FLAGS_PRINTK	(1 << 0) /* printk() a message using the format */
 #define _DPRINTK_FLAGS_INCL_MODNAME	(1<<1)
 #define _DPRINTK_FLAGS_INCL_FUNCNAME	(1<<2)
 #define _DPRINTK_FLAGS_INCL_LINENO	(1<<3)
@@ -44,8 +44,10 @@ struct _ddebug {
 	 _DPRINTK_FLAGS_INCL_LINENO  | _DPRINTK_FLAGS_INCL_TID |\
 	 _DPRINTK_FLAGS_INCL_SOURCENAME)
 
+#define _DPRINTK_FLAGS_ENABLED		_DPRINTK_FLAGS_PRINTK
+
 #if defined DEBUG
-#define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINT
+#define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINTK
 #else
 #define _DPRINTK_FLAGS_DEFAULT 0
 #endif
@@ -199,10 +201,10 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
 
 #ifdef DEBUG
 #define DYNAMIC_DEBUG_BRANCH(descriptor) \
-	likely(descriptor.flags & _DPRINTK_FLAGS_PRINT)
+	likely(descriptor.flags & _DPRINTK_FLAGS_ENABLED)
 #else
 #define DYNAMIC_DEBUG_BRANCH(descriptor) \
-	unlikely(descriptor.flags & _DPRINTK_FLAGS_PRINT)
+	unlikely(descriptor.flags & _DPRINTK_FLAGS_ENABLED)
 #endif
 
 #endif /* CONFIG_JUMP_LABEL */
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 6fba6423cc10..ee0cb37153ef 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -89,7 +89,7 @@ static inline const char *trim_prefix(const char *path)
 }
 
 static const struct { unsigned flag:8; char opt_char; } opt_array[] = {
-	{ _DPRINTK_FLAGS_PRINT, 'p' },
+	{ _DPRINTK_FLAGS_PRINTK, 'p' },
 	{ _DPRINTK_FLAGS_INCL_MODNAME, 'm' },
 	{ _DPRINTK_FLAGS_INCL_FUNCNAME, 'f' },
 	{ _DPRINTK_FLAGS_INCL_SOURCENAME, 's' },
@@ -247,10 +247,10 @@ static int ddebug_change(const struct ddebug_query *query,
 			if (newflags == dp->flags)
 				continue;
 #ifdef CONFIG_JUMP_LABEL
-			if (dp->flags & _DPRINTK_FLAGS_PRINT) {
-				if (!(newflags & _DPRINTK_FLAGS_PRINT))
+			if (dp->flags & _DPRINTK_FLAGS_ENABLED) {
+				if (!(newflags & _DPRINTK_FLAGS_ENABLED))
 					static_branch_disable(&dp->key.dd_key_true);
-			} else if (newflags & _DPRINTK_FLAGS_PRINT) {
+			} else if (newflags & _DPRINTK_FLAGS_ENABLED) {
 				static_branch_enable(&dp->key.dd_key_true);
 			}
 #endif
-- 
2.42.0.869.gea05f2083d-goog

[PATCH v1 02/12] dyndbg: add _DPRINTK_FLAGS_TRACE

[Łukasz Bartosik]

From: Jim Cromie <jim.cromie@gmail.com>

Add new flag, and OR it into _DPRINTK_FLAGS_ENABLED definition

CC: vincent.whitchurch@axis.com
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 include/linux/dynamic_debug.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 7be791af7cf1..497130816e9c 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -33,6 +33,9 @@ struct _ddebug {
 	 */
 #define _DPRINTK_FLAGS_NONE	0
 #define _DPRINTK_FLAGS_PRINTK	(1 << 0) /* printk() a message using the format */
+#define _DPRINTK_FLAGS_TRACE	(1 << 6)
+#define _DPRINTK_FLAGS_ENABLED	(_DPRINTK_FLAGS_PRINTK | _DPRINTK_FLAGS_TRACE)
+
 #define _DPRINTK_FLAGS_INCL_MODNAME	(1<<1)
 #define _DPRINTK_FLAGS_INCL_FUNCNAME	(1<<2)
 #define _DPRINTK_FLAGS_INCL_LINENO	(1<<3)
@@ -44,8 +47,6 @@ struct _ddebug {
 	 _DPRINTK_FLAGS_INCL_LINENO  | _DPRINTK_FLAGS_INCL_TID |\
 	 _DPRINTK_FLAGS_INCL_SOURCENAME)
 
-#define _DPRINTK_FLAGS_ENABLED		_DPRINTK_FLAGS_PRINTK
-
 #if defined DEBUG
 #define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINTK
 #else
-- 
2.42.0.869.gea05f2083d-goog

[PATCH v1 03/12] dyndbg: add write-events-to-tracefs code

[Łukasz Bartosik]

From: Jim Cromie <jim.cromie@gmail.com>

adds: ddebug_trace()
 uses trace_console() temporarily to issue printk:console event
 uses internal-ish __ftrace_trace_stack code:
      4-context buffer stack, barriers per Steve Rostedt

call it from new funcs:
  ddebug_printk() - print to both syslog/tracefs
  ddebug_dev_printk() - dev-print to both syslog/tracefs

These handle both _DPRINTK_FLAGS_PRINTK and _DPRINTK_FLAGS_TRACE
cases, allowing to vsnprintf the message once and use it for both,
skipping past the KERN_DEBUG character for tracing.

Finally, adjust the callers: __ddebug_{pr_debug,{,net,ib}dev_dbg},
replacing printk and dev_printk with the new funcs above.

The _DPRINTK_FLAGS_TRACE flag character is 'T', so the following finds
all callsites enabled for tracing:

  grep -P =p?T /proc/dynamic_debug/control

This patch,~1,~2 are basically copies of: https://lore.kernel.org/lkml/
20200825153338.17061-1-vincent.whitchurch@axis.com

with a few differences:

- s/dynamic_/ddebug_/ on Vincent's additions
- __printf attrs on the _printk funcs
- reuses trace_console() event, not adding a new "printk:dyndbg" event.
  next patch replaces this with 2 new events

CC: vincent.whitchurch@axis.com
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 .../admin-guide/dynamic-debug-howto.rst       |   5 +-
 lib/dynamic_debug.c                           | 156 +++++++++++++++---
 2 files changed, 133 insertions(+), 28 deletions(-)

diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
index 0b3d39c610d9..8a126e10a6c5 100644
--- a/Documentation/admin-guide/dynamic-debug-howto.rst
+++ b/Documentation/admin-guide/dynamic-debug-howto.rst
@@ -209,8 +209,9 @@ of the characters::
 
 The flags are::
 
-  p    enables the pr_debug() callsite.
-  _    enables no flags.
+  p    callsite prints to syslog
+  T    callsite issues a dyndbg:* trace-event
+  _    enables no flags
 
   Decorator flags add to the message-prefix, in order:
   t    Include thread ID, or <intr>
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index ee0cb37153ef..016f33c20251 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 <trace/events/printk.h>
 
 #include <rdma/ib_verbs.h>
 
@@ -90,6 +91,7 @@ static inline const char *trim_prefix(const char *path)
 
 static const struct { unsigned flag:8; char opt_char; } opt_array[] = {
 	{ _DPRINTK_FLAGS_PRINTK, 'p' },
+	{ _DPRINTK_FLAGS_TRACE, 'T' },
 	{ _DPRINTK_FLAGS_INCL_MODNAME, 'm' },
 	{ _DPRINTK_FLAGS_INCL_FUNCNAME, 'f' },
 	{ _DPRINTK_FLAGS_INCL_SOURCENAME, 's' },
@@ -858,6 +860,98 @@ static inline char *dynamic_emit_prefix(struct _ddebug *desc, char *buf)
 	return buf;
 }
 
+/*
+ * This code is heavily based on __ftrace_trace_stack().
+ *
+ * Allow 4 levels of nesting: normal, softirq, irq, NMI.
+ */
+#define DYNAMIC_TRACE_NESTING	4
+
+struct ddebug_trace_buf {
+	char buf[256];
+};
+
+struct ddebug_trace_bufs {
+	struct ddebug_trace_buf bufs[DYNAMIC_TRACE_NESTING];
+};
+
+static DEFINE_PER_CPU(struct ddebug_trace_bufs, ddebug_trace_bufs);
+static DEFINE_PER_CPU(int, ddebug_trace_reserve);
+
+static void ddebug_trace(const char *fmt, va_list args)
+{
+	struct ddebug_trace_buf *buf;
+	int bufidx;
+	int len;
+
+	preempt_disable_notrace();
+
+	bufidx = __this_cpu_inc_return(ddebug_trace_reserve) - 1;
+
+	if (WARN_ON_ONCE(bufidx > DYNAMIC_TRACE_NESTING))
+		goto out;
+
+	/* For the same reasons as in __ftrace_trace_stack(). */
+	barrier();
+
+	buf = this_cpu_ptr(ddebug_trace_bufs.bufs) + bufidx;
+
+	len = vscnprintf(buf->buf, sizeof(buf->buf), fmt, args);
+	trace_console(buf->buf, len);
+
+out:
+	/* As above. */
+	barrier();
+	__this_cpu_dec(ddebug_trace_reserve);
+	preempt_enable_notrace();
+}
+
+__printf(2, 3)
+static void ddebug_printk(unsigned int flags, const char *fmt, ...)
+{
+	if (flags & _DPRINTK_FLAGS_TRACE) {
+		va_list args;
+
+		va_start(args, fmt);
+		/*
+		 * All callers include the KERN_DEBUG prefix to keep the
+		 * vprintk case simple; strip it out for tracing.
+		 */
+		ddebug_trace(fmt + strlen(KERN_DEBUG), args);
+		va_end(args);
+	}
+
+	if (flags & _DPRINTK_FLAGS_PRINTK) {
+		va_list args;
+
+		va_start(args, fmt);
+		vprintk(fmt, args);
+		va_end(args);
+	}
+}
+
+__printf(3, 4)
+static void ddebug_dev_printk(unsigned int flags, const struct device *dev,
+			      const char *fmt, ...)
+{
+
+	if (flags & _DPRINTK_FLAGS_TRACE) {
+		va_list args;
+
+		va_start(args, fmt);
+		ddebug_trace(fmt, args);
+		va_end(args);
+	}
+
+	if (flags & _DPRINTK_FLAGS_PRINTK) {
+		va_list args;
+
+		va_start(args, fmt);
+		dev_vprintk_emit(LOGLEVEL_DEBUG, dev, fmt, args);
+		va_end(args);
+	}
+}
+
 void __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
 {
 	va_list args;
@@ -872,16 +966,18 @@ 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);
+	ddebug_printk(descriptor->flags, KERN_DEBUG "%s%pV",
+		      dynamic_emit_prefix(descriptor, buf), &vaf);
 
 	va_end(args);
 }
 EXPORT_SYMBOL(__dynamic_pr_debug);
 
 void __dynamic_dev_dbg(struct _ddebug *descriptor,
-		      const struct device *dev, const char *fmt, ...)
+		       const struct device *dev, const char *fmt, ...)
 {
 	struct va_format vaf;
+	unsigned int flags;
 	va_list args;
 
 	BUG_ON(!descriptor);
@@ -891,16 +987,18 @@ void __dynamic_dev_dbg(struct _ddebug *descriptor,
 
 	vaf.fmt = fmt;
 	vaf.va = &args;
+	flags = descriptor->flags;
 
 	if (!dev) {
-		printk(KERN_DEBUG "(NULL device *): %pV", &vaf);
+		ddebug_printk(flags, 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);
+		ddebug_dev_printk(flags, dev, "%s%s %s: %pV",
+				  dynamic_emit_prefix(descriptor, buf),
+				  dev_driver_string(dev), dev_name(dev),
+				  &vaf);
 	}
 
 	va_end(args);
@@ -913,6 +1011,7 @@ void __dynamic_netdev_dbg(struct _ddebug *descriptor,
 			  const struct net_device *dev, const char *fmt, ...)
 {
 	struct va_format vaf;
+	unsigned int flags;
 	va_list args;
 
 	BUG_ON(!descriptor);
@@ -922,22 +1021,24 @@ void __dynamic_netdev_dbg(struct _ddebug *descriptor,
 
 	vaf.fmt = fmt;
 	vaf.va = &args;
+	flags = descriptor->flags;
 
 	if (dev && dev->dev.parent) {
 		char buf[PREFIX_SIZE] = "";
 
-		dev_printk_emit(LOGLEVEL_DEBUG, dev->dev.parent,
-				"%s%s %s %s%s: %pV",
-				dynamic_emit_prefix(descriptor, buf),
-				dev_driver_string(dev->dev.parent),
-				dev_name(dev->dev.parent),
-				netdev_name(dev), netdev_reg_state(dev),
-				&vaf);
+		ddebug_dev_printk(flags, dev->dev.parent,
+				   "%s%s %s %s%s: %pV",
+				   dynamic_emit_prefix(descriptor, buf),
+				   dev_driver_string(dev->dev.parent),
+				   dev_name(dev->dev.parent),
+				   netdev_name(dev), netdev_reg_state(dev),
+				   &vaf);
 	} else if (dev) {
-		printk(KERN_DEBUG "%s%s: %pV", netdev_name(dev),
-		       netdev_reg_state(dev), &vaf);
+		ddebug_printk(flags, KERN_DEBUG "%s%s: %pV",
+			       netdev_name(dev), netdev_reg_state(dev), &vaf);
 	} else {
-		printk(KERN_DEBUG "(NULL net_device): %pV", &vaf);
+		ddebug_printk(flags, KERN_DEBUG "(NULL net_device): %pV",
+			       &vaf);
 	}
 
 	va_end(args);
@@ -953,26 +1054,29 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
 {
 	struct va_format vaf;
 	va_list args;
+	unsigned int flags;
 
 	va_start(args, fmt);
 
 	vaf.fmt = fmt;
 	vaf.va = &args;
+	flags = descriptor->flags;
 
 	if (ibdev && ibdev->dev.parent) {
 		char buf[PREFIX_SIZE] = "";
 
-		dev_printk_emit(LOGLEVEL_DEBUG, ibdev->dev.parent,
-				"%s%s %s %s: %pV",
-				dynamic_emit_prefix(descriptor, buf),
-				dev_driver_string(ibdev->dev.parent),
-				dev_name(ibdev->dev.parent),
-				dev_name(&ibdev->dev),
-				&vaf);
+		ddebug_dev_printk(flags, ibdev->dev.parent,
+				  "%s%s %s %s: %pV",
+				  dynamic_emit_prefix(descriptor, buf),
+				  dev_driver_string(ibdev->dev.parent),
+				  dev_name(ibdev->dev.parent),
+				  dev_name(&ibdev->dev),
+				  &vaf);
 	} else if (ibdev) {
-		printk(KERN_DEBUG "%s: %pV", dev_name(&ibdev->dev), &vaf);
+		ddebug_printk(flags, KERN_DEBUG "%s: %pV",
+			      dev_name(&ibdev->dev), &vaf);
 	} else {
-		printk(KERN_DEBUG "(NULL ib_device): %pV", &vaf);
+		ddebug_printk(flags, KERN_DEBUG "(NULL ip_device): %pV", &vaf);
 	}
 
 	va_end(args);
-- 
2.42.0.869.gea05f2083d-goog

[PATCH v1 04/12] dyndbg: add 2 trace-events: pr_debug, dev_dbg

[Łukasz Bartosik]

From: Jim Cromie <jim.cromie@gmail.com>

ddebug_trace() currently issues a single printk:console event.
Replace that event by adding include/trace/events/dyndbg.h,
which defines 2 new trace-events: dyndbg:prdbg & dyndbg:devdbg.

These events get the _ddebug descriptor, so they can access the whole
callsite record: file, line, function, flags.  This allows the
addition of a dynamic prefix later.

So ddebug_trace() gets 2 new args: the descriptor and the device.
And its callers: ddebug_printk(), ddebug_dev_printk() upgrade their
flags param to pass the descriptor itself, and thus also the flags.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 MAINTAINERS                   |  1 +
 include/trace/events/dyndbg.h | 74 +++++++++++++++++++++++++++++++++++
 lib/dynamic_debug.c           | 73 +++++++++++++++++-----------------
 3 files changed, 112 insertions(+), 36 deletions(-)
 create mode 100644 include/trace/events/dyndbg.h

diff --git a/MAINTAINERS b/MAINTAINERS
index dd5de540ec0b..fd02dc86f1fd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -7320,6 +7320,7 @@ M:	Jason Baron <jbaron@akamai.com>
 M:	Jim Cromie <jim.cromie@gmail.com>
 S:	Maintained
 F:	include/linux/dynamic_debug.h
+F:	include/trace/events/dyndbg.h
 F:	lib/dynamic_debug.c
 F:	lib/test_dynamic_debug.c
 
diff --git a/include/trace/events/dyndbg.h b/include/trace/events/dyndbg.h
new file mode 100644
index 000000000000..ccc5bcb070f9
--- /dev/null
+++ b/include/trace/events/dyndbg.h
@@ -0,0 +1,74 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM dyndbg
+
+#if !defined(_TRACE_DYNDBG_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_DYNDBG_H
+
+#include <linux/tracepoint.h>
+
+/* capture pr_debug() callsite descriptor and message */
+TRACE_EVENT(prdbg,
+	    TP_PROTO(const struct _ddebug *desc, const char *text, size_t len),
+
+	    TP_ARGS(desc, text, len),
+
+	    TP_STRUCT__entry(
+			__field(const struct _ddebug *, desc)
+			__dynamic_array(char, msg, len + 1)
+		    ),
+
+	    TP_fast_assign(
+			__entry->desc = desc;
+			/*
+			 * Each trace entry is printed in a new line.
+			 * If the msg finishes with '\n', cut it off
+			 * to avoid blank lines in the trace.
+			 */
+			if (len > 0 && (text[len - 1] == '\n'))
+				len -= 1;
+
+			memcpy(__get_str(msg), text, len);
+			__get_str(msg)[len] = 0;
+		    ),
+
+	    TP_printk("%s.%s %s", __entry->desc->modname,
+		      __entry->desc->function, __get_str(msg))
+);
+
+/* capture dev_dbg() callsite descriptor, device, and message */
+TRACE_EVENT(devdbg,
+	    TP_PROTO(const struct _ddebug *desc, const struct device *dev,
+		     const char *text, size_t len),
+
+	    TP_ARGS(desc, dev, text, len),
+
+	    TP_STRUCT__entry(
+			__field(const struct _ddebug *, desc)
+			__field(const struct device *, dev)
+			__dynamic_array(char, msg, len + 1)
+		    ),
+
+	    TP_fast_assign(
+			__entry->desc = desc;
+			__entry->dev = (struct device *) dev;
+			/*
+			 * Each trace entry is printed in a new line.
+			 * If the msg finishes with '\n', cut it off
+			 * to avoid blank lines in the trace.
+			 */
+			if (len > 0 && (text[len - 1] == '\n'))
+				len -= 1;
+
+			memcpy(__get_str(msg), text, len);
+			__get_str(msg)[len] = 0;
+		    ),
+
+	    TP_printk("%s.%s %s", __entry->desc->modname,
+		      __entry->desc->function, __get_str(msg))
+);
+
+#endif /* _TRACE_DYNDBG_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 016f33c20251..1ed3c4f16f69 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -36,7 +36,9 @@
 #include <linux/sched.h>
 #include <linux/device.h>
 #include <linux/netdevice.h>
-#include <trace/events/printk.h>
+
+#define CREATE_TRACE_POINTS
+#include <trace/events/dyndbg.h>
 
 #include <rdma/ib_verbs.h>
 
@@ -878,7 +880,9 @@ struct ddebug_trace_bufs {
 static DEFINE_PER_CPU(struct ddebug_trace_bufs, ddebug_trace_bufs);
 static DEFINE_PER_CPU(int, ddebug_trace_reserve);
 
-static void ddebug_trace(const char *fmt, va_list args)
+__printf(3, 0)
+static void ddebug_trace(struct _ddebug *desc, const struct device *dev,
+			 const char *fmt, va_list args)
 {
 	struct ddebug_trace_buf *buf;
 	int bufidx;
@@ -897,7 +901,11 @@ static void ddebug_trace(const char *fmt, va_list args)
 	buf = this_cpu_ptr(ddebug_trace_bufs.bufs) + bufidx;
 
 	len = vscnprintf(buf->buf, sizeof(buf->buf), fmt, args);
-	trace_console(buf->buf, len);
+
+	if (!dev)
+		trace_prdbg(desc, buf->buf, len);
+	else
+		trace_devdbg(desc, dev, buf->buf, len);
 
 out:
 	/* As above. */
@@ -907,9 +915,9 @@ static void ddebug_trace(const char *fmt, va_list args)
 }
 
 __printf(2, 3)
-static void ddebug_printk(unsigned int flags, const char *fmt, ...)
+static void ddebug_printk(struct _ddebug *desc, const char *fmt, ...)
 {
-	if (flags & _DPRINTK_FLAGS_TRACE) {
+	if (desc->flags & _DPRINTK_FLAGS_TRACE) {
 		va_list args;
 
 		va_start(args, fmt);
@@ -917,11 +925,11 @@ static void ddebug_printk(unsigned int flags, const char *fmt, ...)
 		 * All callers include the KERN_DEBUG prefix to keep the
 		 * vprintk case simple; strip it out for tracing.
 		 */
-		ddebug_trace(fmt + strlen(KERN_DEBUG), args);
+		ddebug_trace(desc, NULL, fmt + strlen(KERN_DEBUG), args);
 		va_end(args);
 	}
 
-	if (flags & _DPRINTK_FLAGS_PRINTK) {
+	if (desc->flags & _DPRINTK_FLAGS_PRINTK) {
 		va_list args;
 
 		va_start(args, fmt);
@@ -931,19 +939,19 @@ static void ddebug_printk(unsigned int flags, const char *fmt, ...)
 }
 
 __printf(3, 4)
-static void ddebug_dev_printk(unsigned int flags, const struct device *dev,
+static void ddebug_dev_printk(struct _ddebug *desc, const struct device *dev,
 			      const char *fmt, ...)
 {
 
-	if (flags & _DPRINTK_FLAGS_TRACE) {
+	if (desc->flags & _DPRINTK_FLAGS_TRACE) {
 		va_list args;
 
 		va_start(args, fmt);
-		ddebug_trace(fmt, args);
+		ddebug_trace(desc, dev, fmt, args);
 		va_end(args);
 	}
 
-	if (flags & _DPRINTK_FLAGS_PRINTK) {
+	if (desc->flags & _DPRINTK_FLAGS_PRINTK) {
 		va_list args;
 
 		va_start(args, fmt);
@@ -966,7 +974,7 @@ void __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
 	vaf.fmt = fmt;
 	vaf.va = &args;
 
-	ddebug_printk(descriptor->flags, KERN_DEBUG "%s%pV",
+	ddebug_printk(descriptor, KERN_DEBUG "%s%pV",
 		      dynamic_emit_prefix(descriptor, buf), &vaf);
 
 	va_end(args);
@@ -977,7 +985,6 @@ void __dynamic_dev_dbg(struct _ddebug *descriptor,
 		       const struct device *dev, const char *fmt, ...)
 {
 	struct va_format vaf;
-	unsigned int flags;
 	va_list args;
 
 	BUG_ON(!descriptor);
@@ -987,15 +994,14 @@ void __dynamic_dev_dbg(struct _ddebug *descriptor,
 
 	vaf.fmt = fmt;
 	vaf.va = &args;
-	flags = descriptor->flags;
 
 	if (!dev) {
-		ddebug_printk(flags, KERN_DEBUG "(NULL device *): %pV",
-			      &vaf);
+		ddebug_printk(descriptor, KERN_DEBUG "(NULL device *): %pV",
+			       &vaf);
 	} else {
 		char buf[PREFIX_SIZE] = "";
 
-		ddebug_dev_printk(flags, dev, "%s%s %s: %pV",
+		ddebug_dev_printk(descriptor, dev, "%s%s %s: %pV",
 				  dynamic_emit_prefix(descriptor, buf),
 				  dev_driver_string(dev), dev_name(dev),
 				  &vaf);
@@ -1011,7 +1017,6 @@ void __dynamic_netdev_dbg(struct _ddebug *descriptor,
 			  const struct net_device *dev, const char *fmt, ...)
 {
 	struct va_format vaf;
-	unsigned int flags;
 	va_list args;
 
 	BUG_ON(!descriptor);
@@ -1021,24 +1026,22 @@ void __dynamic_netdev_dbg(struct _ddebug *descriptor,
 
 	vaf.fmt = fmt;
 	vaf.va = &args;
-	flags = descriptor->flags;
 
 	if (dev && dev->dev.parent) {
 		char buf[PREFIX_SIZE] = "";
 
-		ddebug_dev_printk(flags, dev->dev.parent,
-				   "%s%s %s %s%s: %pV",
-				   dynamic_emit_prefix(descriptor, buf),
-				   dev_driver_string(dev->dev.parent),
-				   dev_name(dev->dev.parent),
-				   netdev_name(dev), netdev_reg_state(dev),
-				   &vaf);
+		ddebug_dev_printk(descriptor, dev->dev.parent,
+				  "%s%s %s %s%s: %pV",
+				  dynamic_emit_prefix(descriptor, buf),
+				  dev_driver_string(dev->dev.parent),
+				  dev_name(dev->dev.parent),
+				  netdev_name(dev), netdev_reg_state(dev),
+				  &vaf);
 	} else if (dev) {
-		ddebug_printk(flags, KERN_DEBUG "%s%s: %pV",
-			       netdev_name(dev), netdev_reg_state(dev), &vaf);
+		ddebug_dev_printk(descriptor, &dev->dev, KERN_DEBUG "%s%s: %pV",
+				  netdev_name(dev), netdev_reg_state(dev), &vaf);
 	} else {
-		ddebug_printk(flags, KERN_DEBUG "(NULL net_device): %pV",
-			       &vaf);
+		ddebug_printk(descriptor, KERN_DEBUG "(NULL net_device): %pV", &vaf);
 	}
 
 	va_end(args);
@@ -1054,18 +1057,16 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
 {
 	struct va_format vaf;
 	va_list args;
-	unsigned int flags;
 
 	va_start(args, fmt);
 
 	vaf.fmt = fmt;
 	vaf.va = &args;
-	flags = descriptor->flags;
 
 	if (ibdev && ibdev->dev.parent) {
 		char buf[PREFIX_SIZE] = "";
 
-		ddebug_dev_printk(flags, ibdev->dev.parent,
+		ddebug_dev_printk(descriptor, ibdev->dev.parent,
 				  "%s%s %s %s: %pV",
 				  dynamic_emit_prefix(descriptor, buf),
 				  dev_driver_string(ibdev->dev.parent),
@@ -1073,10 +1074,10 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
 				  dev_name(&ibdev->dev),
 				  &vaf);
 	} else if (ibdev) {
-		ddebug_printk(flags, KERN_DEBUG "%s: %pV",
-			      dev_name(&ibdev->dev), &vaf);
+		ddebug_dev_printk(descriptor, &ibdev->dev, KERN_DEBUG "%s: %pV",
+				  dev_name(&ibdev->dev), &vaf);
 	} else {
-		ddebug_printk(flags, KERN_DEBUG "(NULL ip_device): %pV", &vaf);
+		ddebug_printk(descriptor, KERN_DEBUG "(NULL ip_device): %pV", &vaf);
 	}
 
 	va_end(args);
-- 
2.42.0.869.gea05f2083d-goog

Re: [PATCH v1 04/12] dyndbg: add 2 trace-events: pr_debug, dev_dbg

[jim.cromie]

On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
>
> From: Jim Cromie <jim.cromie@gmail.com>
>
> ddebug_trace() currently issues a single printk:console event.
> Replace that event by adding include/trace/events/dyndbg.h,
> which defines 2 new trace-events: dyndbg:prdbg & dyndbg:devdbg.
>
> These events get the _ddebug descriptor, so they can access the whole
> callsite record: file, line, function, flags.  This allows the
> addition of a dynamic prefix later.
>
> So ddebug_trace() gets 2 new args: the descriptor and the device.
> And its callers: ddebug_printk(), ddebug_dev_printk() upgrade their
> flags param to pass the descriptor itself, and thus also the flags.
>
> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>

let me add:

I have doubts about adding the descriptor to the trace record.
For loadable modules in particular, those descriptors will go away at rmmod.

I added it thinking it would support filtering by callsite info.
Its not entirely clear whether it adds any utility, now or potentially.

During dev/hacking, I saw UNSAFE_<mumble> while TP_printk-ing the records,
while banging on it with various hammers (modprobe cycling probably)

maybe on rmmod, they could be "cached"  (or pointers poisoned)
allowing their use until then (and maybe afterwards)


> ---
>  MAINTAINERS                   |  1 +
>  include/trace/events/dyndbg.h | 74 +++++++++++++++++++++++++++++++++++
>  lib/dynamic_debug.c           | 73 +++++++++++++++++-----------------
>  3 files changed, 112 insertions(+), 36 deletions(-)
>  create mode 100644 include/trace/events/dyndbg.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index dd5de540ec0b..fd02dc86f1fd 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7320,6 +7320,7 @@ M:        Jason Baron <jbaron@akamai.com>
>  M:     Jim Cromie <jim.cromie@gmail.com>
>  S:     Maintained
>  F:     include/linux/dynamic_debug.h
> +F:     include/trace/events/dyndbg.h
>  F:     lib/dynamic_debug.c
>  F:     lib/test_dynamic_debug.c
>
> diff --git a/include/trace/events/dyndbg.h b/include/trace/events/dyndbg.h
> new file mode 100644
> index 000000000000..ccc5bcb070f9
> --- /dev/null
> +++ b/include/trace/events/dyndbg.h
> @@ -0,0 +1,74 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#undef TRACE_SYSTEM
> +#define TRACE_SYSTEM dyndbg
> +
> +#if !defined(_TRACE_DYNDBG_H) || defined(TRACE_HEADER_MULTI_READ)
> +#define _TRACE_DYNDBG_H
> +
> +#include <linux/tracepoint.h>
> +
> +/* capture pr_debug() callsite descriptor and message */
> +TRACE_EVENT(prdbg,
> +           TP_PROTO(const struct _ddebug *desc, const char *text, size_t len),
> +
> +           TP_ARGS(desc, text, len),
> +
> +           TP_STRUCT__entry(
> +                       __field(const struct _ddebug *, desc)
> +                       __dynamic_array(char, msg, len + 1)
> +                   ),
> +
> +           TP_fast_assign(
> +                       __entry->desc = desc;
> +                       /*
> +                        * Each trace entry is printed in a new line.
> +                        * If the msg finishes with '\n', cut it off
> +                        * to avoid blank lines in the trace.
> +                        */
> +                       if (len > 0 && (text[len - 1] == '\n'))
> +                               len -= 1;
> +
> +                       memcpy(__get_str(msg), text, len);
> +                       __get_str(msg)[len] = 0;
> +                   ),
> +
> +           TP_printk("%s.%s %s", __entry->desc->modname,
> +                     __entry->desc->function, __get_str(msg))
> +);
> +
> +/* capture dev_dbg() callsite descriptor, device, and message */
> +TRACE_EVENT(devdbg,
> +           TP_PROTO(const struct _ddebug *desc, const struct device *dev,
> +                    const char *text, size_t len),
> +
> +           TP_ARGS(desc, dev, text, len),
> +
> +           TP_STRUCT__entry(
> +                       __field(const struct _ddebug *, desc)
> +                       __field(const struct device *, dev)
> +                       __dynamic_array(char, msg, len + 1)
> +                   ),
> +
> +           TP_fast_assign(
> +                       __entry->desc = desc;
> +                       __entry->dev = (struct device *) dev;
> +                       /*
> +                        * Each trace entry is printed in a new line.
> +                        * If the msg finishes with '\n', cut it off
> +                        * to avoid blank lines in the trace.
> +                        */
> +                       if (len > 0 && (text[len - 1] == '\n'))
> +                               len -= 1;
> +
> +                       memcpy(__get_str(msg), text, len);
> +                       __get_str(msg)[len] = 0;
> +                   ),
> +
> +           TP_printk("%s.%s %s", __entry->desc->modname,
> +                     __entry->desc->function, __get_str(msg))
> +);
> +
> +#endif /* _TRACE_DYNDBG_H */
> +
> +/* This part must be outside protection */
> +#include <trace/define_trace.h>
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index 016f33c20251..1ed3c4f16f69 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -36,7 +36,9 @@
>  #include <linux/sched.h>
>  #include <linux/device.h>
>  #include <linux/netdevice.h>
> -#include <trace/events/printk.h>
> +
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/dyndbg.h>
>
>  #include <rdma/ib_verbs.h>
>
> @@ -878,7 +880,9 @@ struct ddebug_trace_bufs {
>  static DEFINE_PER_CPU(struct ddebug_trace_bufs, ddebug_trace_bufs);
>  static DEFINE_PER_CPU(int, ddebug_trace_reserve);
>
> -static void ddebug_trace(const char *fmt, va_list args)
> +__printf(3, 0)
> +static void ddebug_trace(struct _ddebug *desc, const struct device *dev,
> +                        const char *fmt, va_list args)
>  {
>         struct ddebug_trace_buf *buf;
>         int bufidx;
> @@ -897,7 +901,11 @@ static void ddebug_trace(const char *fmt, va_list args)
>         buf = this_cpu_ptr(ddebug_trace_bufs.bufs) + bufidx;
>
>         len = vscnprintf(buf->buf, sizeof(buf->buf), fmt, args);
> -       trace_console(buf->buf, len);
> +
> +       if (!dev)
> +               trace_prdbg(desc, buf->buf, len);
> +       else
> +               trace_devdbg(desc, dev, buf->buf, len);
>
>  out:
>         /* As above. */
> @@ -907,9 +915,9 @@ static void ddebug_trace(const char *fmt, va_list args)
>  }
>
>  __printf(2, 3)
> -static void ddebug_printk(unsigned int flags, const char *fmt, ...)
> +static void ddebug_printk(struct _ddebug *desc, const char *fmt, ...)
>  {
> -       if (flags & _DPRINTK_FLAGS_TRACE) {
> +       if (desc->flags & _DPRINTK_FLAGS_TRACE) {
>                 va_list args;
>
>                 va_start(args, fmt);
> @@ -917,11 +925,11 @@ static void ddebug_printk(unsigned int flags, const char *fmt, ...)
>                  * All callers include the KERN_DEBUG prefix to keep the
>                  * vprintk case simple; strip it out for tracing.
>                  */
> -               ddebug_trace(fmt + strlen(KERN_DEBUG), args);
> +               ddebug_trace(desc, NULL, fmt + strlen(KERN_DEBUG), args);
>                 va_end(args);
>         }
>
> -       if (flags & _DPRINTK_FLAGS_PRINTK) {
> +       if (desc->flags & _DPRINTK_FLAGS_PRINTK) {
>                 va_list args;
>
>                 va_start(args, fmt);
> @@ -931,19 +939,19 @@ static void ddebug_printk(unsigned int flags, const char *fmt, ...)
>  }
>
>  __printf(3, 4)
> -static void ddebug_dev_printk(unsigned int flags, const struct device *dev,
> +static void ddebug_dev_printk(struct _ddebug *desc, const struct device *dev,
>                               const char *fmt, ...)
>  {
>
> -       if (flags & _DPRINTK_FLAGS_TRACE) {
> +       if (desc->flags & _DPRINTK_FLAGS_TRACE) {
>                 va_list args;
>
>                 va_start(args, fmt);
> -               ddebug_trace(fmt, args);
> +               ddebug_trace(desc, dev, fmt, args);
>                 va_end(args);
>         }
>
> -       if (flags & _DPRINTK_FLAGS_PRINTK) {
> +       if (desc->flags & _DPRINTK_FLAGS_PRINTK) {
>                 va_list args;
>
>                 va_start(args, fmt);
> @@ -966,7 +974,7 @@ void __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
>         vaf.fmt = fmt;
>         vaf.va = &args;
>
> -       ddebug_printk(descriptor->flags, KERN_DEBUG "%s%pV",
> +       ddebug_printk(descriptor, KERN_DEBUG "%s%pV",
>                       dynamic_emit_prefix(descriptor, buf), &vaf);
>
>         va_end(args);
> @@ -977,7 +985,6 @@ void __dynamic_dev_dbg(struct _ddebug *descriptor,
>                        const struct device *dev, const char *fmt, ...)
>  {
>         struct va_format vaf;
> -       unsigned int flags;
>         va_list args;
>
>         BUG_ON(!descriptor);
> @@ -987,15 +994,14 @@ void __dynamic_dev_dbg(struct _ddebug *descriptor,
>
>         vaf.fmt = fmt;
>         vaf.va = &args;
> -       flags = descriptor->flags;
>
>         if (!dev) {
> -               ddebug_printk(flags, KERN_DEBUG "(NULL device *): %pV",
> -                             &vaf);
> +               ddebug_printk(descriptor, KERN_DEBUG "(NULL device *): %pV",
> +                              &vaf);
>         } else {
>                 char buf[PREFIX_SIZE] = "";
>
> -               ddebug_dev_printk(flags, dev, "%s%s %s: %pV",
> +               ddebug_dev_printk(descriptor, dev, "%s%s %s: %pV",
>                                   dynamic_emit_prefix(descriptor, buf),
>                                   dev_driver_string(dev), dev_name(dev),
>                                   &vaf);
> @@ -1011,7 +1017,6 @@ void __dynamic_netdev_dbg(struct _ddebug *descriptor,
>                           const struct net_device *dev, const char *fmt, ...)
>  {
>         struct va_format vaf;
> -       unsigned int flags;
>         va_list args;
>
>         BUG_ON(!descriptor);
> @@ -1021,24 +1026,22 @@ void __dynamic_netdev_dbg(struct _ddebug *descriptor,
>
>         vaf.fmt = fmt;
>         vaf.va = &args;
> -       flags = descriptor->flags;
>
>         if (dev && dev->dev.parent) {
>                 char buf[PREFIX_SIZE] = "";
>
> -               ddebug_dev_printk(flags, dev->dev.parent,
> -                                  "%s%s %s %s%s: %pV",
> -                                  dynamic_emit_prefix(descriptor, buf),
> -                                  dev_driver_string(dev->dev.parent),
> -                                  dev_name(dev->dev.parent),
> -                                  netdev_name(dev), netdev_reg_state(dev),
> -                                  &vaf);
> +               ddebug_dev_printk(descriptor, dev->dev.parent,
> +                                 "%s%s %s %s%s: %pV",
> +                                 dynamic_emit_prefix(descriptor, buf),
> +                                 dev_driver_string(dev->dev.parent),
> +                                 dev_name(dev->dev.parent),
> +                                 netdev_name(dev), netdev_reg_state(dev),
> +                                 &vaf);
>         } else if (dev) {
> -               ddebug_printk(flags, KERN_DEBUG "%s%s: %pV",
> -                              netdev_name(dev), netdev_reg_state(dev), &vaf);
> +               ddebug_dev_printk(descriptor, &dev->dev, KERN_DEBUG "%s%s: %pV",
> +                                 netdev_name(dev), netdev_reg_state(dev), &vaf);
>         } else {
> -               ddebug_printk(flags, KERN_DEBUG "(NULL net_device): %pV",
> -                              &vaf);
> +               ddebug_printk(descriptor, KERN_DEBUG "(NULL net_device): %pV", &vaf);
>         }
>
>         va_end(args);
> @@ -1054,18 +1057,16 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
>  {
>         struct va_format vaf;
>         va_list args;
> -       unsigned int flags;
>
>         va_start(args, fmt);
>
>         vaf.fmt = fmt;
>         vaf.va = &args;
> -       flags = descriptor->flags;
>
>         if (ibdev && ibdev->dev.parent) {
>                 char buf[PREFIX_SIZE] = "";
>
> -               ddebug_dev_printk(flags, ibdev->dev.parent,
> +               ddebug_dev_printk(descriptor, ibdev->dev.parent,
>                                   "%s%s %s %s: %pV",
>                                   dynamic_emit_prefix(descriptor, buf),
>                                   dev_driver_string(ibdev->dev.parent),
> @@ -1073,10 +1074,10 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
>                                   dev_name(&ibdev->dev),
>                                   &vaf);
>         } else if (ibdev) {
> -               ddebug_printk(flags, KERN_DEBUG "%s: %pV",
> -                             dev_name(&ibdev->dev), &vaf);
> +               ddebug_dev_printk(descriptor, &ibdev->dev, KERN_DEBUG "%s: %pV",
> +                                 dev_name(&ibdev->dev), &vaf);
>         } else {
> -               ddebug_printk(flags, KERN_DEBUG "(NULL ip_device): %pV", &vaf);
> +               ddebug_printk(descriptor, KERN_DEBUG "(NULL ip_device): %pV", &vaf);
>         }
>
>         va_end(args);
> --
> 2.42.0.869.gea05f2083d-goog
>

Re: [PATCH v1 04/12] dyndbg: add 2 trace-events: pr_debug, dev_dbg

[Steven Rostedt]

On Fri,  3 Nov 2023 14:10:03 +0100
Łukasz Bartosik <lb@semihalf.com> wrote:

> +/* capture pr_debug() callsite descriptor and message */
> +TRACE_EVENT(prdbg,
> +	    TP_PROTO(const struct _ddebug *desc, const char *text, size_t len),
> +
> +	    TP_ARGS(desc, text, len),
> +
> +	    TP_STRUCT__entry(
> +			__field(const struct _ddebug *, desc)
> +			__dynamic_array(char, msg, len + 1)
> +		    ),
> +
> +	    TP_fast_assign(
> +			__entry->desc = desc;
> +			/*
> +			 * Each trace entry is printed in a new line.
> +			 * If the msg finishes with '\n', cut it off
> +			 * to avoid blank lines in the trace.
> +			 */
> +			if (len > 0 && (text[len - 1] == '\n'))
> +				len -= 1;
> +
> +			memcpy(__get_str(msg), text, len);
> +			__get_str(msg)[len] = 0;
> +		    ),
> +


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

That TP_printk() is dangerous. How do you know __entry->desc still exists
when reading the buffer?

Is the struct _ddebug permanent? Can it be freed? If so, the above can
easily cause a crash.

-- Steve

Re: [PATCH v1 04/12] dyndbg: add 2 trace-events: pr_debug, dev_dbg

[Łukasz Bartosik]

wt., 7 lis 2023 o 00:55 Steven Rostedt <rostedt@goodmis.org> napisał(a):
>
> On Fri,  3 Nov 2023 14:10:03 +0100
> Łukasz Bartosik <lb@semihalf.com> wrote:
>
> > +/* capture pr_debug() callsite descriptor and message */
> > +TRACE_EVENT(prdbg,
> > +         TP_PROTO(const struct _ddebug *desc, const char *text, size_t len),
> > +
> > +         TP_ARGS(desc, text, len),
> > +
> > +         TP_STRUCT__entry(
> > +                     __field(const struct _ddebug *, desc)
> > +                     __dynamic_array(char, msg, len + 1)
> > +                 ),
> > +
> > +         TP_fast_assign(
> > +                     __entry->desc = desc;
> > +                     /*
> > +                      * Each trace entry is printed in a new line.
> > +                      * If the msg finishes with '\n', cut it off
> > +                      * to avoid blank lines in the trace.
> > +                      */
> > +                     if (len > 0 && (text[len - 1] == '\n'))
> > +                             len -= 1;
> > +
> > +                     memcpy(__get_str(msg), text, len);
> > +                     __get_str(msg)[len] = 0;
> > +                 ),
> > +
>
>
> > +         TP_printk("%s.%s %s", __entry->desc->modname,
> > +                   __entry->desc->function, __get_str(msg))
> > +);
> > +
>
> That TP_printk() is dangerous. How do you know __entry->desc still exists
> when reading the buffer?
>
> Is the struct _ddebug permanent? Can it be freed? If so, the above can
> easily cause a crash.
>

I assume that we're talking here about the scenario where TP prdbg is
called and before TP_printk runs _ddebug pointer
becomes invalid, is that correct ? If so then I believe this also
applied to __dynamic_pr_debug and other dyndbg functions because there
is also potential for _ddebug pointer to become invalid (in case of
rrmod) before a function dereferences it.

Would it be acceptable to increase reference count of a module and
hold it until at least one callsite in that module is enabled ?
This  would ensure that passed pointer to a _ddebug struct is valid.

> -- Steve

Re: [PATCH v1 04/12] dyndbg: add 2 trace-events: pr_debug, dev_dbg

[jim.cromie]

On Fri, Nov 10, 2023 at 7:51 AM Łukasz Bartosik <lb@semihalf.com> wrote:
>
> wt., 7 lis 2023 o 00:55 Steven Rostedt <rostedt@goodmis.org> napisał(a):
> >
> > On Fri,  3 Nov 2023 14:10:03 +0100
> > Łukasz Bartosik <lb@semihalf.com> wrote:
> >
> > > +/* capture pr_debug() callsite descriptor and message */
> > > +TRACE_EVENT(prdbg,
> > > +         TP_PROTO(const struct _ddebug *desc, const char *text, size_t len),
> > > +
> > > +         TP_ARGS(desc, text, len),
> > > +
> > > +         TP_STRUCT__entry(
> > > +                     __field(const struct _ddebug *, desc)
> > > +                     __dynamic_array(char, msg, len + 1)
> > > +                 ),
> > > +
> > > +         TP_fast_assign(
> > > +                     __entry->desc = desc;
> > > +                     /*
> > > +                      * Each trace entry is printed in a new line.
> > > +                      * If the msg finishes with '\n', cut it off
> > > +                      * to avoid blank lines in the trace.
> > > +                      */
> > > +                     if (len > 0 && (text[len - 1] == '\n'))
> > > +                             len -= 1;
> > > +
> > > +                     memcpy(__get_str(msg), text, len);
> > > +                     __get_str(msg)[len] = 0;
> > > +                 ),
> > > +
> >
> >
> > > +         TP_printk("%s.%s %s", __entry->desc->modname,
> > > +                   __entry->desc->function, __get_str(msg))
> > > +);
> > > +
> >
> > That TP_printk() is dangerous. How do you know __entry->desc still exists
> > when reading the buffer?
> >
> > Is the struct _ddebug permanent? Can it be freed? If so, the above can
> > easily cause a crash.
> >
>
> I assume that we're talking here about the scenario where TP prdbg is
> called and before TP_printk runs _ddebug pointer
> becomes invalid, is that correct ? If so then I believe this also
> applied to __dynamic_pr_debug and other dyndbg functions because there
> is also potential for _ddebug pointer to become invalid (in case of
> rrmod) before a function dereferences it.
>
> Would it be acceptable to increase reference count of a module and
> hold it until at least one callsite in that module is enabled ?
> This  would ensure that passed pointer to a _ddebug struct is valid.
>

Im not understanding you, but I dont think its on-point -

a loadable module might write lots to trace-log, and each trace-entry
would have the descriptor address, with which it could deref and print 3 fields.
Then rmmod happens, all the module mem is freed, and reused for someth9ing else.

then someone cats trace, and the descriptor addrs are used to render
the tracelog.
BOOM.



> > -- Steve

Re: [PATCH v1 04/12] dyndbg: add 2 trace-events: pr_debug, dev_dbg

[Łukasz Bartosik]

pt., 10 lis 2023 o 20:20 <jim.cromie@gmail.com> napisał(a):
>
> On Fri, Nov 10, 2023 at 7:51 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> >
> > wt., 7 lis 2023 o 00:55 Steven Rostedt <rostedt@goodmis.org> napisał(a):
> > >
> > > On Fri,  3 Nov 2023 14:10:03 +0100
> > > Łukasz Bartosik <lb@semihalf.com> wrote:
> > >
> > > > +/* capture pr_debug() callsite descriptor and message */
> > > > +TRACE_EVENT(prdbg,
> > > > +         TP_PROTO(const struct _ddebug *desc, const char *text, size_t len),
> > > > +
> > > > +         TP_ARGS(desc, text, len),
> > > > +
> > > > +         TP_STRUCT__entry(
> > > > +                     __field(const struct _ddebug *, desc)
> > > > +                     __dynamic_array(char, msg, len + 1)
> > > > +                 ),
> > > > +
> > > > +         TP_fast_assign(
> > > > +                     __entry->desc = desc;
> > > > +                     /*
> > > > +                      * Each trace entry is printed in a new line.
> > > > +                      * If the msg finishes with '\n', cut it off
> > > > +                      * to avoid blank lines in the trace.
> > > > +                      */
> > > > +                     if (len > 0 && (text[len - 1] == '\n'))
> > > > +                             len -= 1;
> > > > +
> > > > +                     memcpy(__get_str(msg), text, len);
> > > > +                     __get_str(msg)[len] = 0;
> > > > +                 ),
> > > > +
> > >
> > >
> > > > +         TP_printk("%s.%s %s", __entry->desc->modname,
> > > > +                   __entry->desc->function, __get_str(msg))
> > > > +);
> > > > +
> > >
> > > That TP_printk() is dangerous. How do you know __entry->desc still exists
> > > when reading the buffer?
> > >
> > > Is the struct _ddebug permanent? Can it be freed? If so, the above can
> > > easily cause a crash.
> > >
> >
> > I assume that we're talking here about the scenario where TP prdbg is
> > called and before TP_printk runs _ddebug pointer
> > becomes invalid, is that correct ? If so then I believe this also
> > applied to __dynamic_pr_debug and other dyndbg functions because there
> > is also potential for _ddebug pointer to become invalid (in case of
> > rrmod) before a function dereferences it.
> >
> > Would it be acceptable to increase reference count of a module and
> > hold it until at least one callsite in that module is enabled ?
> > This  would ensure that passed pointer to a _ddebug struct is valid.
> >
>
> Im not understanding you, but I dont think its on-point -
>
> a loadable module might write lots to trace-log, and each trace-entry
> would have the descriptor address, with which it could deref and print 3 fields.
> Then rmmod happens, all the module mem is freed, and reused for someth9ing else.
>
> then someone cats trace, and the descriptor addrs are used to render
> the tracelog.
> BOOM.
>

Jim, thanks for educating me on this one. I completely missed the fact
that TP_printk is delayed until, for example as you mentioned cat is
run on a trace.

I'll remove passing of _ddebug ptr to trace_prdbg and trace_devdbg
functions. Probably also passing of dev ptr can removed from
trace_devdbg.

>
>
> > > -- Steve

[PATCH v1 05/12] tracefs: add TP_printk_no_nl - RFC

[Łukasz Bartosik]

From: Jim Cromie <jim.cromie@gmail.com>

This variant of TP_printk() does *not* add the trailing newline.  It
is for use by printk/debug-ish events which already have a trailing
newline.  Its here to support:

https://lore.kernel.org/lkml/
20200825153338.17061-1-vincent.whitchurch@axis.com/
which taught dyndbg to send pr_debug() msgs to tracefs, via -x/T flag.

It "reused" the include/trace/events/printk.h console event,
which does the following:

	TP_fast_assign(
		/*
		 * Each trace entry is printed in a new line.
		 * If the msg finishes with '\n', cut it off
		 * to avoid blank lines in the trace.
		 */
		if ((len > 0) && (text[len-1] == '\n'))
			len -= 1;

		memcpy(__get_str(msg), text, len);
		__get_str(msg)[len] = 0;
	),

That trim work could be avoided, *iff* all pr_debug() callers are
known to have no '\n' to strip.  While thats not true for *all*
callsites, it is 99+% true for DRM.debug callsites, and can be made
true for some subsets of prdbg/dyndbg callsites.

WANTED: macros to validate that a literal format-str has or doesn't
have a trailing newline, or to provide or trim trailing newline(s?).
Should be usable in TP_printk* defns, for use in new event defns.

Cc: <rostedt@goodmis.org>
Cc: Vincent Whitchurch <vincent.whitchurch@axis.com>
Cc: <daniel@ffwll.ch>
Cc: <pmladek@suse.com>
Cc: <sergey.senozhatsky@gmail.com>
Cc: <john.ogness@linutronix.de>
Cc: Simon Ser <contact@emersion.fr>
Cc: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 include/trace/stages/stage3_trace_output.h | 3 +++
 include/trace/stages/stage7_class_define.h | 3 +++
 2 files changed, 6 insertions(+)

diff --git a/include/trace/stages/stage3_trace_output.h b/include/trace/stages/stage3_trace_output.h
index c1fb1355d309..5f5c1374fa10 100644
--- a/include/trace/stages/stage3_trace_output.h
+++ b/include/trace/stages/stage3_trace_output.h
@@ -8,6 +8,9 @@
 #undef TP_printk
 #define TP_printk(fmt, args...) fmt "\n", args
 
+#undef TP_printk_no_nl
+#define TP_printk_no_nl(fmt, args...) fmt, args
+
 #undef __get_dynamic_array
 #define __get_dynamic_array(field)	\
 		((void *)__entry + (__entry->__data_loc_##field & 0xffff))
diff --git a/include/trace/stages/stage7_class_define.h b/include/trace/stages/stage7_class_define.h
index bcb960d16fc0..8247e4478f19 100644
--- a/include/trace/stages/stage7_class_define.h
+++ b/include/trace/stages/stage7_class_define.h
@@ -37,3 +37,6 @@
 
 #undef TP_printk
 #define TP_printk(fmt, args...) "\"" fmt "\", "  __stringify(args)
+
+#undef TP_printk_no_nl
+#define TP_printk_no_nl(fmt, args...) "\"" fmt "\", "  __stringify(args)
-- 
2.42.0.869.gea05f2083d-goog

Re: [PATCH v1 05/12] tracefs: add TP_printk_no_nl - RFC

[jim.cromie]

On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
>
> From: Jim Cromie <jim.cromie@gmail.com>
>
> This variant of TP_printk() does *not* add the trailing newline.  It
> is for use by printk/debug-ish events which already have a trailing
> newline.  Its here to support:
>
> https://lore.kernel.org/lkml/
> 20200825153338.17061-1-vincent.whitchurch@axis.com/
> which taught dyndbg to send pr_debug() msgs to tracefs, via -x/T flag.
>
> It "reused" the include/trace/events/printk.h console event,
> which does the following:
>
>         TP_fast_assign(
>                 /*
>                  * Each trace entry is printed in a new line.
>                  * If the msg finishes with '\n', cut it off
>                  * to avoid blank lines in the trace.
>                  */
>                 if ((len > 0) && (text[len-1] == '\n'))
>                         len -= 1;
>
>                 memcpy(__get_str(msg), text, len);
>                 __get_str(msg)[len] = 0;
>         ),
>
> That trim work could be avoided, *iff* all pr_debug() callers are
> known to have no '\n' to strip.  While thats not true for *all*
> callsites, it is 99+% true for DRM.debug callsites, and can be made
> true for some subsets of prdbg/dyndbg callsites.

some or all of DRM.debug messages (that I audited / caught)
were merged by Maxime recently, I;ll go back (later) to see if I missed any.

>
> WANTED: macros to validate that a literal format-str has or doesn't
> have a trailing newline, or to provide or trim trailing newline(s?).
> Should be usable in TP_printk* defns, for use in new event defns.

that might be over-optimizing

Steve,
IIRC you considered adding \n where needed.
is there anything gained (conceivably) by not just adding the trailing
\n when "needed" ?
statistically, macros could get us to 99.99+ "compliance"
IIRC - the "needed" seems correct.


>
> Cc: <rostedt@goodmis.org>
> Cc: Vincent Whitchurch <vincent.whitchurch@axis.com>
> Cc: <daniel@ffwll.ch>
> Cc: <pmladek@suse.com>
> Cc: <sergey.senozhatsky@gmail.com>
> Cc: <john.ogness@linutronix.de>
> Cc: Simon Ser <contact@emersion.fr>
> Cc: Sean Paul <seanpaul@chromium.org>
> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
> ---
>  include/trace/stages/stage3_trace_output.h | 3 +++
>  include/trace/stages/stage7_class_define.h | 3 +++
>  2 files changed, 6 insertions(+)
>
> diff --git a/include/trace/stages/stage3_trace_output.h b/include/trace/stages/stage3_trace_output.h
> index c1fb1355d309..5f5c1374fa10 100644
> --- a/include/trace/stages/stage3_trace_output.h
> +++ b/include/trace/stages/stage3_trace_output.h
> @@ -8,6 +8,9 @@
>  #undef TP_printk
>  #define TP_printk(fmt, args...) fmt "\n", args
>
> +#undef TP_printk_no_nl
> +#define TP_printk_no_nl(fmt, args...) fmt, args
> +
>  #undef __get_dynamic_array
>  #define __get_dynamic_array(field)     \
>                 ((void *)__entry + (__entry->__data_loc_##field & 0xffff))
> diff --git a/include/trace/stages/stage7_class_define.h b/include/trace/stages/stage7_class_define.h
> index bcb960d16fc0..8247e4478f19 100644
> --- a/include/trace/stages/stage7_class_define.h
> +++ b/include/trace/stages/stage7_class_define.h
> @@ -37,3 +37,6 @@
>
>  #undef TP_printk
>  #define TP_printk(fmt, args...) "\"" fmt "\", "  __stringify(args)
> +
> +#undef TP_printk_no_nl
> +#define TP_printk_no_nl(fmt, args...) "\"" fmt "\", "  __stringify(args)
> --
> 2.42.0.869.gea05f2083d-goog
>

Re: [PATCH v1 05/12] tracefs: add TP_printk_no_nl - RFC

[Steven Rostedt]

On Fri, 3 Nov 2023 21:40:13 -0600
jim.cromie@gmail.com wrote:

> Steve,
> IIRC you considered adding \n where needed.
> is there anything gained (conceivably) by not just adding the trailing
> \n when "needed" ?
> statistically, macros could get us to 99.99+ "compliance"
> IIRC - the "needed" seems correct.
> 

I replied to the next patch suggesting to do this on the read side.

-- Steve

[PATCH v1 06/12] trace: use TP_printk_no_nl in dyndbg:prdbg,devdbg

[Łukasz Bartosik]

From: Jim Cromie <jim.cromie@gmail.com>

Recently added dyndbg events: prdbg, devdbg have code to strip the
trailing newline, if its there.  Drop that trimming (minimally), and
use the new TP_printk_no_nl macro instead.  Also converting to a
vstring is deferred for now.

This use is slightly premature/overkill, since some pr_debugs do not
have the expected trailing newline.  While those lacks are arguably
bugs, this doesn't fix them.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 include/trace/events/dyndbg.h | 24 ++----------------------
 1 file changed, 2 insertions(+), 22 deletions(-)

diff --git a/include/trace/events/dyndbg.h b/include/trace/events/dyndbg.h
index ccc5bcb070f9..91dcdbe059c0 100644
--- a/include/trace/events/dyndbg.h
+++ b/include/trace/events/dyndbg.h
@@ -20,20 +20,10 @@ TRACE_EVENT(prdbg,
 
 	    TP_fast_assign(
 			__entry->desc = desc;
-			/*
-			 * Each trace entry is printed in a new line.
-			 * If the msg finishes with '\n', cut it off
-			 * to avoid blank lines in the trace.
-			 */
-			if (len > 0 && (text[len - 1] == '\n'))
-				len -= 1;
-
 			memcpy(__get_str(msg), text, len);
-			__get_str(msg)[len] = 0;
 		    ),
 
-	    TP_printk("%s.%s %s", __entry->desc->modname,
-		      __entry->desc->function, __get_str(msg))
+	    TP_printk_no_nl("%s", __get_str(msg))
 );
 
 /* capture dev_dbg() callsite descriptor, device, and message */
@@ -52,20 +42,10 @@ TRACE_EVENT(devdbg,
 	    TP_fast_assign(
 			__entry->desc = desc;
 			__entry->dev = (struct device *) dev;
-			/*
-			 * Each trace entry is printed in a new line.
-			 * If the msg finishes with '\n', cut it off
-			 * to avoid blank lines in the trace.
-			 */
-			if (len > 0 && (text[len - 1] == '\n'))
-				len -= 1;
-
 			memcpy(__get_str(msg), text, len);
-			__get_str(msg)[len] = 0;
 		    ),
 
-	    TP_printk("%s.%s %s", __entry->desc->modname,
-		      __entry->desc->function, __get_str(msg))
+	    TP_printk_no_nl("%s", __get_str(msg))
 );
 
 #endif /* _TRACE_DYNDBG_H */
-- 
2.42.0.869.gea05f2083d-goog

Re: [PATCH v1 06/12] trace: use TP_printk_no_nl in dyndbg:prdbg,devdbg

[Steven Rostedt]

On Fri,  3 Nov 2023 14:10:05 +0100
Łukasz Bartosik <lb@semihalf.com> wrote:

> index ccc5bcb070f9..91dcdbe059c0 100644
> --- a/include/trace/events/dyndbg.h
> +++ b/include/trace/events/dyndbg.h
> @@ -20,20 +20,10 @@ TRACE_EVENT(prdbg,
>  
>  	    TP_fast_assign(
>  			__entry->desc = desc;
> -			/*
> -			 * Each trace entry is printed in a new line.
> -			 * If the msg finishes with '\n', cut it off
> -			 * to avoid blank lines in the trace.
> -			 */
> -			if (len > 0 && (text[len - 1] == '\n'))
> -				len -= 1;
> -
>  			memcpy(__get_str(msg), text, len);
> -			__get_str(msg)[len] = 0;
>  		    ),
>  
> -	    TP_printk("%s.%s %s", __entry->desc->modname,
> -		      __entry->desc->function, __get_str(msg))
> +	    TP_printk_no_nl("%s", __get_str(msg))
>  );
>  

Instead of adding the TP_printk_no_nl() (Which I still do not like), we
could add a:

	__get_str_strip_nl(msg)

That will do the above loop. Which will move the processing to read side
(slow path).

And then we could update libtraceevent to handle that too.

-- Steve

Re: [PATCH v1 06/12] trace: use TP_printk_no_nl in dyndbg:prdbg,devdbg

[Łukasz Bartosik]

wt., 7 lis 2023 o 01:45 Steven Rostedt <rostedt@goodmis.org> napisał(a):
>
> On Fri,  3 Nov 2023 14:10:05 +0100
> Łukasz Bartosik <lb@semihalf.com> wrote:
>
> > index ccc5bcb070f9..91dcdbe059c0 100644
> > --- a/include/trace/events/dyndbg.h
> > +++ b/include/trace/events/dyndbg.h
> > @@ -20,20 +20,10 @@ TRACE_EVENT(prdbg,
> >
> >           TP_fast_assign(
> >                       __entry->desc = desc;
> > -                     /*
> > -                      * Each trace entry is printed in a new line.
> > -                      * If the msg finishes with '\n', cut it off
> > -                      * to avoid blank lines in the trace.
> > -                      */
> > -                     if (len > 0 && (text[len - 1] == '\n'))
> > -                             len -= 1;
> > -
> >                       memcpy(__get_str(msg), text, len);
> > -                     __get_str(msg)[len] = 0;
> >                   ),
> >
> > -         TP_printk("%s.%s %s", __entry->desc->modname,
> > -                   __entry->desc->function, __get_str(msg))
> > +         TP_printk_no_nl("%s", __get_str(msg))
> >  );
> >
>
> Instead of adding the TP_printk_no_nl() (Which I still do not like), we
> could add a:
>
>         __get_str_strip_nl(msg)
>
> That will do the above loop. Which will move the processing to read side
> (slow path).
>
> And then we could update libtraceevent to handle that too.
>

Thanks Steve.

Jim, if you don't mind I will make the suggested changes ?

> -- Steve

Re: [PATCH v1 06/12] trace: use TP_printk_no_nl in dyndbg:prdbg,devdbg

[jim.cromie]

On Fri, Nov 10, 2023 at 7:51 AM Łukasz Bartosik <lb@semihalf.com> wrote:
>
> wt., 7 lis 2023 o 01:45 Steven Rostedt <rostedt@goodmis.org> napisał(a):
> >
> > On Fri,  3 Nov 2023 14:10:05 +0100
> > Łukasz Bartosik <lb@semihalf.com> wrote:
> >
> > > index ccc5bcb070f9..91dcdbe059c0 100644
> > > --- a/include/trace/events/dyndbg.h
> > > +++ b/include/trace/events/dyndbg.h
> > > @@ -20,20 +20,10 @@ TRACE_EVENT(prdbg,
> > >
> > >           TP_fast_assign(
> > >                       __entry->desc = desc;
> > > -                     /*
> > > -                      * Each trace entry is printed in a new line.
> > > -                      * If the msg finishes with '\n', cut it off
> > > -                      * to avoid blank lines in the trace.
> > > -                      */
> > > -                     if (len > 0 && (text[len - 1] == '\n'))
> > > -                             len -= 1;
> > > -
> > >                       memcpy(__get_str(msg), text, len);
> > > -                     __get_str(msg)[len] = 0;
> > >                   ),
> > >
> > > -         TP_printk("%s.%s %s", __entry->desc->modname,
> > > -                   __entry->desc->function, __get_str(msg))
> > > +         TP_printk_no_nl("%s", __get_str(msg))
> > >  );
> > >
> >
> > Instead of adding the TP_printk_no_nl() (Which I still do not like), we
> > could add a:
> >
> >         __get_str_strip_nl(msg)
> >
> > That will do the above loop. Which will move the processing to read side
> > (slow path).
> >
> > And then we could update libtraceevent to handle that too.
> >
>
> Thanks Steve.
>
> Jim, if you don't mind I will make the suggested changes ?
>

if Steve likes it, Im happy.

> > -- Steve

[PATCH v1 07/12] dyndbg: repack struct _ddebug

[Łukasz Bartosik]

From: Jim Cromie <jim.cromie@gmail.com>

Move the JUMP_LABEL to the top of the struct, since theyre both
align(8) and this closes a pahole (unfortunately trading for padding,
but still).

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

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 497130816e9c..b9237e4ecd1b 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -14,6 +14,12 @@
  * the special section is treated as an array of these.
  */
 struct _ddebug {
+#ifdef CONFIG_JUMP_LABEL
+	union {
+		struct static_key_true dd_key_true;
+		struct static_key_false dd_key_false;
+	} key;
+#endif
 	/*
 	 * These fields are used to drive the user interface
 	 * for selecting and displaying debug callsites.
@@ -53,12 +59,6 @@ struct _ddebug {
 #define _DPRINTK_FLAGS_DEFAULT 0
 #endif
 	unsigned int flags:8;
-#ifdef CONFIG_JUMP_LABEL
-	union {
-		struct static_key_true dd_key_true;
-		struct static_key_false dd_key_false;
-	} key;
-#endif
 } __attribute__((aligned(8)));
 
 enum class_map_type {
-- 
2.42.0.869.gea05f2083d-goog

Re: [PATCH v1 07/12] dyndbg: repack struct _ddebug

[jim.cromie]

On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
>
> From: Jim Cromie <jim.cromie@gmail.com>
>
> Move the JUMP_LABEL to the top of the struct, since theyre both
> align(8) and this closes a pahole (unfortunately trading for padding,
> but still).
>
> Signed-off-by: Jim Cromie <jim.cromie@gmail.com>

let me add, I havent really tested this, nevermind thorough.
specifically, I didnt look for any offset dependence on the static-key
inside their container.
Conversely, maybe theres a free default or something in there.

> ---
>  include/linux/dynamic_debug.h | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
> index 497130816e9c..b9237e4ecd1b 100644
> --- a/include/linux/dynamic_debug.h
> +++ b/include/linux/dynamic_debug.h
> @@ -14,6 +14,12 @@
>   * the special section is treated as an array of these.
>   */
>  struct _ddebug {
> +#ifdef CONFIG_JUMP_LABEL
> +       union {
> +               struct static_key_true dd_key_true;
> +               struct static_key_false dd_key_false;
> +       } key;
> +#endif
>         /*
>          * These fields are used to drive the user interface
>          * for selecting and displaying debug callsites.
> @@ -53,12 +59,6 @@ struct _ddebug {
>  #define _DPRINTK_FLAGS_DEFAULT 0
>  #endif
>         unsigned int flags:8;
> -#ifdef CONFIG_JUMP_LABEL
> -       union {
> -               struct static_key_true dd_key_true;
> -               struct static_key_false dd_key_false;
> -       } key;
> -#endif
>  } __attribute__((aligned(8)));
>
>  enum class_map_type {
> --
> 2.42.0.869.gea05f2083d-goog
>

Re: [PATCH v1 07/12] dyndbg: repack struct _ddebug

[Łukasz Bartosik]

sob., 4 lis 2023 o 02:49 <jim.cromie@gmail.com> napisał(a):
>
> On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> >
> > From: Jim Cromie <jim.cromie@gmail.com>
> >
> > Move the JUMP_LABEL to the top of the struct, since theyre both
> > align(8) and this closes a pahole (unfortunately trading for padding,
> > but still).
> >
> > Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
>
> let me add, I havent really tested this, nevermind thorough.
> specifically, I didnt look for any offset dependence on the static-key
> inside their container.
> Conversely, maybe theres a free default or something in there.
>

Any idea how to properly test the relocation of the key ?



> > ---
> >  include/linux/dynamic_debug.h | 12 ++++++------
> >  1 file changed, 6 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
> > index 497130816e9c..b9237e4ecd1b 100644
> > --- a/include/linux/dynamic_debug.h
> > +++ b/include/linux/dynamic_debug.h
> > @@ -14,6 +14,12 @@
> >   * the special section is treated as an array of these.
> >   */
> >  struct _ddebug {
> > +#ifdef CONFIG_JUMP_LABEL
> > +       union {
> > +               struct static_key_true dd_key_true;
> > +               struct static_key_false dd_key_false;
> > +       } key;
> > +#endif
> >         /*
> >          * These fields are used to drive the user interface
> >          * for selecting and displaying debug callsites.
> > @@ -53,12 +59,6 @@ struct _ddebug {
> >  #define _DPRINTK_FLAGS_DEFAULT 0
> >  #endif
> >         unsigned int flags:8;
> > -#ifdef CONFIG_JUMP_LABEL
> > -       union {
> > -               struct static_key_true dd_key_true;
> > -               struct static_key_false dd_key_false;
> > -       } key;
> > -#endif
> >  } __attribute__((aligned(8)));
> >
> >  enum class_map_type {
> > --
> > 2.42.0.869.gea05f2083d-goog
> >

Re: [PATCH v1 07/12] dyndbg: repack struct _ddebug

[jim.cromie]

On Fri, Nov 10, 2023 at 7:51 AM Łukasz Bartosik <lb@semihalf.com> wrote:
>
> sob., 4 lis 2023 o 02:49 <jim.cromie@gmail.com> napisał(a):
> >
> > On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > >
> > > From: Jim Cromie <jim.cromie@gmail.com>
> > >
> > > Move the JUMP_LABEL to the top of the struct, since theyre both
> > > align(8) and this closes a pahole (unfortunately trading for padding,
> > > but still).
> > >
> > > Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
> >
> > let me add, I havent really tested this, nevermind thorough.
> > specifically, I didnt look for any offset dependence on the static-key
> > inside their container.
> > Conversely, maybe theres a free default or something in there.
> >
>
> Any idea how to properly test the relocation of the key ?

I was hoping Jason knew it from memory.

I have booted dd-kitchen-sink, which includes it, and it didnt melt the box.

I just checked `pahole vmlinux` output for the existence of 0-offset keys.
Its not conclusive, cuz im only looking at x86.

it does occur, but only for "sub-types".

struct static_key_true {
        struct static_key          key;                  /*     0    16 */

        /* size: 16, cachelines: 1, members: 1 */
        /* last cacheline: 16 bytes */
};
struct static_key_false {
        struct static_key          key;                  /*     0    16 */

        /* size: 16, cachelines: 1, members: 1 */
        /* last cacheline: 16 bytes */
};
struct static_key_false_deferred {
        struct static_key_false    key;                  /*     0    16 */
...};
struct static_key_mod {
        struct static_key_mod *    next;                 /*     0     8 */
...};
struct static_key_deferred {
        struct static_key          key;                  /*     0    16 */

Re: [PATCH v1 07/12] dyndbg: repack struct _ddebug

[Łukasz Bartosik]

pt., 10 lis 2023 o 22:01 <jim.cromie@gmail.com> napisał(a):
>
> On Fri, Nov 10, 2023 at 7:51 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> >
> > sob., 4 lis 2023 o 02:49 <jim.cromie@gmail.com> napisał(a):
> > >
> > > On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > > >
> > > > From: Jim Cromie <jim.cromie@gmail.com>
> > > >
> > > > Move the JUMP_LABEL to the top of the struct, since theyre both
> > > > align(8) and this closes a pahole (unfortunately trading for padding,
> > > > but still).
> > > >
> > > > Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
> > >
> > > let me add, I havent really tested this, nevermind thorough.
> > > specifically, I didnt look for any offset dependence on the static-key
> > > inside their container.
> > > Conversely, maybe theres a free default or something in there.
> > >
> >
> > Any idea how to properly test the relocation of the key ?
>
> I was hoping Jason knew it from memory.
>
> I have booted dd-kitchen-sink, which includes it, and it didnt melt the box.
>
> I just checked `pahole vmlinux` output for the existence of 0-offset keys.
> Its not conclusive, cuz im only looking at x86.
>
> it does occur, but only for "sub-types".
>
> struct static_key_true {
>         struct static_key          key;                  /*     0    16 */
>
>         /* size: 16, cachelines: 1, members: 1 */
>         /* last cacheline: 16 bytes */
> };
> struct static_key_false {
>         struct static_key          key;                  /*     0    16 */
>
>         /* size: 16, cachelines: 1, members: 1 */
>         /* last cacheline: 16 bytes */
> };
> struct static_key_false_deferred {
>         struct static_key_false    key;                  /*     0    16 */
> ...};
> struct static_key_mod {
>         struct static_key_mod *    next;                 /*     0     8 */
> ...};
> struct static_key_deferred {
>         struct static_key          key;                  /*     0    16 */

I will test it on arm64.

Re: [PATCH v1 07/12] dyndbg: repack struct _ddebug

[Łukasz Bartosik]

niedz., 12 lis 2023 o 17:28 Łukasz Bartosik <lb@semihalf.com> napisał(a):
>
> pt., 10 lis 2023 o 22:01 <jim.cromie@gmail.com> napisał(a):
> >
> > On Fri, Nov 10, 2023 at 7:51 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > >
> > > sob., 4 lis 2023 o 02:49 <jim.cromie@gmail.com> napisał(a):
> > > >
> > > > On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > > > >
> > > > > From: Jim Cromie <jim.cromie@gmail.com>
> > > > >
> > > > > Move the JUMP_LABEL to the top of the struct, since theyre both
> > > > > align(8) and this closes a pahole (unfortunately trading for padding,
> > > > > but still).
> > > > >
> > > > > Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
> > > >
> > > > let me add, I havent really tested this, nevermind thorough.
> > > > specifically, I didnt look for any offset dependence on the static-key
> > > > inside their container.
> > > > Conversely, maybe theres a free default or something in there.
> > > >
> > >
> > > Any idea how to properly test the relocation of the key ?
> >
> > I was hoping Jason knew it from memory.
> >
> > I have booted dd-kitchen-sink, which includes it, and it didnt melt the box.
> >
> > I just checked `pahole vmlinux` output for the existence of 0-offset keys.
> > Its not conclusive, cuz im only looking at x86.
> >
> > it does occur, but only for "sub-types".
> >
> > struct static_key_true {
> >         struct static_key          key;                  /*     0    16 */
> >
> >         /* size: 16, cachelines: 1, members: 1 */
> >         /* last cacheline: 16 bytes */
> > };
> > struct static_key_false {
> >         struct static_key          key;                  /*     0    16 */
> >
> >         /* size: 16, cachelines: 1, members: 1 */
> >         /* last cacheline: 16 bytes */
> > };
> > struct static_key_false_deferred {
> >         struct static_key_false    key;                  /*     0    16 */
> > ...};
> > struct static_key_mod {
> >         struct static_key_mod *    next;                 /*     0     8 */
> > ...};
> > struct static_key_deferred {
> >         struct static_key          key;                  /*     0    16 */
>
> I will test it on arm64.

Hi Jim,

I verified that relocation of JUMP_LABEL to the top of the _ddebug
struct does not brak dynamic debug functionality on arm64.
I double checked I had CONFIG_JUMP_LABEL enabled in the kernel config for arm64.
I was able to enable/disable callsites and see debug logs being written.

But if you're concerned there might be issue related to that
relocation on other architectures then let's drop this patch
and I will use pahole instead of padding for location of flags and
trace destination fields.
What do you think ?

Thanks,
Lukasz

Re: [PATCH v1 07/12] dyndbg: repack struct _ddebug

[jim.cromie]

On Fri, Nov 24, 2023 at 7:39 AM Łukasz Bartosik <lb@semihalf.com> wrote:
>
> niedz., 12 lis 2023 o 17:28 Łukasz Bartosik <lb@semihalf.com> napisał(a):
> >
> > pt., 10 lis 2023 o 22:01 <jim.cromie@gmail.com> napisał(a):
> > >
> > > On Fri, Nov 10, 2023 at 7:51 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > > >
> > > > sob., 4 lis 2023 o 02:49 <jim.cromie@gmail.com> napisał(a):
> > > > >
> > > > > On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > > > > >
> > > > > > From: Jim Cromie <jim.cromie@gmail.com>
> > > > > >
> > > > > > Move the JUMP_LABEL to the top of the struct, since theyre both
> > > > > > align(8) and this closes a pahole (unfortunately trading for padding,
> > > > > > but still).
> > > > > >
> > > > > > Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
> > > > >
> > > > > let me add, I havent really tested this, nevermind thorough.
> > > > > specifically, I didnt look for any offset dependence on the static-key
> > > > > inside their container.
> > > > > Conversely, maybe theres a free default or something in there.
> > > > >
> > > >
> > > > Any idea how to properly test the relocation of the key ?
> > >
> > > I was hoping Jason knew it from memory.
> > >
> > > I have booted dd-kitchen-sink, which includes it, and it didnt melt the box.
> > >
> > > I just checked `pahole vmlinux` output for the existence of 0-offset keys.
> > > Its not conclusive, cuz im only looking at x86.
> > >
> > > it does occur, but only for "sub-types".
> > >
> > > struct static_key_true {
> > >         struct static_key          key;                  /*     0    16 */
> > >
> > >         /* size: 16, cachelines: 1, members: 1 */
> > >         /* last cacheline: 16 bytes */
> > > };
> > > struct static_key_false {
> > >         struct static_key          key;                  /*     0    16 */
> > >
> > >         /* size: 16, cachelines: 1, members: 1 */
> > >         /* last cacheline: 16 bytes */
> > > };
> > > struct static_key_false_deferred {
> > >         struct static_key_false    key;                  /*     0    16 */
> > > ...};
> > > struct static_key_mod {
> > >         struct static_key_mod *    next;                 /*     0     8 */
> > > ...};
> > > struct static_key_deferred {
> > >         struct static_key          key;                  /*     0    16 */
> >
> > I will test it on arm64.
>
> Hi Jim,
>
> I verified that relocation of JUMP_LABEL to the top of the _ddebug
> struct does not brak dynamic debug functionality on arm64.
> I double checked I had CONFIG_JUMP_LABEL enabled in the kernel config for arm64.
> I was able to enable/disable callsites and see debug logs being written.
>
> But if you're concerned there might be issue related to that
> relocation on other architectures then let's drop this patch
> and I will use pahole instead of padding for location of flags and
> trace destination fields.
> What do you think ?
>


On balance, I think it should go in.
0 - my bias was towards abundance of paranoia
1 - youve done real work to evaluate the actual risk
2 - Jason is on thread, hasnt said WHOA
3 - actual patches have seen some testing (lkp-robot included)
4 - static-keys/jump-labels have been around a long time

One new topic:

Do you have any thoughts or plans wrt self-testing ?

the addition of private instances,
that can be opened & closed, and written to by +T:private_1

would benefit greatly from a test harness to validate it.
so far all Ive done is demo scripts

:-) thanks

> Thanks,
> Lukasz

Re: [PATCH v1 07/12] dyndbg: repack struct _ddebug

[Łukasz Bartosik]

niedz., 26 lis 2023 o 07:00 <jim.cromie@gmail.com> napisał(a):
>
> On Fri, Nov 24, 2023 at 7:39 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> >
> > niedz., 12 lis 2023 o 17:28 Łukasz Bartosik <lb@semihalf.com> napisał(a):
> > >
> > > pt., 10 lis 2023 o 22:01 <jim.cromie@gmail.com> napisał(a):
> > > >
> > > > On Fri, Nov 10, 2023 at 7:51 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > > > >
> > > > > sob., 4 lis 2023 o 02:49 <jim.cromie@gmail.com> napisał(a):
> > > > > >
> > > > > > On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > > > > > >
> > > > > > > From: Jim Cromie <jim.cromie@gmail.com>
> > > > > > >
> > > > > > > Move the JUMP_LABEL to the top of the struct, since theyre both
> > > > > > > align(8) and this closes a pahole (unfortunately trading for padding,
> > > > > > > but still).
> > > > > > >
> > > > > > > Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
> > > > > >
> > > > > > let me add, I havent really tested this, nevermind thorough.
> > > > > > specifically, I didnt look for any offset dependence on the static-key
> > > > > > inside their container.
> > > > > > Conversely, maybe theres a free default or something in there.
> > > > > >
> > > > >
> > > > > Any idea how to properly test the relocation of the key ?
> > > >
> > > > I was hoping Jason knew it from memory.
> > > >
> > > > I have booted dd-kitchen-sink, which includes it, and it didnt melt the box.
> > > >
> > > > I just checked `pahole vmlinux` output for the existence of 0-offset keys.
> > > > Its not conclusive, cuz im only looking at x86.
> > > >
> > > > it does occur, but only for "sub-types".
> > > >
> > > > struct static_key_true {
> > > >         struct static_key          key;                  /*     0    16 */
> > > >
> > > >         /* size: 16, cachelines: 1, members: 1 */
> > > >         /* last cacheline: 16 bytes */
> > > > };
> > > > struct static_key_false {
> > > >         struct static_key          key;                  /*     0    16 */
> > > >
> > > >         /* size: 16, cachelines: 1, members: 1 */
> > > >         /* last cacheline: 16 bytes */
> > > > };
> > > > struct static_key_false_deferred {
> > > >         struct static_key_false    key;                  /*     0    16 */
> > > > ...};
> > > > struct static_key_mod {
> > > >         struct static_key_mod *    next;                 /*     0     8 */
> > > > ...};
> > > > struct static_key_deferred {
> > > >         struct static_key          key;                  /*     0    16 */
> > >
> > > I will test it on arm64.
> >
> > Hi Jim,
> >
> > I verified that relocation of JUMP_LABEL to the top of the _ddebug
> > struct does not brak dynamic debug functionality on arm64.
> > I double checked I had CONFIG_JUMP_LABEL enabled in the kernel config for arm64.
> > I was able to enable/disable callsites and see debug logs being written.
> >
> > But if you're concerned there might be issue related to that
> > relocation on other architectures then let's drop this patch
> > and I will use pahole instead of padding for location of flags and
> > trace destination fields.
> > What do you think ?
> >
>
>
> On balance, I think it should go in.
> 0 - my bias was towards abundance of paranoia
> 1 - youve done real work to evaluate the actual risk
> 2 - Jason is on thread, hasnt said WHOA
> 3 - actual patches have seen some testing (lkp-robot included)
> 4 - static-keys/jump-labels have been around a long time
>
> One new topic:
>
> Do you have any thoughts or plans wrt self-testing ?
>

Actually I didn't think about it at all ;). It is not so common
practice to write tests among kernel developers. Addition of trace
instances & events to the dynamic debug is a major change so I see the
value in having it thoroughly tested. That said I'm not saying no to
writing test harness for that purpose but I wonder if there is any
test framework in the kernel that could be reused or is everyone on
their own when it comes to the testing area ?

Thanks,
Lukasz

> the addition of private instances,
> that can be opened & closed, and written to by +T:private_1
>
> would benefit greatly from a test harness to validate it.
> so far all Ive done is demo scripts
>
> :-) thanks
>
> > Thanks,
> > Lukasz

[PATCH v1 08/12] dyndbg: move flags field to a new structure

[Łukasz Bartosik]

Add a new structure ctrl and place it in 4 padding bytes
of _ddebug struct. Move flags field to the ctrl struct
and create setter and getter for the flags field. Add unused
fields to explicitly emphasise size of each bitfield.
This step prepares for addition of a trace_dst field.

Layout of _ddebug struct after addition of ctrl is:

struct _ddebug {
        union {
                struct static_key_true dd_key_true;      /*     0    16 */
                struct static_key_false dd_key_false;    /*     0    16 */
        } key;                                           /*     0    16 */
        union {
                struct static_key_true     dd_key_true;  /*     0    16 */
                struct static_key_false    dd_key_false; /*     0    16 */
        };

        const char  *              modname;              /*    16     8 */
        const char  *              function;             /*    24     8 */
        const char  *              filename;             /*    32     8 */
        const char  *              format;               /*    40     8 */
        unsigned int               lineno:18;            /*    48: 0  4 */
        unsigned int               class_id:6;           /*    48:18  4 */
        unsigned int               unused:8;             /*    48:24  4 */
        struct dd_ctrl             ctrl;                 /*    52     4 */

        /* size: 56, cachelines: 1, members: 9 */
        /* last cacheline: 56 bytes */
} __attribute__((__aligned__(8)));

Signed-off-by: Łukasz Bartosik <lb@semihalf.com>
---
 include/linux/dynamic_debug.h |  9 +++++--
 lib/dynamic_debug.c           | 44 ++++++++++++++++++++++-------------
 2 files changed, 35 insertions(+), 18 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index b9237e4ecd1b..684766289bfc 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -32,6 +32,8 @@ struct _ddebug {
 #define CLS_BITS 6
 	unsigned int class_id:CLS_BITS;
 #define _DPRINTK_CLASS_DFLT		((1 << CLS_BITS) - 1)
+	unsigned int unused:8;
+
 	/*
 	 * The flags field controls the behaviour at the callsite.
 	 * The bits here are changed dynamically when the user
@@ -58,7 +60,10 @@ struct _ddebug {
 #else
 #define _DPRINTK_FLAGS_DEFAULT 0
 #endif
-	unsigned int flags:8;
+	struct {
+		unsigned int flags:8;
+		unsigned unused:24;
+	} ctrl;
 } __attribute__((aligned(8)));
 
 enum class_map_type {
@@ -171,7 +176,7 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
 		.filename = __FILE__,				\
 		.format = (fmt),				\
 		.lineno = __LINE__,				\
-		.flags = _DPRINTK_FLAGS_DEFAULT,		\
+		.ctrl = { .flags = _DPRINTK_FLAGS_DEFAULT },	\
 		.class_id = cls,				\
 		_DPRINTK_KEY_INIT				\
 	};							\
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 1ed3c4f16f69..ca87adf327df 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -80,6 +80,16 @@ module_param(verbose, int, 0644);
 MODULE_PARM_DESC(verbose, " dynamic_debug/control processing "
 		 "( 0 = off (default), 1 = module add/rm, 2 = >control summary, 3 = parsing, 4 = per-site changes)");
 
+static inline unsigned int get_flags(const struct _ddebug *desc)
+{
+	return desc->ctrl.flags;
+}
+
+static inline void set_flags(struct _ddebug *desc, unsigned int val)
+{
+	desc->ctrl.flags = val;
+}
+
 /* Return the path relative to source root */
 static inline const char *trim_prefix(const char *path)
 {
@@ -247,11 +257,11 @@ static int ddebug_change(const struct ddebug_query *query,
 
 			nfound++;
 
-			newflags = (dp->flags & modifiers->mask) | modifiers->flags;
-			if (newflags == dp->flags)
+			newflags = (get_flags(dp) & modifiers->mask) | modifiers->flags;
+			if (newflags == get_flags(dp))
 				continue;
 #ifdef CONFIG_JUMP_LABEL
-			if (dp->flags & _DPRINTK_FLAGS_ENABLED) {
+			if (get_flags(dp) & _DPRINTK_FLAGS_ENABLED) {
 				if (!(newflags & _DPRINTK_FLAGS_ENABLED))
 					static_branch_disable(&dp->key.dd_key_true);
 			} else if (newflags & _DPRINTK_FLAGS_ENABLED) {
@@ -261,9 +271,9 @@ static int ddebug_change(const struct ddebug_query *query,
 			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(get_flags(dp), &fbuf),
 				  ddebug_describe_flags(newflags, &nbuf));
-			dp->flags = newflags;
+			set_flags(dp, newflags);
 		}
 	}
 	mutex_unlock(&ddebug_lock);
@@ -824,10 +834,11 @@ static int remaining(int wrote)
 
 static char *__dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
 {
+	unsigned int flags = get_flags(desc);
 	int pos_after_tid;
 	int pos = 0;
 
-	if (desc->flags & _DPRINTK_FLAGS_INCL_TID) {
+	if (flags & _DPRINTK_FLAGS_INCL_TID) {
 		if (in_interrupt())
 			pos += snprintf(buf + pos, remaining(pos), "<intr> ");
 		else
@@ -835,16 +846,16 @@ static char *__dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
 					task_pid_vnr(current));
 	}
 	pos_after_tid = pos;
-	if (desc->flags & _DPRINTK_FLAGS_INCL_MODNAME)
+	if (flags & _DPRINTK_FLAGS_INCL_MODNAME)
 		pos += snprintf(buf + pos, remaining(pos), "%s:",
 				desc->modname);
-	if (desc->flags & _DPRINTK_FLAGS_INCL_FUNCNAME)
+	if (flags & _DPRINTK_FLAGS_INCL_FUNCNAME)
 		pos += snprintf(buf + pos, remaining(pos), "%s:",
 				desc->function);
-	if (desc->flags & _DPRINTK_FLAGS_INCL_SOURCENAME)
+	if (flags & _DPRINTK_FLAGS_INCL_SOURCENAME)
 		pos += snprintf(buf + pos, remaining(pos), "%s:",
 				trim_prefix(desc->filename));
-	if (desc->flags & _DPRINTK_FLAGS_INCL_LINENO)
+	if (flags & _DPRINTK_FLAGS_INCL_LINENO)
 		pos += snprintf(buf + pos, remaining(pos), "%d:",
 				desc->lineno);
 	if (pos - pos_after_tid)
@@ -857,7 +868,7 @@ 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(get_flags(desc) & _DPRINTK_FLAGS_INCL_ANY))
 		return __dynamic_emit_prefix(desc, buf);
 	return buf;
 }
@@ -917,7 +928,8 @@ static void ddebug_trace(struct _ddebug *desc, const struct device *dev,
 __printf(2, 3)
 static void ddebug_printk(struct _ddebug *desc, const char *fmt, ...)
 {
-	if (desc->flags & _DPRINTK_FLAGS_TRACE) {
+
+	if (get_flags(desc) & _DPRINTK_FLAGS_TRACE) {
 		va_list args;
 
 		va_start(args, fmt);
@@ -929,7 +941,7 @@ static void ddebug_printk(struct _ddebug *desc, const char *fmt, ...)
 		va_end(args);
 	}
 
-	if (desc->flags & _DPRINTK_FLAGS_PRINTK) {
+	if (get_flags(desc) & _DPRINTK_FLAGS_PRINTK) {
 		va_list args;
 
 		va_start(args, fmt);
@@ -943,7 +955,7 @@ static void ddebug_dev_printk(struct _ddebug *desc, const struct device *dev,
 			      const char *fmt, ...)
 {
 
-	if (desc->flags & _DPRINTK_FLAGS_TRACE) {
+	if (get_flags(desc) & _DPRINTK_FLAGS_TRACE) {
 		va_list args;
 
 		va_start(args, fmt);
@@ -951,7 +963,7 @@ static void ddebug_dev_printk(struct _ddebug *desc, const struct device *dev,
 		va_end(args);
 	}
 
-	if (desc->flags & _DPRINTK_FLAGS_PRINTK) {
+	if (get_flags(desc) & _DPRINTK_FLAGS_PRINTK) {
 		va_list args;
 
 		va_start(args, fmt);
@@ -1247,7 +1259,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
 	seq_printf(m, "%s:%u [%s]%s =%s \"",
 		   trim_prefix(dp->filename), dp->lineno,
 		   iter->table->mod_name, dp->function,
-		   ddebug_describe_flags(dp->flags, &flags));
+		   ddebug_describe_flags(get_flags(dp), &flags));
 	seq_escape_str(m, dp->format, ESCAPE_SPACE, "\t\r\n\"");
 	seq_puts(m, "\"");
 
-- 
2.42.0.869.gea05f2083d-goog

Re: [PATCH v1 08/12] dyndbg: move flags field to a new structure

[kernel test robot]

Hi Łukasz,

kernel test robot noticed the following build warnings:

[auto build test WARNING on akpm-mm/mm-everything]
[also build test WARNING on linus/master v6.6 next-20231103]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/ukasz-Bartosik/dyndbg-add-_DPRINTK_FLAGS_ENABLED/20231103-212105
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20231103131011.1316396-9-lb%40semihalf.com
patch subject: [PATCH v1 08/12] dyndbg: move flags field to a new structure
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20231104/202311040450.FoHhKIIg-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231104/202311040450.FoHhKIIg-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311040450.FoHhKIIg-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from include/asm-generic/bug.h:5,
                    from arch/m68k/include/asm/bug.h:32,
                    from include/linux/bug.h:5,
                    from include/linux/mmdebug.h:5,
                    from include/linux/mm.h:6,
                    from mm/page_alloc.c:19:
   mm/page_alloc.c: In function 'zone_pcp_init':
   include/linux/dynamic_debug.h:213:28: error: 'struct _ddebug' has no member named 'flags'
     213 |         unlikely(descriptor.flags & _DPRINTK_FLAGS_ENABLED)
         |                            ^
   include/linux/compiler.h:77:45: note: in definition of macro 'unlikely'
      77 | # define unlikely(x)    __builtin_expect(!!(x), 0)
         |                                             ^
   include/linux/dynamic_debug.h:231:13: note: in expansion of macro 'DYNAMIC_DEBUG_BRANCH'
     231 |         if (DYNAMIC_DEBUG_BRANCH(id))                           \
         |             ^~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:256:9: note: in expansion of macro '__dynamic_func_call_cls'
     256 |         __dynamic_func_call_cls(__UNIQUE_ID(ddebug), cls, fmt, func, ##__VA_ARGS__)
         |         ^~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:258:9: note: in expansion of macro '_dynamic_func_call_cls'
     258 |         _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__)
         |         ^~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:277:9: note: in expansion of macro '_dynamic_func_call'
     277 |         _dynamic_func_call(fmt, __dynamic_pr_debug,             \
         |         ^~~~~~~~~~~~~~~~~~
   include/linux/printk.h:579:9: note: in expansion of macro 'dynamic_pr_debug'
     579 |         dynamic_pr_debug(fmt, ##__VA_ARGS__)
         |         ^~~~~~~~~~~~~~~~
   mm/page_alloc.c:5691:17: note: in expansion of macro 'pr_debug'
    5691 |                 pr_debug("  %s zone: %lu pages, LIFO batch:%u\n", zone->name,
         |                 ^~~~~~~~
   mm/page_alloc.c: In function 'alloc_contig_dump_pages':
   include/linux/dynamic_debug.h:213:28: error: 'struct _ddebug' has no member named 'flags'
     213 |         unlikely(descriptor.flags & _DPRINTK_FLAGS_ENABLED)
         |                            ^
   include/linux/compiler.h:77:45: note: in definition of macro 'unlikely'
      77 | # define unlikely(x)    __builtin_expect(!!(x), 0)
         |                                             ^
   mm/page_alloc.c:6250:13: note: in expansion of macro 'DYNAMIC_DEBUG_BRANCH'
    6250 |         if (DYNAMIC_DEBUG_BRANCH(descriptor)) {
         |             ^~~~~~~~~~~~~~~~~~~~
   In file included from include/linux/printk.h:564,
                    from include/asm-generic/bug.h:22:
>> mm/page_alloc.c:6248:39: warning: variable 'descriptor' set but not used [-Wunused-but-set-variable]
    6248 |         DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, "migrate failure");
         |                                       ^~~~~~~~~~
   include/linux/dynamic_debug.h:173:31: note: in definition of macro 'DEFINE_DYNAMIC_DEBUG_METADATA_CLS'
     173 |         __section("__dyndbg") name = {                          \
         |                               ^~~~
   mm/page_alloc.c:6248:9: note: in expansion of macro 'DEFINE_DYNAMIC_DEBUG_METADATA'
    6248 |         DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, "migrate failure");
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
--
   In file included from include/linux/build_bug.h:5,
                    from include/linux/container_of.h:5,
                    from include/linux/list.h:5,
                    from include/linux/rculist.h:10,
                    from include/linux/pid.h:5,
                    from include/linux/sched.h:14,
                    from fs/btrfs/delayed-ref.c:6:
   fs/btrfs/delayed-ref.c: In function 'btrfs_check_delayed_seq':
   include/linux/dynamic_debug.h:213:28: error: 'struct _ddebug' has no member named 'flags'
     213 |         unlikely(descriptor.flags & _DPRINTK_FLAGS_ENABLED)
         |                            ^
   include/linux/compiler.h:77:45: note: in definition of macro 'unlikely'
      77 | # define unlikely(x)    __builtin_expect(!!(x), 0)
         |                                             ^
   include/linux/dynamic_debug.h:240:13: note: in expansion of macro 'DYNAMIC_DEBUG_BRANCH'
     240 |         if (DYNAMIC_DEBUG_BRANCH(id))                                   \
         |             ^~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/delayed-ref.c:519:17: note: in expansion of macro 'btrfs_debug'
     519 |                 btrfs_debug(fs_info,
         |                 ^~~~~~~~~~~
   In file included from include/linux/printk.h:564,
                    from include/asm-generic/bug.h:22,
                    from arch/m68k/include/asm/bug.h:32,
                    from include/linux/bug.h:5,
                    from include/linux/thread_info.h:13,
                    from include/asm-generic/preempt.h:5,
                    from ./arch/m68k/include/generated/asm/preempt.h:1,
                    from include/linux/preempt.h:79,
                    from arch/m68k/include/asm/irqflags.h:6,
                    from include/linux/irqflags.h:17,
                    from arch/m68k/include/asm/atomic.h:6,
                    from include/linux/atomic.h:7,
                    from include/linux/rcupdate.h:25,
                    from include/linux/rculist.h:11:
>> include/linux/compiler.h:180:45: warning: variable '__UNIQUE_ID_ddebug322' set but not used [-Wunused-but-set-variable]
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                             ^~~~~~~~~~~~
   include/linux/dynamic_debug.h:173:31: note: in definition of macro 'DEFINE_DYNAMIC_DEBUG_METADATA_CLS'
     173 |         __section("__dyndbg") name = {                          \
         |                               ^~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:29: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                             ^~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:37: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                     ^~~~~~~
   include/linux/dynamic_debug.h:266:41: note: in expansion of macro '__UNIQUE_ID'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |                                         ^~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/delayed-ref.c:519:17: note: in expansion of macro 'btrfs_debug'
     519 |                 btrfs_debug(fs_info,
         |                 ^~~~~~~~~~~
--
   In file included from include/linux/build_bug.h:5,
                    from include/linux/container_of.h:5,
                    from include/linux/list.h:5,
                    from include/linux/rculist.h:10,
                    from include/linux/pid.h:5,
                    from include/linux/sched.h:14,
                    from fs/btrfs/relocation.c:6:
   fs/btrfs/relocation.c: In function 'create_reloc_root':
   include/linux/dynamic_debug.h:213:28: error: 'struct _ddebug' has no member named 'flags'
     213 |         unlikely(descriptor.flags & _DPRINTK_FLAGS_ENABLED)
         |                            ^
   include/linux/compiler.h:77:45: note: in definition of macro 'unlikely'
      77 | # define unlikely(x)    __builtin_expect(!!(x), 0)
         |                                             ^
   include/linux/dynamic_debug.h:240:13: note: in expansion of macro 'DYNAMIC_DEBUG_BRANCH'
     240 |         if (DYNAMIC_DEBUG_BRANCH(id))                                   \
         |             ^~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/transaction.h:222:25: note: in expansion of macro 'btrfs_debug'
     222 |                         btrfs_debug((trans)->fs_info,                   \
         |                         ^~~~~~~~~~~
   fs/btrfs/relocation.c:833:17: note: in expansion of macro 'btrfs_abort_transaction'
     833 |                 btrfs_abort_transaction(trans, ret);
         |                 ^~~~~~~~~~~~~~~~~~~~~~~
   In file included from include/linux/printk.h:564,
                    from include/asm-generic/bug.h:22,
                    from arch/m68k/include/asm/bug.h:32,
                    from include/linux/bug.h:5,
                    from include/linux/thread_info.h:13,
                    from include/asm-generic/preempt.h:5,
                    from ./arch/m68k/include/generated/asm/preempt.h:1,
                    from include/linux/preempt.h:79,
                    from arch/m68k/include/asm/irqflags.h:6,
                    from include/linux/irqflags.h:17,
                    from arch/m68k/include/asm/atomic.h:6,
                    from include/linux/atomic.h:7,
                    from include/linux/rcupdate.h:25,
                    from include/linux/rculist.h:11:
>> include/linux/compiler.h:180:45: warning: variable '__UNIQUE_ID_ddebug320' set but not used [-Wunused-but-set-variable]
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                             ^~~~~~~~~~~~
   include/linux/dynamic_debug.h:173:31: note: in definition of macro 'DEFINE_DYNAMIC_DEBUG_METADATA_CLS'
     173 |         __section("__dyndbg") name = {                          \
         |                               ^~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:29: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                             ^~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:37: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                     ^~~~~~~
   include/linux/dynamic_debug.h:266:41: note: in expansion of macro '__UNIQUE_ID'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |                                         ^~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/transaction.h:222:25: note: in expansion of macro 'btrfs_debug'
     222 |                         btrfs_debug((trans)->fs_info,                   \
         |                         ^~~~~~~~~~~
   fs/btrfs/relocation.c:833:17: note: in expansion of macro 'btrfs_abort_transaction'
     833 |                 btrfs_abort_transaction(trans, ret);
         |                 ^~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/relocation.c: In function 'replace_file_extents':
   include/linux/dynamic_debug.h:213:28: error: 'struct _ddebug' has no member named 'flags'
     213 |         unlikely(descriptor.flags & _DPRINTK_FLAGS_ENABLED)
         |                            ^
   include/linux/compiler.h:77:45: note: in definition of macro 'unlikely'
      77 | # define unlikely(x)    __builtin_expect(!!(x), 0)
         |                                             ^
   include/linux/dynamic_debug.h:240:13: note: in expansion of macro 'DYNAMIC_DEBUG_BRANCH'
     240 |         if (DYNAMIC_DEBUG_BRANCH(id))                                   \
         |             ^~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/transaction.h:222:25: note: in expansion of macro 'btrfs_debug'
     222 |                         btrfs_debug((trans)->fs_info,                   \
         |                         ^~~~~~~~~~~
   fs/btrfs/relocation.c:1167:25: note: in expansion of macro 'btrfs_abort_transaction'
    1167 |                         btrfs_abort_transaction(trans, ret);
         |                         ^~~~~~~~~~~~~~~~~~~~~~~
>> include/linux/compiler.h:180:45: warning: variable '__UNIQUE_ID_ddebug322' set but not used [-Wunused-but-set-variable]
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                             ^~~~~~~~~~~~
   include/linux/dynamic_debug.h:173:31: note: in definition of macro 'DEFINE_DYNAMIC_DEBUG_METADATA_CLS'
     173 |         __section("__dyndbg") name = {                          \
         |                               ^~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:29: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                             ^~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:37: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                     ^~~~~~~
   include/linux/dynamic_debug.h:266:41: note: in expansion of macro '__UNIQUE_ID'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |                                         ^~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/transaction.h:222:25: note: in expansion of macro 'btrfs_debug'
     222 |                         btrfs_debug((trans)->fs_info,                   \
         |                         ^~~~~~~~~~~
   fs/btrfs/relocation.c:1167:25: note: in expansion of macro 'btrfs_abort_transaction'
    1167 |                         btrfs_abort_transaction(trans, ret);
         |                         ^~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:213:28: error: 'struct _ddebug' has no member named 'flags'
     213 |         unlikely(descriptor.flags & _DPRINTK_FLAGS_ENABLED)
         |                            ^
   include/linux/compiler.h:77:45: note: in definition of macro 'unlikely'
      77 | # define unlikely(x)    __builtin_expect(!!(x), 0)
         |                                             ^
   include/linux/dynamic_debug.h:240:13: note: in expansion of macro 'DYNAMIC_DEBUG_BRANCH'
     240 |         if (DYNAMIC_DEBUG_BRANCH(id))                                   \
         |             ^~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/transaction.h:222:25: note: in expansion of macro 'btrfs_debug'
     222 |                         btrfs_debug((trans)->fs_info,                   \
         |                         ^~~~~~~~~~~
   fs/btrfs/relocation.c:1178:25: note: in expansion of macro 'btrfs_abort_transaction'
    1178 |                         btrfs_abort_transaction(trans, ret);
         |                         ^~~~~~~~~~~~~~~~~~~~~~~
>> include/linux/compiler.h:180:45: warning: variable '__UNIQUE_ID_ddebug324' set but not used [-Wunused-but-set-variable]
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                             ^~~~~~~~~~~~
   include/linux/dynamic_debug.h:173:31: note: in definition of macro 'DEFINE_DYNAMIC_DEBUG_METADATA_CLS'
     173 |         __section("__dyndbg") name = {                          \
         |                               ^~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:29: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                             ^~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:37: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                     ^~~~~~~
   include/linux/dynamic_debug.h:266:41: note: in expansion of macro '__UNIQUE_ID'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |                                         ^~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/transaction.h:222:25: note: in expansion of macro 'btrfs_debug'
     222 |                         btrfs_debug((trans)->fs_info,                   \
         |                         ^~~~~~~~~~~
   fs/btrfs/relocation.c:1178:25: note: in expansion of macro 'btrfs_abort_transaction'
    1178 |                         btrfs_abort_transaction(trans, ret);
         |                         ^~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/relocation.c: In function 'replace_path':
   include/linux/dynamic_debug.h:213:28: error: 'struct _ddebug' has no member named 'flags'
     213 |         unlikely(descriptor.flags & _DPRINTK_FLAGS_ENABLED)
         |                            ^
   include/linux/compiler.h:77:45: note: in definition of macro 'unlikely'
      77 | # define unlikely(x)    __builtin_expect(!!(x), 0)
         |                                             ^
   include/linux/dynamic_debug.h:240:13: note: in expansion of macro 'DYNAMIC_DEBUG_BRANCH'
     240 |         if (DYNAMIC_DEBUG_BRANCH(id))                                   \
         |             ^~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/transaction.h:222:25: note: in expansion of macro 'btrfs_debug'
     222 |                         btrfs_debug((trans)->fs_info,                   \
         |                         ^~~~~~~~~~~
   fs/btrfs/relocation.c:1390:25: note: in expansion of macro 'btrfs_abort_transaction'
    1390 |                         btrfs_abort_transaction(trans, ret);
         |                         ^~~~~~~~~~~~~~~~~~~~~~~
>> include/linux/compiler.h:180:45: warning: variable '__UNIQUE_ID_ddebug326' set but not used [-Wunused-but-set-variable]
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                             ^~~~~~~~~~~~
   include/linux/dynamic_debug.h:173:31: note: in definition of macro 'DEFINE_DYNAMIC_DEBUG_METADATA_CLS'
     173 |         __section("__dyndbg") name = {                          \
         |                               ^~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:29: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                             ^~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:37: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                     ^~~~~~~
   include/linux/dynamic_debug.h:266:41: note: in expansion of macro '__UNIQUE_ID'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |                                         ^~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/transaction.h:222:25: note: in expansion of macro 'btrfs_debug'
     222 |                         btrfs_debug((trans)->fs_info,                   \
         |                         ^~~~~~~~~~~
   fs/btrfs/relocation.c:1390:25: note: in expansion of macro 'btrfs_abort_transaction'
    1390 |                         btrfs_abort_transaction(trans, ret);
         |                         ^~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:213:28: error: 'struct _ddebug' has no member named 'flags'
     213 |         unlikely(descriptor.flags & _DPRINTK_FLAGS_ENABLED)
         |                            ^
   include/linux/compiler.h:77:45: note: in definition of macro 'unlikely'
      77 | # define unlikely(x)    __builtin_expect(!!(x), 0)
         |                                             ^
   include/linux/dynamic_debug.h:240:13: note: in expansion of macro 'DYNAMIC_DEBUG_BRANCH'
     240 |         if (DYNAMIC_DEBUG_BRANCH(id))                                   \
         |             ^~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/transaction.h:222:25: note: in expansion of macro 'btrfs_debug'
     222 |                         btrfs_debug((trans)->fs_info,                   \
         |                         ^~~~~~~~~~~
   fs/btrfs/relocation.c:1399:25: note: in expansion of macro 'btrfs_abort_transaction'
    1399 |                         btrfs_abort_transaction(trans, ret);
         |                         ^~~~~~~~~~~~~~~~~~~~~~~
>> include/linux/compiler.h:180:45: warning: variable '__UNIQUE_ID_ddebug328' set but not used [-Wunused-but-set-variable]
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                             ^~~~~~~~~~~~
   include/linux/dynamic_debug.h:173:31: note: in definition of macro 'DEFINE_DYNAMIC_DEBUG_METADATA_CLS'
     173 |         __section("__dyndbg") name = {                          \
         |                               ^~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:29: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                             ^~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:37: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                     ^~~~~~~
   include/linux/dynamic_debug.h:266:41: note: in expansion of macro '__UNIQUE_ID'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |                                         ^~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/transaction.h:222:25: note: in expansion of macro 'btrfs_debug'
     222 |                         btrfs_debug((trans)->fs_info,                   \
         |                         ^~~~~~~~~~~
   fs/btrfs/relocation.c:1399:25: note: in expansion of macro 'btrfs_abort_transaction'
    1399 |                         btrfs_abort_transaction(trans, ret);
         |                         ^~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:213:28: error: 'struct _ddebug' has no member named 'flags'
     213 |         unlikely(descriptor.flags & _DPRINTK_FLAGS_ENABLED)
         |                            ^
   include/linux/compiler.h:77:45: note: in definition of macro 'unlikely'
      77 | # define unlikely(x)    __builtin_expect(!!(x), 0)
         |                                             ^
   include/linux/dynamic_debug.h:240:13: note: in expansion of macro 'DYNAMIC_DEBUG_BRANCH'
     240 |         if (DYNAMIC_DEBUG_BRANCH(id))                                   \
         |             ^~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/transaction.h:222:25: note: in expansion of macro 'btrfs_debug'
     222 |                         btrfs_debug((trans)->fs_info,                   \
         |                         ^~~~~~~~~~~
   fs/btrfs/relocation.c:1409:25: note: in expansion of macro 'btrfs_abort_transaction'
    1409 |                         btrfs_abort_transaction(trans, ret);
         |                         ^~~~~~~~~~~~~~~~~~~~~~~
>> include/linux/compiler.h:180:45: warning: variable '__UNIQUE_ID_ddebug330' set but not used [-Wunused-but-set-variable]
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                             ^~~~~~~~~~~~
   include/linux/dynamic_debug.h:173:31: note: in definition of macro 'DEFINE_DYNAMIC_DEBUG_METADATA_CLS'
     173 |         __section("__dyndbg") name = {                          \
         |                               ^~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:29: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                             ^~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:37: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                     ^~~~~~~
   include/linux/dynamic_debug.h:266:41: note: in expansion of macro '__UNIQUE_ID'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |                                         ^~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/transaction.h:222:25: note: in expansion of macro 'btrfs_debug'
     222 |                         btrfs_debug((trans)->fs_info,                   \
         |                         ^~~~~~~~~~~
   fs/btrfs/relocation.c:1409:25: note: in expansion of macro 'btrfs_abort_transaction'
    1409 |                         btrfs_abort_transaction(trans, ret);
         |                         ^~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:213:28: error: 'struct _ddebug' has no member named 'flags'
     213 |         unlikely(descriptor.flags & _DPRINTK_FLAGS_ENABLED)
         |                            ^
   include/linux/compiler.h:77:45: note: in definition of macro 'unlikely'
      77 | # define unlikely(x)    __builtin_expect(!!(x), 0)
         |                                             ^
   include/linux/dynamic_debug.h:240:13: note: in expansion of macro 'DYNAMIC_DEBUG_BRANCH'
     240 |         if (DYNAMIC_DEBUG_BRANCH(id))                                   \
         |             ^~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/transaction.h:222:25: note: in expansion of macro 'btrfs_debug'
     222 |                         btrfs_debug((trans)->fs_info,                   \
         |                         ^~~~~~~~~~~
   fs/btrfs/relocation.c:1419:25: note: in expansion of macro 'btrfs_abort_transaction'
    1419 |                         btrfs_abort_transaction(trans, ret);
         |                         ^~~~~~~~~~~~~~~~~~~~~~~
>> include/linux/compiler.h:180:45: warning: variable '__UNIQUE_ID_ddebug332' set but not used [-Wunused-but-set-variable]
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                             ^~~~~~~~~~~~
   include/linux/dynamic_debug.h:173:31: note: in definition of macro 'DEFINE_DYNAMIC_DEBUG_METADATA_CLS'
     173 |         __section("__dyndbg") name = {                          \
         |                               ^~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:29: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                             ^~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:37: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                     ^~~~~~~
   include/linux/dynamic_debug.h:266:41: note: in expansion of macro '__UNIQUE_ID'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |                                         ^~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/transaction.h:222:25: note: in expansion of macro 'btrfs_debug'
     222 |                         btrfs_debug((trans)->fs_info,                   \
         |                         ^~~~~~~~~~~
   fs/btrfs/relocation.c:1419:25: note: in expansion of macro 'btrfs_abort_transaction'
    1419 |                         btrfs_abort_transaction(trans, ret);
         |                         ^~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/relocation.c: In function 'merge_reloc_root':
   include/linux/dynamic_debug.h:213:28: error: 'struct _ddebug' has no member named 'flags'
     213 |         unlikely(descriptor.flags & _DPRINTK_FLAGS_ENABLED)
         |                            ^
   include/linux/compiler.h:77:45: note: in definition of macro 'unlikely'
      77 | # define unlikely(x)    __builtin_expect(!!(x), 0)
         |                                             ^
   include/linux/dynamic_debug.h:240:13: note: in expansion of macro 'DYNAMIC_DEBUG_BRANCH'
     240 |         if (DYNAMIC_DEBUG_BRANCH(id))                                   \
         |             ^~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/transaction.h:222:25: note: in expansion of macro 'btrfs_debug'
     222 |                         btrfs_debug((trans)->fs_info,                   \
         |                         ^~~~~~~~~~~
   fs/btrfs/relocation.c:1843:25: note: in expansion of macro 'btrfs_abort_transaction'
    1843 |                         btrfs_abort_transaction(trans, ret);
         |                         ^~~~~~~~~~~~~~~~~~~~~~~
>> include/linux/compiler.h:180:45: warning: variable '__UNIQUE_ID_ddebug336' set but not used [-Wunused-but-set-variable]
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                             ^~~~~~~~~~~~
   include/linux/dynamic_debug.h:173:31: note: in definition of macro 'DEFINE_DYNAMIC_DEBUG_METADATA_CLS'
     173 |         __section("__dyndbg") name = {                          \
         |                               ^~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:29: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                             ^~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:37: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                     ^~~~~~~
   include/linux/dynamic_debug.h:266:41: note: in expansion of macro '__UNIQUE_ID'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |                                         ^~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/transaction.h:222:25: note: in expansion of macro 'btrfs_debug'
     222 |                         btrfs_debug((trans)->fs_info,                   \
         |                         ^~~~~~~~~~~
   fs/btrfs/relocation.c:1843:25: note: in expansion of macro 'btrfs_abort_transaction'
    1843 |                         btrfs_abort_transaction(trans, ret);
         |                         ^~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/relocation.c: In function 'prepare_to_merge':
   include/linux/dynamic_debug.h:213:28: error: 'struct _ddebug' has no member named 'flags'
     213 |         unlikely(descriptor.flags & _DPRINTK_FLAGS_ENABLED)
         |                            ^
   include/linux/compiler.h:77:45: note: in definition of macro 'unlikely'
      77 | # define unlikely(x)    __builtin_expect(!!(x), 0)
         |                                             ^
   include/linux/dynamic_debug.h:240:13: note: in expansion of macro 'DYNAMIC_DEBUG_BRANCH'
     240 |         if (DYNAMIC_DEBUG_BRANCH(id))                                   \
         |             ^~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/transaction.h:222:25: note: in expansion of macro 'btrfs_debug'
     222 |                         btrfs_debug((trans)->fs_info,                   \
         |                         ^~~~~~~~~~~
   fs/btrfs/relocation.c:1914:25: note: in expansion of macro 'btrfs_abort_transaction'
    1914 |                         btrfs_abort_transaction(trans, (int)PTR_ERR(root));
         |                         ^~~~~~~~~~~~~~~~~~~~~~~
>> include/linux/compiler.h:180:45: warning: variable '__UNIQUE_ID_ddebug338' set but not used [-Wunused-but-set-variable]
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                             ^~~~~~~~~~~~
   include/linux/dynamic_debug.h:173:31: note: in definition of macro 'DEFINE_DYNAMIC_DEBUG_METADATA_CLS'
     173 |         __section("__dyndbg") name = {                          \
         |                               ^~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:29: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                             ^~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:37: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                     ^~~~~~~
   include/linux/dynamic_debug.h:266:41: note: in expansion of macro '__UNIQUE_ID'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |                                         ^~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/transaction.h:222:25: note: in expansion of macro 'btrfs_debug'
     222 |                         btrfs_debug((trans)->fs_info,                   \
         |                         ^~~~~~~~~~~
   fs/btrfs/relocation.c:1914:25: note: in expansion of macro 'btrfs_abort_transaction'
    1914 |                         btrfs_abort_transaction(trans, (int)PTR_ERR(root));
         |                         ^~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:213:28: error: 'struct _ddebug' has no member named 'flags'
     213 |         unlikely(descriptor.flags & _DPRINTK_FLAGS_ENABLED)
         |                            ^
   include/linux/compiler.h:77:45: note: in definition of macro 'unlikely'
      77 | # define unlikely(x)    __builtin_expect(!!(x), 0)
         |                                             ^
   include/linux/dynamic_debug.h:240:13: note: in expansion of macro 'DYNAMIC_DEBUG_BRANCH'
     240 |         if (DYNAMIC_DEBUG_BRANCH(id))                                   \
         |             ^~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/transaction.h:222:25: note: in expansion of macro 'btrfs_debug'
     222 |                         btrfs_debug((trans)->fs_info,                   \
         |                         ^~~~~~~~~~~
   fs/btrfs/relocation.c:1947:25: note: in expansion of macro 'btrfs_abort_transaction'
    1947 |                         btrfs_abort_transaction(trans, -EUCLEAN);
         |                         ^~~~~~~~~~~~~~~~~~~~~~~
>> include/linux/compiler.h:180:45: warning: variable '__UNIQUE_ID_ddebug340' set but not used [-Wunused-but-set-variable]
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                             ^~~~~~~~~~~~
   include/linux/dynamic_debug.h:173:31: note: in definition of macro 'DEFINE_DYNAMIC_DEBUG_METADATA_CLS'
     173 |         __section("__dyndbg") name = {                          \
         |                               ^~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:29: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                             ^~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:37: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                     ^~~~~~~
   include/linux/dynamic_debug.h:266:41: note: in expansion of macro '__UNIQUE_ID'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |                                         ^~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/transaction.h:222:25: note: in expansion of macro 'btrfs_debug'
     222 |                         btrfs_debug((trans)->fs_info,                   \
         |                         ^~~~~~~~~~~
   fs/btrfs/relocation.c:1947:25: note: in expansion of macro 'btrfs_abort_transaction'
    1947 |                         btrfs_abort_transaction(trans, -EUCLEAN);
         |                         ^~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:213:28: error: 'struct _ddebug' has no member named 'flags'
     213 |         unlikely(descriptor.flags & _DPRINTK_FLAGS_ENABLED)
         |                            ^
   include/linux/compiler.h:77:45: note: in definition of macro 'unlikely'
      77 | # define unlikely(x)    __builtin_expect(!!(x), 0)
         |                                             ^
   include/linux/dynamic_debug.h:240:13: note: in expansion of macro 'DYNAMIC_DEBUG_BRANCH'
     240 |         if (DYNAMIC_DEBUG_BRANCH(id))                                   \
         |             ^~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/transaction.h:222:25: note: in expansion of macro 'btrfs_debug'
     222 |                         btrfs_debug((trans)->fs_info,                   \
         |                         ^~~~~~~~~~~
   fs/btrfs/relocation.c:1969:25: note: in expansion of macro 'btrfs_abort_transaction'
    1969 |                         btrfs_abort_transaction(trans, ret);
         |                         ^~~~~~~~~~~~~~~~~~~~~~~
>> include/linux/compiler.h:180:45: warning: variable '__UNIQUE_ID_ddebug342' set but not used [-Wunused-but-set-variable]
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                             ^~~~~~~~~~~~
   include/linux/dynamic_debug.h:173:31: note: in definition of macro 'DEFINE_DYNAMIC_DEBUG_METADATA_CLS'
     173 |         __section("__dyndbg") name = {                          \
         |                               ^~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:29: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                             ^~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:37: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                     ^~~~~~~
   include/linux/dynamic_debug.h:266:41: note: in expansion of macro '__UNIQUE_ID'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |                                         ^~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/transaction.h:222:25: note: in expansion of macro 'btrfs_debug'
     222 |                         btrfs_debug((trans)->fs_info,                   \
         |                         ^~~~~~~~~~~
   fs/btrfs/relocation.c:1969:25: note: in expansion of macro 'btrfs_abort_transaction'
    1969 |                         btrfs_abort_transaction(trans, ret);
         |                         ^~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/relocation.c: In function 'do_relocation':
   include/linux/dynamic_debug.h:213:28: error: 'struct _ddebug' has no member named 'flags'
     213 |         unlikely(descriptor.flags & _DPRINTK_FLAGS_ENABLED)
         |                            ^
   include/linux/compiler.h:77:45: note: in definition of macro 'unlikely'
      77 | # define unlikely(x)    __builtin_expect(!!(x), 0)
         |                                             ^
   include/linux/dynamic_debug.h:240:13: note: in expansion of macro 'DYNAMIC_DEBUG_BRANCH'
     240 |         if (DYNAMIC_DEBUG_BRANCH(id))                                   \
         |             ^~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/transaction.h:222:25: note: in expansion of macro 'btrfs_debug'
     222 |                         btrfs_debug((trans)->fs_info,                   \
         |                         ^~~~~~~~~~~
   fs/btrfs/relocation.c:2532:33: note: in expansion of macro 'btrfs_abort_transaction'
    2532 |                                 btrfs_abort_transaction(trans, ret);
         |                                 ^~~~~~~~~~~~~~~~~~~~~~~
>> include/linux/compiler.h:180:45: warning: variable '__UNIQUE_ID_ddebug344' set but not used [-Wunused-but-set-variable]
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                             ^~~~~~~~~~~~
   include/linux/dynamic_debug.h:173:31: note: in definition of macro 'DEFINE_DYNAMIC_DEBUG_METADATA_CLS'
     173 |         __section("__dyndbg") name = {                          \
         |                               ^~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:29: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                             ^~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:37: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                     ^~~~~~~
   include/linux/dynamic_debug.h:266:41: note: in expansion of macro '__UNIQUE_ID'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |                                         ^~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/transaction.h:222:25: note: in expansion of macro 'btrfs_debug'
     222 |                         btrfs_debug((trans)->fs_info,                   \
         |                         ^~~~~~~~~~~
   fs/btrfs/relocation.c:2532:33: note: in expansion of macro 'btrfs_abort_transaction'
    2532 |                                 btrfs_abort_transaction(trans, ret);
         |                                 ^~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/relocation.c: In function 'delete_orphan_inode':
   include/linux/dynamic_debug.h:213:28: error: 'struct _ddebug' has no member named 'flags'
     213 |         unlikely(descriptor.flags & _DPRINTK_FLAGS_ENABLED)
         |                            ^
   include/linux/compiler.h:77:45: note: in definition of macro 'unlikely'
      77 | # define unlikely(x)    __builtin_expect(!!(x), 0)
         |                                             ^
   include/linux/dynamic_debug.h:240:13: note: in expansion of macro 'DYNAMIC_DEBUG_BRANCH'
     240 |         if (DYNAMIC_DEBUG_BRANCH(id))                                   \
         |             ^~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/transaction.h:222:25: note: in expansion of macro 'btrfs_debug'
     222 |                         btrfs_debug((trans)->fs_info,                   \
         |                         ^~~~~~~~~~~
   fs/btrfs/relocation.c:3868:17: note: in expansion of macro 'btrfs_abort_transaction'
    3868 |                 btrfs_abort_transaction(trans, ret);
         |                 ^~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler.h:180:45: warning: variable '__UNIQUE_ID_ddebug353' set but not used [-Wunused-but-set-variable]
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                             ^~~~~~~~~~~~
   include/linux/dynamic_debug.h:173:31: note: in definition of macro 'DEFINE_DYNAMIC_DEBUG_METADATA_CLS'
     173 |         __section("__dyndbg") name = {                          \
         |                               ^~~~
   include/linux/dynamic_debug.h:266:9: note: in expansion of macro '__dynamic_func_call_cls_no_desc'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:29: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                             ^~~~~~~
   include/linux/compiler_types.h:75:22: note: in expansion of macro '___PASTE'
      75 | #define __PASTE(a,b) ___PASTE(a,b)
         |                      ^~~~~~~~
   include/linux/compiler.h:180:37: note: in expansion of macro '__PASTE'
     180 | #define __UNIQUE_ID(prefix) __PASTE(__PASTE(__UNIQUE_ID_, prefix), __COUNTER__)
         |                                     ^~~~~~~
   include/linux/dynamic_debug.h:266:41: note: in expansion of macro '__UNIQUE_ID'
     266 |         __dynamic_func_call_cls_no_desc(__UNIQUE_ID(ddebug), cls, fmt,  \
         |                                         ^~~~~~~~~~~
   include/linux/dynamic_debug.h:269:9: note: in expansion of macro '_dynamic_func_call_cls_no_desc'
     269 |         _dynamic_func_call_cls_no_desc(_DPRINTK_CLASS_DFLT, fmt,        \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/messages.h:110:9: note: in expansion of macro '_dynamic_func_call_no_desc'
     110 |         _dynamic_func_call_no_desc(fmt, btrfs_printk,                   \
         |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
   fs/btrfs/transaction.h:222:25: note: in expansion of macro 'btrfs_debug'
     222 |                         btrfs_debug((trans)->fs_info,                   \
         |                         ^~~~~~~~~~~
   fs/btrfs/relocation.c:3868:17: note: in expansion of macro 'btrfs_abort_transaction'
    3868 |                 btrfs_abort_transaction(trans, ret);
         |                 ^~~~~~~~~~~~~~~~~~~~~~~
..


vim +/descriptor +6248 mm/page_alloc.c

e95d372c4cd46b6 Kefeng Wang     2023-05-16  6243  
8df995f6bde01de Alexandre Ghiti 2019-05-13  6244  #ifdef CONFIG_CONTIG_ALLOC
a1394bddf9b60e9 Minchan Kim     2021-04-29  6245  /* Usage: See admin-guide/dynamic-debug-howto.rst */
a1394bddf9b60e9 Minchan Kim     2021-04-29  6246  static void alloc_contig_dump_pages(struct list_head *page_list)
a1394bddf9b60e9 Minchan Kim     2021-04-29  6247  {
a1394bddf9b60e9 Minchan Kim     2021-04-29 @6248  	DEFINE_DYNAMIC_DEBUG_METADATA(descriptor, "migrate failure");
a1394bddf9b60e9 Minchan Kim     2021-04-29  6249  
a1394bddf9b60e9 Minchan Kim     2021-04-29 @6250  	if (DYNAMIC_DEBUG_BRANCH(descriptor)) {
a1394bddf9b60e9 Minchan Kim     2021-04-29  6251  		struct page *page;
a1394bddf9b60e9 Minchan Kim     2021-04-29  6252  
a1394bddf9b60e9 Minchan Kim     2021-04-29  6253  		dump_stack();
a1394bddf9b60e9 Minchan Kim     2021-04-29  6254  		list_for_each_entry(page, page_list, lru)
a1394bddf9b60e9 Minchan Kim     2021-04-29  6255  			dump_page(page, "migration failure");
a1394bddf9b60e9 Minchan Kim     2021-04-29  6256  	}
a1394bddf9b60e9 Minchan Kim     2021-04-29  6257  }
a1394bddf9b60e9 Minchan Kim     2021-04-29  6258  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v1 09/12] dyndbg: add trace destination field to _ddebug

[Łukasz Bartosik]

Add trace destination field (trace_dst) to the _ddebug structure.
The trace destination field is used to determine output of debug
logs when +T is set. Setting trace_dst value to 0 (default) enables
output to prdbg and devdbg trace events. Setting trace_dst value to
a value in range of [1..255] enables output to trace instance.

Signed-off-by: Łukasz Bartosik <lb@semihalf.com>
---
 include/linux/dynamic_debug.h | 13 +++++++++++--
 lib/dynamic_debug.c           | 28 +++++++++++++++++++---------
 2 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 684766289bfc..3084302876b4 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -60,9 +60,18 @@ struct _ddebug {
 #else
 #define _DPRINTK_FLAGS_DEFAULT 0
 #endif
-	struct {
+	struct dd_ctrl {
 		unsigned int flags:8;
-		unsigned unused:24;
+	/*
+	 * The trace destination field is used to determine output of debug
+	 * logs when +T is set. Setting trace_dst value to 0 (default) enables
+	 * output to prdbg and devdbg trace events. Setting trace_dst value to
+	 * a value in range of [1..255] enables output to trace instance.
+	 */
+#define TRACE_DST_BITS 8
+		unsigned int trace_dst:TRACE_DST_BITS;
+#define TRACE_DST_MAX	((1 << TRACE_DST_BITS) - 1)
+		unsigned unused:16;
 	} ctrl;
 } __attribute__((aligned(8)));
 
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index ca87adf327df..3218ab078a76 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -80,14 +80,24 @@ module_param(verbose, int, 0644);
 MODULE_PARM_DESC(verbose, " dynamic_debug/control processing "
 		 "( 0 = off (default), 1 = module add/rm, 2 = >control summary, 3 = parsing, 4 = per-site changes)");
 
+static inline struct dd_ctrl *get_ctrl(struct _ddebug *desc)
+{
+	return &desc->ctrl;
+}
+
+static inline void set_ctrl(struct _ddebug *desc, struct dd_ctrl *ctrl)
+{
+	desc->ctrl = *ctrl;
+}
+
 static inline unsigned int get_flags(const struct _ddebug *desc)
 {
 	return desc->ctrl.flags;
 }
 
-static inline void set_flags(struct _ddebug *desc, unsigned int val)
+static inline unsigned int get_trace_dst(const struct _ddebug *desc)
 {
-	desc->ctrl.flags = val;
+	return desc->ctrl.trace_dst;
 }
 
 /* Return the path relative to source root */
@@ -190,8 +200,8 @@ static int ddebug_change(const struct ddebug_query *query,
 {
 	int i;
 	struct ddebug_table *dt;
-	unsigned int newflags;
 	unsigned int nfound = 0;
+	struct dd_ctrl nctrl = {0};
 	struct flagsbuf fbuf, nbuf;
 	struct ddebug_class_map *map = NULL;
 	int __outvar valid_class;
@@ -257,14 +267,14 @@ static int ddebug_change(const struct ddebug_query *query,
 
 			nfound++;
 
-			newflags = (get_flags(dp) & modifiers->mask) | modifiers->flags;
-			if (newflags == get_flags(dp))
+			nctrl.flags = (get_flags(dp) & modifiers->mask) | modifiers->flags;
+			if (!memcmp(&nctrl, get_ctrl(dp), sizeof(struct dd_ctrl)))
 				continue;
 #ifdef CONFIG_JUMP_LABEL
 			if (get_flags(dp) & _DPRINTK_FLAGS_ENABLED) {
-				if (!(newflags & _DPRINTK_FLAGS_ENABLED))
+				if (!(nctrl.flags & _DPRINTK_FLAGS_ENABLED))
 					static_branch_disable(&dp->key.dd_key_true);
-			} else if (newflags & _DPRINTK_FLAGS_ENABLED) {
+			} else if (nctrl.flags & _DPRINTK_FLAGS_ENABLED) {
 				static_branch_enable(&dp->key.dd_key_true);
 			}
 #endif
@@ -272,8 +282,8 @@ static int ddebug_change(const struct ddebug_query *query,
 				  trim_prefix(dp->filename), dp->lineno,
 				  dt->mod_name, dp->function,
 				  ddebug_describe_flags(get_flags(dp), &fbuf),
-				  ddebug_describe_flags(newflags, &nbuf));
-			set_flags(dp, newflags);
+				  ddebug_describe_flags(nctrl.flags, &nbuf));
+			set_ctrl(dp, &nctrl);
 		}
 	}
 	mutex_unlock(&ddebug_lock);
-- 
2.42.0.869.gea05f2083d-goog

Re: [PATCH v1 09/12] dyndbg: add trace destination field to _ddebug

[jim.cromie]

On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
>
> Add trace destination field (trace_dst) to the _ddebug structure.
> The trace destination field is used to determine output of debug
> logs when +T is set. Setting trace_dst value to 0 (default) enables
> output to prdbg and devdbg trace events. Setting trace_dst value to
> a value in range of [1..255] enables output to trace instance.


should we do some expectation setting here ?
255 is something of a promise to more than tom,dick,harry.
16-64 is more suggestive of a limited resource,
might encourage more judicious use.

will look further later

Re: [PATCH v1 09/12] dyndbg: add trace destination field to _ddebug

[Łukasz Bartosik]

sob., 4 lis 2023 o 02:39 <jim.cromie@gmail.com> napisał(a):
>
> On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> >
> > Add trace destination field (trace_dst) to the _ddebug structure.
> > The trace destination field is used to determine output of debug
> > logs when +T is set. Setting trace_dst value to 0 (default) enables
> > output to prdbg and devdbg trace events. Setting trace_dst value to
> > a value in range of [1..255] enables output to trace instance.
>
>
> should we do some expectation setting here ?
> 255 is something of a promise to more than tom,dick,harry.
> 16-64 is more suggestive of a limited resource,
> might encourage more judicious use.
>

How about making it configurable in kernel Kconfig with default value
set to 16 or 32 ?

> will look further later

Re: [PATCH v1 09/12] dyndbg: add trace destination field to _ddebug

[jim.cromie]

On Fri, Nov 10, 2023 at 7:52 AM Łukasz Bartosik <lb@semihalf.com> wrote:
>
> sob., 4 lis 2023 o 02:39 <jim.cromie@gmail.com> napisał(a):
> >
> > On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > >
> > > Add trace destination field (trace_dst) to the _ddebug structure.
> > > The trace destination field is used to determine output of debug
> > > logs when +T is set. Setting trace_dst value to 0 (default) enables
> > > output to prdbg and devdbg trace events. Setting trace_dst value to
> > > a value in range of [1..255] enables output to trace instance.
> >
> >
> > should we do some expectation setting here ?
> > 255 is something of a promise to more than tom,dick,harry.
> > 16-64 is more suggestive of a limited resource,
> > might encourage more judicious use.
> >
>
> How about making it configurable in kernel Kconfig with default value
> set to 16 or 32 ?
>

given the general dislike of extra knobs, it's not the battle I would choose.
ISTM we could start small, add bits later (if needed)
maybe 16 is too parsimonious; esp if DRM wants multiple instances per
driver (device?)



> > will look further later

Re: [PATCH v1 09/12] dyndbg: add trace destination field to _ddebug

[Łukasz Bartosik]

pt., 10 lis 2023 o 20:37 <jim.cromie@gmail.com> napisał(a):
>
> On Fri, Nov 10, 2023 at 7:52 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> >
> > sob., 4 lis 2023 o 02:39 <jim.cromie@gmail.com> napisał(a):
> > >
> > > On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > > >
> > > > Add trace destination field (trace_dst) to the _ddebug structure.
> > > > The trace destination field is used to determine output of debug
> > > > logs when +T is set. Setting trace_dst value to 0 (default) enables
> > > > output to prdbg and devdbg trace events. Setting trace_dst value to
> > > > a value in range of [1..255] enables output to trace instance.
> > >
> > >
> > > should we do some expectation setting here ?
> > > 255 is something of a promise to more than tom,dick,harry.
> > > 16-64 is more suggestive of a limited resource,
> > > might encourage more judicious use.
> > >
> >
> > How about making it configurable in kernel Kconfig with default value
> > set to 16 or 32 ?
> >
>
> given the general dislike of extra knobs, it's not the battle I would choose.
> ISTM we could start small, add bits later (if needed)
> maybe 16 is too parsimonious; esp if DRM wants multiple instances per
> driver (device?)
>

If we don't want the extra config knob then I would opt for 64.

>
>
> > > will look further later

Re: [PATCH v1 09/12] dyndbg: add trace destination field to _ddebug

[jim.cromie]

On Sun, Nov 12, 2023 at 9:29 AM Łukasz Bartosik <lb@semihalf.com> wrote:
>
> pt., 10 lis 2023 o 20:37 <jim.cromie@gmail.com> napisał(a):
> >
> > On Fri, Nov 10, 2023 at 7:52 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > >
> > > sob., 4 lis 2023 o 02:39 <jim.cromie@gmail.com> napisał(a):
> > > >
> > > > On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > > > >
> > > > > Add trace destination field (trace_dst) to the _ddebug structure.
> > > > > The trace destination field is used to determine output of debug
> > > > > logs when +T is set. Setting trace_dst value to 0 (default) enables
> > > > > output to prdbg and devdbg trace events. Setting trace_dst value to
> > > > > a value in range of [1..255] enables output to trace instance.
> > > >
> > > >
> > > > should we do some expectation setting here ?
> > > > 255 is something of a promise to more than tom,dick,harry.
> > > > 16-64 is more suggestive of a limited resource,
> > > > might encourage more judicious use.
> > > >
> > >
> > > How about making it configurable in kernel Kconfig with default value
> > > set to 16 or 32 ?
> > >
> >
> > given the general dislike of extra knobs, it's not the battle I would choose.
> > ISTM we could start small, add bits later (if needed)
> > maybe 16 is too parsimonious; esp if DRM wants multiple instances per
> > driver (device?)
> >
>
> If we don't want the extra config knob then I would opt for 64.

I think I could live with that.
And I dont foresee other uses for spare bits atm.

[PATCH v1 10/12] dyndbg: add processing of T(race) flag argument

[Łukasz Bartosik]

Add processing of argument provided to T(race) flag.
The argument value determines destination of debug logs:

0 - debug logs will be written to prdbg and devdbg trace events
[1..255] - debug logs will be written to trace instance

A user can provide trace destination by folowing T flag with
":" and trace destination value in range [0..255], for example:

echo "module thunderbolt =pT:7" > /sys/kernel/debug/dynamic_debug/control
echo "module thunderbolt =lT:7,p" > /sys/kernel/debug/dynamic_debug/control

When T flag with argument is followed by other flags then the next flag has
to be preceded with ",".

When no value is provided trace destination defaults to 0, for example:

echo "module thunderbolt =T" > /sys/kernel/debug/dynamic_debug/control
echo "module thunderbolt =lTp" > /sys/kernel/debug/dynamic_debug/control

Signed-off-by: Łukasz Bartosik <lb@semihalf.com>
---
 lib/dynamic_debug.c | 105 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 90 insertions(+), 15 deletions(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 3218ab078a76..c5cd28e74a02 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -71,6 +71,7 @@ struct ddebug_iter {
 struct flag_settings {
 	unsigned int flags;
 	unsigned int mask;
+	unsigned int trace_dst;
 };
 
 static DEFINE_MUTEX(ddebug_lock);
@@ -111,9 +112,67 @@ static inline const char *trim_prefix(const char *path)
 	return path + skip;
 }
 
-static const struct { unsigned flag:8; char opt_char; } opt_array[] = {
+typedef const char* (*read_flag_args_f)(const char *, struct flag_settings *);
+typedef char* (*show_flag_args_f)(struct dd_ctrl *, char *);
+
+/*
+ * Maximum number of characters representing value
+ * of flag T argument in human readable form - ":255,"
+ */
+#define FLAG_T_ARGS_LEN 5
+
+static const
+char *read_T_args(const char *str, struct flag_settings *modifiers)
+{
+	char *end, args[FLAG_T_ARGS_LEN];
+	int len;
+
+	if (*(str+1) != ':')
+		return str;
+
+	str += 2;
+	end = strchr(str, ',');
+	if (end && *(end + 1) == '\0')
+		return NULL;
+
+	if (end)
+		len = end - str;
+	else
+		len = strlen(str);
+
+	if (len > FLAG_T_ARGS_LEN - 1)
+		return NULL;
+
+	memcpy(args, str, len);
+	args[len] = '\0';
+	if (kstrtouint(args, 10, &modifiers->trace_dst) < 0)
+		return NULL;
+
+	if (modifiers->trace_dst > TRACE_DST_MAX)
+		return NULL;
+
+	return end ? end : str + len;
+}
+
+char *show_T_args(struct dd_ctrl *ctrl, char *p)
+{
+	int n;
+
+	n = snprintf(p, FLAG_T_ARGS_LEN, ":%u", ctrl->trace_dst);
+	WARN_ONCE(n < 0, "printing T flag args value failed\n");
+
+	return n < 0 ? p : p + n;
+}
+
+static const struct
+{
+	unsigned flag:8;
+	char opt_char;
+	read_flag_args_f read_args;
+	show_flag_args_f show_args;
+} opt_array[] = {
 	{ _DPRINTK_FLAGS_PRINTK, 'p' },
-	{ _DPRINTK_FLAGS_TRACE, 'T' },
+	{ _DPRINTK_FLAGS_TRACE, 'T', read_T_args, show_T_args},
 	{ _DPRINTK_FLAGS_INCL_MODNAME, 'm' },
 	{ _DPRINTK_FLAGS_INCL_FUNCNAME, 'f' },
 	{ _DPRINTK_FLAGS_INCL_SOURCENAME, 's' },
@@ -122,22 +181,30 @@ static const struct { unsigned flag:8; char opt_char; } opt_array[] = {
 	{ _DPRINTK_FLAGS_NONE, '_' },
 };
 
-struct flagsbuf { char buf[ARRAY_SIZE(opt_array)+1]; };
+struct ctrlbuf { char buf[ARRAY_SIZE(opt_array)+FLAG_T_ARGS_LEN+1]; };
 
 /* format a string into buf[] which describes the _ddebug's flags */
-static char *ddebug_describe_flags(unsigned int flags, struct flagsbuf *fb)
+static char *ddebug_describe_ctrl(struct dd_ctrl *ctrl, struct ctrlbuf *cb)
 {
-	char *p = fb->buf;
+	show_flag_args_f show_args = NULL;
+	char *p = cb->buf;
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(opt_array); ++i)
-		if (flags & opt_array[i].flag)
+		if (ctrl->flags & opt_array[i].flag) {
+			if (show_args)
+				*p++ = ',';
 			*p++ = opt_array[i].opt_char;
-	if (p == fb->buf)
+			show_args = opt_array[i].show_args;
+			if (show_args)
+				p = show_args(ctrl, p);
+		}
+
+	if (p == cb->buf)
 		*p++ = '_';
 	*p = '\0';
 
-	return fb->buf;
+	return cb->buf;
 }
 
 #define vnpr_info(lvl, fmt, ...)				\
@@ -202,7 +269,7 @@ static int ddebug_change(const struct ddebug_query *query,
 	struct ddebug_table *dt;
 	unsigned int nfound = 0;
 	struct dd_ctrl nctrl = {0};
-	struct flagsbuf fbuf, nbuf;
+	struct ctrlbuf cbuf, nbuf;
 	struct ddebug_class_map *map = NULL;
 	int __outvar valid_class;
 
@@ -268,7 +335,8 @@ static int ddebug_change(const struct ddebug_query *query,
 			nfound++;
 
 			nctrl.flags = (get_flags(dp) & modifiers->mask) | modifiers->flags;
-			if (!memcmp(&nctrl, get_ctrl(dp), sizeof(struct dd_ctrl)))
+			nctrl.trace_dst = modifiers->trace_dst;
+			if (!memcmp(&nctrl, get_ctrl(dp), sizeof(nctrl)))
 				continue;
 #ifdef CONFIG_JUMP_LABEL
 			if (get_flags(dp) & _DPRINTK_FLAGS_ENABLED) {
@@ -281,8 +349,8 @@ static int ddebug_change(const struct ddebug_query *query,
 			v4pr_info("changed %s:%d [%s]%s %s => %s\n",
 				  trim_prefix(dp->filename), dp->lineno,
 				  dt->mod_name, dp->function,
-				  ddebug_describe_flags(get_flags(dp), &fbuf),
-				  ddebug_describe_flags(nctrl.flags, &nbuf));
+				  ddebug_describe_ctrl(&dp->ctrl, &cbuf),
+				  ddebug_describe_ctrl(&nctrl, &nbuf));
 			set_ctrl(dp, &nctrl);
 		}
 	}
@@ -507,6 +575,7 @@ static int ddebug_parse_query(char *words[], int nwords,
  */
 static int ddebug_parse_flags(const char *str, struct flag_settings *modifiers)
 {
+	read_flag_args_f read_args;
 	int op, i;
 
 	switch (*str) {
@@ -525,6 +594,12 @@ static int ddebug_parse_flags(const char *str, struct flag_settings *modifiers)
 		for (i = ARRAY_SIZE(opt_array) - 1; i >= 0; i--) {
 			if (*str == opt_array[i].opt_char) {
 				modifiers->flags |= opt_array[i].flag;
+				read_args = opt_array[i].read_args;
+				if (read_args) {
+					str = read_args(str, modifiers);
+					if (!str)
+						return -EINVAL;
+				}
 				break;
 			}
 		}
@@ -533,7 +608,7 @@ static int ddebug_parse_flags(const char *str, struct flag_settings *modifiers)
 			return -EINVAL;
 		}
 	}
-	v3pr_info("flags=0x%x\n", modifiers->flags);
+	v3pr_info("flags=0x%x, trace dest=0x%x\n", modifiers->flags, modifiers->trace_dst);
 
 	/* calculate final flags, mask based upon op */
 	switch (op) {
@@ -1257,7 +1332,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
 {
 	struct ddebug_iter *iter = m->private;
 	struct _ddebug *dp = p;
-	struct flagsbuf flags;
+	struct ctrlbuf cbuf;
 	char const *class;
 
 	if (p == SEQ_START_TOKEN) {
@@ -1269,7 +1344,7 @@ static int ddebug_proc_show(struct seq_file *m, void *p)
 	seq_printf(m, "%s:%u [%s]%s =%s \"",
 		   trim_prefix(dp->filename), dp->lineno,
 		   iter->table->mod_name, dp->function,
-		   ddebug_describe_flags(get_flags(dp), &flags));
+		   ddebug_describe_ctrl(&dp->ctrl, &cbuf));
 	seq_escape_str(m, dp->format, ESCAPE_SPACE, "\t\r\n\"");
 	seq_puts(m, "\"");
 
-- 
2.42.0.869.gea05f2083d-goog

Re: [PATCH v1 10/12] dyndbg: add processing of T(race) flag argument

[kernel test robot]

Hi Łukasz,

kernel test robot noticed the following build warnings:

[auto build test WARNING on akpm-mm/mm-everything]
[also build test WARNING on linus/master v6.6 next-20231103]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/ukasz-Bartosik/dyndbg-add-_DPRINTK_FLAGS_ENABLED/20231103-212105
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20231103131011.1316396-11-lb%40semihalf.com
patch subject: [PATCH v1 10/12] dyndbg: add processing of T(race) flag argument
config: loongarch-randconfig-002-20231103 (https://download.01.org/0day-ci/archive/20231104/202311040120.oiJ1m9Pw-lkp@intel.com/config)
compiler: loongarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231104/202311040120.oiJ1m9Pw-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311040120.oiJ1m9Pw-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> lib/dynamic_debug.c:157:7: warning: no previous prototype for 'show_T_args' [-Wmissing-prototypes]
     157 | char *show_T_args(struct dd_ctrl *ctrl, char *p)
         |       ^~~~~~~~~~~


vim +/show_T_args +157 lib/dynamic_debug.c

   156	
 > 157	char *show_T_args(struct dd_ctrl *ctrl, char *p)
   158	{
   159		int n;
   160	
   161		n = snprintf(p, FLAG_T_ARGS_LEN, ":%u", ctrl->trace_dst);
   162		WARN_ONCE(n < 0, "printing T flag args value failed\n");
   163	
   164		return n < 0 ? p : p + n;
   165	}
   166	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v1 10/12] dyndbg: add processing of T(race) flag argument

[jim.cromie]

On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
>
> Add processing of argument provided to T(race) flag.
> The argument value determines destination of debug logs:
>
> 0 - debug logs will be written to prdbg and devdbg trace events
> [1..255] - debug logs will be written to trace instance
>
> A user can provide trace destination by folowing T flag with
> ":" and trace destination value in range [0..255], for example:
>
> echo "module thunderbolt =pT:7" > /sys/kernel/debug/dynamic_debug/control
> echo "module thunderbolt =lT:7,p" > /sys/kernel/debug/dynamic_debug/control
>
> When T flag with argument is followed by other flags then the next flag has
> to be preceded with ",".
>

the trailing , seems punctuation heavy.
Could we just stipulate that any :string  (leading : trailing anything)
be the last flag in the spec ?
bare T flags are not constrained otherwise.
seems fine as API-spec-by-error-codes.




> When no value is provided trace destination defaults to 0, for example:
>
> echo "module thunderbolt =T" > /sys/kernel/debug/dynamic_debug/control
> echo "module thunderbolt =lTp" > /sys/kernel/debug/dynamic_debug/control

no colon after T means p is a flag, not a destination name

Re: [PATCH v1 10/12] dyndbg: add processing of T(race) flag argument

[Łukasz Bartosik]

sob., 4 lis 2023 o 04:06 <jim.cromie@gmail.com> napisał(a):
>
> On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> >
> > Add processing of argument provided to T(race) flag.
> > The argument value determines destination of debug logs:
> >
> > 0 - debug logs will be written to prdbg and devdbg trace events
> > [1..255] - debug logs will be written to trace instance
> >
> > A user can provide trace destination by folowing T flag with
> > ":" and trace destination value in range [0..255], for example:
> >
> > echo "module thunderbolt =pT:7" > /sys/kernel/debug/dynamic_debug/control
> > echo "module thunderbolt =lT:7,p" > /sys/kernel/debug/dynamic_debug/control
> >
> > When T flag with argument is followed by other flags then the next flag has
> > to be preceded with ",".
> >
>
> the trailing , seems punctuation heavy.
> Could we just stipulate that any :string  (leading : trailing anything)
> be the last flag in the spec ?
> bare T flags are not constrained otherwise.
> seems fine as API-spec-by-error-codes.
>

I followed Jason's suggestion to use "," when T flag is not the last
flag and destination is explicitly provided for the T flag, like in
the example above
"echo "module thunderbolt =lT:7,p" > /sys/kernel/debug/dynamic_debug/control".

With "," we can have the following cases:
- when T is the last flag then it doesn't need to be followed by ","
even if destination is explicitly provided, for example "lpT:7",
- when T is not the last flag and destination is explicitly provided
then "," has to be used before next flag, for example "lT:7,p",
- when T is not the last flag and destination is not explicitly
provided then "," is not required, for example "lTp",

Jim, Jason, would you please come to terms if we want to use "," or
just assume that T has to be the last flag in the spec ?

>
>
>
> > When no value is provided trace destination defaults to 0, for example:
> >
> > echo "module thunderbolt =T" > /sys/kernel/debug/dynamic_debug/control
> > echo "module thunderbolt =lTp" > /sys/kernel/debug/dynamic_debug/control
>
> no colon after T means p is a flag, not a destination name

Yes, in this case p is a flag because when T is not followed
explicitly by destination then next character would be treated as
another flag.

Re: [PATCH v1 10/12] dyndbg: add processing of T(race) flag argument

[jim.cromie]

On Fri, Nov 10, 2023 at 7:52 AM Łukasz Bartosik <lb@semihalf.com> wrote:
>
> sob., 4 lis 2023 o 04:06 <jim.cromie@gmail.com> napisał(a):
> >
> > On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > >
> > > Add processing of argument provided to T(race) flag.
> > > The argument value determines destination of debug logs:
> > >
> > > 0 - debug logs will be written to prdbg and devdbg trace events
> > > [1..255] - debug logs will be written to trace instance
> > >
> > > A user can provide trace destination by folowing T flag with
> > > ":" and trace destination value in range [0..255], for example:
> > >
> > > echo "module thunderbolt =pT:7" > /sys/kernel/debug/dynamic_debug/control
> > > echo "module thunderbolt =lT:7,p" > /sys/kernel/debug/dynamic_debug/control
> > >
> > > When T flag with argument is followed by other flags then the next flag has
> > > to be preceded with ",".
> > >
> >
> > the trailing , seems punctuation heavy.
> > Could we just stipulate that any :string  (leading : trailing anything)
> > be the last flag in the spec ?
> > bare T flags are not constrained otherwise.
> > seems fine as API-spec-by-error-codes.
> >
>
> I followed Jason's suggestion to use "," when T flag is not the last
> flag and destination is explicitly provided for the T flag, like in
> the example above
> "echo "module thunderbolt =lT:7,p" > /sys/kernel/debug/dynamic_debug/control".
>
> With "," we can have the following cases:
> - when T is the last flag then it doesn't need to be followed by ","
> even if destination is explicitly provided, for example "lpT:7",
> - when T is not the last flag and destination is explicitly provided
> then "," has to be used before next flag, for example "lT:7,p",
> - when T is not the last flag and destination is not explicitly
> provided then "," is not required, for example "lTp",
>
> Jim, Jason, would you please come to terms if we want to use "," or
> just assume that T has to be the last flag in the spec ?
>

Im fine either way -   eliminating punctuation has a cost too,
it adds some order dependency which isnt there now.
If that complicates the code, no-good.


> >
> >
> >
> > > When no value is provided trace destination defaults to 0, for example:

That seems wrong now - it should default to whatever it was previously set to,

this allows setting a non-default dest while disabling the site:
   echo class DRM_UT_CORE -T:core-log  > /proc/dynamic_debug/control

then just enabling it later, to use the preset dest
   echo class DRM_UT_CORE +T  > /proc/dynamic_debug/control
or more likely:
   echo 0x01 > /sys/module/drm/parameters/debug_trace

this way, debug_trace is just like debug, but still can write to the
separate trace-instances

> > >
> > > echo "module thunderbolt =T" > /sys/kernel/debug/dynamic_debug/control
> > > echo "module thunderbolt =lTp" > /sys/kernel/debug/dynamic_debug/control
> >
> > no colon after T means p is a flag, not a destination name
>
> Yes, in this case p is a flag because when T is not followed
> explicitly by destination then next character would be treated as
> another flag.

Re: [PATCH v1 10/12] dyndbg: add processing of T(race) flag argument

[Łukasz Bartosik]

pt., 10 lis 2023 o 20:51 <jim.cromie@gmail.com> napisał(a):
>
> On Fri, Nov 10, 2023 at 7:52 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> >
> > sob., 4 lis 2023 o 04:06 <jim.cromie@gmail.com> napisał(a):
> > >
> > > On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > > >
> > > > Add processing of argument provided to T(race) flag.
> > > > The argument value determines destination of debug logs:
> > > >
> > > > 0 - debug logs will be written to prdbg and devdbg trace events
> > > > [1..255] - debug logs will be written to trace instance
> > > >
> > > > A user can provide trace destination by folowing T flag with
> > > > ":" and trace destination value in range [0..255], for example:
> > > >
> > > > echo "module thunderbolt =pT:7" > /sys/kernel/debug/dynamic_debug/control
> > > > echo "module thunderbolt =lT:7,p" > /sys/kernel/debug/dynamic_debug/control
> > > >
> > > > When T flag with argument is followed by other flags then the next flag has
> > > > to be preceded with ",".
> > > >
> > >
> > > the trailing , seems punctuation heavy.
> > > Could we just stipulate that any :string  (leading : trailing anything)
> > > be the last flag in the spec ?
> > > bare T flags are not constrained otherwise.
> > > seems fine as API-spec-by-error-codes.
> > >
> >
> > I followed Jason's suggestion to use "," when T flag is not the last
> > flag and destination is explicitly provided for the T flag, like in
> > the example above
> > "echo "module thunderbolt =lT:7,p" > /sys/kernel/debug/dynamic_debug/control".
> >
> > With "," we can have the following cases:
> > - when T is the last flag then it doesn't need to be followed by ","
> > even if destination is explicitly provided, for example "lpT:7",
> > - when T is not the last flag and destination is explicitly provided
> > then "," has to be used before next flag, for example "lT:7,p",
> > - when T is not the last flag and destination is not explicitly
> > provided then "," is not required, for example "lTp",
> >
> > Jim, Jason, would you please come to terms if we want to use "," or
> > just assume that T has to be the last flag in the spec ?
> >
>
> Im fine either way -   eliminating punctuation has a cost too,
> it adds some order dependency which isnt there now.
> If that complicates the code, no-good.
>

Ok, I will keep the option to use "," to separate T with explicitly
provided destination from a next flag.

>
> > >
> > >
> > >
> > > > When no value is provided trace destination defaults to 0, for example:
>
> That seems wrong now - it should default to whatever it was previously set to,
>

It was in my original proposal before you suggested to create
open/close commands.


> this allows setting a non-default dest while disabling the site:
>    echo class DRM_UT_CORE -T:core-log  > /proc/dynamic_debug/control
>
> then just enabling it later, to use the preset dest
>    echo class DRM_UT_CORE +T  > /proc/dynamic_debug/control
> or more likely:
>    echo 0x01 > /sys/module/drm/parameters/debug_trace
>
> this way, debug_trace is just like debug, but still can write to the
> separate trace-instances
>

Ack, I also clarified my suggestion related to this topic in patch 11.



> > > >
> > > > echo "module thunderbolt =T" > /sys/kernel/debug/dynamic_debug/control
> > > > echo "module thunderbolt =lTp" > /sys/kernel/debug/dynamic_debug/control
> > >
> > > no colon after T means p is a flag, not a destination name
> >
> > Yes, in this case p is a flag because when T is not followed
> > explicitly by destination then next character would be treated as
> > another flag.

Re: [PATCH v1 10/12] dyndbg: add processing of T(race) flag argument

[jim.cromie]

On Sun, Nov 12, 2023 at 9:29 AM Łukasz Bartosik <lb@semihalf.com> wrote:
>
> pt., 10 lis 2023 o 20:51 <jim.cromie@gmail.com> napisał(a):
> >
> > On Fri, Nov 10, 2023 at 7:52 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > >
> > > sob., 4 lis 2023 o 04:06 <jim.cromie@gmail.com> napisał(a):
> > > >
> > > > On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > > > >
> > > > > Add processing of argument provided to T(race) flag.
> > > > > The argument value determines destination of debug logs:
> > > > >
> > > > > 0 - debug logs will be written to prdbg and devdbg trace events
> > > > > [1..255] - debug logs will be written to trace instance
> > > > >
> > > > > A user can provide trace destination by folowing T flag with
> > > > > ":" and trace destination value in range [0..255], for example:
> > > > >
> > > > > echo "module thunderbolt =pT:7" > /sys/kernel/debug/dynamic_debug/control
> > > > > echo "module thunderbolt =lT:7,p" > /sys/kernel/debug/dynamic_debug/control
> > > > >
> > > > > When T flag with argument is followed by other flags then the next flag has
> > > > > to be preceded with ",".
> > > > >
> > > >
> > > > the trailing , seems punctuation heavy.
> > > > Could we just stipulate that any :string  (leading : trailing anything)
> > > > be the last flag in the spec ?
> > > > bare T flags are not constrained otherwise.
> > > > seems fine as API-spec-by-error-codes.
> > > >
> > >
> > > I followed Jason's suggestion to use "," when T flag is not the last
> > > flag and destination is explicitly provided for the T flag, like in
> > > the example above
> > > "echo "module thunderbolt =lT:7,p" > /sys/kernel/debug/dynamic_debug/control".
> > >
> > > With "," we can have the following cases:
> > > - when T is the last flag then it doesn't need to be followed by ","
> > > even if destination is explicitly provided, for example "lpT:7",
> > > - when T is not the last flag and destination is explicitly provided
> > > then "," has to be used before next flag, for example "lT:7,p",
> > > - when T is not the last flag and destination is not explicitly
> > > provided then "," is not required, for example "lTp",
> > >
> > > Jim, Jason, would you please come to terms if we want to use "," or
> > > just assume that T has to be the last flag in the spec ?
> > >
> >
> > Im fine either way -   eliminating punctuation has a cost too,
> > it adds some order dependency which isnt there now.
> > If that complicates the code, no-good.
> >
>
> Ok, I will keep the option to use "," to separate T with explicitly
> provided destination from a next flag.
>
> >
> > > >
> > > >
> > > >
> > > > > When no value is provided trace destination defaults to 0, for example:
> >
> > That seems wrong now - it should default to whatever it was previously set to,
> >
>
> It was in my original proposal before you suggested to create
> open/close commands.
>
>
> > this allows setting a non-default dest while disabling the site:
> >    echo class DRM_UT_CORE -T:core-log  > /proc/dynamic_debug/control
> >
> > then just enabling it later, to use the preset dest
> >    echo class DRM_UT_CORE +T  > /proc/dynamic_debug/control
> > or more likely:
> >    echo 0x01 > /sys/module/drm/parameters/debug_trace
> >
> > this way, debug_trace is just like debug, but still can write to the
> > separate trace-instances
> >
>
> Ack, I also clarified my suggestion related to this topic in patch 11.
>

to reiterate and add context:

echo 0x01 > /sys/module/drm/parameters/debug
is the legacy way of enabling DRM_UT_CORE logging.
in CONFIG_DRM_USE_DYNAMIC_DEBUG=y builds,
this uses classmaps, but controls only the +p flag.

echo 0x01 > /sys/module/drm/parameters/debug_trace
doesnt exist yet, but is simple to add with the classmap impl.

this legacy interface cannot handle private trace-instances.
only >control  can do that.
ISTM thats fine.

Re: [PATCH v1 10/12] dyndbg: add processing of T(race) flag argument

[kernel test robot]

Hi Łukasz,

kernel test robot noticed the following build warnings:

[auto build test WARNING on akpm-mm/mm-everything]
[also build test WARNING on linus/master v6.6 next-20231103]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/ukasz-Bartosik/dyndbg-add-_DPRINTK_FLAGS_ENABLED/20231103-212105
base:   https://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm.git mm-everything
patch link:    https://lore.kernel.org/r/20231103131011.1316396-11-lb%40semihalf.com
patch subject: [PATCH v1 10/12] dyndbg: add processing of T(race) flag argument
config: i386-randconfig-063-20231104 (https://download.01.org/0day-ci/archive/20231104/202311041243.SboyHnXN-lkp@intel.com/config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231104/202311041243.SboyHnXN-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311041243.SboyHnXN-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> lib/dynamic_debug.c:157:6: sparse: sparse: symbol 'show_T_args' was not declared. Should it be static?

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH v1 11/12] dyndbg: write debug logs to trace instance

[Łukasz Bartosik]

When trace is enabled (T flag is set) and trace_dst field is set
to value greater than 0 (0 is reserved for trace events) then
debug logs will be written to trace instance pointed by trace_dst
value, for example when trace_dst value is 2 then debug logs will
be written to <debugfs>/tracing/instances/dyndbg_inst_2 instance.
Given trace instance will not be initialized until debug logs are
requested to be written to it and afer init will persist until
reboot.

Signed-off-by: Łukasz Bartosik <lb@semihalf.com>
---
 lib/Kconfig.debug   |  1 +
 lib/dynamic_debug.c | 79 ++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 76 insertions(+), 4 deletions(-)

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index fa307f93fa2e..9617e92c046d 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -181,6 +181,7 @@ config DYNAMIC_DEBUG_CORE
 	bool "Enable core function of dynamic debug support"
 	depends on PRINTK
 	depends on (DEBUG_FS || PROC_FS)
+	depends on TRACING
 	help
 	  Enable core functional support of dynamic debug. It is useful
 	  when you want to tie dynamic debug to your kernel modules with
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index c5cd28e74a02..541d9d522b3b 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>
 
 #define CREATE_TRACE_POINTS
 #include <trace/events/dyndbg.h>
@@ -81,6 +82,18 @@ module_param(verbose, int, 0644);
 MODULE_PARM_DESC(verbose, " dynamic_debug/control processing "
 		 "( 0 = off (default), 1 = module add/rm, 2 = >control summary, 3 = parsing, 4 = per-site changes)");
 
+/*
+ * When trace is enabled (T flag is set) and trace_dst field is set
+ * to value greater than 0 (0 is reserved for trace events) then
+ * debug logs will be written to trace instance pointed by trace_dst
+ * value, for example when trace_dst value is 2 then debug logs will
+ * be written to <debugfs>/tracing/instances/dyndbg_inst_2 instance.
+ * Given trace instance will not be initialized until debug logs are
+ * requested to be written to it and afer init will persist until
+ * reboot.
+ */
+static struct trace_array *trace_arr[TRACE_DST_MAX];
+
 static inline struct dd_ctrl *get_ctrl(struct _ddebug *desc)
 {
 	return &desc->ctrl;
@@ -255,6 +268,45 @@ static struct ddebug_class_map *ddebug_find_valid_class(struct ddebug_table cons
 	return NULL;
 }
 
+static int handle_trace_dst(struct dd_ctrl *ctrl)
+{
+#define TRACE_INST_NAME_LEN 16
+	char instance_name[TRACE_INST_NAME_LEN];
+	struct trace_array *arr;
+	int ret = -EINVAL;
+
+	/* check if trace (T flag) is enabled */
+	if (!(ctrl->flags & _DPRINTK_FLAGS_TRACE))
+		return 0;
+
+	/* check if trace destination are trace events */
+	if (!ctrl->trace_dst)
+		return 0;
+
+	/* check if trace instance is already set up */
+	if (trace_arr[ctrl->trace_dst])
+		return 0;
+
+	snprintf(instance_name, TRACE_INST_NAME_LEN,
+		 "dyndbg_inst_%u", ctrl->trace_dst);
+	arr = trace_array_get_by_name(instance_name);
+	if (!arr)
+		goto err;
+
+	ret = trace_array_init_printk(arr);
+	if (ret)
+		goto err_init;
+
+	trace_arr[ctrl->trace_dst] = arr;
+	return 0;
+
+err_init:
+	trace_array_put(arr);
+	trace_array_destroy(arr);
+err:
+	return ret;
+}
+
 #define __outvar /* filled by callee */
 /*
  * Search the tables for _ddebug's which match the given `query' and
@@ -338,6 +390,9 @@ static int ddebug_change(const struct ddebug_query *query,
 			nctrl.trace_dst = modifiers->trace_dst;
 			if (!memcmp(&nctrl, get_ctrl(dp), sizeof(nctrl)))
 				continue;
+
+			if (handle_trace_dst(&nctrl))
+				continue;
 #ifdef CONFIG_JUMP_LABEL
 			if (get_flags(dp) & _DPRINTK_FLAGS_ENABLED) {
 				if (!(nctrl.flags & _DPRINTK_FLAGS_ENABLED))
@@ -977,8 +1032,8 @@ static DEFINE_PER_CPU(struct ddebug_trace_bufs, ddebug_trace_bufs);
 static DEFINE_PER_CPU(int, ddebug_trace_reserve);
 
 __printf(3, 0)
-static void ddebug_trace(struct _ddebug *desc, const struct device *dev,
-			 const char *fmt, va_list args)
+static void ddebug_trace_event(struct _ddebug *desc, const struct device *dev,
+			       const char *fmt, va_list args)
 {
 	struct ddebug_trace_buf *buf;
 	int bufidx;
@@ -1010,6 +1065,15 @@ static void ddebug_trace(struct _ddebug *desc, const struct device *dev,
 	preempt_enable_notrace();
 }
 
+__printf(2, 0)
+static void ddebug_trace_instance(struct _ddebug *desc, const char *fmt,
+				  va_list *args)
+{
+	struct va_format vaf = { .fmt = fmt, .va = args};
+
+	trace_array_printk(trace_arr[get_trace_dst(desc)], _THIS_IP_, "%pV", &vaf);
+}
+
 __printf(2, 3)
 static void ddebug_printk(struct _ddebug *desc, const char *fmt, ...)
 {
@@ -1022,7 +1086,11 @@ static void ddebug_printk(struct _ddebug *desc, const char *fmt, ...)
 		 * All callers include the KERN_DEBUG prefix to keep the
 		 * vprintk case simple; strip it out for tracing.
 		 */
-		ddebug_trace(desc, NULL, fmt + strlen(KERN_DEBUG), args);
+		if (get_trace_dst(desc))
+			ddebug_trace_instance(desc, fmt, &args);
+		else
+			ddebug_trace_event(desc, NULL,
+					   fmt + strlen(KERN_DEBUG), args);
 		va_end(args);
 	}
 
@@ -1044,7 +1112,10 @@ static void ddebug_dev_printk(struct _ddebug *desc, const struct device *dev,
 		va_list args;
 
 		va_start(args, fmt);
-		ddebug_trace(desc, dev, fmt, args);
+		if (get_trace_dst(desc))
+			ddebug_trace_instance(desc, fmt, &args);
+		else
+			ddebug_trace_event(desc, dev, fmt, args);
 		va_end(args);
 	}
 
-- 
2.42.0.869.gea05f2083d-goog

Re: [PATCH v1 11/12] dyndbg: write debug logs to trace instance

[jim.cromie]

On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
>
> When trace is enabled (T flag is set) and trace_dst field is set
> to value greater than 0 (0 is reserved for trace events) then
> debug logs will be written to trace instance pointed by trace_dst
> value, for example when trace_dst value is 2 then debug logs will
> be written to <debugfs>/tracing/instances/dyndbg_inst_2 instance.
> Given trace instance will not be initialized until debug logs are
> requested to be written to it and afer init will persist until
> reboot.
>

restating 00 comments -

you can get rid of integer destination ids by adding a new command: open/close

$> echo  \
 open kms-instance \;\
 class DRM_UT_KMS -T:kms-instance  # preset-dests-disable-sites \;\
> /proc/dynamic_debug/control

echo "class DRM_UT_KMS +T # enable sites, with the preselected data" >
/proc/dynamic_debug/control

open - assign name to id, reserve it, hide it from user
+T:valid-name # fail command if name wasnt opened

and +T  w/o dest means use existing setting, not just 0 (unless thats
the existing setting)

> Signed-off-by: Łukasz Bartosik <lb@semihalf.com>
> ---
>  lib/Kconfig.debug   |  1 +
>  lib/dynamic_debug.c | 79 ++++++++++++++++++++++++++++++++++++++++++---
>  2 files changed, 76 insertions(+), 4 deletions(-)
>
> diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> index fa307f93fa2e..9617e92c046d 100644
> --- a/lib/Kconfig.debug
> +++ b/lib/Kconfig.debug
> @@ -181,6 +181,7 @@ config DYNAMIC_DEBUG_CORE
>         bool "Enable core function of dynamic debug support"
>         depends on PRINTK
>         depends on (DEBUG_FS || PROC_FS)
> +       depends on TRACING
>         help
>           Enable core functional support of dynamic debug. It is useful
>           when you want to tie dynamic debug to your kernel modules with
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index c5cd28e74a02..541d9d522b3b 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>
>
>  #define CREATE_TRACE_POINTS
>  #include <trace/events/dyndbg.h>
> @@ -81,6 +82,18 @@ module_param(verbose, int, 0644);
>  MODULE_PARM_DESC(verbose, " dynamic_debug/control processing "
>                  "( 0 = off (default), 1 = module add/rm, 2 = >control summary, 3 = parsing, 4 = per-site changes)");
>
> +/*
> + * When trace is enabled (T flag is set) and trace_dst field is set
> + * to value greater than 0 (0 is reserved for trace events) then
> + * debug logs will be written to trace instance pointed by trace_dst
> + * value, for example when trace_dst value is 2 then debug logs will
> + * be written to <debugfs>/tracing/instances/dyndbg_inst_2 instance.
> + * Given trace instance will not be initialized until debug logs are
> + * requested to be written to it and afer init will persist until
> + * reboot.
> + */
> +static struct trace_array *trace_arr[TRACE_DST_MAX];
> +
>  static inline struct dd_ctrl *get_ctrl(struct _ddebug *desc)
>  {
>         return &desc->ctrl;
> @@ -255,6 +268,45 @@ static struct ddebug_class_map *ddebug_find_valid_class(struct ddebug_table cons
>         return NULL;
>  }
>
> +static int handle_trace_dst(struct dd_ctrl *ctrl)
> +{
> +#define TRACE_INST_NAME_LEN 16
> +       char instance_name[TRACE_INST_NAME_LEN];
> +       struct trace_array *arr;
> +       int ret = -EINVAL;
> +
> +       /* check if trace (T flag) is enabled */
> +       if (!(ctrl->flags & _DPRINTK_FLAGS_TRACE))
> +               return 0;
> +
> +       /* check if trace destination are trace events */
> +       if (!ctrl->trace_dst)
> +               return 0;
> +
> +       /* check if trace instance is already set up */
> +       if (trace_arr[ctrl->trace_dst])
> +               return 0;
> +
> +       snprintf(instance_name, TRACE_INST_NAME_LEN,
> +                "dyndbg_inst_%u", ctrl->trace_dst);
> +       arr = trace_array_get_by_name(instance_name);
> +       if (!arr)
> +               goto err;
> +
> +       ret = trace_array_init_printk(arr);
> +       if (ret)
> +               goto err_init;
> +
> +       trace_arr[ctrl->trace_dst] = arr;
> +       return 0;
> +
> +err_init:
> +       trace_array_put(arr);
> +       trace_array_destroy(arr);
> +err:
> +       return ret;
> +}
> +
>  #define __outvar /* filled by callee */
>  /*
>   * Search the tables for _ddebug's which match the given `query' and
> @@ -338,6 +390,9 @@ static int ddebug_change(const struct ddebug_query *query,
>                         nctrl.trace_dst = modifiers->trace_dst;
>                         if (!memcmp(&nctrl, get_ctrl(dp), sizeof(nctrl)))
>                                 continue;
> +
> +                       if (handle_trace_dst(&nctrl))
> +                               continue;
>  #ifdef CONFIG_JUMP_LABEL
>                         if (get_flags(dp) & _DPRINTK_FLAGS_ENABLED) {
>                                 if (!(nctrl.flags & _DPRINTK_FLAGS_ENABLED))
> @@ -977,8 +1032,8 @@ static DEFINE_PER_CPU(struct ddebug_trace_bufs, ddebug_trace_bufs);
>  static DEFINE_PER_CPU(int, ddebug_trace_reserve);
>
>  __printf(3, 0)
> -static void ddebug_trace(struct _ddebug *desc, const struct device *dev,
> -                        const char *fmt, va_list args)
> +static void ddebug_trace_event(struct _ddebug *desc, const struct device *dev,
> +                              const char *fmt, va_list args)
>  {
>         struct ddebug_trace_buf *buf;
>         int bufidx;
> @@ -1010,6 +1065,15 @@ static void ddebug_trace(struct _ddebug *desc, const struct device *dev,
>         preempt_enable_notrace();
>  }
>
> +__printf(2, 0)
> +static void ddebug_trace_instance(struct _ddebug *desc, const char *fmt,
> +                                 va_list *args)
> +{
> +       struct va_format vaf = { .fmt = fmt, .va = args};
> +
> +       trace_array_printk(trace_arr[get_trace_dst(desc)], _THIS_IP_, "%pV", &vaf);
> +}
> +
>  __printf(2, 3)
>  static void ddebug_printk(struct _ddebug *desc, const char *fmt, ...)
>  {
> @@ -1022,7 +1086,11 @@ static void ddebug_printk(struct _ddebug *desc, const char *fmt, ...)
>                  * All callers include the KERN_DEBUG prefix to keep the
>                  * vprintk case simple; strip it out for tracing.
>                  */
> -               ddebug_trace(desc, NULL, fmt + strlen(KERN_DEBUG), args);
> +               if (get_trace_dst(desc))
> +                       ddebug_trace_instance(desc, fmt, &args);
> +               else
> +                       ddebug_trace_event(desc, NULL,
> +                                          fmt + strlen(KERN_DEBUG), args);
>                 va_end(args);
>         }
>
> @@ -1044,7 +1112,10 @@ static void ddebug_dev_printk(struct _ddebug *desc, const struct device *dev,
>                 va_list args;
>
>                 va_start(args, fmt);
> -               ddebug_trace(desc, dev, fmt, args);
> +               if (get_trace_dst(desc))
> +                       ddebug_trace_instance(desc, fmt, &args);
> +               else
> +                       ddebug_trace_event(desc, dev, fmt, args);
>                 va_end(args);
>         }
>
> --
> 2.42.0.869.gea05f2083d-goog
>

Re: [PATCH v1 11/12] dyndbg: write debug logs to trace instance

[Łukasz Bartosik]

sob., 4 lis 2023 o 22:49 <jim.cromie@gmail.com> napisał(a):
>
> On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> >
> > When trace is enabled (T flag is set) and trace_dst field is set
> > to value greater than 0 (0 is reserved for trace events) then
> > debug logs will be written to trace instance pointed by trace_dst
> > value, for example when trace_dst value is 2 then debug logs will
> > be written to <debugfs>/tracing/instances/dyndbg_inst_2 instance.
> > Given trace instance will not be initialized until debug logs are
> > requested to be written to it and afer init will persist until
> > reboot.
> >
>
> restating 00 comments -
>
> you can get rid of integer destination ids by adding a new command: open/close
>
> $> echo  \
>  open kms-instance \;\
>  class DRM_UT_KMS -T:kms-instance  # preset-dests-disable-sites \;\
> > /proc/dynamic_debug/control
>

Instead of using above command to preset destination we could preset
destination with open command. I mean last successful
open would preset destination ? What do you think ?

>
> and +T  w/o dest means use existing setting, not just 0 (unless thats
> the existing setting)
>

Sounds good.




> > Signed-off-by: Łukasz Bartosik <lb@semihalf.com>
> > ---
> >  lib/Kconfig.debug   |  1 +
> >  lib/dynamic_debug.c | 79 ++++++++++++++++++++++++++++++++++++++++++---
> >  2 files changed, 76 insertions(+), 4 deletions(-)
> >
> > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
> > index fa307f93fa2e..9617e92c046d 100644
> > --- a/lib/Kconfig.debug
> > +++ b/lib/Kconfig.debug
> > @@ -181,6 +181,7 @@ config DYNAMIC_DEBUG_CORE
> >         bool "Enable core function of dynamic debug support"
> >         depends on PRINTK
> >         depends on (DEBUG_FS || PROC_FS)
> > +       depends on TRACING
> >         help
> >           Enable core functional support of dynamic debug. It is useful
> >           when you want to tie dynamic debug to your kernel modules with
> > diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> > index c5cd28e74a02..541d9d522b3b 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>
> >
> >  #define CREATE_TRACE_POINTS
> >  #include <trace/events/dyndbg.h>
> > @@ -81,6 +82,18 @@ module_param(verbose, int, 0644);
> >  MODULE_PARM_DESC(verbose, " dynamic_debug/control processing "
> >                  "( 0 = off (default), 1 = module add/rm, 2 = >control summary, 3 = parsing, 4 = per-site changes)");
> >
> > +/*
> > + * When trace is enabled (T flag is set) and trace_dst field is set
> > + * to value greater than 0 (0 is reserved for trace events) then
> > + * debug logs will be written to trace instance pointed by trace_dst
> > + * value, for example when trace_dst value is 2 then debug logs will
> > + * be written to <debugfs>/tracing/instances/dyndbg_inst_2 instance.
> > + * Given trace instance will not be initialized until debug logs are
> > + * requested to be written to it and afer init will persist until
> > + * reboot.
> > + */
> > +static struct trace_array *trace_arr[TRACE_DST_MAX];
> > +
> >  static inline struct dd_ctrl *get_ctrl(struct _ddebug *desc)
> >  {
> >         return &desc->ctrl;
> > @@ -255,6 +268,45 @@ static struct ddebug_class_map *ddebug_find_valid_class(struct ddebug_table cons
> >         return NULL;
> >  }
> >
> > +static int handle_trace_dst(struct dd_ctrl *ctrl)
> > +{
> > +#define TRACE_INST_NAME_LEN 16
> > +       char instance_name[TRACE_INST_NAME_LEN];
> > +       struct trace_array *arr;
> > +       int ret = -EINVAL;
> > +
> > +       /* check if trace (T flag) is enabled */
> > +       if (!(ctrl->flags & _DPRINTK_FLAGS_TRACE))
> > +               return 0;
> > +
> > +       /* check if trace destination are trace events */
> > +       if (!ctrl->trace_dst)
> > +               return 0;
> > +
> > +       /* check if trace instance is already set up */
> > +       if (trace_arr[ctrl->trace_dst])
> > +               return 0;
> > +
> > +       snprintf(instance_name, TRACE_INST_NAME_LEN,
> > +                "dyndbg_inst_%u", ctrl->trace_dst);
> > +       arr = trace_array_get_by_name(instance_name);
> > +       if (!arr)
> > +               goto err;
> > +
> > +       ret = trace_array_init_printk(arr);
> > +       if (ret)
> > +               goto err_init;
> > +
> > +       trace_arr[ctrl->trace_dst] = arr;
> > +       return 0;
> > +
> > +err_init:
> > +       trace_array_put(arr);
> > +       trace_array_destroy(arr);
> > +err:
> > +       return ret;
> > +}
> > +
> >  #define __outvar /* filled by callee */
> >  /*
> >   * Search the tables for _ddebug's which match the given `query' and
> > @@ -338,6 +390,9 @@ static int ddebug_change(const struct ddebug_query *query,
> >                         nctrl.trace_dst = modifiers->trace_dst;
> >                         if (!memcmp(&nctrl, get_ctrl(dp), sizeof(nctrl)))
> >                                 continue;
> > +
> > +                       if (handle_trace_dst(&nctrl))
> > +                               continue;
> >  #ifdef CONFIG_JUMP_LABEL
> >                         if (get_flags(dp) & _DPRINTK_FLAGS_ENABLED) {
> >                                 if (!(nctrl.flags & _DPRINTK_FLAGS_ENABLED))
> > @@ -977,8 +1032,8 @@ static DEFINE_PER_CPU(struct ddebug_trace_bufs, ddebug_trace_bufs);
> >  static DEFINE_PER_CPU(int, ddebug_trace_reserve);
> >
> >  __printf(3, 0)
> > -static void ddebug_trace(struct _ddebug *desc, const struct device *dev,
> > -                        const char *fmt, va_list args)
> > +static void ddebug_trace_event(struct _ddebug *desc, const struct device *dev,
> > +                              const char *fmt, va_list args)
> >  {
> >         struct ddebug_trace_buf *buf;
> >         int bufidx;
> > @@ -1010,6 +1065,15 @@ static void ddebug_trace(struct _ddebug *desc, const struct device *dev,
> >         preempt_enable_notrace();
> >  }
> >
> > +__printf(2, 0)
> > +static void ddebug_trace_instance(struct _ddebug *desc, const char *fmt,
> > +                                 va_list *args)
> > +{
> > +       struct va_format vaf = { .fmt = fmt, .va = args};
> > +
> > +       trace_array_printk(trace_arr[get_trace_dst(desc)], _THIS_IP_, "%pV", &vaf);
> > +}
> > +
> >  __printf(2, 3)
> >  static void ddebug_printk(struct _ddebug *desc, const char *fmt, ...)
> >  {
> > @@ -1022,7 +1086,11 @@ static void ddebug_printk(struct _ddebug *desc, const char *fmt, ...)
> >                  * All callers include the KERN_DEBUG prefix to keep the
> >                  * vprintk case simple; strip it out for tracing.
> >                  */
> > -               ddebug_trace(desc, NULL, fmt + strlen(KERN_DEBUG), args);
> > +               if (get_trace_dst(desc))
> > +                       ddebug_trace_instance(desc, fmt, &args);
> > +               else
> > +                       ddebug_trace_event(desc, NULL,
> > +                                          fmt + strlen(KERN_DEBUG), args);
> >                 va_end(args);
> >         }
> >
> > @@ -1044,7 +1112,10 @@ static void ddebug_dev_printk(struct _ddebug *desc, const struct device *dev,
> >                 va_list args;
> >
> >                 va_start(args, fmt);
> > -               ddebug_trace(desc, dev, fmt, args);
> > +               if (get_trace_dst(desc))
> > +                       ddebug_trace_instance(desc, fmt, &args);
> > +               else
> > +                       ddebug_trace_event(desc, dev, fmt, args);
> >                 va_end(args);
> >         }
> >
> > --
> > 2.42.0.869.gea05f2083d-goog
> >

Re: [PATCH v1 11/12] dyndbg: write debug logs to trace instance

[jim.cromie]

On Fri, Nov 10, 2023 at 7:53 AM Łukasz Bartosik <lb@semihalf.com> wrote:
>
> sob., 4 lis 2023 o 22:49 <jim.cromie@gmail.com> napisał(a):
> >
> > On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > >
> > > When trace is enabled (T flag is set) and trace_dst field is set
> > > to value greater than 0 (0 is reserved for trace events) then
> > > debug logs will be written to trace instance pointed by trace_dst
> > > value, for example when trace_dst value is 2 then debug logs will
> > > be written to <debugfs>/tracing/instances/dyndbg_inst_2 instance.
> > > Given trace instance will not be initialized until debug logs are
> > > requested to be written to it and afer init will persist until
> > > reboot.
> > >
> >
> > restating 00 comments -
> >
> > you can get rid of integer destination ids by adding a new command: open/close
> >
> > $> echo  \
> >  open kms-instance \;\
> >  class DRM_UT_KMS -T:kms-instance  # preset-dests-disable-sites \;\
> > > /proc/dynamic_debug/control
> >
>
> Instead of using above command to preset destination we could preset
> destination with open command. I mean last successful
> open would preset destination ? What do you think ?
>

I dont think it works - if open maps to a dest-number, (or implicit as
TOP-of-stack)
then you just have +T<dest-number>  (or +T <implicit tos>)
rather than +T:dest-name
and you still have to keep track of what dest-numbers were already used.
(or every new dest needs an explicit OPEN before it)

and how do you then get back to default instance ?
open 0 ?
close <previous-handle> ?


by using names, all opens can be at the top,
(and thus document in 1 block all the named-instances)
and any named dest that hasnt been opened is an error
(not just reusing previous OPEN)


> >
> > and +T  w/o dest means use existing setting, not just 0 (unless thats
> > the existing setting)
> >
>
> Sounds good.
>

:-)

Re: [PATCH v1 11/12] dyndbg: write debug logs to trace instance

[Łukasz Bartosik]

pt., 10 lis 2023 o 21:03 <jim.cromie@gmail.com> napisał(a):
>
> On Fri, Nov 10, 2023 at 7:53 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> >
> > sob., 4 lis 2023 o 22:49 <jim.cromie@gmail.com> napisał(a):
> > >
> > > On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > > >
> > > > When trace is enabled (T flag is set) and trace_dst field is set
> > > > to value greater than 0 (0 is reserved for trace events) then
> > > > debug logs will be written to trace instance pointed by trace_dst
> > > > value, for example when trace_dst value is 2 then debug logs will
> > > > be written to <debugfs>/tracing/instances/dyndbg_inst_2 instance.
> > > > Given trace instance will not be initialized until debug logs are
> > > > requested to be written to it and afer init will persist until
> > > > reboot.
> > > >
> > >
> > > restating 00 comments -
> > >
> > > you can get rid of integer destination ids by adding a new command: open/close
> > >
> > > $> echo  \
> > >  open kms-instance \;\
> > >  class DRM_UT_KMS -T:kms-instance  # preset-dests-disable-sites \;\
> > > > /proc/dynamic_debug/control
> > >
> >
> > Instead of using above command to preset destination we could preset
> > destination with open command. I mean last successful
> > open would preset destination ? What do you think ?
> >
>
> I dont think it works - if open maps to a dest-number, (or implicit as
> TOP-of-stack)
> then you just have +T<dest-number>  (or +T <implicit tos>)
> rather than +T:dest-name
> and you still have to keep track of what dest-numbers were already used.
> (or every new dest needs an explicit OPEN before it)
>
> and how do you then get back to default instance ?
> open 0 ?
> close <previous-handle> ?
>
>
> by using names, all opens can be at the top,
> (and thus document in 1 block all the named-instances)
> and any named dest that hasnt been opened is an error
> (not just reusing previous OPEN)
>

Sorry, I should have been more specific with my proposal. Let me use
an example to clarify it:
open usb -> create trace instance "usb" and make it default
echo module usbcore +T > /proc/dynamic_debug/control --> write usbcore
debug logs to trace instance named usb
open tbt --> create trace instance "tbt" and make it default
echo module aaa +T:usb > /proc/dynamic_debug/control --> write aaa
debug logs to trace instance named usb, instance usb has to be used
explicitly

                         because now tbt is default trace instance
echo module bbb +T > /proc/dynamic_debug/control --> write bbb debug
logs to trace instance named tbt
close tbt --> close tbt trace instance, I omit this step but in order
for an instance to be successful closed it must not be used by any
callsite, after
                    closing tbt instance the usb becomes default instance

I agree that your method of setting default trace instance is more flexible:
class DRM_UT_KMS -T:kms-instance  # preset-dests-disable-sites

Maybe we can combine both to set default trace destination ?

Also I think we need a reserved keyword for writing debug logs to
trace events - maybe "event" keyword ?


>
> > >
> > > and +T  w/o dest means use existing setting, not just 0 (unless thats
> > > the existing setting)
> > >
> >
> > Sounds good.
> >
>
> :-)

Re: [PATCH v1 11/12] dyndbg: write debug logs to trace instance

[jim.cromie]

On Sun, Nov 12, 2023 at 9:32 AM Łukasz Bartosik <lb@semihalf.com> wrote:
>
> pt., 10 lis 2023 o 21:03 <jim.cromie@gmail.com> napisał(a):
> >
> > On Fri, Nov 10, 2023 at 7:53 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > >
> > > sob., 4 lis 2023 o 22:49 <jim.cromie@gmail.com> napisał(a):
> > > >
> > > > On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > > > >
> > > > > When trace is enabled (T flag is set) and trace_dst field is set
> > > > > to value greater than 0 (0 is reserved for trace events) then
> > > > > debug logs will be written to trace instance pointed by trace_dst
> > > > > value, for example when trace_dst value is 2 then debug logs will
> > > > > be written to <debugfs>/tracing/instances/dyndbg_inst_2 instance.
> > > > > Given trace instance will not be initialized until debug logs are
> > > > > requested to be written to it and afer init will persist until
> > > > > reboot.
> > > > >
> > > >
> > > > restating 00 comments -
> > > >
> > > > you can get rid of integer destination ids by adding a new command: open/close
> > > >
> > > > $> echo  \
> > > >  open kms-instance \;\
> > > >  class DRM_UT_KMS -T:kms-instance  # preset-dests-disable-sites \;\
> > > > > /proc/dynamic_debug/control
> > > >
> > >
> > > Instead of using above command to preset destination we could preset
> > > destination with open command. I mean last successful
> > > open would preset destination ? What do you think ?
> > >
> >
> > I dont think it works - if open maps to a dest-number, (or implicit as
> > TOP-of-stack)
> > then you just have +T<dest-number>  (or +T <implicit tos>)
> > rather than +T:dest-name
> > and you still have to keep track of what dest-numbers were already used.
> > (or every new dest needs an explicit OPEN before it)
> >
> > and how do you then get back to default instance ?
> > open 0 ?
> > close <previous-handle> ?
> >
> >
> > by using names, all opens can be at the top,
> > (and thus document in 1 block all the named-instances)
> > and any named dest that hasnt been opened is an error
> > (not just reusing previous OPEN)
> >
>
> Sorry, I should have been more specific with my proposal. Let me use
> an example to clarify it:
> open usb    # -> create trace instance "usb" and make it default
> echo module usbcore +T > /proc/dynamic_debug/control ## --> write usbcore
> ## debug logs to trace instance named usb
> open tbt --> create trace instance "tbt" and make it default
> echo module aaa +T:usb > /proc/dynamic_debug/control --> write aaa
> debug logs to trace instance named usb, instance usb has to be used
> explicitly
>
>                          because now tbt is default trace instance

that feels too magical/ action at a distance.

ISTM it also muddles what the "default" is:

my-default:
what each callsite's current/preset dest is/was
the only way to set it is with explicit [-+]T:outstream

your-default:
whatever was last opened. whether it was 2 or 50 lines above,
or set weeks ago, the last time somebody opened an instance.

and as more instances are created
(potentially by different users?
after all there are separate instances,
and presumably separate interests),
the default gets less predictable.


> echo module bbb +T > /proc/dynamic_debug/control --> write bbb debug
> logs to trace instance named tbt
> close tbt --> close tbt trace instance, I omit this step but in order
> for an instance to be successful closed it must not be used by any
> callsite, after
>                     closing tbt instance the usb becomes default instance

so after 'close tbt',  the previous 'open usb' is now top-of-stack ?

how does that affect all existing callsite-users of tbt ?
do they continue to use the trace-instance theyve been writing to ?
If not, then reverting to the global instance seems much more predictable.

Or are you proposing that the close fails because the instance is still in use ?
this seems least surprising,
and more robust in the face of the next 'open foo'
which could otherwize reuse the dst_id mapped to tbt


>
> I agree that your method of setting default trace instance is more flexible:
> class DRM_UT_KMS -T:kms-instance  # preset-dests-disable-sites
>
> Maybe we can combine both to set default trace destination ?
>
> Also I think we need a reserved keyword for writing debug logs to
> trace events - maybe "event" keyword ?

do you mean "event" as a selector, like module, function, class, etc ?
if so, what are the values ?
any event under  /sys/kernel/debug/tracing/events/ ?

how does this get used ?

>
>
> >
> > > >
> > > > and +T  w/o dest means use existing setting, not just 0 (unless thats
> > > > the existing setting)
> > > >
> > >
> > > Sounds good.
> > >
> >
> > :-)

Re: [PATCH v1 11/12] dyndbg: write debug logs to trace instance

[Łukasz Bartosik]

pon., 13 lis 2023 o 19:59 <jim.cromie@gmail.com> napisał(a):
>
> On Sun, Nov 12, 2023 at 9:32 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> >
> > pt., 10 lis 2023 o 21:03 <jim.cromie@gmail.com> napisał(a):
> > >
> > > On Fri, Nov 10, 2023 at 7:53 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > > >
> > > > sob., 4 lis 2023 o 22:49 <jim.cromie@gmail.com> napisał(a):
> > > > >
> > > > > On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > > > > >
> > > > > > When trace is enabled (T flag is set) and trace_dst field is set
> > > > > > to value greater than 0 (0 is reserved for trace events) then
> > > > > > debug logs will be written to trace instance pointed by trace_dst
> > > > > > value, for example when trace_dst value is 2 then debug logs will
> > > > > > be written to <debugfs>/tracing/instances/dyndbg_inst_2 instance.
> > > > > > Given trace instance will not be initialized until debug logs are
> > > > > > requested to be written to it and afer init will persist until
> > > > > > reboot.
> > > > > >
> > > > >
> > > > > restating 00 comments -
> > > > >
> > > > > you can get rid of integer destination ids by adding a new command: open/close
> > > > >
> > > > > $> echo  \
> > > > >  open kms-instance \;\
> > > > >  class DRM_UT_KMS -T:kms-instance  # preset-dests-disable-sites \;\
> > > > > > /proc/dynamic_debug/control
> > > > >
> > > >
> > > > Instead of using above command to preset destination we could preset
> > > > destination with open command. I mean last successful
> > > > open would preset destination ? What do you think ?
> > > >
> > >
> > > I dont think it works - if open maps to a dest-number, (or implicit as
> > > TOP-of-stack)
> > > then you just have +T<dest-number>  (or +T <implicit tos>)
> > > rather than +T:dest-name
> > > and you still have to keep track of what dest-numbers were already used.
> > > (or every new dest needs an explicit OPEN before it)
> > >
> > > and how do you then get back to default instance ?
> > > open 0 ?
> > > close <previous-handle> ?
> > >
> > >
> > > by using names, all opens can be at the top,
> > > (and thus document in 1 block all the named-instances)
> > > and any named dest that hasnt been opened is an error
> > > (not just reusing previous OPEN)
> > >
> >
> > Sorry, I should have been more specific with my proposal. Let me use
> > an example to clarify it:
> > open usb    # -> create trace instance "usb" and make it default
> > echo module usbcore +T > /proc/dynamic_debug/control ## --> write usbcore
> > ## debug logs to trace instance named usb
> > open tbt --> create trace instance "tbt" and make it default
> > echo module aaa +T:usb > /proc/dynamic_debug/control --> write aaa
> > debug logs to trace instance named usb, instance usb has to be used
> > explicitly
> >
> >                          because now tbt is default trace instance
>
> that feels too magical/ action at a distance.
>
> ISTM it also muddles what the "default" is:
>
> my-default:
> what each callsite's current/preset dest is/was
> the only way to set it is with explicit [-+]T:outstream
>

I see your point, I will follow your suggestion.

> your-default:
> whatever was last opened. whether it was 2 or 50 lines above,
> or set weeks ago, the last time somebody opened an instance.
>
> and as more instances are created
> (potentially by different users?
> after all there are separate instances,
> and presumably separate interests),
> the default gets less predictable.
>
>
> > echo module bbb +T > /proc/dynamic_debug/control --> write bbb debug
> > logs to trace instance named tbt
> > close tbt --> close tbt trace instance, I omit this step but in order
> > for an instance to be successful closed it must not be used by any
> > callsite, after
> >                     closing tbt instance the usb becomes default instance
>
> so after 'close tbt',  the previous 'open usb' is now top-of-stack ?
>
> how does that affect all existing callsite-users of tbt ?
> do they continue to use the trace-instance theyve been writing to ?
> If not, then reverting to the global instance seems much more predictable.
>
> Or are you proposing that the close fails because the instance is still in use ?
> this seems least surprising,
> and more robust in the face of the next 'open foo'
> which could otherwize reuse the dst_id mapped to tbt
>
>
> >
> > I agree that your method of setting default trace instance is more flexible:
> > class DRM_UT_KMS -T:kms-instance  # preset-dests-disable-sites
> >
> > Maybe we can combine both to set default trace destination ?
> >
> > Also I think we need a reserved keyword for writing debug logs to
> > trace events - maybe "event" keyword ?
>
> do you mean "event" as a selector, like module, function, class, etc ?
> if so, what are the values ?
> any event under  /sys/kernel/debug/tracing/events/ ?
>
> how does this get used ?
>

I meant that we need to reserve name/keyword to enable writing debug
logs to trace events (prdbg and devdbg), for example
echo module usbcore +T:event > /proc/dynamic_debug/control

Or do you anticipate other way to do it ?

> >
> >
> > >
> > > > >
> > > > > and +T  w/o dest means use existing setting, not just 0 (unless thats
> > > > > the existing setting)
> > > > >
> > > >
> > > > Sounds good.
> > > >
> > >
> > > :-)

Re: [PATCH v1 11/12] dyndbg: write debug logs to trace instance

[jim.cromie]

On Mon, Nov 13, 2023 at 4:44 PM Łukasz Bartosik <lb@semihalf.com> wrote:
>
> pon., 13 lis 2023 o 19:59 <jim.cromie@gmail.com> napisał(a):
> >
> > On Sun, Nov 12, 2023 at 9:32 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > >
> > > pt., 10 lis 2023 o 21:03 <jim.cromie@gmail.com> napisał(a):
> > > >
> > > > On Fri, Nov 10, 2023 at 7:53 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > > > >
> > > > > sob., 4 lis 2023 o 22:49 <jim.cromie@gmail.com> napisał(a):
> > > > > >
> > > > > > On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > > > > > >
> > > > > > > When trace is enabled (T flag is set) and trace_dst field is set
> > > > > > > to value greater than 0 (0 is reserved for trace events) then
> > > > > > > debug logs will be written to trace instance pointed by trace_dst
> > > > > > > value, for example when trace_dst value is 2 then debug logs will
> > > > > > > be written to <debugfs>/tracing/instances/dyndbg_inst_2 instance.
> > > > > > > Given trace instance will not be initialized until debug logs are
> > > > > > > requested to be written to it and afer init will persist until
> > > > > > > reboot.
> > > > > > >
> > > > > >
> > > > > > restating 00 comments -
> > > > > >
> > > > > > you can get rid of integer destination ids by adding a new command: open/close
> > > > > >
> > > > > > $> echo  \
> > > > > >  open kms-instance \;\
> > > > > >  class DRM_UT_KMS -T:kms-instance  # preset-dests-disable-sites \;\
> > > > > > > /proc/dynamic_debug/control
> > > > > >
> > > > >
> > > > > Instead of using above command to preset destination we could preset
> > > > > destination with open command. I mean last successful
> > > > > open would preset destination ? What do you think ?
> > > > >
> > > >
> > > > I dont think it works - if open maps to a dest-number, (or implicit as
> > > > TOP-of-stack)
> > > > then you just have +T<dest-number>  (or +T <implicit tos>)
> > > > rather than +T:dest-name
> > > > and you still have to keep track of what dest-numbers were already used.
> > > > (or every new dest needs an explicit OPEN before it)
> > > >
> > > > and how do you then get back to default instance ?
> > > > open 0 ?
> > > > close <previous-handle> ?
> > > >
> > > >
> > > > by using names, all opens can be at the top,
> > > > (and thus document in 1 block all the named-instances)
> > > > and any named dest that hasnt been opened is an error
> > > > (not just reusing previous OPEN)
> > > >
> > >
> > > Sorry, I should have been more specific with my proposal. Let me use
> > > an example to clarify it:
> > > open usb    # -> create trace instance "usb" and make it default
> > > echo module usbcore +T > /proc/dynamic_debug/control ## --> write usbcore
> > > ## debug logs to trace instance named usb
> > > open tbt --> create trace instance "tbt" and make it default
> > > echo module aaa +T:usb > /proc/dynamic_debug/control --> write aaa
> > > debug logs to trace instance named usb, instance usb has to be used
> > > explicitly
> > >
> > >                          because now tbt is default trace instance
> >
> > that feels too magical/ action at a distance.
> >
> > ISTM it also muddles what the "default" is:
> >
> > my-default:
> > what each callsite's current/preset dest is/was
> > the only way to set it is with explicit [-+]T:outstream
> >
>
> I see your point, I will follow your suggestion.
>
> > your-default:
> > whatever was last opened. whether it was 2 or 50 lines above,
> > or set weeks ago, the last time somebody opened an instance.
> >
> > and as more instances are created
> > (potentially by different users?
> > after all there are separate instances,
> > and presumably separate interests),
> > the default gets less predictable.
> >
> >
> > > echo module bbb +T > /proc/dynamic_debug/control --> write bbb debug
> > > logs to trace instance named tbt
> > > close tbt --> close tbt trace instance, I omit this step but in order
> > > for an instance to be successful closed it must not be used by any
> > > callsite, after
> > >                     closing tbt instance the usb becomes default instance
> >
> > so after 'close tbt',  the previous 'open usb' is now top-of-stack ?
> >
> > how does that affect all existing callsite-users of tbt ?
> > do they continue to use the trace-instance theyve been writing to ?
> > If not, then reverting to the global instance seems much more predictable.
> >
> > Or are you proposing that the close fails because the instance is still in use ?
> > this seems least surprising,
> > and more robust in the face of the next 'open foo'
> > which could otherwize reuse the dst_id mapped to tbt
> >
> >
> > >
> > > I agree that your method of setting default trace instance is more flexible:
> > > class DRM_UT_KMS -T:kms-instance  # preset-dests-disable-sites
> > >
> > > Maybe we can combine both to set default trace destination ?
> > >
> > > Also I think we need a reserved keyword for writing debug logs to
> > > trace events - maybe "event" keyword ?
> >
> > do you mean "event" as a selector, like module, function, class, etc ?
> > if so, what are the values ?
> > any event under  /sys/kernel/debug/tracing/events/ ?
> >
> > how does this get used ?
> >
>
> I meant that we need to reserve name/keyword to enable writing debug
> logs to trace events (prdbg and devdbg), for example
> echo module usbcore +T:event > /proc/dynamic_debug/control
>
> Or do you anticipate other way to do it ?

way back, when I had even fewer clues,
I sent patches to call trace-printk when +T was set.
Steve didnt like it, I think cuz it could flood the tracebuf.

Thats why I added the prdbg and devdbg event-types,
so that they could be disabled easily using /sys/kernel/debug/tracing/
putting them squarely under trace-control.

Note that this puts 2 off-switches in series,
both tracefs and >control can disable all the pr_debug traffic,
tracefs by event-type, and >control at individual callsite level.

echo 1 > /sys/kernel/debug/tracing/dyndbg/enable
echo 1 > /sys/kernel/debug/tracing/dyndbg/prdbg/enable
echo 1 > /sys/kernel/debug/tracing/dyndbg/devdbg/enable

I briefly thought about linking the 2 off-switches,
but punted cuz I thought it complicated things,
(how exactly would they get coupled?)
and I didnt want to distract from larger goals

Does that address your question ?

On a related point, I also added drm_dbg and drm_devdbg.
Those are issued from __drm_dbg & __drm_dev_dbg
 respectively when CONFIG_DRM_USE_DYNAMIC_DEBUG=n.

Im not sure theyre more useful than confusing yet.

>
> > >
> > >
> > > >
> > > > > >
> > > > > > and +T  w/o dest means use existing setting, not just 0 (unless thats
> > > > > > the existing setting)
> > > > > >
> > > > >
> > > > > Sounds good.
> > > > >
> > > >
> > > > :-)

Re: [PATCH v1 11/12] dyndbg: write debug logs to trace instance

[Łukasz Bartosik]

wt., 14 lis 2023 o 02:08 <jim.cromie@gmail.com> napisał(a):
>
> On Mon, Nov 13, 2023 at 4:44 PM Łukasz Bartosik <lb@semihalf.com> wrote:
> >
> > pon., 13 lis 2023 o 19:59 <jim.cromie@gmail.com> napisał(a):
> > >
> > > On Sun, Nov 12, 2023 at 9:32 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > > >
> > > > pt., 10 lis 2023 o 21:03 <jim.cromie@gmail.com> napisał(a):
> > > > >
> > > > > On Fri, Nov 10, 2023 at 7:53 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > > > > >
> > > > > > sob., 4 lis 2023 o 22:49 <jim.cromie@gmail.com> napisał(a):
> > > > > > >
> > > > > > > On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> > > > > > > >
> > > > > > > > When trace is enabled (T flag is set) and trace_dst field is set
> > > > > > > > to value greater than 0 (0 is reserved for trace events) then
> > > > > > > > debug logs will be written to trace instance pointed by trace_dst
> > > > > > > > value, for example when trace_dst value is 2 then debug logs will
> > > > > > > > be written to <debugfs>/tracing/instances/dyndbg_inst_2 instance.
> > > > > > > > Given trace instance will not be initialized until debug logs are
> > > > > > > > requested to be written to it and afer init will persist until
> > > > > > > > reboot.
> > > > > > > >
> > > > > > >
> > > > > > > restating 00 comments -
> > > > > > >
> > > > > > > you can get rid of integer destination ids by adding a new command: open/close
> > > > > > >
> > > > > > > $> echo  \
> > > > > > >  open kms-instance \;\
> > > > > > >  class DRM_UT_KMS -T:kms-instance  # preset-dests-disable-sites \;\
> > > > > > > > /proc/dynamic_debug/control
> > > > > > >
> > > > > >
> > > > > > Instead of using above command to preset destination we could preset
> > > > > > destination with open command. I mean last successful
> > > > > > open would preset destination ? What do you think ?
> > > > > >
> > > > >
> > > > > I dont think it works - if open maps to a dest-number, (or implicit as
> > > > > TOP-of-stack)
> > > > > then you just have +T<dest-number>  (or +T <implicit tos>)
> > > > > rather than +T:dest-name
> > > > > and you still have to keep track of what dest-numbers were already used.
> > > > > (or every new dest needs an explicit OPEN before it)
> > > > >
> > > > > and how do you then get back to default instance ?
> > > > > open 0 ?
> > > > > close <previous-handle> ?
> > > > >
> > > > >
> > > > > by using names, all opens can be at the top,
> > > > > (and thus document in 1 block all the named-instances)
> > > > > and any named dest that hasnt been opened is an error
> > > > > (not just reusing previous OPEN)
> > > > >
> > > >
> > > > Sorry, I should have been more specific with my proposal. Let me use
> > > > an example to clarify it:
> > > > open usb    # -> create trace instance "usb" and make it default
> > > > echo module usbcore +T > /proc/dynamic_debug/control ## --> write usbcore
> > > > ## debug logs to trace instance named usb
> > > > open tbt --> create trace instance "tbt" and make it default
> > > > echo module aaa +T:usb > /proc/dynamic_debug/control --> write aaa
> > > > debug logs to trace instance named usb, instance usb has to be used
> > > > explicitly
> > > >
> > > >                          because now tbt is default trace instance
> > >
> > > that feels too magical/ action at a distance.
> > >
> > > ISTM it also muddles what the "default" is:
> > >
> > > my-default:
> > > what each callsite's current/preset dest is/was
> > > the only way to set it is with explicit [-+]T:outstream
> > >
> >
> > I see your point, I will follow your suggestion.
> >
> > > your-default:
> > > whatever was last opened. whether it was 2 or 50 lines above,
> > > or set weeks ago, the last time somebody opened an instance.
> > >
> > > and as more instances are created
> > > (potentially by different users?
> > > after all there are separate instances,
> > > and presumably separate interests),
> > > the default gets less predictable.
> > >
> > >
> > > > echo module bbb +T > /proc/dynamic_debug/control --> write bbb debug
> > > > logs to trace instance named tbt
> > > > close tbt --> close tbt trace instance, I omit this step but in order
> > > > for an instance to be successful closed it must not be used by any
> > > > callsite, after
> > > >                     closing tbt instance the usb becomes default instance
> > >
> > > so after 'close tbt',  the previous 'open usb' is now top-of-stack ?
> > >
> > > how does that affect all existing callsite-users of tbt ?
> > > do they continue to use the trace-instance theyve been writing to ?
> > > If not, then reverting to the global instance seems much more predictable.
> > >
> > > Or are you proposing that the close fails because the instance is still in use ?
> > > this seems least surprising,
> > > and more robust in the face of the next 'open foo'
> > > which could otherwize reuse the dst_id mapped to tbt
> > >
> > >
> > > >
> > > > I agree that your method of setting default trace instance is more flexible:
> > > > class DRM_UT_KMS -T:kms-instance  # preset-dests-disable-sites
> > > >
> > > > Maybe we can combine both to set default trace destination ?
> > > >
> > > > Also I think we need a reserved keyword for writing debug logs to
> > > > trace events - maybe "event" keyword ?
> > >
> > > do you mean "event" as a selector, like module, function, class, etc ?
> > > if so, what are the values ?
> > > any event under  /sys/kernel/debug/tracing/events/ ?
> > >
> > > how does this get used ?
> > >
> >
> > I meant that we need to reserve name/keyword to enable writing debug
> > logs to trace events (prdbg and devdbg), for example
> > echo module usbcore +T:event > /proc/dynamic_debug/control
> >
> > Or do you anticipate other way to do it ?
>
> way back, when I had even fewer clues,
> I sent patches to call trace-printk when +T was set.
> Steve didnt like it, I think cuz it could flood the tracebuf.
>
> Thats why I added the prdbg and devdbg event-types,
> so that they could be disabled easily using /sys/kernel/debug/tracing/
> putting them squarely under trace-control.
>
> Note that this puts 2 off-switches in series,
> both tracefs and >control can disable all the pr_debug traffic,
> tracefs by event-type, and >control at individual callsite level.
>
> echo 1 > /sys/kernel/debug/tracing/dyndbg/enable
> echo 1 > /sys/kernel/debug/tracing/dyndbg/prdbg/enable
> echo 1 > /sys/kernel/debug/tracing/dyndbg/devdbg/enable
>
> I briefly thought about linking the 2 off-switches,
> but punted cuz I thought it complicated things,
> (how exactly would they get coupled?)
> and I didnt want to distract from larger goals
>
> Does that address your question ?
>

Jim,

Thanks but it doesn't answer my question.

How do you plan to enable output to tracefs event at a callsite level ?

In my original proposal it was enabled by setting trace destination to
0. Since we are moving to names instead of numbers now I guess we need
to reserve a name for it not to clash with trace instance names
provided by users. That's why I proposed to reserve name "event" for
that purpose and be used as +T:event.

Or did I miss answer to that in our long discussion :> ?

Thanks,
Lukasz

> On a related point, I also added drm_dbg and drm_devdbg.
> Those are issued from __drm_dbg & __drm_dev_dbg
>  respectively when CONFIG_DRM_USE_DYNAMIC_DEBUG=n.
>
> Im not sure theyre more useful than confusing yet.
>
> >
> > > >
> > > >
> > > > >
> > > > > > >
> > > > > > > and +T  w/o dest means use existing setting, not just 0 (unless thats
> > > > > > > the existing setting)
> > > > > > >
> > > > > >
> > > > > > Sounds good.
> > > > > >
> > > > >
> > > > > :-)

Re: [PATCH v1 11/12] dyndbg: write debug logs to trace instance

[jim.cromie]

On Tue, Nov 14, 2023 at 12:45 AM Łukasz Bartosik <lb@semihalf.com> wrote:
>
> wt., 14 lis 2023 o 02:08 <jim.cromie@gmail.com> napisał(a):
> >

> > > > > Also I think we need a reserved keyword for writing debug logs to
> > > > > trace events - maybe "event" keyword ?
> > > >
> > > > do you mean "event" as a selector, like module, function, class, etc ?
> > > > if so, what are the values ?
> > > > any event under  /sys/kernel/debug/tracing/events/ ?
> > > >
> > > > how does this get used ?
> > > >
> > >
> > > I meant that we need to reserve name/keyword to enable writing debug
> > > logs to trace events (prdbg and devdbg), for example
> > > echo module usbcore +T:event > /proc/dynamic_debug/control
> > >
> > > Or do you anticipate other way to do it ?
> >
> > way back, when I had even fewer clues,
> > I sent patches to call trace-printk when +T was set.
> > Steve didnt like it, I think cuz it could flood the tracebuf.
> >
> > Thats why I added the prdbg and devdbg event-types,
> > so that they could be disabled easily using /sys/kernel/debug/tracing/
> > putting them squarely under trace-control.
> >
> > Note that this puts 2 off-switches in series,
> > both tracefs and >control can disable all the pr_debug traffic,
> > tracefs by event-type, and >control at individual callsite level.
> >
> > echo 1 > /sys/kernel/debug/tracing/dyndbg/enable
> > echo 1 > /sys/kernel/debug/tracing/dyndbg/prdbg/enable
> > echo 1 > /sys/kernel/debug/tracing/dyndbg/devdbg/enable
> >
> > I briefly thought about linking the 2 off-switches,
> > but punted cuz I thought it complicated things,
> > (how exactly would they get coupled?)
> > and I didnt want to distract from larger goals
> >
> > Does that address your question ?
> >
>
> Jim,
>
> Thanks but it doesn't answer my question.
>
> How do you plan to enable output to tracefs event at a callsite level ?
>
> In my original proposal it was enabled by setting trace destination to
> 0. Since we are moving to names instead of numbers now I guess we need
> to reserve a name for it not to clash with trace instance names
> provided by users. That's why I proposed to reserve name "event" for
> that purpose and be used as +T:event.
>

Ok, I got your point now.

how about we call it "0" ?
it should be an obvious "magical" value,
cuz it doesnt need to be open'd, and cant be close'd

then we can revert to global tracebuf by its "name"
echo module foo +T:0 > /proc/dynamic_debug/control

we probably should also limit the trace-instance-names to ^\w+

> Or did I miss answer to that in our long discussion :> ?

nope :-)

thanks,
Jim

>
> Thanks,
> Lukasz
>
> > On a related point, I also added drm_dbg and drm_devdbg.
> > Those are issued from __drm_dbg & __drm_dev_dbg
> >  respectively when CONFIG_DRM_USE_DYNAMIC_DEBUG=n.
> >
> > Im not sure theyre more useful than confusing yet.
> >
> > >
> > > > >
> > > > >
> > > > > >
> > > > > > > >
> > > > > > > > and +T  w/o dest means use existing setting, not just 0 (unless thats
> > > > > > > > the existing setting)
> > > > > > > >
> > > > > > >
> > > > > > > Sounds good.
> > > > > > >
> > > > > >
> > > > > > :-)

Re: [PATCH v1 11/12] dyndbg: write debug logs to trace instance

[Łukasz Bartosik]

wt., 14 lis 2023 o 16:41 <jim.cromie@gmail.com> napisał(a):
>
> On Tue, Nov 14, 2023 at 12:45 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> >
> > wt., 14 lis 2023 o 02:08 <jim.cromie@gmail.com> napisał(a):
> > >
>
> > > > > > Also I think we need a reserved keyword for writing debug logs to
> > > > > > trace events - maybe "event" keyword ?
> > > > >
> > > > > do you mean "event" as a selector, like module, function, class, etc ?
> > > > > if so, what are the values ?
> > > > > any event under  /sys/kernel/debug/tracing/events/ ?
> > > > >
> > > > > how does this get used ?
> > > > >
> > > >
> > > > I meant that we need to reserve name/keyword to enable writing debug
> > > > logs to trace events (prdbg and devdbg), for example
> > > > echo module usbcore +T:event > /proc/dynamic_debug/control
> > > >
> > > > Or do you anticipate other way to do it ?
> > >
> > > way back, when I had even fewer clues,
> > > I sent patches to call trace-printk when +T was set.
> > > Steve didnt like it, I think cuz it could flood the tracebuf.
> > >
> > > Thats why I added the prdbg and devdbg event-types,
> > > so that they could be disabled easily using /sys/kernel/debug/tracing/
> > > putting them squarely under trace-control.
> > >
> > > Note that this puts 2 off-switches in series,
> > > both tracefs and >control can disable all the pr_debug traffic,
> > > tracefs by event-type, and >control at individual callsite level.
> > >
> > > echo 1 > /sys/kernel/debug/tracing/dyndbg/enable
> > > echo 1 > /sys/kernel/debug/tracing/dyndbg/prdbg/enable
> > > echo 1 > /sys/kernel/debug/tracing/dyndbg/devdbg/enable
> > >
> > > I briefly thought about linking the 2 off-switches,
> > > but punted cuz I thought it complicated things,
> > > (how exactly would they get coupled?)
> > > and I didnt want to distract from larger goals
> > >
> > > Does that address your question ?
> > >
> >
> > Jim,
> >
> > Thanks but it doesn't answer my question.
> >
> > How do you plan to enable output to tracefs event at a callsite level ?
> >
> > In my original proposal it was enabled by setting trace destination to
> > 0. Since we are moving to names instead of numbers now I guess we need
> > to reserve a name for it not to clash with trace instance names
> > provided by users. That's why I proposed to reserve name "event" for
> > that purpose and be used as +T:event.
> >
>
> Ok, I got your point now.
>
> how about we call it "0" ?
> it should be an obvious "magical" value,
> cuz it doesnt need to be open'd, and cant be close'd
>
> then we can revert to global tracebuf by its "name"
> echo module foo +T:0 > /proc/dynamic_debug/control
>

I like it. It resembles surprise emoji :0

> we probably should also limit the trace-instance-names to ^\w+
>

Ack

I think this closes the last open topic to discuss for now.
I will get back with next patchset version to you soon.

> > Or did I miss answer to that in our long discussion :> ?
>
> nope :-)
>
> thanks,
> Jim
>
> >
> > Thanks,
> > Lukasz
> >
> > > On a related point, I also added drm_dbg and drm_devdbg.
> > > Those are issued from __drm_dbg & __drm_dev_dbg
> > >  respectively when CONFIG_DRM_USE_DYNAMIC_DEBUG=n.
> > >
> > > Im not sure theyre more useful than confusing yet.
> > >
> > > >
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > > >
> > > > > > > > > and +T  w/o dest means use existing setting, not just 0 (unless thats
> > > > > > > > > the existing setting)
> > > > > > > > >
> > > > > > > >
> > > > > > > > Sounds good.
> > > > > > > >
> > > > > > >
> > > > > > > :-)

[PATCH v1 12/12] dyndbg: add trace support for hexdump

[Łukasz Bartosik]

Add support for writing hexdump debug logs to trace.

Signed-off-by: Łukasz Bartosik <lb@semihalf.com>
---
 include/linux/dynamic_debug.h | 16 ++++++++++------
 lib/dynamic_debug.c           | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 6 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 3084302876b4..e01b529dcd09 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -298,12 +298,16 @@ void __dynamic_ibdev_dbg(struct _ddebug *descriptor,
 	_dynamic_func_call(fmt, __dynamic_ibdev_dbg,		\
 			   dev, fmt, ##__VA_ARGS__)
 
-#define dynamic_hex_dump(prefix_str, prefix_type, rowsize,		\
-			 groupsize, buf, len, ascii)			\
-	_dynamic_func_call_no_desc(__builtin_constant_p(prefix_str) ? prefix_str : "hexdump", \
-				   print_hex_dump,			\
-				   KERN_DEBUG, prefix_str, prefix_type,	\
-				   rowsize, groupsize, buf, len, ascii)
+void _print_hex_dump(struct _ddebug *descriptor, const char *level,
+		     const char *prefix_str, int prefix_type, int rowsize,
+		     int groupsize, const void *buf, size_t len, bool ascii);
+
+#define dynamic_hex_dump(prefix_str, prefix_type, rowsize,				\
+			 groupsize, buf, len, ascii)					\
+	_dynamic_func_call(__builtin_constant_p(prefix_str) ? prefix_str : "hexdump",	\
+			   _print_hex_dump,						\
+			   KERN_DEBUG, prefix_str, prefix_type,				\
+			   rowsize, groupsize, buf, len, ascii)
 
 /* for test only, generally expect drm.debug style macro wrappers */
 #define __pr_debug_cls(cls, fmt, ...) do {			\
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 541d9d522b3b..fb2c6e2909bb 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -1128,6 +1128,41 @@ static void ddebug_dev_printk(struct _ddebug *desc, const struct device *dev,
 	}
 }
 
+void _print_hex_dump(struct _ddebug *descriptor, const char *level,
+		     const char *prefix_str, int prefix_type, int rowsize,
+		     int groupsize, const void *buf, size_t len, bool ascii)
+{
+	const u8 *ptr = buf;
+	int i, linelen, remaining = len;
+	unsigned char linebuf[32 * 3 + 2 + 32 + 1];
+
+	if (rowsize != 16 && rowsize != 32)
+		rowsize = 16;
+
+	for (i = 0; i < len; i += rowsize) {
+		linelen = min(remaining, rowsize);
+		remaining -= rowsize;
+
+		hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
+				   linebuf, sizeof(linebuf), ascii);
+
+		switch (prefix_type) {
+		case DUMP_PREFIX_ADDRESS:
+			ddebug_printk(descriptor, "%s%s%p: %s\n",
+				      level, prefix_str, ptr + i, linebuf);
+			break;
+		case DUMP_PREFIX_OFFSET:
+			ddebug_printk(descriptor, "%s%s%.8x: %s\n",
+				      level, prefix_str, i, linebuf);
+			break;
+		default:
+			ddebug_printk(descriptor, "%s%s%s\n",
+				      level, prefix_str, linebuf);
+			break;
+		}
+	}
+}
+
 void __dynamic_pr_debug(struct _ddebug *descriptor, const char *fmt, ...)
 {
 	va_list args;
-- 
2.42.0.869.gea05f2083d-goog

Re: [PATCH v1 00/12] dyndbg: add support for writing debug logs to trace

[jim.cromie]

Hi Łukasz,

can I haz a git remote url ?
no webmail antics that way.

On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
>
> Add support for writing debug logs to trace events and trace instances.
> The rationale behing this feature is to be able to redirect debug logs
> (per each callsite indivdually) to trace to aid in debugging. The debug
> logs output to trace can be enabled with T flag. Additionally trace
> destination can be provided to the T flag after ":". The trace destination
> field is used to determine where debug logs will be written. Setting trace
> destination value to 0 (default) enables output to prdbg and devdbg trace

isnt +p independent of dest var ?  its just "on" to syslog.

> events. Setting trace destination value to a value in range of [1..255]
> enables output to trace instance identified by trace destination value.
> For example when trace destination value is 2 then debug logs will
> be written to <debugfs>/tracing/instances/dyndbg_inst_2 instance.
>
> Usage examples:
>
> localhost ~ # echo "module thunderbolt =pT:7" >
>                                 <debugfs>/dynamic_debug/control
>
> This will enable output of debug logs to trace instance
> <debugfs>/tracing/instances/dyndbg_inst_7 and debug logs will
> be written to the syslog also because p flag is set.
>
> localhost ~ # echo "module thunderbolt =pT:7,l" >
>                                 <debugfs>/dynamic_debug/control
>
> When trace destination is followed by another flag then trace
> destination has to be followed by ",".
>
> localhost ~ # echo "module thunderbolt =pTl" >
>                                 <debugfs>/dynamic_debug/control
>
> When trace destination is not provided explicitly then its value
> defaults to 0. In this case debug logs will be written to the prdbg
> and devdbg trace events.
>
> localhost ~ # echo "module thunderbolt =T:25" >
>                                 <debugfs>/dynamic_debug/control
>
> This will enable output of debug logs to trace instance
> <debugfs>/tracing/instances/dyndbg_inst_25 with debug logs output
> to syslog disabled.
>
> Given trace instance will not be initialized until debug logs are
> requested to be written to it and afer init it will persist until
> reboot.
>

that (delayed init) might be a problem,
user side will have to look for the appearance of traces ?

Also, I have some reservations about exposing numeric destinations -
the user(space) must then decide/coordinate what dest-number
is used for which instance/purpose.

It would be fine for 1 customer, but might be a little tedious for many,
who now have to coordinate.  A bit like a shared/party line in the early
days of rural telephone.

As I recall, an early early version of classmaps used numeric classes,
(taken straight from drm_debug_category).
As Jason noted (more diplomatically than I assimilated)
it was kind of arbitrary and obscure/obtuse/unhelpful numbering.

It is why I added classnames, with the bonus that
the name->num mapping was also a validation step
against known CLASSMAP_DEFINE-itions
(if you knew DRM drivers knew "DRM_KMS_CORE",
 you knew what you were asking for)

Your earlier version had a dest keyword which maybe fits better with this point.
that said, it was in a selector position, so it doesnt work grammatically.

So, what do you think about a new command:

echo <<EoCMDBlk
open kms-stream
class DRM_UT_CORE +T  # global
class DRM_UT_KMS +T:kms-stream
EoCMDBlk \
> /proc/dynamic/debug

this allows tracking names, assigning ids, erroring when all used,
and validating names > control
without exposing the numbers.

the open/close changes are (would be) persistent

the thing it doesnt allow is pre-selecting the destination,
then arming it later with a +T

so it doesnt (wouldnt) play super-nice with
echo 0x1F > /sys/module/drm/parameters/debug_trace

that said, we can preset the dst:

echo <<EoCMDBlk
open drm-kms-stream
open drm-core-stream
class DRM_UT_CORE -T:drm-core-stream
class DRM_UT_KMS -T:drm-kms-stream
EoCMDBlk \
> /proc/dynamic/debug

then enable whatever is preset selectively:

echo $I_forgot_the_bit > /sys/module/drm/parameters/debug_trace
OR
echo class DRM_UT_KMS +T > /proc/dynamic/debug







> Please note that output of debug logs to syslog (p flag) and trace
> (T flag) can be independently enabled/disabled for each callsite.
>

so its the specific wording I previously grumbled about, I think.

>
>
> Jim I took the liberty and based my work on your patches you pointed me
> to https://github.com/jimc/linux/tree/dd-kitchen-sink. I picked up
> the commits relevant to trace from the dd-kitchen-sink branch.
> The only changes I introduced in your commits were related to checkpatch
> complains. There are two errors still left:

Bah - macros !
I'll look at your diffs in git :-)

>
> 1)
> ERROR: need consistent spacing around '*' (ctx:WxV)
> 140: FILE: lib/dynamic_debug.c:1070:
> +                                 va_list *args)
>
> Which seems to be a false positive to me.
>
> 2)
> ERROR: Macros with complex values should be enclosed in parentheses
> 62: FILE: include/trace/stages/stage3_trace_output.h:12:
> +#define TP_printk_no_nl(fmt, args...) fmt, args
>
> I have not figured out how to fix it.

those 2  no_nl   patches were pretty exploratory,
IIRC, Steve was inclined to add the \n  when not already in the format.
It would be variation-proof


>
> Changes:
> V1) Major rework after receiving feedback in
> https://lore.kernel.org/all/20230915154856.1896062-1-lb@semihalf.com/
>
> Jim Cromie (7):
>   dyndbg: add _DPRINTK_FLAGS_ENABLED
>   dyndbg: add _DPRINTK_FLAGS_TRACE
>   dyndbg: add write-events-to-tracefs code
>   dyndbg: add 2 trace-events: pr_debug, dev_dbg
>   tracefs: add TP_printk_no_nl - RFC
>   trace: use TP_printk_no_nl in dyndbg:prdbg,devdbg
>   dyndbg: repack struct _ddebug
>
> Łukasz Bartosik (5):
>   dyndbg: move flags field to a new structure
>   dyndbg: add trace destination field to _ddebug
>   dyndbg: add processing of T(race) flag argument
>   dyndbg: write debug logs to trace instance
>   dyndbg: add trace support for hexdump
>
>  .../admin-guide/dynamic-debug-howto.rst       |   5 +-
>  MAINTAINERS                                   |   1 +
>  include/linux/dynamic_debug.h                 |  57 ++-
>  include/trace/events/dyndbg.h                 |  54 +++
>  include/trace/stages/stage3_trace_output.h    |   3 +
>  include/trace/stages/stage7_class_define.h    |   3 +
>  lib/Kconfig.debug                             |   1 +
>  lib/dynamic_debug.c                           | 414 +++++++++++++++---
>  8 files changed, 465 insertions(+), 73 deletions(-)
>  create mode 100644 include/trace/events/dyndbg.h
>
> --
> 2.42.0.869.gea05f2083d-goog
>

Re: [PATCH v1 00/12] dyndbg: add support for writing debug logs to trace

[Łukasz Bartosik]

sob., 4 lis 2023 o 02:25 <jim.cromie@gmail.com> napisał(a):
>
> Hi Łukasz,
>
> can I haz a git remote url ?
> no webmail antics that way.
>

Here you are https://chromium.googlesource.com/chromiumos/third_party/kernel/+log/refs/sandbox/ukaszb/dyndbg_trace_v1

> On Fri, Nov 3, 2023 at 7:10 AM Łukasz Bartosik <lb@semihalf.com> wrote:
> >
> > Add support for writing debug logs to trace events and trace instances.
> > The rationale behing this feature is to be able to redirect debug logs
> > (per each callsite indivdually) to trace to aid in debugging. The debug
> > logs output to trace can be enabled with T flag. Additionally trace
> > destination can be provided to the T flag after ":". The trace destination
> > field is used to determine where debug logs will be written. Setting trace
> > destination value to 0 (default) enables output to prdbg and devdbg trace
>
> isnt +p independent of dest var ?  its just "on" to syslog.
>

Yes +p would be independent from +T so that a given callsite debug log
could be written both to syslog and trace.

> > events. Setting trace destination value to a value in range of [1..255]
> > enables output to trace instance identified by trace destination value.
> > For example when trace destination value is 2 then debug logs will
> > be written to <debugfs>/tracing/instances/dyndbg_inst_2 instance.
> >
> > Usage examples:
> >
> > localhost ~ # echo "module thunderbolt =pT:7" >
> >                                 <debugfs>/dynamic_debug/control
> >
> > This will enable output of debug logs to trace instance
> > <debugfs>/tracing/instances/dyndbg_inst_7 and debug logs will
> > be written to the syslog also because p flag is set.
> >
> > localhost ~ # echo "module thunderbolt =pT:7,l" >
> >                                 <debugfs>/dynamic_debug/control
> >
> > When trace destination is followed by another flag then trace
> > destination has to be followed by ",".
> >
> > localhost ~ # echo "module thunderbolt =pTl" >
> >                                 <debugfs>/dynamic_debug/control
> >
> > When trace destination is not provided explicitly then its value
> > defaults to 0. In this case debug logs will be written to the prdbg
> > and devdbg trace events.
> >
> > localhost ~ # echo "module thunderbolt =T:25" >
> >                                 <debugfs>/dynamic_debug/control
> >
> > This will enable output of debug logs to trace instance
> > <debugfs>/tracing/instances/dyndbg_inst_25 with debug logs output
> > to syslog disabled.
> >
> > Given trace instance will not be initialized until debug logs are
> > requested to be written to it and afer init it will persist until
> > reboot.
> >
>
> that (delayed init) might be a problem,
> user side will have to look for the appearance of traces ?
>

With open/close commands you proposed this will become obsolete.

> Also, I have some reservations about exposing numeric destinations -
> the user(space) must then decide/coordinate what dest-number
> is used for which instance/purpose.
>
> It would be fine for 1 customer, but might be a little tedious for many,
> who now have to coordinate.  A bit like a shared/party line in the early
> days of rural telephone.
>

Nice comparison :). But it was pretty "socializing" as everyone could
hear a conversation which took place on such a line.

> As I recall, an early early version of classmaps used numeric classes,
> (taken straight from drm_debug_category).
> As Jason noted (more diplomatically than I assimilated)
> it was kind of arbitrary and obscure/obtuse/unhelpful numbering.
>
> It is why I added classnames, with the bonus that
> the name->num mapping was also a validation step
> against known CLASSMAP_DEFINE-itions
> (if you knew DRM drivers knew "DRM_KMS_CORE",
>  you knew what you were asking for)
>
> Your earlier version had a dest keyword which maybe fits better with this point.
> that said, it was in a selector position, so it doesnt work grammatically.
>
> So, what do you think about a new command:
>
> echo <<EoCMDBlk
> open kms-stream
> class DRM_UT_CORE +T  # global
> class DRM_UT_KMS +T:kms-stream
> EoCMDBlk \
> > /proc/dynamic/debug
>
> this allows tracking names, assigning ids, erroring when all used,
> and validating names > control
> without exposing the numbers.
>
> the open/close changes are (would be) persistent
>
> the thing it doesnt allow is pre-selecting the destination,
> then arming it later with a +T
>
> so it doesnt (wouldnt) play super-nice with
> echo 0x1F > /sys/module/drm/parameters/debug_trace
>
> that said, we can preset the dst:
>
> echo <<EoCMDBlk
> open drm-kms-stream
> open drm-core-stream
> class DRM_UT_CORE -T:drm-core-stream
> class DRM_UT_KMS -T:drm-kms-stream
> EoCMDBlk \
> > /proc/dynamic/debug
>
> then enable whatever is preset selectively:
>
> echo $I_forgot_the_bit > /sys/module/drm/parameters/debug_trace
> OR
> echo class DRM_UT_KMS +T > /proc/dynamic/debug
>

I agree that numbers are not very meaningful, I asked question related to your
proposal in revisited comment.


>
>
>
>
>
>
> > Please note that output of debug logs to syslog (p flag) and trace
> > (T flag) can be independently enabled/disabled for each callsite.
> >
>
> so its the specific wording I previously grumbled about, I think.
>
> >
> >
> > Jim I took the liberty and based my work on your patches you pointed me
> > to https://github.com/jimc/linux/tree/dd-kitchen-sink. I picked up
> > the commits relevant to trace from the dd-kitchen-sink branch.
> > The only changes I introduced in your commits were related to checkpatch
> > complains. There are two errors still left:
>
> Bah - macros !
> I'll look at your diffs in git :-)
>
> >
> > 1)
> > ERROR: need consistent spacing around '*' (ctx:WxV)
> > 140: FILE: lib/dynamic_debug.c:1070:
> > +                                 va_list *args)
> >
> > Which seems to be a false positive to me.
> >
> > 2)
> > ERROR: Macros with complex values should be enclosed in parentheses
> > 62: FILE: include/trace/stages/stage3_trace_output.h:12:
> > +#define TP_printk_no_nl(fmt, args...) fmt, args
> >
> > I have not figured out how to fix it.
>
> those 2  no_nl   patches were pretty exploratory,
> IIRC, Steve was inclined to add the \n  when not already in the format.
> It would be variation-proof
>
>
> >
> > Changes:
> > V1) Major rework after receiving feedback in
> > https://lore.kernel.org/all/20230915154856.1896062-1-lb@semihalf.com/
> >
> > Jim Cromie (7):
> >   dyndbg: add _DPRINTK_FLAGS_ENABLED
> >   dyndbg: add _DPRINTK_FLAGS_TRACE
> >   dyndbg: add write-events-to-tracefs code
> >   dyndbg: add 2 trace-events: pr_debug, dev_dbg
> >   tracefs: add TP_printk_no_nl - RFC
> >   trace: use TP_printk_no_nl in dyndbg:prdbg,devdbg
> >   dyndbg: repack struct _ddebug
> >
> > Łukasz Bartosik (5):
> >   dyndbg: move flags field to a new structure
> >   dyndbg: add trace destination field to _ddebug
> >   dyndbg: add processing of T(race) flag argument
> >   dyndbg: write debug logs to trace instance
> >   dyndbg: add trace support for hexdump
> >
> >  .../admin-guide/dynamic-debug-howto.rst       |   5 +-
> >  MAINTAINERS                                   |   1 +
> >  include/linux/dynamic_debug.h                 |  57 ++-
> >  include/trace/events/dyndbg.h                 |  54 +++
> >  include/trace/stages/stage3_trace_output.h    |   3 +
> >  include/trace/stages/stage7_class_define.h    |   3 +
> >  lib/Kconfig.debug                             |   1 +
> >  lib/dynamic_debug.c                           | 414 +++++++++++++++---
> >  8 files changed, 465 insertions(+), 73 deletions(-)
> >  create mode 100644 include/trace/events/dyndbg.h
> >
> > --
> > 2.42.0.869.gea05f2083d-goog
> >