All of lore.kernel.org
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: Zhan Xusheng <zhanxusheng1024@gmail.com>
Cc: irogers@google.com, acme@kernel.org, changbin.du@huawei.com,
	adrian.hunter@intel.com, jolsa@kernel.org, mingo@redhat.com,
	peterz@infradead.org, linux-perf-users@vger.kernel.org,
	linux-kernel@vger.kernel.org, zhanxusheng@xiaomi.com
Subject: Re: [PATCH v3] perf symbols: Apply the symfs flat layout to the composed filename
Date: Thu, 24 Sep 2026 15:30:48 -0700	[thread overview]
Message-ID: <arWkmNKRbzF2FhoA@google.com> (raw)
In-Reply-To: <20260922131952.2922816-1-zhanxusheng@xiaomi.com>

Hello,

On Tue, Sep 22, 2026 at 09:19:52PM +0800, Zhan Xusheng wrote:
> --symfs=<dir>,flat documents itself as matching only the base name, so a
> debuginfo lookup for /usr/lib/x86_64-linux-gnu/libc.so.6 should land on
> <dir>/libc.so.6.debug.  It lands on
> <dir>/debug/usr/lib/x86_64-linux-gnu/libc.so.6.debug instead, because the
> layout is applied to a fixed prefix and the file name arrives afterwards:
> 
> 	len = __symbol__join_symfs(filename, size, "/usr/lib/debug");
> 	snprintf(filename + len, size - len, "%s.debug", dso__long_name(dso));
> 
> perf_basename() only ever sees "/usr/lib/debug", so neither layout comes
> out of it.
> 
> Compose the path as it would appear on the profiled system and join it
> once, which puts the layout on the whole name.  That covers
> FEDORA_DEBUGINFO, UBUNTU_DEBUGINFO, MIXEDUP_UBUNTU_DEBUGINFO,
> OPENEMBEDDED_DEBUGINFO and BUILDID_DEBUGINFO, plus the same composition in
> build_id_cache__find_debug().
> 
> BUILDID_DEBUGINFO needs one more step: the build id cache splits the first
> two characters of the build id off into a directory name, so the basename
> of the hierarchy path is only part of the build id.  Name the flat file
> after the whole build id.
> 
> dso__disassemble_filename() passes a path from perf's own build id cache
> rather than one on the profiled system, and every entry there is named
> "elf" or "debug".  Use path__join() so the layout does not apply.
> 
> With --symfs /s,flat the lookups are now /s/libc.so.6, /s/libc.so.6.debug,
> /s/sleep and /s/<build id>.debug.  hierarchy is unchanged.
> 
> Signed-off-by: Zhan Xusheng <zhanxusheng@xiaomi.com>

Reviewed-by: Namhyung Kim <namhyung@kernel.org>

Thanks,
Namhyung

