linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] dyndbg: add source file name to prefix
@ 2023-01-30  2:01 Thomas Weißschuh
  2023-01-30  2:01 ` [PATCH 1/3] dyndbg: constify opt_array Thomas Weißschuh
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Thomas Weißschuh @ 2023-01-30  2:01 UTC (permalink / raw)
  To: Jason Baron, Jim Cromie, Jonathan Corbet
  Cc: linux-kernel, linux-doc, Thomas Weißschuh

Currently dyndbg has no facility to print the source file name of a
debug statement.
Without the source file the line number is of limited use.
Also the dyndbg control file uses the filename as the primary index, so
having it in the logmessage makes the relation clearer.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
Thomas Weißschuh (3):
      dyndbg: constify opt_array
      dyndbg: increase PREFIX_SIZE to 128
      dyndbg: add source filename to prefix

 Documentation/admin-guide/dynamic-debug-howto.rst | 5 +++--
 include/linux/dynamic_debug.h                     | 4 +++-
 lib/dynamic_debug.c                               | 8 ++++++--
 3 files changed, 12 insertions(+), 5 deletions(-)
---
base-commit: 6d796c50f84ca79f1722bb131799e5a5710c4700
change-id: 20221223-dyndbg-filename-02e0879dae4b

Best regards,
-- 
Thomas Weißschuh <linux@weissschuh.net>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/3] dyndbg: constify opt_array
  2023-01-30  2:01 [PATCH 0/3] dyndbg: add source file name to prefix Thomas Weißschuh
@ 2023-01-30  2:01 ` Thomas Weißschuh
  2023-01-30  2:01 ` [PATCH 2/3] dyndbg: increase PREFIX_SIZE to 128 Thomas Weißschuh
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Thomas Weißschuh @ 2023-01-30  2:01 UTC (permalink / raw)
  To: Jason Baron, Jim Cromie, Jonathan Corbet
  Cc: linux-kernel, linux-doc, Thomas Weißschuh

It is never modified, so mark it const.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 lib/dynamic_debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 009f2ead09c1..6915e088bed6 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -88,7 +88,7 @@ static inline const char *trim_prefix(const char *path)
 	return path + skip;
 }
 
-static struct { unsigned flag:8; char opt_char; } opt_array[] = {
+static const struct { unsigned flag:8; char opt_char; } opt_array[] = {
 	{ _DPRINTK_FLAGS_PRINT, 'p' },
 	{ _DPRINTK_FLAGS_INCL_MODNAME, 'm' },
 	{ _DPRINTK_FLAGS_INCL_FUNCNAME, 'f' },

-- 
2.39.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/3] dyndbg: increase PREFIX_SIZE to 128
  2023-01-30  2:01 [PATCH 0/3] dyndbg: add source file name to prefix Thomas Weißschuh
  2023-01-30  2:01 ` [PATCH 1/3] dyndbg: constify opt_array Thomas Weißschuh
@ 2023-01-30  2:01 ` Thomas Weißschuh
  2023-01-30  2:01 ` [PATCH 3/3] dyndbg: add source filename to prefix Thomas Weißschuh
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Thomas Weißschuh @ 2023-01-30  2:01 UTC (permalink / raw)
  To: Jason Baron, Jim Cromie, Jonathan Corbet
  Cc: linux-kernel, linux-doc, Thomas Weißschuh

A follow-up patch will add the possibility to print the filename as part
of the prefix.
Increase the maximum prefix size to accommodate this.

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 lib/dynamic_debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 6915e088bed6..e96ea427d8af 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -808,7 +808,7 @@ const struct kernel_param_ops param_ops_dyndbg_classes = {
 };
 EXPORT_SYMBOL(param_ops_dyndbg_classes);
 
-#define PREFIX_SIZE 64
+#define PREFIX_SIZE 128
 
 static int remaining(int wrote)
 {

-- 
2.39.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/3] dyndbg: add source filename to prefix
  2023-01-30  2:01 [PATCH 0/3] dyndbg: add source file name to prefix Thomas Weißschuh
  2023-01-30  2:01 ` [PATCH 1/3] dyndbg: constify opt_array Thomas Weißschuh
  2023-01-30  2:01 ` [PATCH 2/3] dyndbg: increase PREFIX_SIZE to 128 Thomas Weißschuh
@ 2023-01-30  2:01 ` Thomas Weißschuh
  2023-02-03 16:45   ` Jason Baron
  2023-03-20 22:13 ` [PATCH 0/3] dyndbg: add source file name " Thomas Weißschuh
  2023-06-27 14:30 ` Thomas Weißschuh
  4 siblings, 1 reply; 12+ messages in thread
