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(-)
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.0On 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
>
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
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
© 2016 - 2026 Red Hat, Inc.