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

Łukasz Bartosik posted 12 patches 2 years, 1 month ago
There is a newer version of this series
[PATCH v1 10/12] dyndbg: add processing of T(race) flag argument
Posted by Łukasz Bartosik 2 years, 1 month ago
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
Posted by kernel test robot 2 years, 1 month ago
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
Re: [PATCH v1 10/12] dyndbg: add processing of T(race) flag argument
Posted by jim.cromie@gmail.com 2 years, 1 month ago
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
Posted by Łukasz Bartosik 2 years, 1 month ago
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
Posted by jim.cromie@gmail.com 2 years, 1 month ago
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
Posted by Łukasz Bartosik 2 years, 1 month ago
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
Posted by jim.cromie@gmail.com 2 years, 1 month ago
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
Posted by kernel test robot 2 years, 1 month ago
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