| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAP-5=fUN7=cQ9X10tH1qpmzpJoSoqHH5CV5MVwhqXkQ8vkbwoA@mail.gmail.com>
Date: Wed, 26 Feb 2025 21:42:31 -0800
From: Ian Rogers <irogers@...gle.com>
To: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>, Namhyung Kim <namhyung@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
Ian Rogers <irogers@...gle.com>, Adrian Hunter <adrian.hunter@...el.com>,
Kan Liang <kan.liang@...ux.intel.com>, James Clark <james.clark@...aro.org>,
linux-kernel@...r.kernel.org, linux-perf-users@...r.kernel.org
Subject: Re: [PATCH v1] perf tests: Fix data symbol test with LTO builds
On Wed, Feb 26, 2025 at 3:01 PM Ian Rogers <irogers@...gle.com> wrote:
>
> With LTO builds, although regular builds could also see this as
> all the code is in one file, the datasym workload can realize the
> buf1.reserved data is never accessed. The compiler moves the
> variable to bss and only keeps the data1 and data2 parts as
> separate variables. This causes the symbol check to fail in the
> test. Make the variable volatile to disable the more aggressive
> optimization. Rename the variable to make which buf1 in perf is
> being referred to.
>
> Before:
> ```
> $ perf test -vv "data symbol"
> 126: Test data symbol:
> --- start ---
> test child forked, pid 299808
> perf does not have symbol 'buf1'
> perf is missing symbols - skipping test
> ---- end(-2) ----
> 126: Test data symbol : Skip
> $ nm perf|grep buf1
> 0000000000a5fa40 b buf1.0
> 0000000000a5fa48 b buf1.1
> ```
>
> After:
> ```
> $ nm perf|grep buf1
> 0000000000a53a00 d buf1
> $ perf test -vv "data symbol"126: Test data symbol:
> --- start ---
> test child forked, pid 302166
> a53a00-a53a39 l buf1
> perf does have symbol 'buf1'
> Recording workload...
> Waiting for "perf record has started" message
> OK
> Cleaning up files...
> ---- end(0) ----
> 126: Test data symbol : Ok
> ```
>
> Fixes: 3dfc01fe9d12 ("perf test: Add 'datasym' test workload")
> Signed-off-by: Ian Rogers <irogers@...gle.com>
Ah, I see we're trying to force -O0 and -fno-inline in the Makefile
(git.kernel.org is giving 403s):
https://github.com/torvalds/linux/blob/master/tools/perf/tests/workloads/Build#L11
Which LTO later undoes. I'm not seeing LTO breakages for brstack.c and
the shell test "Check branch stack sampling". I think LTO is able to
optimize this case as it is a variable/struct being optimized, so the
"-O0" and "-fno-inline" mustn't be being made to apply. Not a wholly
satisfactory reason to add the volatile, but I'm short on
alternatives.
Thanks,
Ian
> ---
> tools/perf/tests/shell/test_data_symbol.sh | 17 +++++++++--------
> tools/perf/tests/workloads/datasym.c | 11 ++++++-----
> 2 files changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/tools/perf/tests/shell/test_data_symbol.sh b/tools/perf/tests/shell/test_data_symbol.sh
> index c86da0235059..7da606db97cb 100755
> --- a/tools/perf/tests/shell/test_data_symbol.sh
> +++ b/tools/perf/tests/shell/test_data_symbol.sh
> @@ -18,7 +18,7 @@ skip_if_no_mem_event() {
>
> skip_if_no_mem_event || exit 2
>
> -skip_test_missing_symbol buf1
> +skip_test_missing_symbol workload_datasym_buf1
>
> TEST_PROGRAM="perf test -w datasym"
> PERF_DATA=$(mktemp /tmp/__perf_test.perf.data.XXXXX)
> @@ -26,18 +26,19 @@ ERR_FILE=$(mktemp /tmp/__perf_test.stderr.XXXXX)
>
> check_result() {
> # The memory report format is as below:
> - # 99.92% ... [.] buf1+0x38
> + # 99.92% ... [.] workload_datasym_buf1+0x38
> result=$(perf mem report -i ${PERF_DATA} -s symbol_daddr -q 2>&1 |
> - awk '/buf1/ { print $4 }')
> + awk '/workload_datasym_buf1/ { print $4 }')
>
> - # Testing is failed if has no any sample for "buf1"
> + # Testing is failed if has no any sample for "workload_datasym_buf1"
> [ -z "$result" ] && return 1
>
> while IFS= read -r line; do
> - # The "data1" and "data2" fields in structure "buf1" have
> - # offset "0x0" and "0x38", returns failure if detect any
> - # other offset value.
> - if [ "$line" != "buf1+0x0" ] && [ "$line" != "buf1+0x38" ]; then
> + # The "data1" and "data2" fields in structure
> + # "workload_datasym_buf1" have offset "0x0" and "0x38", returns
> + # failure if detect any other offset value.
> + if [ "$line" != "workload_datasym_buf1+0x0" ] && \
> + [ "$line" != "workload_datasym_buf1+0x38" ]; then
> return 1
> fi
> done <<< "$result"
> diff --git a/tools/perf/tests/workloads/datasym.c b/tools/perf/tests/workloads/datasym.c
> index 8e08fc75a973..5074b3439835 100644
> --- a/tools/perf/tests/workloads/datasym.c
> +++ b/tools/perf/tests/workloads/datasym.c
> @@ -7,7 +7,8 @@ typedef struct _buf {
> char data2;
> } buf __attribute__((aligned(64)));
>
> -static buf buf1 = {
> +/* volatile to try to avoid the compiler seeing reserved as unused. */
> +static volatile buf workload_datasym_buf1 = {
> /* to have this in the data section */
> .reserved[0] = 1,
> };
> @@ -15,8 +16,8 @@ static buf buf1 = {
> static int datasym(int argc __maybe_unused, const char **argv __maybe_unused)
> {
> for (;;) {
> - buf1.data1++;
> - if (buf1.data1 == 123) {
> + workload_datasym_buf1.data1++;
> + if (workload_datasym_buf1.data1 == 123) {
> /*
> * Add some 'noise' in the loop to work around errata
> * 1694299 on Arm N1.
> @@ -30,9 +31,9 @@ static int datasym(int argc __maybe_unused, const char **argv __maybe_unused)
> * longer a continuous repeating pattern that interacts
> * badly with the bias.
> */
> - buf1.data1++;
> + workload_datasym_buf1.data1++;
> }
> - buf1.data2 += buf1.data1;
> + workload_datasym_buf1.data2 += workload_datasym_buf1.data1;
> }
> return 0;
> }
> --
> 2.48.1.711.g2feabab25a-goog
>
Powered by blists - more mailing lists