All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] perf test: Fix errors when deleting data in perf record tests
@ 2026-09-23 21:52 Namhyung Kim
  2026-09-23 21:52 ` [PATCH 2/3] perf test: Fix record tests on Intel Broadwell Namhyung Kim
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Namhyung Kim @ 2026-09-23 21:52 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ian Rogers, Jiri Olsa, Adrian Hunter, James Clark, Peter Zijlstra,
	Ingo Molnar, LKML, linux-perf-users

perf record with multithreads would create a data directory instead of a
file.  But the cleanup and retry logic expect a file and try to delete
them with 'rm -f' which would fail.  Add '-r' flag to remove directories.

  rm: cannot remove '/tmp/__perf_test.perf.data.ZVHTw.old': Is a directory

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/tests/shell/lib/perf_record.sh | 2 +-
 tools/perf/tests/shell/record.sh          | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/tools/perf/tests/shell/lib/perf_record.sh b/tools/perf/tests/shell/lib/perf_record.sh
index 2b9e11b66dc7a801..a95fafb8b6fa8b40 100644
--- a/tools/perf/tests/shell/lib/perf_record.sh
+++ b/tools/perf/tests/shell/lib/perf_record.sh
@@ -30,7 +30,7 @@ perf_record_with_retry() {
   fi
 
   for duration in 0.01 0.1 0.3 1.0 2.0; do
-    rm -f "${perfdata}".old
+    rm -rf "${perfdata}".old
     ${cmd_prefix} "$@" -o "${perfdata}" ${testprog_base} ${duration} > "$logfile" 2>&1
     local record_exit=$?
 
diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
index 4a4931cb5ba91849..cf8ba3d3411b0bcc 100755
--- a/tools/perf/tests/shell/record.sh
+++ b/tools/perf/tests/shell/record.sh
@@ -39,8 +39,8 @@ default_fd_limit=$(ulimit -Sn)
 min_fd_limit=$(($(getconf _NPROCESSORS_ONLN) * 16))
 
 cleanup() {
-  rm -f "${perfdata}"
-  rm -f "${perfdata}".old
+  rm -rf "${perfdata}"
+  rm -rf "${perfdata}".old
   rm -f "${script_output}"
   perf_record_cleanup
 
-- 
2.56.0.rc1.310.g51773c2048-goog


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/3] perf test: Fix record tests on Intel Broadwell
  2026-09-23 21:52 [PATCH 1/3] perf test: Fix errors when deleting data in perf record tests Namhyung Kim
@ 2026-09-23 21:52 ` Namhyung Kim
  2026-09-23 22:01   ` sashiko-bot
  2026-09-24  3:45   ` Ian Rogers
  2026-09-23 21:52 ` [PATCH 3/3] perf test: Fix record tests on hybrid machines Namhyung Kim
                   ` (3 subsequent siblings)
  4 siblings, 2 replies; 11+ messages in thread
From: Namhyung Kim @ 2026-09-23 21:52 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ian Rogers, Jiri Olsa, Adrian Hunter, James Clark, Peter Zijlstra,
	Ingo Molnar, LKML, linux-perf-users

I noticed it failed for leader sampling tests on Broadwell.  It's
because the period of groups members are a little bit different.  Maybe
there's an hardware issue but anyway it's close enough to treat them as
same.  I printed the value like below.

  $ perf test record
  ...
  prev: 462507, curr: 462506
  prev: 494783, curr: 494782
  prev: 454350, curr: 454349
  prev: 502782, curr: 502781
  prev: 566693, curr: 566692
  prev: 606558, curr: 606557
  prev: 559770, curr: 559769
  prev: 595737, curr: 595736
  prev: 643386, curr: 643385
  prev: 663672, curr: 663671
  prev: 620622, curr: 620621
  prev: 650419, curr: 650418
  Leader sampling [Failed inconsistent cycles count]

Let's update the logic to compare values to allow 1% of errors.