From: Thomas Weißschuh @ 2023-01-30  2:01 UTC (permalink / raw)
  To: Jason Baron, Jim Cromie, Jonathan Corbet
  Cc: linux-kernel, linux-doc, Thomas Weißschuh

Printing the line number without the file is of limited usefulness.

Knowing the filename also makes it also easier to relate the logged
information to the controlfile.

Example:

    # modprobe test_dynamic_debug
    # echo 'file test_dynamic_debug.c =pfsl' > /proc/dynamic_debug/control
    # echo 1 > /sys/module/test_dynamic_debug/parameters/do_prints
    # dmesg | tail -2
    [   71.802212] do_cats:lib/test_dynamic_debug.c:103: test_dd: doing categories
    [   71.802227] do_levels:lib/test_dynamic_debug.c:123: test_dd: doing levels

Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
 Documentation/admin-guide/dynamic-debug-howto.rst | 5 +++--
 include/linux/dynamic_debug.h                     | 4 +++-
 lib/dynamic_debug.c                               | 4 ++++
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
index faa22f77847a..f9fa8163fba6 100644
--- a/Documentation/admin-guide/dynamic-debug-howto.rst
+++ b/Documentation/admin-guide/dynamic-debug-howto.rst
@@ -216,13 +216,14 @@ The flags are::
   t    Include thread ID, or <intr>
   m    Include module name
   f    Include the function name
+  s    Include the source file name
   l    Include line number
 
 For ``print_hex_dump_debug()`` and ``print_hex_dump_bytes()``, only
 the ``p`` flag has meaning, other flags are ignored.
 
