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@xxxxxxxxx> wrote:
>
> From: Zhan Xusheng <zhanxusheng1024@xxxxxxxxx>
>
> From: Zhan Xusheng <zhanxusheng@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx
>
>  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
>