> ---
> v2->v3: different approach.  v1 and v2 took the four sites out of the
> layout; Namhyung asked for the layout to be made to work instead, so v3
> composes the filename first and joins once.
> 
> v2: https://lore.kernel.org/r/20260819094621.844115-1-zhanxusheng@xiaomi.com
> 
> Traced openat()/newfstatat() under --symfs /s,flat and /s,hierarchy with
> perf record -- sleep 0.3 before and after: flat goes from 21 paths to 15,
> hierarchy is identical at 21.
>  tools/perf/util/build-id.c | 12 +++++++----
>  tools/perf/util/disasm.c   |  8 ++++++-
>  tools/perf/util/dso.c      | 43 +++++++++++++++++++++++++-------------
>  3 files changed, 44 insertions(+), 19 deletions(-)
> 
> diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c
> index eb95ab90f974..02233ef0eaff 100644
> --- a/tools/perf/util/build-id.c
> +++ b/tools/perf/util/build-id.c
> @@ -586,9 +586,9 @@ static char *build_id_cache__find_debug(const char *sbuild_id,
>  	const char *dirname = "/usr/lib/debug/.build-id/";
>  	char *realname = NULL;
>  	char dirbuf[PATH_MAX];
> +	char pathbuf[PATH_MAX];
>  	char *debugfile;
>  	struct nscookie nsc;
> -	size_t len = 0;
>  
>  	debugfile = calloc(1, PATH_MAX);
>  	if (!debugfile)
> @@ -599,9 +599,13 @@ static char *build_id_cache__find_debug(const char *sbuild_id,
>  		dirname = dirbuf;
>  	}
>  
> -	len = __symbol__join_symfs(debugfile, PATH_MAX, dirname);
> -	snprintf(debugfile + len, PATH_MAX - len, "%.2s/%s.debug", sbuild_id,
> -		 sbuild_id + 2);
> +	/* See the build id note in dso__read_binary_type_filename(). */
> +	if (symbol_conf.symfs_layout_flat)
> +		scnprintf(pathbuf, PATH_MAX, "/%s.debug", sbuild_id);
> +	else
> +		scnprintf(pathbuf, PATH_MAX, "%s%.2s/%s.debug", dirname,
> +			  sbuild_id, sbuild_id + 2);
> +	__symbol__join_symfs(debugfile, PATH_MAX, pathbuf);
>  
>  	nsinfo__mountns_enter(nsi, &nsc);
>  	realname = realpath(debugfile, NULL);
> diff --git a/tools/perf/util/disasm.c b/tools/perf/util/disasm.c
> index 6cfdbabbb8c7..1148dde6c975 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,12 @@ 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);
> +		/*
> +		 * This is a path in perf's own build id cache, not a path on
> +		 * the profiled system, so the symfs layout does not apply.
> +		 */
> +		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 42bfe30a3b51..e52de7cc1bc6 100644
> --- a/tools/perf/util/dso.c
> +++ b/tools/perf/util/dso.c
> @@ -115,6 +115,7 @@ int dso__read_binary_type_filename(const struct dso *dso,
>  				   const char *root_dir, char *filename, size_t size)
>  {
>  	char build_id_hex[SBUILD_ID_SIZE];
> +	char relative[PATH_MAX];
>  	int ret = 0;
>  	size_t len;
>  
> @@ -167,13 +168,15 @@ 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");
> -		snprintf(filename + len, size - len, "%s.debug", dso__long_name(dso));
> +		snprintf(relative, sizeof(relative), "/usr/lib/debug%s.debug",
> +			 dso__long_name(dso));
> +		__symbol__join_symfs(filename, size, relative);
>  		break;
>  
>  	case DSO_BINARY_TYPE__UBUNTU_DEBUGINFO:
> -		len = __symbol__join_symfs(filename, size, "/usr/lib/debug");
> -		snprintf(filename + len, size - len, "%s", dso__long_name(dso));
> +		snprintf(relative, sizeof(relative), "/usr/lib/debug%s",
> +			 dso__long_name(dso));
> +		__symbol__join_symfs(filename, size, relative);
>  		break;
>  
>  	case DSO_BINARY_TYPE__MIXEDUP_UBUNTU_DEBUGINFO:
> @@ -187,8 +190,9 @@ int dso__read_binary_type_filename(const struct dso *dso,
>  			ret = -1;
>  			break;
>  		}
> -		len = __symbol__join_symfs(filename, size, "/usr/lib/debug");
> -		snprintf(filename + len, size - len, "%s", dso__long_name(dso) + 4);
> +		snprintf(relative, sizeof(relative), "/usr/lib/debug%s",
> +			 dso__long_name(dso) + 4);
> +		__symbol__join_symfs(filename, size, relative);
>  		break;
>  
>  	case DSO_BINARY_TYPE__OPENEMBEDDED_DEBUGINFO:
> @@ -200,15 +204,15 @@ 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, "");
>  		dir_size = last_slash - dso__long_name(dso) + 2;
> -		if (dir_size > (size - len)) {
> +		if (dir_size > sizeof(relative)) {
>  			ret = -1;
>  			break;
>  		}
> -		len += scnprintf(filename + len, dir_size, "%s",  dso__long_name(dso));
> -		len += scnprintf(filename + len , size - len, ".debug%s",
> -								last_slash);
> +		len = scnprintf(relative, dir_size, "%s", dso__long_name(dso));
> +		scnprintf(relative + len, sizeof(relative) - len, ".debug%s",
> +			  last_slash);
> +		__symbol__join_symfs(filename, size, relative);
>  		break;
>  	}
>  
> @@ -219,9 +223,20 @@ 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/");
> -		snprintf(filename + len, size - len, "%.2s/%s.debug",
> -			 build_id_hex, build_id_hex + 2);
> +		/*
> +		 * The build id cache layout splits the first two characters off
> +		 * into a directory name, so the basename of the hierarchy path
> +		 * is only part of the build id. Name the flat file after the
> +		 * whole build id instead.
> +		 */
> +		if (symbol_conf.symfs_layout_flat)
> +			snprintf(relative, sizeof(relative), "/%s.debug",
> +				 build_id_hex);
> +		else
> +			snprintf(relative, sizeof(relative),
> +				 "/usr/lib/debug/.build-id/%.2s/%s.debug",
> +				 build_id_hex, build_id_hex + 2);
> +		__symbol__join_symfs(filename, size, relative);
>  		break;
>  
>  	case DSO_BINARY_TYPE__VMLINUX:
> 
> base-commit: f0100363d8c374bd8e9ea7c9ba02744f0b802ca4
> -- 
> 2.43.0
> 

  parent reply	other threads:[~2026-09-24 22:30 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-22 13:19 [PATCH v3] perf symbols: Apply the symfs flat layout to the composed filename Zhan Xusheng
2026-09-22 13:27 ` sashiko-bot
2026-09-24 22:30 ` Namhyung Kim [this message]
2026-09-25 12:07   ` Arnaldo Carvalho de Melo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=arWkmNKRbzF2FhoA@google.com \
    --to=namhyung@kernel.org \
    --cc=acme@kernel.org \
    --cc=adrian.hunter@intel.com \
    --cc=changbin.du@huawei.com \
    --cc=irogers@google.com \
    --cc=jolsa@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-perf-users@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=zhanxusheng1024@gmail.com \
    --cc=zhanxusheng@xiaomi.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.