-Note the regexp ``^[-+=][flmpt_]+$`` matches a flags specification.
-To clear all flags at once, use ``=_`` or ``-flmpt``.
+Note the regexp ``^[-+=][fslmpt_]+$`` matches a flags specification.
+To clear all flags at once, use ``=_`` or ``-fslmpt``.
 
 
 Debug messages during Boot Process
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 41682278d2e8..0c77105d583c 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -37,10 +37,12 @@ struct _ddebug {
 #define _DPRINTK_FLAGS_INCL_FUNCNAME	(1<<2)
 #define _DPRINTK_FLAGS_INCL_LINENO	(1<<3)
 #define _DPRINTK_FLAGS_INCL_TID		(1<<4)
+#define _DPRINTK_FLAGS_INCL_SOURCENAME	(1<<5)
 
 #define _DPRINTK_FLAGS_INCL_ANY		\
 	(_DPRINTK_FLAGS_INCL_MODNAME | _DPRINTK_FLAGS_INCL_FUNCNAME |\
-	 _DPRINTK_FLAGS_INCL_LINENO  | _DPRINTK_FLAGS_INCL_TID)
+	 _DPRINTK_FLAGS_INCL_LINENO  | _DPRINTK_FLAGS_INCL_TID |\
+	 _DPRINTK_FLAGS_INCL_SOURCENAME)
 
 #if defined DEBUG
 #define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINT
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index e96ea427d8af..fa7418e35197 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -92,6 +92,7 @@ static const struct { unsigned flag:8; char opt_char; } opt_array[] = {
 	{ _DPRINTK_FLAGS_PRINT, 'p' },
 	{ _DPRINTK_FLAGS_INCL_MODNAME, 'm' },
 	{ _DPRINTK_FLAGS_INCL_FUNCNAME, 'f' },
+	{ _DPRINTK_FLAGS_INCL_SOURCENAME, 's' },
 	{ _DPRINTK_FLAGS_INCL_LINENO, 'l' },
 	{ _DPRINTK_FLAGS_INCL_TID, 't' },
 	{ _DPRINTK_FLAGS_NONE, '_' },
@@ -836,6 +837,9 @@ static char *__dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
 	if (desc->flags & _DPRINTK_FLAGS_INCL_FUNCNAME)
 		pos += snprintf(buf + pos, remaining(pos), "%s:",
 				desc->function);
+	if (desc->flags & _DPRINTK_FLAGS_INCL_SOURCENAME)
+		pos += snprintf(buf + pos, remaining(pos), "%s:",
+				trim_prefix(desc->filename));
 	if (desc->flags & _DPRINTK_FLAGS_INCL_LINENO)
 		pos += snprintf(buf + pos, remaining(pos), "%d:",
 				desc->lineno);

-- 
2.39.1


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] dyndbg: add source filename to prefix
  2023-01-30  2:01 ` [PATCH 3/3] dyndbg: add source filename to prefix Thomas Weißschuh
@ 2023-02-03 16:45   ` Jason Baron
  2023-02-04 14:49     ` linux
  0 siblings, 1 reply; 12+ messages in thread
From: Jason Baron @ 2023-02-03 16:45 UTC (permalink / raw)
  To: Thomas Weißschuh, Jim Cromie, Jonathan Corbet
  Cc: linux-kernel, linux-doc

Hi Thomas,

Thanks for this series, this series is fine with me:
Acked-by: Jason Baron <jbaron@akamai.com>

Your comment about making the output more relatable to the control file 
made me think if we should try and make the logged output look more like 
the control file:

# cat /proc/dynamic_debug/control
# filename:lineno [module]function flags format

So for your example, I think that would look like:

[   71.802212] lib/test_dynamic_debug.c:103 do_cats: test_dd: doing 
categories
[   71.802227] lib/test_dynamic_debug.c:123 do_levels: doing levels

But even if we think it looks better, there maybe too many dependencies 
on the current output format...

Thanks,

-Jason

On 1/29/23 9:01 PM, Thomas Weißschuh wrote:
> Printing the line number without the file is of limited usefulness.
> 
> Knowing the filename also makes it also easier to relate the logged
> information to the controlfile.
> 
> Example:
> 
>      # modprobe test_dynamic_debug
>      # echo 'file test_dynamic_debug.c =pfsl' > /proc/dynamic_debug/control
>      # echo 1 > /sys/module/test_dynamic_debug/parameters/do_prints
>      # dmesg | tail -2
>      [   71.802212] do_cats:lib/test_dynamic_debug.c:103: test_dd: doing categories
>      [   71.802227] do_levels:lib/test_dynamic_debug.c:123: test_dd: doing levels
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
>   Documentation/admin-guide/dynamic-debug-howto.rst | 5 +++--
>   include/linux/dynamic_debug.h                     | 4 +++-
>   lib/dynamic_debug.c                               | 4 ++++
>   3 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
> index faa22f77847a..f9fa8163fba6 100644
> --- a/Documentation/admin-guide/dynamic-debug-howto.rst
> +++ b/Documentation/admin-guide/dynamic-debug-howto.rst
> @@ -216,13 +216,14 @@ The flags are::
>     t    Include thread ID, or <intr>
>     m    Include module name
>     f    Include the function name
> +  s    Include the source file name
>     l    Include line number
>   
>   For ``print_hex_dump_debug()`` and ``print_hex_dump_bytes()``, only
>   the ``p`` flag has meaning, other flags are ignored.
>   
> -Note the regexp ``^[-+=][flmpt_]+$`` matches a flags specification.
> -To clear all flags at once, use ``=_`` or ``-flmpt``.
> +Note the regexp ``^[-+=][fslmpt_]+$`` matches a flags specification.
> +To clear all flags at once, use ``=_`` or ``-fslmpt``.
>   
>   
>   Debug messages during Boot Process
> diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
> index 41682278d2e8..0c77105d583c 100644
> --- a/include/linux/dynamic_debug.h
> +++ b/include/linux/dynamic_debug.h
> @@ -37,10 +37,12 @@ struct _ddebug {
>   #define _DPRINTK_FLAGS_INCL_FUNCNAME	(1<<2)
>   #define _DPRINTK_FLAGS_INCL_LINENO	(1<<3)
>   #define _DPRINTK_FLAGS_INCL_TID		(1<<4)
> +#define _DPRINTK_FLAGS_INCL_SOURCENAME	(1<<5)
>   
>   #define _DPRINTK_FLAGS_INCL_ANY		\
>   	(_DPRINTK_FLAGS_INCL_MODNAME | _DPRINTK_FLAGS_INCL_FUNCNAME |\
> -	 _DPRINTK_FLAGS_INCL_LINENO  | _DPRINTK_FLAGS_INCL_TID)
> +	 _DPRINTK_FLAGS_INCL_LINENO  | _DPRINTK_FLAGS_INCL_TID |\
> +	 _DPRINTK_FLAGS_INCL_SOURCENAME)
>   
>   #if defined DEBUG
>   #define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINT
> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> index e96ea427d8af..fa7418e35197 100644
> --- a/lib/dynamic_debug.c
> +++ b/lib/dynamic_debug.c
> @@ -92,6 +92,7 @@ static const struct { unsigned flag:8; char opt_char; } opt_array[] = {
>   	{ _DPRINTK_FLAGS_PRINT, 'p' },
>   	{ _DPRINTK_FLAGS_INCL_MODNAME, 'm' },
>   	{ _DPRINTK_FLAGS_INCL_FUNCNAME, 'f' },
> +	{ _DPRINTK_FLAGS_INCL_SOURCENAME, 's' },
>   	{ _DPRINTK_FLAGS_INCL_LINENO, 'l' },
>   	{ _DPRINTK_FLAGS_INCL_TID, 't' },
>   	{ _DPRINTK_FLAGS_NONE, '_' },
> @@ -836,6 +837,9 @@ static char *__dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
>   	if (desc->flags & _DPRINTK_FLAGS_INCL_FUNCNAME)
>   		pos += snprintf(buf + pos, remaining(pos), "%s:",
>   				desc->function);
> +	if (desc->flags & _DPRINTK_FLAGS_INCL_SOURCENAME)
> +		pos += snprintf(buf + pos, remaining(pos), "%s:",
> +				trim_prefix(desc->filename));
>   	if (desc->flags & _DPRINTK_FLAGS_INCL_LINENO)
>   		pos += snprintf(buf + pos, remaining(pos), "%d:",
>   				desc->lineno);
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] dyndbg: add source filename to prefix
  2023-02-03 16:45   ` Jason Baron
