[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZpCyf6ulH-8dRBu4@google.com>
Date: Thu, 11 Jul 2024 21:35:11 -0700
From: Namhyung Kim <namhyung@...nel.org>
To: Haoze Xie <royenheart@...il.com>
Cc: vmolnaro@...hat.com, peterz@...radead.org, mingo@...hat.com,
acme@...nel.org, mark.rutland@....com,
alexander.shishkin@...ux.intel.com, jolsa@...nel.org,
irogers@...gle.com, adrian.hunter@...el.com,
kan.liang@...ux.intel.com, linux-perf-users@...r.kernel.org,
linux-kernel@...r.kernel.org, Yuan Tan <tanyuan@...ylab.org>
Subject: Re: [PATCH 1/1] perf archive: unpack to correct dir given by perf
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@...il.com>
> Signed-off-by: Yuan Tan <tanyuan@...ylab.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
>
Powered by blists - more mailing lists