[PATCH 01/16] dyndbg: drop trim_prefix, obsoleted by __FILE__s relative path

[PATCH 01/16] dyndbg: drop trim_prefix, obsoleted by __FILE__s relative path

[Jim Cromie]

Regarding:
commit 2b6783191da7 ("dynamic_debug: add trim_prefix() to provide source-root relative paths")
commit a73619a845d5 ("kbuild: use -fmacro-prefix-map to make __FILE__ a relative path")

2nd commit broke dynamic-debug's "file $fullpath" query form, but
nobody noticed because 1st commit trimmed prefixes from control-file
output, so the click-copy-pasting of fullpaths into new queries had
ceased; that query form became unused.

So remove the function and callers; its purpose was to strip the
prefix from __FILE__, which is now gone.  Further, there is no loss of
query selectivity, and no real value in iteratively trimming ^/\w+
prefix and testing for a match.

Also drop "file $fullpath" from docs, and tweak example to cite column
1 of control-file as valid "file $input".

cc: clang-built-linux@googlegroups.com

I built one clang-kernel a while ago to see if this patch broke
things, it did not, but I may have messed up something.

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
---
 .../admin-guide/dynamic-debug-howto.rst       | 19 +++++++++----------
 lib/dynamic_debug.c                           | 19 +++----------------
 2 files changed, 12 insertions(+), 26 deletions(-)

diff --git a/Documentation/admin-guide/dynamic-debug-howto.rst b/Documentation/admin-guide/dynamic-debug-howto.rst
index 252e5ef324e5..e011f8907116 100644
--- a/Documentation/admin-guide/dynamic-debug-howto.rst
+++ b/Documentation/admin-guide/dynamic-debug-howto.rst
@@ -62,10 +62,10 @@ statements via::
 
   nullarbor:~ # cat <debugfs>/dynamic_debug/control
   # filename:lineno [module]function flags format
-  /usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svc_rdma.c:323 [svcxprt_rdma]svc_rdma_cleanup =_ "SVCRDMA Module Removed, deregister RPC RDMA transport\012"
-  /usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svc_rdma.c:341 [svcxprt_rdma]svc_rdma_init =_ "\011max_inline       : %d\012"
-  /usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svc_rdma.c:340 [svcxprt_rdma]svc_rdma_init =_ "\011sq_depth         : %d\012"
-  /usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svc_rdma.c:338 [svcxprt_rdma]svc_rdma_init =_ "\011max_requests     : %d\012"
+  net/sunrpc/svc_rdma.c:323 [svcxprt_rdma]svc_rdma_cleanup =_ "SVCRDMA Module Removed, deregister RPC RDMA transport\012"
+  net/sunrpc/svc_rdma.c:341 [svcxprt_rdma]svc_rdma_init =_ "\011max_inline       : %d\012"
+  net/sunrpc/svc_rdma.c:340 [svcxprt_rdma]svc_rdma_init =_ "\011sq_depth         : %d\012"
+  net/sunrpc/svc_rdma.c:338 [svcxprt_rdma]svc_rdma_init =_ "\011max_requests     : %d\012"
   ...
 
 
@@ -85,7 +85,7 @@ the debug statement callsites with any non-default flags::
 
   nullarbor:~ # awk '$3 != "=_"' <debugfs>/dynamic_debug/control
   # filename:lineno [module]function flags format
-  /usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svcsock.c:1603 [sunrpc]svc_send p "svc_process: st_sendto returned %d\012"
+  net/sunrpc/svcsock.c:1603 [sunrpc]svc_send p "svc_process: st_sendto returned %d\012"
 
 Command Language Reference
 ==========================
@@ -158,13 +158,12 @@ func
 	func svc_tcp_accept
 
 file
-    The given string is compared against either the full pathname, the
-    src-root relative pathname, or the basename of the source file of
-    each callsite.  Examples::
+    The given string is compared against either the src-root relative
+    pathname, or the basename of the source file of each callsite.
+    Examples::
 
 	file svcsock.c
-	file kernel/freezer.c
-	file /usr/src/packages/BUILD/sgi-enhancednfs-1.4/default/net/sunrpc/svcsock.c
+	file kernel/freezer.c	# ie column 1 of control file
 
 module
     The given string is compared against the module name
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index c60409138e13..ba35199edd9c 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -67,17 +67,6 @@ static LIST_HEAD(ddebug_tables);
 static int verbose;
 module_param(verbose, int, 0644);
 