@ 2023-02-04 14:49     ` linux
  2023-03-22 15:54       ` jim.cromie
  0 siblings, 1 reply; 12+ messages in thread
From: linux @ 2023-02-04 14:49 UTC (permalink / raw)
  To: Jason Baron
  Cc: Thomas Weißschuh, Jim Cromie, Jonathan Corbet, linux-kernel,
	linux-doc

Hi Jason,


Feb 3, 2023 10:45:49 Jason Baron <jbaron@akamai.com>:

> Hi Thomas,
>
> Thanks for this series, this series is fine with me:
> Acked-by: Jason Baron <jbaron@akamai.com>

Thanks!

> Your comment about making the output more relatable to the control file made me think if we should try and make the logged output look more like the control file:
>
> # cat /proc/dynamic_debug/control
> # filename:lineno [module]function flags format
>
> So for your example, I think that would look like:
>
> [   71.802212] lib/test_dynamic_debug.c:103 do_cats: test_dd: doing categories
> [   71.802227] lib/test_dynamic_debug.c:123 do_levels: doing levels
>
> But even if we think it looks better, there maybe too many dependencies on the current output format...

I agree on both points.

An alternative could be a new flag that prints the
full format from the control file.
The control file even has a format header that
tools could use to parse out the fields, making it
extensible.

Not sure it's worth it though.
And it should be in addition to this series in my
opinion.

Thomas