Also I think it should compare only if sees the non-leader samples so
split the check of (($index % 2)).

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/tests/shell/record.sh | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
index cf8ba3d3411b0bcc..04df35b30b0c15fd 100755
--- a/tools/perf/tests/shell/record.sh
+++ b/tools/perf/tests/shell/record.sh
@@ -550,11 +550,14 @@ test_leader_sampling() {
   while IFS= read -r line
   do
     cycles=$(echo $line | awk '{for(i=1;i<=NF;i++) if($i=="cycles:") print $(i-1)}')
-    if [ $(($index%2)) -ne 0 ] && [ ${cycles}x != ${prev_cycles}x ]
+    if [ $(($index%2)) -ne 0 ]
     then
-      invalid_counts=$(($invalid_counts+1))
-    else
-      valid_counts=$(($valid_counts+1))
+      if (( $(bc <<< "scale=4; r = ${cycles} / ${prev_cycles}; r >= 0.99 && r <= 1.01") ))
+      then
+        valid_counts=$(($valid_counts+1))
+      else
+        invalid_counts=$(($invalid_counts+1))
+      fi
     fi
     index=$(($index+1))
     prev_cycles=$cycles
-- 
2.56.0.rc1.310.g51773c2048-goog


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/3] perf test: Fix record tests on hybrid machines
  2026-09-23 21:52 [PATCH 1/3] perf test: Fix errors when deleting data in perf record tests Namhyung Kim
  2026-09-23 21:52 ` [PATCH 2/3] perf test: Fix record tests on Intel Broadwell Namhyung Kim
@ 2026-09-23 21:52 ` Namhyung Kim
  2026-09-23 21:59   ` sashiko-bot
  2026-09-24  3:44   ` Ian Rogers
  2026-09-23 22:00 ` [PATCH 1/3] perf test: Fix errors when deleting data in perf record tests sashiko-bot
                   ` (2 subsequent siblings)
  4 siblings, 2 replies; 11+ messages in thread
From: Namhyung Kim @ 2026-09-23 21:52 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ian Rogers, Jiri Olsa, Adrian Hunter, James Clark, Peter Zijlstra,
	Ingo Molnar, LKML, linux-perf-users

Currently the leader sampling test looks for "cycles:" string to match
the event and retrieve the value before the event name.  But it won't
work on hybrid systems as the event name is different like below.

  $ perf script | grep -m 10 brstack
      perf 3322152 1873980.978431:     625379 cpu_atom/cycles/:      558200cc40cf brstack_bench+0x31 (/home/namhyung/project/linux/tools/perf/perf)
      perf 3322152 1873980.978431:     625379 cpu_atom/cycles/:      558200cc40cf brstack_bench+0x31 (/home/namhyung/project/linux/tools/perf/perf)
      perf 3322152 1873980.978598:     612177 cpu_atom/cycles/:      558200cc4082 brstack_foo+0x17 (/home/namhyung/project/linux/tools/perf/perf)
      perf 3322152 1873980.978598:     612177 cpu_atom/cycles/:      558200cc4082 brstack_foo+0x17 (/home/namhyung/project/linux/tools/perf/perf)
      perf 3322152 1873980.978762:     591859 cpu_atom/cycles/:      558200cc403d brstack_bar+0x0 (/home/namhyung/project/linux/tools/perf/perf)
      perf 3322152 1873980.978762:     591859 cpu_atom/cycles/:      558200cc403d brstack_bar+0x0 (/home/namhyung/project/linux/tools/perf/perf)
      perf 3322152 1873980.978948:     628505 cpu_atom/cycles/:      558200cc403e brstack_bar+0x1 (/home/namhyung/project/linux/tools/perf/perf)
      perf 3322152 1873980.978948:     628505 cpu_atom/cycles/:      558200cc403e brstack_bar+0x1 (/home/namhyung/project/linux/tools/perf/perf)
      perf 3322152 1873980.979146:     669816 cpu_atom/cycles/:      558200cc40e5 brstack_bench+0x47 (/home/namhyung/project/linux/tools/perf/perf)
      perf 3322152 1873980.979146:     669816 cpu_atom/cycles/:      558200cc40e5 brstack_bench+0x47 (/home/namhyung/project/linux/tools/perf/perf)