-/* Return the path relative to source root */
-static inline const char *trim_prefix(const char *path)
-{
-	int skip = strlen(__FILE__) - strlen("lib/dynamic_debug.c");
-
-	if (strncmp(path, __FILE__, skip))
-		skip = 0; /* prefix mismatch, don't skip */
-
-	return path + skip;
-}
-
 static struct { unsigned flag:8; char opt_char; } opt_array[] = {
 	{ _DPRINTK_FLAGS_PRINT, 'p' },
 	{ _DPRINTK_FLAGS_INCL_MODNAME, 'm' },
@@ -162,9 +151,7 @@ static int ddebug_change(const struct ddebug_query *query,
 			if (query->filename &&
 			    !match_wildcard(query->filename, dp->filename) &&
 			    !match_wildcard(query->filename,
-					   kbasename(dp->filename)) &&
-			    !match_wildcard(query->filename,
-					   trim_prefix(dp->filename)))
+					    kbasename(dp->filename)))
 				continue;
 
 			/* match against the function */
@@ -199,7 +186,7 @@ static int ddebug_change(const struct ddebug_query *query,
 #endif
 			dp->flags = newflags;
 			vpr_info("changed %s:%d [%s]%s =%s\n",
-				 trim_prefix(dp->filename), dp->lineno,
+				 dp->filename, dp->lineno,
 				 dt->mod_name, dp->function,
 				 ddebug_describe_flags(dp, flagbuf,
 						       sizeof(flagbuf)));
@@ -827,7 +814,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,
+		   dp->filename, dp->lineno,
 		   iter->table->mod_name, dp->function,
 		   ddebug_describe_flags(dp, flagsbuf, sizeof(flagsbuf)));
 	seq_escape(m, dp->format, "\t\r\n\"");
-- 
2.21.0

Re: [PATCH 01/16] dyndbg: drop trim_prefix, obsoleted by __FILE__s relative path

[Rasmus Villemoes]

On 29/10/2019 21.00, Jim Cromie wrote:
> Regarding:
> commit 2b6783191da7 ("dynamic_debug: add trim_prefix() to provide source-root relative paths")
> commit a73619a845d5 ("kbuild: use -fmacro-prefix-map to make __FILE__ a relative path")
> 
> 2nd commit broke dynamic-debug's "file $fullpath" query form, but
> nobody noticed because 1st commit trimmed prefixes from control-file
> output, so the click-copy-pasting of fullpaths into new queries had
> ceased; that query form became unused.
> 
> So remove the function and callers; its purpose was to strip the
> prefix from __FILE__, which is now gone.

I agree with the intent, but I wonder if this is premature. I mean, the
-fmacro-prefix-map is only for gcc 8+, so this ends up printing the full
paths when the compiler is just a little old (and the kernel was built
out-of-tree).

I don't think keeping the current workaround for a year or two more
should hurt; when int skip = strlen(__FILE__) -
strlen("lib/dynamic_debug.c"); evaluates to 0 (in-tree build, or build
with new enough gcc), I'm pretty sure gcc optimizes the rest of the
function away (it should know that strncmp(x, y, 0) gives 0, and even if
it didn't, the conditional assigns 0 to skip which it already is, so gcc
just needs to know that strncmp() is pure).

> 
> Also drop "file $fullpath" from docs, and tweak example to cite column
> 1 of control-file as valid "file $input".

That part certainly makes sense, since the $fullpath hasn't actually
been in the control file for a long time.

Rasmus

Re: [PATCH 01/16] dyndbg: drop trim_prefix, obsoleted by __FILE__s relative path

[jim.cromie]

Cool, thanks!


On Wed, Oct 30, 2019 at 3:20 PM Rasmus Villemoes
<linux@rasmusvillemoes.dk> wrote:
>
> On 29/10/2019 21.00, Jim Cromie wrote:
> > Regarding:
> > commit 2b6783191da7 ("dynamic_debug: add trim_prefix() to provide source-root relative paths")
> > commit a73619a845d5 ("kbuild: use -fmacro-prefix-map to make __FILE__ a relative path")
> >
> > 2nd commit broke dynamic-debug's "file $fullpath" query form, but
> > nobody noticed because 1st commit trimmed prefixes from control-file
> > output, so the click-copy-pasting of fullpaths into new queries had
> > ceased; that query form became unused.
> >
> > So remove the function and callers; its purpose was to strip the
> > prefix from __FILE__, which is now gone.
>
> I agree with the intent, but I wonder if this is premature. I mean, the
> -fmacro-prefix-map is only for gcc 8+, so this ends up printing the full
> paths when the compiler is just a little old (and the kernel was built
> out-of-tree).
>
> I don't think keeping the current workaround for a year or two more
> should hurt; when int skip = strlen(__FILE__) -
> strlen("lib/dynamic_debug.c"); evaluates to 0 (in-tree build, or build
> with new enough gcc), I'm pretty sure gcc optimizes the rest of the
> function away (it should know that strncmp(x, y, 0) gives 0, and even if
> it didn't, the conditional assigns 0 to skip which it already is, so gcc
> just needs to know that strncmp() is pure).
>
> >
> > Also drop "file $fullpath" from docs, and tweak example to cite column
> > 1 of control-file as valid "file $input".
>
> That part certainly makes sense, since the $fullpath hasn't actually
> been in the control file for a long time.
>
> Rasmus
>


I agree.  Ive split the patch, am keeping the doc change.