[PATCH v2] perf symbols: Don't apply the symfs layout to synthesised paths

[PATCH v2] perf symbols: Don't apply the symfs layout to synthesised paths

[Zhan Xusheng]

From: Zhan Xusheng <zhanxusheng1024@gmail.com>

From: Zhan Xusheng <zhanxusheng@xiaomi.com>

The flat symfs layout was implemented inside __symbol__join_symfs() alone,
which went from

        return path__join(bf, size, symbol_conf.symfs, path);

to taking perf_basename(path) first.  No caller was changed, so all of them
got it.  Most pass dso__long_name(), which is what the option is about, but
some pass a path perf built itself:

  dso.c        "/usr/lib/debug"             -> "debug"
  dso.c        "/usr/lib/debug/.build-id/"  -> ""
  build-id.c   "/usr/lib/debug/.build-id/"  -> ""
  disasm.c     <file under buildid_dir>     -> last component

perf_basename() yields "" for a path ending in '/'.

Tracing the lookups against an empty symfs,

  perf record -o pd.data -- sleep 0.3
  strace -f -e trace=openat,newfstatat \
      perf report -i pd.data --symfs <symfs>,flat --stdio

15 paths get tried.  Four of them differ, in every case only in the
prefix, which the patch restores:

  FEDORA, UBUNTU, MIXEDUP_UBUNTU
    <symfs>/debug/  ->  <symfs>//usr/lib/debug/
  BUILDID_DEBUGINFO
    <symfs>/        ->  <symfs>//usr/lib/debug/.build-id/

The tail is dso__long_name() for the first three and the build-id file for
the fourth, the same either way, so the build-id file was being looked for
in the symfs root and the distro debuginfo under <symfs>/debug/.  Those
prefixes follow neither layout: perf's own prefix is flattened while
dso__long_name() is still appended whole.

Among the 11 paths that do not change are the basename lookups the option
is for, <symfs>/sleep and <symfs>/vmlinux.  The same trace with
,hierarchy is identical between the two builds.

Use path__join() at those sites, which is what the helper did for them
before.  A hierarchy layout is unaffected, being that same call.  The
OPENEMBEDDED site passes "", where perf_basename() is already a no-op;
it is converted for uniformity.  Every remaining __symbol__join_symfs()
caller passes a path from the profiled system.

Fixes: f182573e06ab ("perf tools: Add layout support for --symfs option")
Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
---
v1->v2:
- Added the traced before/after to the changelog, as asked by Ian Rogers.
  No code change.

v1: https://lore.kernel.org/r/20260811050046.4019578-1-zhanxusheng@xiaomi.com

 tools/perf/util/build-id.c |  2 +-
 tools/perf/util/disasm.c   |  4 +++-
 tools/perf/util/dso.c      | 11 ++++++-----
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
index eb95ab90f974..0c89fe75a650 100644
--- a/tools/perf/util/build-id.c
+++ b/tools/perf/util/build-id.c
@@ -599,7 +599,7 @@ static char *build_id_cache__find_debug(const char *sbuild_id,
 		dirname = dirbuf;
 	}
 
-	len = __symbol__join_symfs(debugfile, PATH_MAX, dirname);
+	len = path__join(debugfile, PATH_MAX, symbol_conf.symfs, dirname);
 	snprintf(debugfile + len, PATH_MAX - len, "%.2s/%s.debug", sbuild_id,
 		 sbuild_id + 2);
 
diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
index 0a1a7e9cf3ef..decf9348d735 100644
--- a/tools/perf/util/disasm.c
+++ b/tools/perf/util/disasm.c
@@ -31,6 +31,7 @@
 #include "map.h"
 #include "maps.h"
 #include "namespaces.h"
+#include "path.h"
 #include "srcline.h"
 #include "symbol.h"
 #include "thread.h"