> Thanks,
>
> -Jason
>
> On 1/29/23 9:01 PM, Thomas Weißschuh wrote:
>> Printing the line number without the file is of limited usefulness.
>> Knowing the filename also makes it also easier to relate the logged
>> information to the controlfile.
>> Example:
>>      # modprobe test_dynamic_debug
>>      # echo 'file test_dynamic_debug.c =pfsl' > /proc/dynamic_debug/control
>>      # echo 1 > /sys/module/test_dynamic_debug/parameters/do_prints
>>      # dmesg | tail -2
>>      [   71.802212] do_cats:lib/test_dynamic_debug.c:103: test_dd: doing categories
>>      [   71.802227] do_levels:lib/test_dynamic_debug.c:123: test_dd: doing levels
>> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
>> ---
>>   Documentation/admin-guide/dynamic-debug-howto.rst | 5 +++--
>>   include/linux/dynamic_debug.h                     | 4 +++-
>>   lib/dynamic_debug.c                               | 4 ++++
>>   3 files changed, 10 insertions(+), 3 deletions(-)
>> diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
>> index faa22f77847a..f9fa8163fba6 100644
>> --- a/Documentation/admin-guide/dynamic-debug-howto.rst
>> +++ b/Documentation/admin-guide/dynamic-debug-howto.rst
>> @@ -216,13 +216,14 @@ The flags are::
>>     t    Include thread ID, or <intr>
>>     m    Include module name
>>     f    Include the function name
>> +  s    Include the source file name
>>     l    Include line number
>>     For ``print_hex_dump_debug()`` and ``print_hex_dump_bytes()``, only
>>   the ``p`` flag has meaning, other flags are ignored.
>>   -Note the regexp ``^[-+=][flmpt_]+$`` matches a flags specification.
>> -To clear all flags at once, use ``=_`` or ``-flmpt``.
>> +Note the regexp ``^[-+=][fslmpt_]+$`` matches a flags specification.
>> +To clear all flags at once, use ``=_`` or ``-fslmpt``.
>>       Debug messages during Boot Process
>> diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
>> index 41682278d2e8..0c77105d583c 100644
>> --- a/include/linux/dynamic_debug.h
>> +++ b/include/linux/dynamic_debug.h
>> @@ -37,10 +37,12 @@ struct _ddebug {
>>   #define _DPRINTK_FLAGS_INCL_FUNCNAME  (1<<2)
>>   #define _DPRINTK_FLAGS_INCL_LINENO    (1<<3)
>>   #define _DPRINTK_FLAGS_INCL_TID       (1<<4)
>> +#define _DPRINTK_FLAGS_INCL_SOURCENAME (1<<5)
>>     #define _DPRINTK_FLAGS_INCL_ANY     \
>>     (_DPRINTK_FLAGS_INCL_MODNAME | _DPRINTK_FLAGS_INCL_FUNCNAME |\
>> -    _DPRINTK_FLAGS_INCL_LINENO  | _DPRINTK_FLAGS_INCL_TID)
>> +    _DPRINTK_FLAGS_INCL_LINENO  | _DPRINTK_FLAGS_INCL_TID |\
>> +    _DPRINTK_FLAGS_INCL_SOURCENAME)
>>     #if defined DEBUG
>>   #define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINT
>> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
>> index e96ea427d8af..fa7418e35197 100644
>> --- a/lib/dynamic_debug.c
>> +++ b/lib/dynamic_debug.c
>> @@ -92,6 +92,7 @@ static const struct { unsigned flag:8; char opt_char; } opt_array[] = {
>>     { _DPRINTK_FLAGS_PRINT, 'p' },
>>     { _DPRINTK_FLAGS_INCL_MODNAME, 'm' },
>>     { _DPRINTK_FLAGS_INCL_FUNCNAME, 'f' },
>> +   { _DPRINTK_FLAGS_INCL_SOURCENAME, 's' },
>>     { _DPRINTK_FLAGS_INCL_LINENO, 'l' },
>>     { _DPRINTK_FLAGS_INCL_TID, 't' },
>>     { _DPRINTK_FLAGS_NONE, '_' },
>> @@ -836,6 +837,9 @@ static char *__dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
>>     if (desc->flags & _DPRINTK_FLAGS_INCL_FUNCNAME)
>>         pos += snprintf(buf + pos, remaining(pos), "%s:",
>>                 desc->function);
>> +   if (desc->flags & _DPRINTK_FLAGS_INCL_SOURCENAME)
>> +       pos += snprintf(buf + pos, remaining(pos), "%s:",
>> +               trim_prefix(desc->filename));
>>     if (desc->flags & _DPRINTK_FLAGS_INCL_LINENO)
>>         pos += snprintf(buf + pos, remaining(pos), "%d:",
>>                 desc->lineno);
>>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] dyndbg: add source file name to prefix
  2023-01-30  2:01 [PATCH 0/3] dyndbg: add source file name to prefix Thomas Weißschuh
                   ` (2 preceding siblings ...)
  2023-01-30  2:01 ` [PATCH 3/3] dyndbg: add source filename to prefix Thomas Weißschuh
@ 2023-03-20 22:13 ` Thomas Weißschuh
  2023-06-27 14:30 ` Thomas Weißschuh
  4 siblings, 0 replies; 12+ messages in thread
From: Thomas Weißschuh @ 2023-03-20 22:13 UTC (permalink / raw)
  To: Jim Cromie; +Cc: linux-kernel, linux-doc, Jonathan Corbet, Jason Baron

Hi Jim,

did you get a chance to look at this yet?

Thanks,
Thomas

On Mon, Jan 30, 2023 at 02:01:17AM +0000, Thomas Weißschuh wrote:
> Currently dyndbg has no facility to print the source file name of a
> debug statement.
> Without the source file the line number is of limited use.
> Also the dyndbg control file uses the filename as the primary index, so
> having it in the logmessage makes the relation clearer.
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
> Thomas Weißschuh (3):
>       dyndbg: constify opt_array
>       dyndbg: increase PREFIX_SIZE to 128
>       dyndbg: add source filename to prefix
> 
>  Documentation/admin-guide/dynamic-debug-howto.rst | 5 +++--
>  include/linux/dynamic_debug.h                     | 4 +++-
>  lib/dynamic_debug.c                               | 8 ++++++--
>  3 files changed, 12 insertions(+), 5 deletions(-)
> ---
> base-commit: 6d796c50f84ca79f1722bb131799e5a5710c4700
> change-id: 20221223-dyndbg-filename-02e0879dae4b
> 
> Best regards,
> -- 
> Thomas Weißschuh <linux@weissschuh.net>
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/3] dyndbg: add source filename to prefix
  2023-02-04 14:49     ` linux
@ 2023-03-22 15:54       ` jim.cromie
  0 siblings, 0 replies; 12+ messages in thread
