[PATCH 1/1] perf archive: unpack to correct dir given by perf

[PATCH 1/1] perf archive: unpack to correct dir given by perf

[Haoze Xie]

In perf-archive.sh, the code segment that defines 'PERF_BUILDID_DIR' is
advanced before 'unpack' operation for subsequent use, followed by a
'mkdir' operation to ensure that the dir exists. Symbols in 'unpack' will
be extracted to correct dir given by perf.

When '--unpack' param is appointed, the symbols are extracted to '~/.debug'
folder by default, without using 'PERF_BUILDID_DIR' given by perf. This
will cause perf to be unable to find the correct buildid's path when users
configured buildid.dir in 'perf config' or used '--buildid-dir' cli param,
since perf will read these params and put them in 'PERF_BUILDID_DIR' env.
'perf script' and 'perf report' will use the env as the basis for buildid
indexing.

Fixes: e43c64c971e4 ("perf archive: Add new option '--unpack' to expand tarballs")
Signed-off-by: Haoze Xie <royenheart@gmail.com>
Signed-off-by: Yuan Tan <tanyuan@tinylab.org>
---
 tools/perf/perf-archive.sh | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/tools/perf/perf-archive.sh b/tools/perf/perf-archive.sh
index 6ed7e52ab881..f29bbc129056 100755
--- a/tools/perf/perf-archive.sh
+++ b/tools/perf/perf-archive.sh
@@ -23,6 +23,19 @@ while [ $# -gt 0 ] ; do
 	fi
 done
 
+#
+# PERF_BUILDID_DIR environment variable set by perf
+# path to buildid directory, default to $HOME/.debug
+#
+if [ -z $PERF_BUILDID_DIR ]; then
+	PERF_BUILDID_DIR=~/.debug/
+else
+	# append / to make substitutions work
+	PERF_BUILDID_DIR=$PERF_BUILDID_DIR/
+fi
+
+mkdir -p $PERF_BUILDID_DIR
+
 if [ $UNPACK -eq 1 ]; then
 	if [ ! -z "$UNPACK_TAR" ]; then					# tar given as an argument
 		if [ ! -e "$UNPACK_TAR" ]; then
@@ -65,25 +78,14 @@ if [ $UNPACK -eq 1 ]; then
 		fi
 
 		# unzip the perf.data file in the current working directory	and debug symbols in ~/.debug directory
-		tar xvf $TARGET && tar xvf $PERF_SYMBOLS.tar.bz2 -C ~/.debug
+		tar xvf $TARGET && tar xvf $PERF_SYMBOLS.tar.bz2 -C $PERF_BUILDID_DIR
 
 	else																# perf tar generated by perf archive (contains only debug symbols)
-		tar xvf $TARGET -C ~/.debug
+		tar xvf $TARGET -C $PERF_BUILDID_DIR
 	fi
 	exit 0
 fi
 
-#
-# PERF_BUILDID_DIR environment variable set by perf
-# path to buildid directory, default to $HOME/.debug
-#
-if [ -z $PERF_BUILDID_DIR ]; then
-	PERF_BUILDID_DIR=~/.debug/
-else
-        # append / to make substitutions work
-        PERF_BUILDID_DIR=$PERF_BUILDID_DIR/
-fi
-
 BUILDIDS=$(mktemp /tmp/perf-archive-buildids.XXXXXX)
 
 perf buildid-list -i $PERF_DATA --with-hits | grep -v "^ " > $BUILDIDS
-- 
2.25.1

Re: [PATCH 1/1] perf archive: unpack to correct dir given by perf

[Namhyung Kim]

Hello,

On Mon, Jul 08, 2024 at 02:04:31AM +0800, Haoze Xie wrote:
> In perf-archive.sh, the code segment that defines 'PERF_BUILDID_DIR' is
> advanced before 'unpack' operation for subsequent use, followed by a
> 'mkdir' operation to ensure that the dir exists. Symbols in 'unpack' will
> be extracted to correct dir given by perf.
> 
> When '--unpack' param is appointed, the symbols are extracted to '~/.debug'
> folder by default, without using 'PERF_BUILDID_DIR' given by perf. This
> will cause perf to be unable to find the correct buildid's path when users
> configured buildid.dir in 'perf config' or used '--buildid-dir' cli param,
> since perf will read these params and put them in 'PERF_BUILDID_DIR' env.
> 'perf script' and 'perf report' will use the env as the basis for buildid
> indexing.

Can you please add an example command line and the output for the error
case?  It'd be helpful to understand the problem more intuitively.

> 
> Fixes: e43c64c971e4 ("perf archive: Add new option '--unpack' to expand tarballs")
> Signed-off-by: Haoze Xie <royenheart@gmail.com>
> Signed-off-by: Yuan Tan <tanyuan@tinylab.org>
> ---
>  tools/perf/perf-archive.sh | 28 +++++++++++++++-------------
>  1 file changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/tools/perf/perf-archive.sh b/tools/perf/perf-archive.sh
> index 6ed7e52ab881..f29bbc129056 100755
> --- a/tools/perf/perf-archive.sh
> +++ b/tools/perf/perf-archive.sh
> @@ -23,6 +23,19 @@ while [ $# -gt 0 ] ; do
>  	fi
>  done
>  
> +#
> +# PERF_BUILDID_DIR environment variable set by perf
> +# path to buildid directory, default to $HOME/.debug
> +#
> +if [ -z $PERF_BUILDID_DIR ]; then
> +	PERF_BUILDID_DIR=~/.debug/
> +else
> +	# append / to make substitutions work
> +	PERF_BUILDID_DIR=$PERF_BUILDID_DIR/
> +fi
> +
> +mkdir -p $PERF_BUILDID_DIR
> +
>  if [ $UNPACK -eq 1 ]; then
>  	if [ ! -z "$UNPACK_TAR" ]; then					# tar given as an argument
>  		if [ ! -e "$UNPACK_TAR" ]; then
> @@ -65,25 +78,14 @@ if [ $UNPACK -eq 1 ]; then
>  		fi
>  
>  		# unzip the perf.data file in the current working directory	and debug symbols in ~/.debug directory
> -		tar xvf $TARGET && tar xvf $PERF_SYMBOLS.tar.bz2 -C ~/.debug
> +		tar xvf $TARGET && tar xvf $PERF_SYMBOLS.tar.bz2 -C $PERF_BUILDID_DIR
>  
>  	else																# perf tar generated by perf archive (contains only debug symbols)

Off-topic.  I'm surprised by the comment placement.
It'd be nice if you (or someone else) can update the whole file and
remove the unnecessary whitespaces altogether.

Thanks,
Namhyung


> -		tar xvf $TARGET -C ~/.debug
> +		tar xvf $TARGET -C $PERF_BUILDID_DIR
>  	fi
>  	exit 0
>  fi
>  
> -#
> -# PERF_BUILDID_DIR environment variable set by perf
> -# path to buildid directory, default to $HOME/.debug
> -#
> -if [ -z $PERF_BUILDID_DIR ]; then
> -	PERF_BUILDID_DIR=~/.debug/
> -else
> -        # append / to make substitutions work
> -        PERF_BUILDID_DIR=$PERF_BUILDID_DIR/
> -fi
> -
>  BUILDIDS=$(mktemp /tmp/perf-archive-buildids.XXXXXX)
>  
>  perf buildid-list -i $PERF_DATA --with-hits | grep -v "^ " > $BUILDIDS
> -- 
> 2.25.1
> 

Re: [PATCH 1/1] perf archive: unpack to correct dir given by perf

[royenheart]

On 2024/7/12 12:35, Namhyung Kim wrote:
> Hello,
> 
> On Mon, Jul 08, 2024 at 02:04:31AM +0800, Haoze Xie wrote:
>> In perf-archive.sh, the code segment that defines 'PERF_BUILDID_DIR' is
>> advanced before 'unpack' operation for subsequent use, followed by a
>> 'mkdir' operation to ensure that the dir exists. Symbols in 'unpack' will
>> be extracted to correct dir given by perf.
>>
>> When '--unpack' param is appointed, the symbols are extracted to '~/.debug'
>> folder by default, without using 'PERF_BUILDID_DIR' given by perf. This
>> will cause perf to be unable to find the correct buildid's path when users
>> configured buildid.dir in 'perf config' or used '--buildid-dir' cli param,
>> since perf will read these params and put them in 'PERF_BUILDID_DIR' env.
>> 'perf script' and 'perf report' will use the env as the basis for buildid
>> indexing.
> 
> Can you please add an example command line and the output for the error
> case?  It'd be helpful to understand the problem more intuitively.
> 
>>
>> Fixes: e43c64c971e4 ("perf archive: Add new option '--unpack' to expand tarballs")
>> Signed-off-by: Haoze Xie <royenheart@gmail.com>
>> Signed-off-by: Yuan Tan <tanyuan@tinylab.org>
>> ---
>>  tools/perf/perf-archive.sh | 28 +++++++++++++++-------------
>>  1 file changed, 15 insertions(+), 13 deletions(-)
>>
>> diff --git a/tools/perf/perf-archive.sh b/tools/perf/perf-archive.sh
>> index 6ed7e52ab881..f29bbc129056 100755
>> --- a/tools/perf/perf-archive.sh
>> +++ b/tools/perf/perf-archive.sh
>> @@ -23,6 +23,19 @@ while [ $# -gt 0 ] ; do
>>  	fi
>>  done
>>  
>> +#
>> +# PERF_BUILDID_DIR environment variable set by perf
>> +# path to buildid directory, default to $HOME/.debug
>> +#
>> +if [ -z $PERF_BUILDID_DIR ]; then
>> +	PERF_BUILDID_DIR=~/.debug/
>> +else
>> +	# append / to make substitutions work
>> +	PERF_BUILDID_DIR=$PERF_BUILDID_DIR/
>> +fi
>> +
>> +mkdir -p $PERF_BUILDID_DIR
>> +
>>  if [ $UNPACK -eq 1 ]; then
>>  	if [ ! -z "$UNPACK_TAR" ]; then					# tar given as an argument
>>  		if [ ! -e "$UNPACK_TAR" ]; then
>> @@ -65,25 +78,14 @@ if [ $UNPACK -eq 1 ]; then
>>  		fi
>>  
>>  		# unzip the perf.data file in the current working directory	and debug symbols in ~/.debug directory
>> -		tar xvf $TARGET && tar xvf $PERF_SYMBOLS.tar.bz2 -C ~/.debug
>> +		tar xvf $TARGET && tar xvf $PERF_SYMBOLS.tar.bz2 -C $PERF_BUILDID_DIR
>>  
>>  	else																# perf tar generated by perf archive (contains only debug symbols)
> 
> Off-topic.  I'm surprised by the comment placement.
> It'd be nice if you (or someone else) can update the whole file and
> remove the unnecessary whitespaces altogether.
> 
> Thanks,
> Namhyung
> 
> 
>> -		tar xvf $TARGET -C ~/.debug
>> +		tar xvf $TARGET -C $PERF_BUILDID_DIR
>>  	fi
>>  	exit 0
>>  fi
>>  
>> -#
>> -# PERF_BUILDID_DIR environment variable set by perf
>> -# path to buildid directory, default to $HOME/.debug
>> -#
>> -if [ -z $PERF_BUILDID_DIR ]; then
>> -	PERF_BUILDID_DIR=~/.debug/
>> -else
>> -        # append / to make substitutions work
>> -        PERF_BUILDID_DIR=$PERF_BUILDID_DIR/
>> -fi
>> -
>>  BUILDIDS=$(mktemp /tmp/perf-archive-buildids.XXXXXX)
>>  
>>  perf buildid-list -i $PERF_DATA --with-hits | grep -v "^ " > $BUILDIDS
>> -- 
>> 2.25.1
>>

I have two machines A and B, I use 'perf record' and 'perf archive' to get
a buildid archive 'perf.data.tar.bz2' on A: 

$ perf record -a -g -- sleep 1
$ perf archive

then I transfer 'perf.data' and its buildid archive to B. On machine B, I
use 'perf archive' to extract the buildid:

$ perf archive --unpack
$   Found target file for unpacking: ./perf.data.tar.bz2
$   .build-id/d1/a727ab038085dbbb50e74c812e5a6e8502e8c7
$   ...

But when I use 'perf script' to view hotspots, no function names are shown:

$ perf script
$   No kallsyms or vmlinux with build-id 
$   251c1248b97a17df394058a189dffe381169ddcd was found
$   perf    1770 [000] 1022235.467607:          1 cycles:P: 
$           ffffffff8ae9ceb6 [unknown] ([kernel.kallsyms])
$           ffffffff8ae15af5 [unknown] ([kernel.kallsyms])
$           ffffffff8ae0f83b [unknown] ([kernel.kallsyms])
$   ......

The problem is, I used 'perf config' to change buildid search path before:

$ perf config --user buildid.dir=/usr/local/symbols
$ cat ~/.perfconfig
$   # this file is auto-generated.
$   [buildid]
$           dir = /usr/local/symbols

But 'perf archive --unpack' just extract them to '~/.debug', which makes
perf can't found right path to search for buildid info. If I add
'buildid-dir' param to replace buildid path defined in perfconfig, problem
can be solved temporarily:

$ ./perf --buildid-dir ~/.debug script
$   perf    1770 [000] 1022235.467607:          1 cycles:P: 
$           ffffffff8ae9ceb6 native_write_msr+0x6 ([kernel.kallsyms])
$           ffffffff8ae15af5 intel_pmu_enable_all+0x15 ([kernel.kallsyms])
$           ffffffff8ae0f83b x86_pmu_enable+0x1ab ([kernel.kallsyms])

The code that determines the buildid path is in the 
'util/config.c:set_buildid_dir' function, which will generate
'PERF_BUILDID_DIR' var.

Re: [PATCH 1/1] perf archive: unpack to correct dir given by perf

[Michael Petlan]

On Tue, 16 Jul 2024, royenheart wrote:
> On 2024/7/12 12:35, Namhyung Kim wrote:
> > Hello,
> > 
> > On Mon, Jul 08, 2024 at 02:04:31AM +0800, Haoze Xie wrote:
> >> In perf-archive.sh, the code segment that defines 'PERF_BUILDID_DIR' is
> >> advanced before 'unpack' operation for subsequent use, followed by a
> >> 'mkdir' operation to ensure that the dir exists. Symbols in 'unpack' will
> >> be extracted to correct dir given by perf.
> >>
> >> When '--unpack' param is appointed, the symbols are extracted to '~/.debug'
> >> folder by default, without using 'PERF_BUILDID_DIR' given by perf. This
> >> will cause perf to be unable to find the correct buildid's path when users
> >> configured buildid.dir in 'perf config' or used '--buildid-dir' cli param,
> >> since perf will read these params and put them in 'PERF_BUILDID_DIR' env.
> >> 'perf script' and 'perf report' will use the env as the basis for buildid
> >> indexing.
> > 
> > Can you please add an example command line and the output for the error
> > case?  It'd be helpful to understand the problem more intuitively.
> > 
> >>

[...]

> 
> I have two machines A and B, I use 'perf record' and 'perf archive' to get
> a buildid archive 'perf.data.tar.bz2' on A: 
> 
> $ perf record -a -g -- sleep 1
> $ perf archive
> 
> then I transfer 'perf.data' and its buildid archive to B. On machine B, I
> use 'perf archive' to extract the buildid:
> 
> $ perf archive --unpack
> $   Found target file for unpacking: ./perf.data.tar.bz2
> $   .build-id/d1/a727ab038085dbbb50e74c812e5a6e8502e8c7
> $   ...
> 
> But when I use 'perf script' to view hotspots, no function names are shown:
> 
> $ perf script
> $   No kallsyms or vmlinux with build-id 
> $   251c1248b97a17df394058a189dffe381169ddcd was found
> $   perf    1770 [000] 1022235.467607:          1 cycles:P: 
> $           ffffffff8ae9ceb6 [unknown] ([kernel.kallsyms])
> $           ffffffff8ae15af5 [unknown] ([kernel.kallsyms])
> $           ffffffff8ae0f83b [unknown] ([kernel.kallsyms])
> $   ......
> 
> The problem is, I used 'perf config' to change buildid search path before:
> 
> $ perf config --user buildid.dir=/usr/local/symbols
> $ cat ~/.perfconfig
> $   # this file is auto-generated.
> $   [buildid]
> $           dir = /usr/local/symbols
> 

Shouldn't then perf-archive just determine the directory from perf-config
without any additional env var?

> But 'perf archive --unpack' just extract them to '~/.debug', which makes
> perf can't found right path to search for buildid info. If I add
> 'buildid-dir' param to replace buildid path defined in perfconfig, problem
> can be solved temporarily:
> 
> $ ./perf --buildid-dir ~/.debug script
> $   perf    1770 [000] 1022235.467607:          1 cycles:P: 
> $           ffffffff8ae9ceb6 native_write_msr+0x6 ([kernel.kallsyms])
> $           ffffffff8ae15af5 intel_pmu_enable_all+0x15 ([kernel.kallsyms])
> $           ffffffff8ae0f83b x86_pmu_enable+0x1ab ([kernel.kallsyms])
> 
> The code that determines the buildid path is in the 
> 'util/config.c:set_buildid_dir' function, which will generate
> 'PERF_BUILDID_DIR' var.

As above, the var seems to be a duplicate to what is already stored in
perf-config...

> 
> 

Regards,
Michael

Re: [PATCH 1/1] perf archive: unpack to correct dir given by perf

[Haoze Xie]

在 2024/7/16 20:29, Michael Petlan 写道:
> On Tue, 16 Jul 2024, royenheart wrote:
>> On 2024/7/12 12:35, Namhyung Kim wrote:
>>> Hello,
>>>
>>> On Mon, Jul 08, 2024 at 02:04:31AM +0800, Haoze Xie wrote:
>>>> In perf-archive.sh, the code segment that defines 'PERF_BUILDID_DIR' is
>>>> advanced before 'unpack' operation for subsequent use, followed by a
>>>> 'mkdir' operation to ensure that the dir exists. Symbols in 'unpack' will
>>>> be extracted to correct dir given by perf.
>>>>
>>>> When '--unpack' param is appointed, the symbols are extracted to '~/.debug'
>>>> folder by default, without using 'PERF_BUILDID_DIR' given by perf. This
>>>> will cause perf to be unable to find the correct buildid's path when users
>>>> configured buildid.dir in 'perf config' or used '--buildid-dir' cli param,
>>>> since perf will read these params and put them in 'PERF_BUILDID_DIR' env.
>>>> 'perf script' and 'perf report' will use the env as the basis for buildid
>>>> indexing.
>>>
>>> Can you please add an example command line and the output for the error
>>> case?  It'd be helpful to understand the problem more intuitively.
>>>
>>>>
> 
> [...]
> 
>>
>> I have two machines A and B, I use 'perf record' and 'perf archive' to get
>> a buildid archive 'perf.data.tar.bz2' on A: 
>>
>> $ perf record -a -g -- sleep 1
>> $ perf archive
>>
>> then I transfer 'perf.data' and its buildid archive to B. On machine B, I
>> use 'perf archive' to extract the buildid:
>>
>> $ perf archive --unpack
>> $   Found target file for unpacking: ./perf.data.tar.bz2
>> $   .build-id/d1/a727ab038085dbbb50e74c812e5a6e8502e8c7
>> $   ...
>>
>> But when I use 'perf script' to view hotspots, no function names are shown:
>>
>> $ perf script
>> $   No kallsyms or vmlinux with build-id 
>> $   251c1248b97a17df394058a189dffe381169ddcd was found
>> $   perf    1770 [000] 1022235.467607:          1 cycles:P: 
>> $           ffffffff8ae9ceb6 [unknown] ([kernel.kallsyms])
>> $           ffffffff8ae15af5 [unknown] ([kernel.kallsyms])
>> $           ffffffff8ae0f83b [unknown] ([kernel.kallsyms])
>> $   ......
>>
>> The problem is, I used 'perf config' to change buildid search path before:
>>
>> $ perf config --user buildid.dir=/usr/local/symbols
>> $ cat ~/.perfconfig
>> $   # this file is auto-generated.
>> $   [buildid]
>> $           dir = /usr/local/symbols
>>
> 
> Shouldn't then perf-archive just determine the directory from perf-config
> without any additional env var?
> 
>> But 'perf archive --unpack' just extract them to '~/.debug', which makes
>> perf can't found right path to search for buildid info. If I add
>> 'buildid-dir' param to replace buildid path defined in perfconfig, problem
>> can be solved temporarily:
>>
>> $ ./perf --buildid-dir ~/.debug script
>> $   perf    1770 [000] 1022235.467607:          1 cycles:P: 
>> $           ffffffff8ae9ceb6 native_write_msr+0x6 ([kernel.kallsyms])
>> $           ffffffff8ae15af5 intel_pmu_enable_all+0x15 ([kernel.kallsyms])
>> $           ffffffff8ae0f83b x86_pmu_enable+0x1ab ([kernel.kallsyms])
>>
>> The code that determines the buildid path is in the 
>> 'util/config.c:set_buildid_dir' function, which will generate
>> 'PERF_BUILDID_DIR' var.
> 
> As above, the var seems to be a duplicate to what is already stored in
> perf-config...
> 
>>
>>
> 
> Regards,
> Michael
> 

Sorry for the delay, I read about the codes, in 'perf.c:main', it will call
'perf_config' function at first, which will call 'config:set_buildid_dir'
to set an env 'PERF_BUILDID_DIR' for external commands(including
perf-archive). Codes are:

$ // perf.c
$ int main(int argc, const char **argv) {
$   // ...
$   err = perf_config(perf_default_config, NULL);
$   // ...
$ }
$
$ ...
$ // config.c
$ void set_buildid_dir(const char *dir) {
$   if (dir)
$	    scnprintf(buildid_dir, MAXPATHLEN, "%s", dir);
$
$	/* default to $HOME/.debug */
$	if (buildid_dir[0] == '\0') {
$		char *home = getenv("HOME");
$
$		if (home) {
$			snprintf(buildid_dir, MAXPATHLEN, "%s/%s",
$				 home, DEBUG_CACHE_DIR);
$		} else {
$			strncpy(buildid_dir, DEBUG_CACHE_DIR, MAXPATHLEN-1);
$		}
$		buildid_dir[MAXPATHLEN-1] = '\0';
$	}
$	/* for communicating with external commands */
$	setenv("PERF_BUILDID_DIR", buildid_dir, 1);
$ }

Or it will set to '${HOME}/.debug'('.debug' if ${HOME} not set).
'--buildid_dir' param will override it. So perf has already read the config
for us and set the 'PERF_BUILDID_DIR' env intentionally at first. I think
it's ok to use the env for indicating where to extract.