@@ -1173,7 +1174,8 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
 
 	build_id_filename = dso__build_id_filename(dso, NULL, 0, false);
 	if (build_id_filename) {
-		__symbol__join_symfs(filename, filename_size, build_id_filename);
+		path__join(filename, filename_size, symbol_conf.symfs,
+			   build_id_filename);
 		free(build_id_filename);
 	} else {
 		if (dso__has_build_id(dso))
diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
index 2309196d8df3..314c9a0edf28 100644
--- a/tools/perf/util/dso.c
+++ b/tools/perf/util/dso.c
@@ -167,12 +167,12 @@ int dso__read_binary_type_filename(const struct dso *dso,
 		break;
 
 	case DSO_BINARY_TYPE__FEDORA_DEBUGINFO:
-		len = __symbol__join_symfs(filename, size, "/usr/lib/debug");
+		len = path__join(filename, size, symbol_conf.symfs, "/usr/lib/debug");
 		snprintf(filename + len, size - len, "%s.debug", dso__long_name(dso));
 		break;
 
 	case DSO_BINARY_TYPE__UBUNTU_DEBUGINFO:
-		len = __symbol__join_symfs(filename, size, "/usr/lib/debug");
+		len = path__join(filename, size, symbol_conf.symfs, "/usr/lib/debug");
 		snprintf(filename + len, size - len, "%s", dso__long_name(dso));
 		break;
 
@@ -187,7 +187,7 @@ int dso__read_binary_type_filename(const struct dso *dso,
 			ret = -1;
 			break;
 		}
-		len = __symbol__join_symfs(filename, size, "/usr/lib/debug");
+		len = path__join(filename, size, symbol_conf.symfs, "/usr/lib/debug");
 		snprintf(filename + len, size - len, "%s", dso__long_name(dso) + 4);
 		break;
 
@@ -200,7 +200,7 @@ int dso__read_binary_type_filename(const struct dso *dso,
 		while (last_slash != dso__long_name(dso) && *last_slash != '/')
 			last_slash--;
 
-		len = __symbol__join_symfs(filename, size, "");
+		len = path__join(filename, size, symbol_conf.symfs, "");
 		dir_size = last_slash - dso__long_name(dso) + 2;
 		if (dir_size > (size - len)) {
 			ret = -1;
@@ -219,7 +219,8 @@ int dso__read_binary_type_filename(const struct dso *dso,
 		}
 
 		build_id__snprintf(dso__bid(dso), build_id_hex, sizeof(build_id_hex));
-		len = __symbol__join_symfs(filename, size, "/usr/lib/debug/.build-id/");
+		len = path__join(filename, size, symbol_conf.symfs,
+				 "/usr/lib/debug/.build-id/");
 		snprintf(filename + len, size - len, "%.2s/%s.debug",
 			 build_id_hex, build_id_hex + 2);
 		break;
-- 
2.43.0

Re: [PATCH v2] perf symbols: Don't apply the symfs layout to synthesised paths

[Ian Rogers]

On Wed, Aug 19, 2026 at 2:46 AM Zhan Xusheng <zhanxusheng1024@gmail.com> wrote:
>
> From: Zhan Xusheng <zhanxusheng1024@gmail.com>
>
> From: Zhan Xusheng <zhanxusheng@xiaomi.com>
>
> The flat symfs layout was implemented inside __symbol__join_symfs() alone,
> which went from
>
>         return path__join(bf, size, symbol_conf.symfs, path);
>
> to taking perf_basename(path) first.  No caller was changed, so all of them
> got it.  Most pass dso__long_name(), which is what the option is about, but
> some pass a path perf built itself:
>
>   dso.c        "/usr/lib/debug"             -> "debug"
>   dso.c        "/usr/lib/debug/.build-id/"  -> ""
>   build-id.c   "/usr/lib/debug/.build-id/"  -> ""
>   disasm.c     <file under buildid_dir>     -> last component
>
> perf_basename() yields "" for a path ending in '/'.
>
> Tracing the lookups against an empty symfs,
>
>   perf record -o pd.data -- sleep 0.3
>   strace -f -e trace=openat,newfstatat \
>       perf report -i pd.data --symfs <symfs>,flat --stdio
>
> 15 paths get tried.  Four of them differ, in every case only in the
> prefix, which the patch restores:
>
>   FEDORA, UBUNTU, MIXEDUP_UBUNTU
>     <symfs>/debug/  ->  <symfs>//usr/lib/debug/
>   BUILDID_DEBUGINFO
>     <symfs>/        ->  <symfs>//usr/lib/debug/.build-id/
>
> The tail is dso__long_name() for the first three and the build-id file for
> the fourth, the same either way, so the build-id file was being looked for
> in the symfs root and the distro debuginfo under <symfs>/debug/.  Those
> prefixes follow neither layout: perf's own prefix is flattened while
> dso__long_name() is still appended whole.

Hi Zhan,

I'm having a hard time understanding the commit message and what this
patch changes. Thanks for providing the before and after in v1. In
those before and after examples, the 'before' case seems to better
match what the user is requesting on the command line, so I think I'm
misunderstanding something. Could you help clarify the details.

Thanks,
Ian

> Among the 11 paths that do not change are the basename lookups the option
> is for, <symfs>/sleep and <symfs>/vmlinux.  The same trace with
> ,hierarchy is identical between the two builds.
>
> Use path__join() at those sites, which is what the helper did for them
> before.  A hierarchy layout is unaffected, being that same call.  The
> OPENEMBEDDED site passes "", where perf_basename() is already a no-op;
> it is converted for uniformity.  Every remaining __symbol__join_symfs()
> caller passes a path from the profiled system.
>
> Fixes: f182573e06ab ("perf tools: Add layout support for --symfs option")
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>
> ---
> v1->v2:
> - Added the traced before/after to the changelog, as asked by Ian Rogers.
>   No code change.
>
> v1: https://lore.kernel.org/r/20260811050046.4019578-1-zhanxusheng@xiaomi.com
>
>  tools/perf/util/build-id.c |  2 +-
>  tools/perf/util/disasm.c   |  4 +++-
>  tools/perf/util/dso.c      | 11 ++++++-----
>  3 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> index eb95ab90f974..0c89fe75a650 100644
> --- a/tools/perf/util/build-id.c
> +++ b/tools/perf/util/build-id.c
> @@ -599,7 +599,7 @@ static char *build_id_cache__find_debug(const char *sbuild_id,
>                 dirname = dirbuf;
>         }
>
> -       len = __symbol__join_symfs(debugfile, PATH_MAX, dirname);
> +       len = path__join(debugfile, PATH_MAX, symbol_conf.symfs, dirname);
>         snprintf(debugfile + len, PATH_MAX - len, "%.2s/%s.debug", sbuild_id,
>                  sbuild_id + 2);
>
> diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
> index 0a1a7e9cf3ef..decf9348d735 100644
> --- a/tools/perf/util/disasm.c
> +++ b/tools/perf/util/disasm.c
> @@ -31,6 +31,7 @@
>  #include "map.h"
>  #include "maps.h"
>  #include "namespaces.h"
> +#include "path.h"
>  #include "srcline.h"
>  #include "symbol.h"
>  #include "thread.h"
> @@ -1173,7 +1174,8 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil
>
>         build_id_filename = dso__build_id_filename(dso, NULL, 0, false);
>         if (build_id_filename) {
> -               __symbol__join_symfs(filename, filename_size, build_id_filename);
> +               path__join(filename, filename_size, symbol_conf.symfs,
> +                          build_id_filename);
>                 free(build_id_filename);
>         } else {
>                 if (dso__has_build_id(dso))
> diff --git a/tools/perf/util/dso.c b/tools/perf/util/dso.c
> index 2309196d8df3..314c9a0edf28 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -167,12 +167,12 @@ int dso__read_binary_type_filename(const struct dso *dso,
>                 break;
>
>         case DSO_BINARY_TYPE__FEDORA_DEBUGINFO:
> -               len = __symbol__join_symfs(filename, size, "/usr/lib/debug");
> +               len = path__join(filename, size, symbol_conf.symfs, "/usr/lib/debug");
>                 snprintf(filename + len, size - len, "%s.debug", dso__long_name(dso));
>                 break;
>
>         case DSO_BINARY_TYPE__UBUNTU_DEBUGINFO:
> -               len = __symbol__join_symfs(filename, size, "/usr/lib/debug");
> +               len = path__join(filename, size, symbol_conf.symfs, "/usr/lib/debug");
>                 snprintf(filename + len, size - len, "%s", dso__long_name(dso));
>                 break;
>
> @@ -187,7 +187,7 @@ int dso__read_binary_type_filename(const struct dso *dso,
>                         ret = -1;
>                         break;
>                 }
> -               len = __symbol__join_symfs(filename, size, "/usr/lib/debug");
> +               len = path__join(filename, size, symbol_conf.symfs, "/usr/lib/debug");
>                 snprintf(filename + len, size - len, "%s", dso__long_name(dso) + 4);
>                 break;
>
> @@ -200,7 +200,7 @@ int dso__read_binary_type_filename(const struct dso *dso,
>                 while (last_slash != dso__long_name(dso) && *last_slash != '/')
>                         last_slash--;
>
> -               len = __symbol__join_symfs(filename, size, "");
> +               len = path__join(filename, size, symbol_conf.symfs, "");
>                 dir_size = last_slash - dso__long_name(dso) + 2;
>                 if (dir_size > (size - len)) {
>                         ret = -1;
> @@ -219,7 +219,8 @@ int dso__read_binary_type_filename(const struct dso *dso,
>                 }
>
>                 build_id__snprintf(dso__bid(dso), build_id_hex, sizeof(build_id_hex));
> -               len = __symbol__join_symfs(filename, size, "/usr/lib/debug/.build-id/");
> +               len = path__join(filename, size, symbol_conf.symfs,
> +                                "/usr/lib/debug/.build-id/");
>                 snprintf(filename + len, size - len, "%.2s/%s.debug",
>                          build_id_hex, build_id_hex + 2);
>                 break;
> --
> 2.43.0
>

Re: [PATCH v2] perf symbols: Don't apply the symfs layout to synthesised paths

[Zhan Xusheng]

On Tue, Sep 15, 2026 at 01:31:16PM -0700, Ian Rogers wrote:
> In those before and after examples, the 'before' case seems to better
> match what the user is requesting on the command line, so I think I'm
> misunderstanding something.

Neither side is the flat layout.  The changelog showed the prefixes and
not the full paths they end up in, so there was nothing in it to see that
from.

For /usr/lib/x86_64-linux-gnu/libc.so.6 the base name is libc.so.6.debug,
so the flat lookup is /s/libc.so.6.debug.  With --symfs /s,flat, the
FEDORA_DEBUGINFO path actually tried is

  before  /s/debug/usr/lib/x86_64-linux-gnu/libc.so.6.debug
  after   /s//usr/lib/debug/usr/lib/x86_64-linux-gnu/libc.so.6.debug

Both carry /usr/lib/x86_64-linux-gnu/libc.so.6 whole.  perf_basename()
only ever saw the prefix, because the DSO path arrives after it:

	len = __symbol__join_symfs(filename, size, "/usr/lib/debug");
	snprintf(filename + len, size - len, "%s.debug", dso__long_name(dso));

Before is shorter in its first component, which is what makes it read as
flatter, but the layout that was asked for is on neither side.

BUILDID_DEBUGINFO is the one I would not try to defend as intended.  Its
prefix is "/usr/lib/debug/.build-id/", and perf_basename() of a path
ending in '/' is "", so the prefix does not become shorter, it disappears:

  before  <symfs>/ab/cdef...ff.debug
  after   <symfs>//usr/lib/debug/.build-id/ab/cdef...ff.debug

Where the argument is the file being looked for, the option does what it
says and the patch changes nothing:

	__symbol__join_symfs(filename, size, dso__long_name(dso));

/s/libc.so.6, /s/ld-linux-x86-64.so.2, /s/sleep.

So the patch is narrow: it keeps a flat request from rewriting perf's own
fixed prefixes.  It does not make flat find distro debuginfo -- for a user
whose debug files really are flat under <symfs>, both columns miss.  That
needs the base name taken from the composed filename instead of the
prefix, which is a different change and not a Fixes:.  I can write that
one instead if you would rather have it.

8 paths change across those 4 sites and 13 are untouched; hierarchy is
identical between the two builds at all 21.  The doubled slash comes from
path__join(), it predates this and shows up in untouched paths too.

The v2 changelog also said four paths differ while listing four call
sites; it is eight paths.  That did not help.

Thanks,
Zhan Xusheng

Re: [PATCH v2] perf symbols: Don't apply the symfs layout to synthesised paths

[Namhyung Kim]

Hello,

On Wed, Sep 16, 2026 at 11:36:47AM +0800, Zhan Xusheng wrote:
> On Tue, Sep 15, 2026 at 01:31:16PM -0700, Ian Rogers wrote:
> > In those before and after examples, the 'before' case seems to better
> > match what the user is requesting on the command line, so I think I'm
> > misunderstanding something.
> 
> Neither side is the flat layout.  The changelog showed the prefixes and
> not the full paths they end up in, so there was nothing in it to see that
> from.
> 
> For /usr/lib/x86_64-linux-gnu/libc.so.6 the base name is libc.so.6.debug,
> so the flat lookup is /s/libc.so.6.debug.  With --symfs /s,flat, the
> FEDORA_DEBUGINFO path actually tried is
> 
>   before  /s/debug/usr/lib/x86_64-linux-gnu/libc.so.6.debug
>   after   /s//usr/lib/debug/usr/lib/x86_64-linux-gnu/libc.so.6.debug

For the original purpose of the flat symfs, I believe it should be

  /s/libc.so.6.debug

> 
> Both carry /usr/lib/x86_64-linux-gnu/libc.so.6 whole.  perf_basename()
> only ever saw the prefix, because the DSO path arrives after it:
> 
> 	len = __symbol__join_symfs(filename, size, "/usr/lib/debug");
> 	snprintf(filename + len, size - len, "%s.debug", dso__long_name(dso));
> 
> Before is shorter in its first component, which is what makes it read as
> flatter, but the layout that was asked for is on neither side.
> 
> BUILDID_DEBUGINFO is the one I would not try to defend as intended.  Its
> prefix is "/usr/lib/debug/.build-id/", and perf_basename() of a path
> ending in '/' is "", so the prefix does not become shorter, it disappears:
> 
>   before  <symfs>/ab/cdef...ff.debug
>   after   <symfs>//usr/lib/debug/.build-id/ab/cdef...ff.debug

This is somewhat confusing as it uses the first 2 character as a
directory name.  But I believe this should be

  /s/abcdef..ff.debug

> 
> Where the argument is the file being looked for, the option does what it
> says and the patch changes nothing:
> 
> 	__symbol__join_symfs(filename, size, dso__long_name(dso));
> 
> /s/libc.so.6, /s/ld-linux-x86-64.so.2, /s/sleep.

These look fine.

> 
> So the patch is narrow: it keeps a flat request from rewriting perf's own
> fixed prefixes.  It does not make flat find distro debuginfo -- for a user
> whose debug files really are flat under <symfs>, both columns miss.  That
> needs the base name taken from the composed filename instead of the
> prefix, which is a different change and not a Fixes:.  I can write that
> one instead if you would rather have it.
> 
> 8 paths change across those 4 sites and 13 are untouched; hierarchy is
> identical between the two builds at all 21.  The doubled slash comes from
> path__join(), it predates this and shows up in untouched paths too.
> 
> The v2 changelog also said four paths differ while listing four call
> sites; it is eight paths.  That did not help.

Can you please change the symfs join function just to use the basename
so that it can really have the flat directory structure inside symfs?

Thanks,
Namhyung