From: jim.cromie @ 2023-03-22 15:54 UTC (permalink / raw)
  To: linux; +Cc: Jason Baron, Jonathan Corbet, linux-kernel, linux-doc

On Sat, Feb 4, 2023 at 7:49 AM <linux@weissschuh.net> wrote:
>
> Hi Jason,
>
>
> Feb 3, 2023 10:45:49 Jason Baron <jbaron@akamai.com>:
>
> > Hi Thomas,
> >
> > Thanks for this series, this series is fine with me:
> > Acked-by: Jason Baron <jbaron@akamai.com>
>
> Thanks!
>
> > Your comment about making the output more relatable to the control file made me think if we should try and make the logged output look more like the control file:
> >
> > # cat /proc/dynamic_debug/control
> > # filename:lineno [module]function flags format
> >
> > So for your example, I think that would look like:
> >
> > [   71.802212] lib/test_dynamic_debug.c:103 do_cats: test_dd: doing categories
> > [   71.802227] lib/test_dynamic_debug.c:123 do_levels: doing levels
> >
> > But even if we think it looks better, there maybe too many dependencies on the current output format...
>
> I agree on both points.
>
> An alternative could be a new flag that prints the
> full format from the control file.
> The control file even has a format header that
> tools could use to parse out the fields, making it
> extensible.
>
> Not sure it's worth it though.
> And it should be in addition to this series in my
> opinion.

Boy howdy, I was hoping you were gonna do it. :-)

I think we need the 's' flag.
it displays properly as "$src:$line"

I agree that shuffling the "$src:$line" part before the "$mod:$func"
part would be ideal
(reluctantly) its a separate patch

Acked-by: Jim Cromie <jim.cromie@gmail.com>

(resisting the urge to hijack this thread for "designing")