But we can control the output format of perf script using -F option.  So
we knows where the period is in the output.  To filter by symbol name,
"ip" and "sym" fields are required as well.

  $ perf script -F period,ip,sym | grep -m 10 brstack
      625379      558200cc40cf brstack_bench
      625379      558200cc40cf brstack_bench
      612177      558200cc4082 brstack_foo
      612177      558200cc4082 brstack_foo
      591859      558200cc403d brstack_bar
      591859      558200cc403d brstack_bar
      628505      558200cc403e brstack_bar
      628505      558200cc403e brstack_bar
      669816      558200cc40e5 brstack_bench
      669816      558200cc40e5 brstack_bench

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/tests/shell/record.sh | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
index 04df35b30b0c15fd..fa8d17e389a0116d 100755
--- a/tools/perf/tests/shell/record.sh
+++ b/tools/perf/tests/shell/record.sh
@@ -537,7 +537,7 @@ test_leader_sampling() {
     err=1
     return
   fi
-  perf script -i "${perfdata}" | grep brstack > $script_output
+  perf script -i "${perfdata}" -F period,ip,sym | grep brstack > $script_output
   # Check if the two instruction counts are equal in each record.
   # However, the throttling code doesn't consider event grouping. During throttling, only the
   # leader is stopped, causing the slave's counts significantly higher. To temporarily solve this,
@@ -549,7 +549,7 @@ test_leader_sampling() {
   tolerance_rate=0.8
   while IFS= read -r line
   do
-    cycles=$(echo $line | awk '{for(i=1;i<=NF;i++) if($i=="cycles:") print $(i-1)}')
+    cycles=$(echo $line | awk '{ print $1 }')
     if [ $(($index%2)) -ne 0 ]
     then
       if (( $(bc <<< "scale=4; r = ${cycles} / ${prev_cycles}; r >= 0.99 && r <= 1.01") ))
-- 
2.56.0.rc1.310.g51773c2048-goog


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] perf test: Fix record tests on hybrid machines
  2026-09-23 21:52 ` [PATCH 3/3] perf test: Fix record tests on hybrid machines Namhyung Kim
@ 2026-09-23 21:59   ` sashiko-bot
  2026-09-24  3:44   ` Ian Rogers
  1 sibling, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-09-23 21:59 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: linux-perf-users

> Currently the leader sampling test looks for "cycles:" string to match
> the event and retrieve the value before the event name.  But it won't
> work on hybrid systems as the event name is different like below.
> 
>   $ perf script | grep -m 10 brstack
>       perf 3322152 1873980.978431:     625379 cpu_atom/cycles/:      558200cc40cf brstack_bench+0x31 (/home/namhyung/project/linux/tools/perf/perf)
>       perf 3322152 1873980.978431:     625379 cpu_atom/cycles/:      558200cc40cf brstack_bench+0x31 (/home/namhyung/project/linux/tools/perf/perf)
>       perf 3322152 1873980.978598:     612177 cpu_atom/cycles/:      558200cc4082 brstack_foo+0x17 (/home/namhyung/project/linux/tools/perf/perf)
>       perf 3322152 1873980.978598:     612177 cpu_atom/cycles/:      558200cc4082 brstack_foo+0x17 (/home/namhyung/project/linux/tools/perf/perf)
>       perf 3322152 1873980.978762:     591859 cpu_atom/cycles/:      558200cc403d brstack_bar+0x0 (/home/namhyung/project/linux/tools/perf/perf)
>       perf 3322152 1873980.978762:     591859 cpu_atom/cycles/:      558200cc403d brstack_bar+0x0 (/home/namhyung/project/linux/tools/perf/perf)
>       perf 3322152 1873980.978948:     628505 cpu_atom/cycles/:      558200cc403e brstack_bar+0x1 (/home/namhyung/project/linux/tools/perf/perf)
>       perf 3322152 1873980.978948:     628505 cpu_atom/cycles/:      558200cc403e brstack_bar+0x1 (/home/namhyung/project/linux/tools/perf/perf)
>       perf 3322152 1873980.979146:     669816 cpu_atom/cycles/:      558200cc40e5 brstack_bench+0x47 (/home/namhyung/project/linux/tools/perf/perf)
>       perf 3322152 1873980.979146:     669816 cpu_atom/cycles/:      558200cc40e5 brstack_bench+0x47 (/home/namhyung/project/linux/tools/perf/perf)
> [ ... ]
>       628505      558200cc403e brstack_bar
>       669816      558200cc40e5 brstack_bench
>       669816      558200cc40e5 brstack_bench
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923215255.3329181-1-namhyung@kernel.org?part=3


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] perf test: Fix errors when deleting data in perf record tests
  2026-09-23 21:52 [PATCH 1/3] perf test: Fix errors when deleting data in perf record tests Namhyung Kim
  2026-09-23 21:52 ` [PATCH 2/3] perf test: Fix record tests on Intel Broadwell Namhyung Kim
  2026-09-23 21:52 ` [PATCH 3/3] perf test: Fix record tests on hybrid machines Namhyung Kim
@ 2026-09-23 22:00 ` sashiko-bot
  2026-09-24  3:39 ` Ian Rogers
  2026-09-25 10:51 ` Arnaldo Carvalho de Melo
  4 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-09-23 22:00 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: linux-perf-users

> perf record with multithreads would create a data directory instead of a
> file.  But the cleanup and retry logic expect a file and try to delete
> them with 'rm -f' which would fail.  Add '-r' flag to remove directories.
> 
>   rm: cannot remove '/tmp/__perf_test.perf.data.ZVHTw.old': Is a directory
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923215255.3329181-1-namhyung@kernel.org?part=1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] perf test: Fix record tests on Intel Broadwell
  2026-09-23 21:52 ` [PATCH 2/3] perf test: Fix record tests on Intel Broadwell Namhyung Kim
@ 2026-09-23 22:01   ` sashiko-bot
  2026-09-24  3:45   ` Ian Rogers
  1 sibling, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-09-23 22:01 UTC (permalink / raw)
  To: Namhyung Kim; +Cc: linux-perf-users

> I noticed it failed for leader sampling tests on Broadwell.  It's
> because the period of groups members are a little bit different.  Maybe
> there's an hardware issue but anyway it's close enough to treat them as
> same.  I printed the value like below.
> 
>   $ perf test record
>   ...
>   prev: 462507, curr: 462506
>   prev: 494783, curr: 494782
>   prev: 454350, curr: 454349
>   prev: 502782, curr: 502781
>   prev: 566693, curr: 566692
>   prev: 606558, curr: 606557
>   prev: 559770, curr: 559769
>   prev: 595737, curr: 595736
>   prev: 643386, curr: 643385
>   prev: 663672, curr: 663671
>   prev: 620622, curr: 620621
>   prev: 650419, curr: 650418
>   Leader sampling [Failed inconsistent cycles count]
> 
> Let's update the logic to compare values to allow 1% of errors.
> 
> Also I think it should compare only if sees the non-leader samples so
> split the check of (($index % 2)).
> 
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260923215255.3329181-1-namhyung@kernel.org?part=2


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] perf test: Fix errors when deleting data in perf record tests
  2026-09-23 21:52 [PATCH 1/3] perf test: Fix errors when deleting data in perf record tests Namhyung Kim
                   ` (2 preceding siblings ...)
  2026-09-23 22:00 ` [PATCH 1/3] perf test: Fix errors when deleting data in perf record tests sashiko-bot
@ 2026-09-24  3:39 ` Ian Rogers
  2026-09-25 10:51 ` Arnaldo Carvalho de Melo
  4 siblings, 0 replies; 11+ messages in thread
From: Ian Rogers @ 2026-09-24  3:39 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Adrian Hunter, James Clark,
	Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users

On Wed, Sep 23, 2026 at 2:52 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> perf record with multithreads would create a data directory instead of a
> file.  But the cleanup and retry logic expect a file and try to delete
> them with 'rm -f' which would fail.  Add '-r' flag to remove directories.
>
>   rm: cannot remove '/tmp/__perf_test.perf.data.ZVHTw.old': Is a directory
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Tested-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/tests/shell/lib/perf_record.sh | 2 +-
>  tools/perf/tests/shell/record.sh          | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/tools/perf/tests/shell/lib/perf_record.sh b/tools/perf/tests/shell/lib/perf_record.sh
> index 2b9e11b66dc7a801..a95fafb8b6fa8b40 100644
> --- a/tools/perf/tests/shell/lib/perf_record.sh
> +++ b/tools/perf/tests/shell/lib/perf_record.sh
> @@ -30,7 +30,7 @@ perf_record_with_retry() {
>    fi
>
>    for duration in 0.01 0.1 0.3 1.0 2.0; do
> -    rm -f "${perfdata}".old
> +    rm -rf "${perfdata}".old
>      ${cmd_prefix} "$@" -o "${perfdata}" ${testprog_base} ${duration} > "$logfile" 2>&1
>      local record_exit=$?
>
> diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
> index 4a4931cb5ba91849..cf8ba3d3411b0bcc 100755
> --- a/tools/perf/tests/shell/record.sh
> +++ b/tools/perf/tests/shell/record.sh
> @@ -39,8 +39,8 @@ default_fd_limit=$(ulimit -Sn)
>  min_fd_limit=$(($(getconf _NPROCESSORS_ONLN) * 16))
>
>  cleanup() {
> -  rm -f "${perfdata}"
> -  rm -f "${perfdata}".old
> +  rm -rf "${perfdata}"
> +  rm -rf "${perfdata}".old
>    rm -f "${script_output}"
>    perf_record_cleanup
>
> --
> 2.56.0.rc1.310.g51773c2048-goog
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 3/3] perf test: Fix record tests on hybrid machines
  2026-09-23 21:52 ` [PATCH 3/3] perf test: Fix record tests on hybrid machines Namhyung Kim
  2026-09-23 21:59   ` sashiko-bot
@ 2026-09-24  3:44   ` Ian Rogers
  1 sibling, 0 replies; 11+ messages in thread
From: Ian Rogers @ 2026-09-24  3:44 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Adrian Hunter, James Clark,
	Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users

On Wed, Sep 23, 2026 at 2:52 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> Currently the leader sampling test looks for "cycles:" string to match
> the event and retrieve the value before the event name.  But it won't
> work on hybrid systems as the event name is different like below.
>
>   $ perf script | grep -m 10 brstack
>       perf 3322152 1873980.978431:     625379 cpu_atom/cycles/:      558200cc40cf brstack_bench+0x31 (/home/namhyung/project/linux/tools/perf/perf)
>       perf 3322152 1873980.978431:     625379 cpu_atom/cycles/:      558200cc40cf brstack_bench+0x31 (/home/namhyung/project/linux/tools/perf/perf)
>       perf 3322152 1873980.978598:     612177 cpu_atom/cycles/:      558200cc4082 brstack_foo+0x17 (/home/namhyung/project/linux/tools/perf/perf)
>       perf 3322152 1873980.978598:     612177 cpu_atom/cycles/:      558200cc4082 brstack_foo+0x17 (/home/namhyung/project/linux/tools/perf/perf)
>       perf 3322152 1873980.978762:     591859 cpu_atom/cycles/:      558200cc403d brstack_bar+0x0 (/home/namhyung/project/linux/tools/perf/perf)
>       perf 3322152 1873980.978762:     591859 cpu_atom/cycles/:      558200cc403d brstack_bar+0x0 (/home/namhyung/project/linux/tools/perf/perf)
>       perf 3322152 1873980.978948:     628505 cpu_atom/cycles/:      558200cc403e brstack_bar+0x1 (/home/namhyung/project/linux/tools/perf/perf)
>       perf 3322152 1873980.978948:     628505 cpu_atom/cycles/:      558200cc403e brstack_bar+0x1 (/home/namhyung/project/linux/tools/perf/perf)
>       perf 3322152 1873980.979146:     669816 cpu_atom/cycles/:      558200cc40e5 brstack_bench+0x47 (/home/namhyung/project/linux/tools/perf/perf)
>       perf 3322152 1873980.979146:     669816 cpu_atom/cycles/:      558200cc40e5 brstack_bench+0x47 (/home/namhyung/project/linux/tools/perf/perf)
>
> But we can control the output format of perf script using -F option.  So
> we knows where the period is in the output.  To filter by symbol name,
> "ip" and "sym" fields are required as well.
>
>   $ perf script -F period,ip,sym | grep -m 10 brstack
>       625379      558200cc40cf brstack_bench
>       625379      558200cc40cf brstack_bench
>       612177      558200cc4082 brstack_foo
>       612177      558200cc4082 brstack_foo
>       591859      558200cc403d brstack_bar
>       591859      558200cc403d brstack_bar
>       628505      558200cc403e brstack_bar
>       628505      558200cc403e brstack_bar
>       669816      558200cc40e5 brstack_bench
>       669816      558200cc40e5 brstack_bench
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