>
> Thomas
>
> > Thanks,
> >
> > -Jason
> >
> > On 1/29/23 9:01 PM, Thomas Weißschuh wrote:
> >> Printing the line number without the file is of limited usefulness.
> >> Knowing the filename also makes it also easier to relate the logged
> >> information to the controlfile.
> >> Example:
> >>      # modprobe test_dynamic_debug
> >>      # echo 'file test_dynamic_debug.c =pfsl' > /proc/dynamic_debug/control
> >>      # echo 1 > /sys/module/test_dynamic_debug/parameters/do_prints
> >>      # dmesg | tail -2
> >>      [   71.802212] do_cats:lib/test_dynamic_debug.c:103: test_dd: doing categories
> >>      [   71.802227] do_levels:lib/test_dynamic_debug.c:123: test_dd: doing levels
> >> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> >> ---
> >>   Documentation/admin-guide/dynamic-debug-howto.rst | 5 +++--
> >>   include/linux/dynamic_debug.h                     | 4 +++-
> >>   lib/dynamic_debug.c                               | 4 ++++
> >>   3 files changed, 10 insertions(+), 3 deletions(-)
> >> diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
> >> index faa22f77847a..f9fa8163fba6 100644
> >> --- a/Documentation/admin-guide/dynamic-debug-howto.rst
> >> +++ b/Documentation/admin-guide/dynamic-debug-howto.rst
> >> @@ -216,13 +216,14 @@ The flags are::
> >>     t    Include thread ID, or <intr>
> >>     m    Include module name
> >>     f    Include the function name
> >> +  s    Include the source file name
> >>     l    Include line number
> >>     For ``print_hex_dump_debug()`` and ``print_hex_dump_bytes()``, only
> >>   the ``p`` flag has meaning, other flags are ignored.
> >>   -Note the regexp ``^[-+=][flmpt_]+$`` matches a flags specification.
> >> -To clear all flags at once, use ``=_`` or ``-flmpt``.
> >> +Note the regexp ``^[-+=][fslmpt_]+$`` matches a flags specification.
> >> +To clear all flags at once, use ``=_`` or ``-fslmpt``.
> >>       Debug messages during Boot Process
> >> diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
> >> index 41682278d2e8..0c77105d583c 100644
> >> --- a/include/linux/dynamic_debug.h
> >> +++ b/include/linux/dynamic_debug.h
> >> @@ -37,10 +37,12 @@ struct _ddebug {
> >>   #define _DPRINTK_FLAGS_INCL_FUNCNAME  (1<<2)
> >>   #define _DPRINTK_FLAGS_INCL_LINENO    (1<<3)
> >>   #define _DPRINTK_FLAGS_INCL_TID       (1<<4)
> >> +#define _DPRINTK_FLAGS_INCL_SOURCENAME (1<<5)
> >>     #define _DPRINTK_FLAGS_INCL_ANY     \
> >>     (_DPRINTK_FLAGS_INCL_MODNAME | _DPRINTK_FLAGS_INCL_FUNCNAME |\
> >> -    _DPRINTK_FLAGS_INCL_LINENO  | _DPRINTK_FLAGS_INCL_TID)
> >> +    _DPRINTK_FLAGS_INCL_LINENO  | _DPRINTK_FLAGS_INCL_TID |\
> >> +    _DPRINTK_FLAGS_INCL_SOURCENAME)
> >>     #if defined DEBUG
> >>   #define _DPRINTK_FLAGS_DEFAULT _DPRINTK_FLAGS_PRINT
> >> diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
> >> index e96ea427d8af..fa7418e35197 100644
> >> --- a/lib/dynamic_debug.c
> >> +++ b/lib/dynamic_debug.c
> >> @@ -92,6 +92,7 @@ static const struct { unsigned flag:8; char opt_char; } opt_array[] = {
> >>     { _DPRINTK_FLAGS_PRINT, 'p' },
> >>     { _DPRINTK_FLAGS_INCL_MODNAME, 'm' },
> >>     { _DPRINTK_FLAGS_INCL_FUNCNAME, 'f' },
> >> +   { _DPRINTK_FLAGS_INCL_SOURCENAME, 's' },
> >>     { _DPRINTK_FLAGS_INCL_LINENO, 'l' },
> >>     { _DPRINTK_FLAGS_INCL_TID, 't' },
> >>     { _DPRINTK_FLAGS_NONE, '_' },
> >> @@ -836,6 +837,9 @@ static char *__dynamic_emit_prefix(const struct _ddebug *desc, char *buf)
> >>     if (desc->flags & _DPRINTK_FLAGS_INCL_FUNCNAME)
> >>         pos += snprintf(buf + pos, remaining(pos), "%s:",
> >>                 desc->function);
> >> +   if (desc->flags & _DPRINTK_FLAGS_INCL_SOURCENAME)
> >> +       pos += snprintf(buf + pos, remaining(pos), "%s:",
> >> +               trim_prefix(desc->filename));
> >>     if (desc->flags & _DPRINTK_FLAGS_INCL_LINENO)
> >>         pos += snprintf(buf + pos, remaining(pos), "%d:",
> >>                 desc->lineno);
> >>
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] dyndbg: add source file name to prefix
  2023-01-30  2:01 [PATCH 0/3] dyndbg: add source file name to prefix Thomas Weißschuh
                   ` (3 preceding siblings ...)
  2023-03-20 22:13 ` [PATCH 0/3] dyndbg: add source file name " Thomas Weißschuh
@ 2023-06-27 14:30 ` Thomas Weißschuh
  2023-06-27 14:47   ` Greg Kroah-Hartman
  4 siblings, 1 reply; 12+ messages in thread
From: Thomas Weißschuh @ 2023-06-27 14:30 UTC (permalink / raw)
  To: Luis Chamberlain, Greg Kroah-Hartman
  Cc: Jason Baron, Jim Cromie, Jonathan Corbet, linux-kernel, linux-doc

Hi Luis and Greg,

it seems you are the ones picking up patches for dyndbg.

Could you take a look at this series?
It has Acks from both Jason and Jim.

Thanks,
Thomas