Tested-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/tests/shell/record.sh | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
> index 04df35b30b0c15fd..fa8d17e389a0116d 100755
> --- a/tools/perf/tests/shell/record.sh
> +++ b/tools/perf/tests/shell/record.sh
> @@ -537,7 +537,7 @@ test_leader_sampling() {
>      err=1
>      return
>    fi
> -  perf script -i "${perfdata}" | grep brstack > $script_output
> +  perf script -i "${perfdata}" -F period,ip,sym | grep brstack > $script_output
>    # Check if the two instruction counts are equal in each record.
>    # However, the throttling code doesn't consider event grouping. During throttling, only the
>    # leader is stopped, causing the slave's counts significantly higher. To temporarily solve this,
> @@ -549,7 +549,7 @@ test_leader_sampling() {
>    tolerance_rate=0.8
>    while IFS= read -r line
>    do
> -    cycles=$(echo $line | awk '{for(i=1;i<=NF;i++) if($i=="cycles:") print $(i-1)}')
> +    cycles=$(echo $line | awk '{ print $1 }')
>      if [ $(($index%2)) -ne 0 ]
>      then
>        if (( $(bc <<< "scale=4; r = ${cycles} / ${prev_cycles}; r >= 0.99 && r <= 1.01") ))
> --
> 2.56.0.rc1.310.g51773c2048-goog
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] perf test: Fix record tests on Intel Broadwell
  2026-09-23 21:52 ` [PATCH 2/3] perf test: Fix record tests on Intel Broadwell Namhyung Kim
  2026-09-23 22:01   ` sashiko-bot
@ 2026-09-24  3:45   ` Ian Rogers
  2026-09-24  6:55     ` Mi, Dapeng
  1 sibling, 1 reply; 11+ messages in thread
From: Ian Rogers @ 2026-09-24  3:45 UTC (permalink / raw)
  To: Namhyung Kim, Dapeng Mi
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Adrian Hunter, James Clark,
	Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users

On Wed, Sep 23, 2026 at 2:53 PM Namhyung Kim <namhyung@kernel.org> wrote:
>
> I noticed it failed for leader sampling tests on Broadwell.  It's
> because the period of groups members are a little bit different.  Maybe
> there's an hardware issue but anyway it's close enough to treat them as
> same.  I printed the value like below.
>
>   $ perf test record
>   ...
>   prev: 462507, curr: 462506
>   prev: 494783, curr: 494782
>   prev: 454350, curr: 454349
>   prev: 502782, curr: 502781
>   prev: 566693, curr: 566692
>   prev: 606558, curr: 606557
>   prev: 559770, curr: 559769
>   prev: 595737, curr: 595736
>   prev: 643386, curr: 643385
>   prev: 663672, curr: 663671
>   prev: 620622, curr: 620621
>   prev: 650419, curr: 650418
>   Leader sampling [Failed inconsistent cycles count]
>
> Let's update the logic to compare values to allow 1% of errors.
>
> Also I think it should compare only if sees the non-leader samples so
> split the check of (($index % 2)).
>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>

+Dapeng Mi

Tested-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  tools/perf/tests/shell/record.sh | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
> index cf8ba3d3411b0bcc..04df35b30b0c15fd 100755
> --- a/tools/perf/tests/shell/record.sh
> +++ b/tools/perf/tests/shell/record.sh
> @@ -550,11 +550,14 @@ test_leader_sampling() {
>    while IFS= read -r line
>    do
>      cycles=$(echo $line | awk '{for(i=1;i<=NF;i++) if($i=="cycles:") print $(i-1)}')
> -    if [ $(($index%2)) -ne 0 ] && [ ${cycles}x != ${prev_cycles}x ]
> +    if [ $(($index%2)) -ne 0 ]
>      then
> -      invalid_counts=$(($invalid_counts+1))
> -    else
> -      valid_counts=$(($valid_counts+1))
> +      if (( $(bc <<< "scale=4; r = ${cycles} / ${prev_cycles}; r >= 0.99 && r <= 1.01") ))
> +      then
> +        valid_counts=$(($valid_counts+1))
> +      else
> +        invalid_counts=$(($invalid_counts+1))
> +      fi
>      fi
>      index=$(($index+1))
>      prev_cycles=$cycles
> --
> 2.56.0.rc1.310.g51773c2048-goog
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 2/3] perf test: Fix record tests on Intel Broadwell
  2026-09-24  3:45   ` Ian Rogers
@ 2026-09-24  6:55     ` Mi, Dapeng
  0 siblings, 0 replies; 11+ messages in thread
From: Mi, Dapeng @ 2026-09-24  6:55 UTC (permalink / raw)
  To: Ian Rogers, Namhyung Kim
  Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Adrian Hunter, James Clark,
	Peter Zijlstra, Ingo Molnar, LKML, linux-perf-users


On 9/24/2026 11:45 AM, Ian Rogers wrote:
> On Wed, Sep 23, 2026 at 2:53 PM Namhyung Kim <namhyung@kernel.org> wrote:
>> I noticed it failed for leader sampling tests on Broadwell.  It's
>> because the period of groups members are a little bit different.  Maybe
>> there's an hardware issue but anyway it's close enough to treat them as
>> same.  I printed the value like below.
>>
>>   $ perf test record
>>   ...
>>   prev: 462507, curr: 462506
>>   prev: 494783, curr: 494782
>>   prev: 454350, curr: 454349
>>   prev: 502782, curr: 502781
>>   prev: 566693, curr: 566692
>>   prev: 606558, curr: 606557
>>   prev: 559770, curr: 559769
>>   prev: 595737, curr: 595736
>>   prev: 643386, curr: 643385
>>   prev: 663672, curr: 663671
>>   prev: 620622, curr: 620621
>>   prev: 650419, curr: 650418
>>   Leader sampling [Failed inconsistent cycles count]

Reviewed-by: Dapeng Mi <dapeng1.mi@linux.intel.com>

It looks a hardware overcount issue.  I don't see this issue on recent
platforms, like SPR+, so I suppose the issue got fixed. 

BTW, recently I found the SAMPLE_READ may report invalid counts in some
PEBS based corner sampling cases, like ACR events + SAMPLE_READ, or
multiple PEBS events simultaneously run SAMPLE_READ, I'm working on a patch
series to fix the issues.

Thanks.


>> Let's update the logic to compare values to allow 1% of errors.
>>
>> Also I think it should compare only if sees the non-leader samples so
>> split the check of (($index % 2)).
>>
>> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> +Dapeng Mi
>
> Tested-by: Ian Rogers <irogers@google.com>
>
> Thanks,
> Ian
>
>> ---
>>  tools/perf/tests/shell/record.sh | 11 +++++++----
>>  1 file changed, 7 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
>> index cf8ba3d3411b0bcc..04df35b30b0c15fd 100755
>> --- a/tools/perf/tests/shell/record.sh
>> +++ b/tools/perf/tests/shell/record.sh
>> @@ -550,11 +550,14 @@ test_leader_sampling() {
>>    while IFS= read -r line
>>    do
>>      cycles=$(echo $line | awk '{for(i=1;i<=NF;i++) if($i=="cycles:") print $(i-1)}')
>> -    if [ $(($index%2)) -ne 0 ] && [ ${cycles}x != ${prev_cycles}x ]
>> +    if [ $(($index%2)) -ne 0 ]
>>      then
>> -      invalid_counts=$(($invalid_counts+1))
>> -    else
>> -      valid_counts=$(($valid_counts+1))
>> +      if (( $(bc <<< "scale=4; r = ${cycles} / ${prev_cycles}; r >= 0.99 && r <= 1.01") ))
>> +      then
>> +        valid_counts=$(($valid_counts+1))
>> +      else
>> +        invalid_counts=$(($invalid_counts+1))
>> +      fi
>>      fi
>>      index=$(($index+1))
>>      prev_cycles=$cycles
>> --
>> 2.56.0.rc1.310.g51773c2048-goog
>>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH 1/3] perf test: Fix errors when deleting data in perf record tests
  2026-09-23 21:52 [PATCH 1/3] perf test: Fix errors when deleting data in perf record tests Namhyung Kim
                   ` (3 preceding siblings ...)
  2026-09-24  3:39 ` Ian Rogers
@ 2026-09-25 10:51 ` Arnaldo Carvalho de Melo
  4 siblings, 0 replies; 11+ messages in thread
From: Arnaldo Carvalho de Melo @ 2026-09-25 10:51 UTC (permalink / raw)
  To: Namhyung Kim
  Cc: Ian Rogers, Jiri Olsa, Adrian Hunter, James Clark, Peter Zijlstra,
	Ingo Molnar, LKML, linux-perf-users

On Wed, Sep 23, 2026 at 02:52:53PM -0700, Namhyung Kim wrote:
> perf record with multithreads would create a data directory instead of a
> file.  But the cleanup and retry logic expect a file and try to delete
> them with 'rm -f' which would fail.  Add '-r' flag to remove directories.
> 
>   rm: cannot remove '/tmp/__perf_test.perf.data.ZVHTw.old': Is a directory
 

Thanks, applied the series to perf-tools-next, for v7.4.

- Arnaldo

> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/tests/shell/lib/perf_record.sh | 2 +-
>  tools/perf/tests/shell/record.sh          | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/perf/tests/shell/lib/perf_record.sh b/tools/perf/tests/shell/lib/perf_record.sh
> index 2b9e11b66dc7a801..a95fafb8b6fa8b40 100644
> --- a/tools/perf/tests/shell/lib/perf_record.sh
> +++ b/tools/perf/tests/shell/lib/perf_record.sh
> @@ -30,7 +30,7 @@ perf_record_with_retry() {
>    fi
>  
>    for duration in 0.01 0.1 0.3 1.0 2.0; do
> -    rm -f "${perfdata}".old
> +    rm -rf "${perfdata}".old
>      ${cmd_prefix} "$@" -o "${perfdata}" ${testprog_base} ${duration} > "$logfile" 2>&1
>      local record_exit=$?
>  
> diff --git a/tools/perf/tests/shell/record.sh b/tools/perf/tests/shell/record.sh
> index 4a4931cb5ba91849..cf8ba3d3411b0bcc 100755
> --- a/tools/perf/tests/shell/record.sh
> +++ b/tools/perf/tests/shell/record.sh
> @@ -39,8 +39,8 @@ default_fd_limit=$(ulimit -Sn)
>  min_fd_limit=$(($(getconf _NPROCESSORS_ONLN) * 16))
>  
>  cleanup() {
> -  rm -f "${perfdata}"
> -  rm -f "${perfdata}".old
> +  rm -rf "${perfdata}"
> +  rm -rf "${perfdata}".old
>    rm -f "${script_output}"
>    perf_record_cleanup
>  
> -- 
> 2.56.0.rc1.310.g51773c2048-goog

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-09-25 10:51 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-23 21:52 [PATCH 1/3] perf test: Fix errors when deleting data in perf record tests Namhyung Kim
2026-09-23 21:52 ` [PATCH 2/3] perf test: Fix record tests on Intel Broadwell Namhyung Kim
2026-09-23 22:01   ` sashiko-bot
2026-09-24  3:45   ` Ian Rogers
2026-09-24  6:55     ` Mi, Dapeng
2026-09-23 21:52 ` [PATCH 3/3] perf test: Fix record tests on hybrid machines Namhyung Kim
2026-09-23 21:59   ` sashiko-bot
2026-09-24  3:44   ` Ian Rogers
2026-09-23 22:00 ` [PATCH 1/3] perf test: Fix errors when deleting data in perf record tests sashiko-bot
2026-09-24  3:39 ` Ian Rogers
2026-09-25 10:51 ` Arnaldo Carvalho de Melo

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.