On 2023-01-30 02:01:17+0000, Thomas Weißschuh wrote:
> Currently dyndbg has no facility to print the source file name of a
> debug statement.
> Without the source file the line number is of limited use.
> Also the dyndbg control file uses the filename as the primary index, so
> having it in the logmessage makes the relation clearer.
> 
> Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
> ---
> Thomas Weißschuh (3):
>       dyndbg: constify opt_array
>       dyndbg: increase PREFIX_SIZE to 128
>       dyndbg: add source filename to prefix
> 
>  Documentation/admin-guide/dynamic-debug-howto.rst | 5 +++--
>  include/linux/dynamic_debug.h                     | 4 +++-
>  lib/dynamic_debug.c                               | 8 ++++++--
>  3 files changed, 12 insertions(+), 5 deletions(-)
> ---
> base-commit: 6d796c50f84ca79f1722bb131799e5a5710c4700
> change-id: 20221223-dyndbg-filename-02e0879dae4b
> 
> Best regards,
> -- 
> Thomas Weißschuh <linux@weissschuh.net>
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] dyndbg: add source file name to prefix
  2023-06-27 14:30 ` Thomas Weißschuh
@ 2023-06-27 14:47   ` Greg Kroah-Hartman
  2023-06-27 14:57     ` Thomas Weißschuh
  0 siblings, 1 reply; 12+ messages in thread
From: Greg Kroah-Hartman @ 2023-06-27 14:47 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Luis Chamberlain, Jason Baron, Jim Cromie, Jonathan Corbet,
	linux-kernel, linux-doc

On Tue, Jun 27, 2023 at 04:30:56PM +0200, Thomas Weißschuh wrote:
> Hi Luis and Greg,
> 
> it seems you are the ones picking up patches for dyndbg.
> 
> Could you take a look at this series?
> It has Acks from both Jason and Jim.

It's the middle of the merge window, we can't take anything new, sorry.

Care to submit it after 6.5-rc1 is out?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] dyndbg: add source file name to prefix
  2023-06-27 14:47   ` Greg Kroah-Hartman
@ 2023-06-27 14:57     ` Thomas Weißschuh
  2023-06-27 15:01       ` Greg Kroah-Hartman
  0 siblings, 1 reply; 12+ messages in thread
From: Thomas Weißschuh @ 2023-06-27 14:57 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Luis Chamberlain, Jason Baron, Jim Cromie, Jonathan Corbet,
	linux-kernel, linux-doc

On 2023-06-27 16:47:26+0200, Greg Kroah-Hartman wrote:
> On Tue, Jun 27, 2023 at 04:30:56PM +0200, Thomas Weißschuh wrote:
> > Hi Luis and Greg,
> > 
> > it seems you are the ones picking up patches for dyndbg.
> > 
> > Could you take a look at this series?
> > It has Acks from both Jason and Jim.
> 
> It's the middle of the merge window, we can't take anything new, sorry.

Just noticed this, too. Sorry for the noise.

> Care to submit it after 6.5-rc1 is out?

I'll ping again should it still apply cleanly to 6.5-rc1
othewise I'll resend.

Thanks,
Thomas

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 0/3] dyndbg: add source file name to prefix
  2023-06-27 14:57     ` Thomas Weißschuh
@ 2023-06-27 15:01       ` Greg Kroah-Hartman
  0 siblings, 0 replies; 12+ messages in thread
From: Greg Kroah-Hartman @ 2023-06-27 15:01 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Luis Chamberlain, Jason Baron, Jim Cromie, Jonathan Corbet,
	linux-kernel, linux-doc

On Tue, Jun 27, 2023 at 04:57:35PM +0200, Thomas Weißschuh wrote:
> On 2023-06-27 16:47:26+0200, Greg Kroah-Hartman wrote:
> > On Tue, Jun 27, 2023 at 04:30:56PM +0200, Thomas Weißschuh wrote:
> > > Hi Luis and Greg,
> > > 
> > > it seems you are the ones picking up patches for dyndbg.
> > > 
> > > Could you take a look at this series?
> > > It has Acks from both Jason and Jim.
> > 
> > It's the middle of the merge window, we can't take anything new, sorry.
> 
> Just noticed this, too. Sorry for the noise.
> 
> > Care to submit it after 6.5-rc1 is out?
> 
> I'll ping again should it still apply cleanly to 6.5-rc1
> othewise I'll resend.

Resend would be good as this is long gone from out inboxes...

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2023-06-27 15:01 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-30  2:01 [PATCH 0/3] dyndbg: add source file name to prefix Thomas Weißschuh
2023-01-30  2:01 ` [PATCH 1/3] dyndbg: constify opt_array Thomas Weißschuh
2023-01-30  2:01 ` [PATCH 2/3] dyndbg: increase PREFIX_SIZE to 128 Thomas Weißschuh
2023-01-30  2:01 ` [PATCH 3/3] dyndbg: add source filename to prefix Thomas Weißschuh
2023-02-03 16:45   ` Jason Baron
2023-02-04 14:49     ` linux
2023-03-22 15:54       ` jim.cromie
2023-03-20 22:13 ` [PATCH 0/3] dyndbg: add source file name " Thomas Weißschuh
2023-06-27 14:30 ` Thomas Weißschuh
2023-06-27 14:47   ` Greg Kroah-Hartman
2023-06-27 14:57     ` Thomas Weißschuh
2023-06-27 15:01       ` Greg Kroah-Hartman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).