[PATCH] perf tests switch-tracking: Fix timestamp comparison

[PATCH] perf tests switch-tracking: Fix timestamp comparison

[Leo Yan]

The test might fail on the Arm64 platform with the error:

  perf test -vvv "Track with sched_switch"
  Missing sched_switch events

The issue is caused by incorrect handling of timestamp comparisons. The
comparison result, a signed 64-bit value, was being directly cast to an
int, leading to incorrect sorting for sched events.

Fix this by explicitly returning 0, 1, or -1 based on whether the result
is zero, positive, or negative.

Fixes: d44bc5582972 ("perf tests: Add a test for tracking with sched_switch")
Signed-off-by: Leo Yan <leo.yan@arm.com>
---
 tools/perf/tests/switch-tracking.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/perf/tests/switch-tracking.c b/tools/perf/tests/switch-tracking.c
index 8df3f9d9ffd2..6b3aac283c37 100644
--- a/tools/perf/tests/switch-tracking.c
+++ b/tools/perf/tests/switch-tracking.c
@@ -264,7 +264,7 @@ static int compar(const void *a, const void *b)
 	const struct event_node *nodeb = b;
 	s64 cmp = nodea->event_time - nodeb->event_time;
 
-	return cmp;
+	return cmp < 0 ? -1 : (cmp > 0 ? 1 : 0);
 }
 
 static int process_events(struct evlist *evlist,
-- 
2.34.1

Re: [PATCH] perf tests switch-tracking: Fix timestamp comparison

[Ian Rogers]

On Mon, Mar 31, 2025 at 10:28 AM Leo Yan <leo.yan@arm.com> wrote:
>
> The test might fail on the Arm64 platform with the error:
>
>   perf test -vvv "Track with sched_switch"
>   Missing sched_switch events
>
> The issue is caused by incorrect handling of timestamp comparisons. The
> comparison result, a signed 64-bit value, was being directly cast to an
> int, leading to incorrect sorting for sched events.
>
> Fix this by explicitly returning 0, 1, or -1 based on whether the result
> is zero, positive, or negative.

I'm reminded of a Java check I wrote for this:
https://errorprone.info/bugpattern/BadComparable
In clang -Wshorten-64-to-32 looks to cover this. I'll see if we can
clean those warnings up a bit.

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

Thanks,
Ian

> Fixes: d44bc5582972 ("perf tests: Add a test for tracking with sched_switch")
> Signed-off-by: Leo Yan <leo.yan@arm.com>
> ---
>  tools/perf/tests/switch-tracking.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/perf/tests/switch-tracking.c b/tools/perf/tests/switch-tracking.c
> index 8df3f9d9ffd2..6b3aac283c37 100644
> --- a/tools/perf/tests/switch-tracking.c
> +++ b/tools/perf/tests/switch-tracking.c
> @@ -264,7 +264,7 @@ static int compar(const void *a, const void *b)
>         const struct event_node *nodeb = b;
>         s64 cmp = nodea->event_time - nodeb->event_time;
>
> -       return cmp;
> +       return cmp < 0 ? -1 : (cmp > 0 ? 1 : 0);
>  }
>
>  static int process_events(struct evlist *evlist,
> --
> 2.34.1
>

Re: [PATCH] perf tests switch-tracking: Fix timestamp comparison

[Leo Yan]

On Mon, Mar 31, 2025 at 01:18:31PM -0700, Ian Rogers wrote:

[...]

> I'm reminded of a Java check I wrote for this:

Nice short article.

> In clang -Wshorten-64-to-32 looks to cover this. I'll see if we can
> clean those warnings up a bit.

I checked a bit and seems GCC has no this flag, but it makes sense for
me to enable the flag for Clang.

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

Thanks a lot, Ian.

Leo

Re: [PATCH] perf tests switch-tracking: Fix timestamp comparison

[Ian Rogers]

On Tue, Apr 1, 2025 at 2:14 AM Leo Yan <leo.yan@arm.com> wrote:
>
> On Mon, Mar 31, 2025 at 01:18:31PM -0700, Ian Rogers wrote:
>
> [...]
>
> > I'm reminded of a Java check I wrote for this:
>
> Nice short article.
>
> > In clang -Wshorten-64-to-32 looks to cover this. I'll see if we can
> > clean those warnings up a bit.
>
> I checked a bit and seems GCC has no this flag, but it makes sense for
> me to enable the flag for Clang.
>
> > Reviewed-by: Ian Rogers <irogers@google.com>
>
> Thanks a lot, Ian.

I made a small variation to the change in:
https://lore.kernel.org/lkml/20250401182347.3422199-10-irogers@google.com/
to avoid a subtract and just directly compare the values.

Thanks,
Ian

> Leo

Re: [PATCH] perf tests switch-tracking: Fix timestamp comparison

[Leo Yan]

On Tue, Apr 01, 2025 at 12:54:12PM -0700, Ian Rogers wrote:
> On Tue, Apr 1, 2025 at 2:14 AM Leo Yan <leo.yan@arm.com> wrote:
> >
> > On Mon, Mar 31, 2025 at 01:18:31PM -0700, Ian Rogers wrote:
> >
> > [...]
> >
> > > I'm reminded of a Java check I wrote for this:
> >
> > Nice short article.
> >
> > > In clang -Wshorten-64-to-32 looks to cover this. I'll see if we can
> > > clean those warnings up a bit.
> >
> > I checked a bit and seems GCC has no this flag, but it makes sense for
> > me to enable the flag for Clang.
> >
> > > Reviewed-by: Ian Rogers <irogers@google.com>
> >
> > Thanks a lot, Ian.
> 
> I made a small variation to the change in:
> https://lore.kernel.org/lkml/20250401182347.3422199-10-irogers@google.com/
> to avoid a subtract and just directly compare the values.

Fine by me.  I reviewed your patch, the direct comparing LGTM.

Thanks,
Leo

Re: [PATCH] perf tests switch-tracking: Fix timestamp comparison

[Leo Yan]

Hi Ian, Namhyung,

[ - Mailing list ]

On Wed, Apr 02, 2025 at 10:05:16AM +0100, Leo Yan wrote:
> On Tue, Apr 01, 2025 at 12:54:12PM -0700, Ian Rogers wrote:
> > On Tue, Apr 1, 2025 at 2:14 AM Leo Yan <leo.yan@arm.com> wrote:
> > >
> > > On Mon, Mar 31, 2025 at 01:18:31PM -0700, Ian Rogers wrote:
> > >
> > > [...]
> > >
> > > > I'm reminded of a Java check I wrote for this:
> > >
> > > Nice short article.
> > >
> > > > In clang -Wshorten-64-to-32 looks to cover this. I'll see if we can
> > > > clean those warnings up a bit.
> > >
> > > I checked a bit and seems GCC has no this flag, but it makes sense for
> > > me to enable the flag for Clang.
> > >
> > > > Reviewed-by: Ian Rogers <irogers@google.com>
> > >
> > > Thanks a lot, Ian.
> > 
> > I made a small variation to the change in:
> > https://lore.kernel.org/lkml/20250401182347.3422199-10-irogers@google.com/
> > to avoid a subtract and just directly compare the values.

Do you mind to pick up my this patch? :) Our internal CI reports the
test case 109_Track_with_sched_switch failure daily, I am just wandering
if we could apply the fix quickly.

Ian is working on a patch series for resolving the Clang warning which
also includes a fix [1], if Ian could extract the fix for the compar()
function in switch-tracking.c, this either would be fine for me.

Thanks a lot for your helping!

Leo

[1] https://lore.kernel.org/linux-perf-users/20250401182347.3422199-10-irogers@google.com/

Re: [PATCH] perf tests switch-tracking: Fix timestamp comparison

[Namhyung Kim]

Hi Leo,

On Fri, May 16, 2025 at 04:31:58PM +0100, Leo Yan wrote:
> Hi Ian, Namhyung,
> 
> [ - Mailing list ]
> 
> On Wed, Apr 02, 2025 at 10:05:16AM +0100, Leo Yan wrote:
> > On Tue, Apr 01, 2025 at 12:54:12PM -0700, Ian Rogers wrote:
> > > On Tue, Apr 1, 2025 at 2:14 AM Leo Yan <leo.yan@arm.com> wrote:
> > > >
> > > > On Mon, Mar 31, 2025 at 01:18:31PM -0700, Ian Rogers wrote:
> > > >
> > > > [...]
> > > >
> > > > > I'm reminded of a Java check I wrote for this:
> > > >
> > > > Nice short article.
> > > >
> > > > > In clang -Wshorten-64-to-32 looks to cover this. I'll see if we can
> > > > > clean those warnings up a bit.
> > > >
> > > > I checked a bit and seems GCC has no this flag, but it makes sense for
> > > > me to enable the flag for Clang.
> > > >
> > > > > Reviewed-by: Ian Rogers <irogers@google.com>
> > > >
> > > > Thanks a lot, Ian.
> > > 
> > > I made a small variation to the change in:
> > > https://lore.kernel.org/lkml/20250401182347.3422199-10-irogers@google.com/
> > > to avoid a subtract and just directly compare the values.
> 
> Do you mind to pick up my this patch? :) Our internal CI reports the
> test case 109_Track_with_sched_switch failure daily, I am just wandering
> if we could apply the fix quickly.

Arnaldo is taking care of patches for v6.15.

Arnaldo, can you please take this?

Thanks,
Namhyung

> 
> Ian is working on a patch series for resolving the Clang warning which
> also includes a fix [1], if Ian could extract the fix for the compar()
> function in switch-tracking.c, this either would be fine for me.
> 
> Thanks a lot for your helping!
> 
> Leo
> 
> [1] https://lore.kernel.org/linux-perf-users/20250401182347.3422199-10-irogers@google.com/

Re: [PATCH] perf tests switch-tracking: Fix timestamp comparison

[Arnaldo Carvalho de Melo]

On Mon, May 19, 2025 at 06:52:45PM -0700, Namhyung Kim wrote:
> Hi Leo,
> 
> On Fri, May 16, 2025 at 04:31:58PM +0100, Leo Yan wrote:
> > Hi Ian, Namhyung,
> > 
> > [ - Mailing list ]
> > 
> > On Wed, Apr 02, 2025 at 10:05:16AM +0100, Leo Yan wrote:
> > > On Tue, Apr 01, 2025 at 12:54:12PM -0700, Ian Rogers wrote:
> > > > On Tue, Apr 1, 2025 at 2:14 AM Leo Yan <leo.yan@arm.com> wrote:
> > > > >
> > > > > On Mon, Mar 31, 2025 at 01:18:31PM -0700, Ian Rogers wrote:
> > > > >
> > > > > [...]
> > > > >
> > > > > > I'm reminded of a Java check I wrote for this:
> > > > >
> > > > > Nice short article.
> > > > >
> > > > > > In clang -Wshorten-64-to-32 looks to cover this. I'll see if we can
> > > > > > clean those warnings up a bit.
> > > > >
> > > > > I checked a bit and seems GCC has no this flag, but it makes sense for
> > > > > me to enable the flag for Clang.
> > > > >
> > > > > > Reviewed-by: Ian Rogers <irogers@google.com>
> > > > >
> > > > > Thanks a lot, Ian.
> > > > 
> > > > I made a small variation to the change in:
> > > > https://lore.kernel.org/lkml/20250401182347.3422199-10-irogers@google.com/
> > > > to avoid a subtract and just directly compare the values.
> > 
> > Do you mind to pick up my this patch? :) Our internal CI reports the
> > test case 109_Track_with_sched_switch failure daily, I am just wandering
> > if we could apply the fix quickly.
> 
> Arnaldo is taking care of patches for v6.15.
> 
> Arnaldo, can you please take this?

From what I understood it people agreed to pick Ian's patch, ok.

- Arnaldo
 
> Thanks,
> Namhyung
> 
> > 
> > Ian is working on a patch series for resolving the Clang warning which
> > also includes a fix [1], if Ian could extract the fix for the compar()
> > function in switch-tracking.c, this either would be fine for me.
> > 
> > Thanks a lot for your helping!
> > 
> > Leo
> > 
> > [1] https://lore.kernel.org/linux-perf-users/20250401182347.3422199-10-irogers@google.com/

Re: [PATCH] perf tests switch-tracking: Fix timestamp comparison

[Arnaldo Carvalho de Melo]

On Thu, May 22, 2025 at 10:49:31PM -0300, Arnaldo Carvalho de Melo wrote:
> On Mon, May 19, 2025 at 06:52:45PM -0700, Namhyung Kim wrote:
> > Hi Leo,
> > 
> > On Fri, May 16, 2025 at 04:31:58PM +0100, Leo Yan wrote:
> > > Hi Ian, Namhyung,
> > > 
> > > [ - Mailing list ]
> > > 
> > > On Wed, Apr 02, 2025 at 10:05:16AM +0100, Leo Yan wrote:
> > > > On Tue, Apr 01, 2025 at 12:54:12PM -0700, Ian Rogers wrote:
> > > > > On Tue, Apr 1, 2025 at 2:14 AM Leo Yan <leo.yan@arm.com> wrote:
> > > > > >
> > > > > > On Mon, Mar 31, 2025 at 01:18:31PM -0700, Ian Rogers wrote:
> > > > > >
> > > > > > [...]
> > > > > >
> > > > > > > I'm reminded of a Java check I wrote for this:
> > > > > >
> > > > > > Nice short article.
> > > > > >
> > > > > > > In clang -Wshorten-64-to-32 looks to cover this. I'll see if we can
> > > > > > > clean those warnings up a bit.
> > > > > >
> > > > > > I checked a bit and seems GCC has no this flag, but it makes sense for
> > > > > > me to enable the flag for Clang.
> > > > > >
> > > > > > > Reviewed-by: Ian Rogers <irogers@google.com>
> > > > > >
> > > > > > Thanks a lot, Ian.
> > > > > 
> > > > > I made a small variation to the change in:
> > > > > https://lore.kernel.org/lkml/20250401182347.3422199-10-irogers@google.com/
> > > > > to avoid a subtract and just directly compare the values.
> > > 
> > > Do you mind to pick up my this patch? :) Our internal CI reports the
> > > test case 109_Track_with_sched_switch failure daily, I am just wandering
> > > if we could apply the fix quickly.
> > 
> > Arnaldo is taking care of patches for v6.15.
> > 
> > Arnaldo, can you please take this?
> 
> >From what I understood it people agreed to pick Ian's patch, ok.

But that is part of a larger patch, so since Ian reviewed Leo's patch,
I'll end up picking the more contained one, no bandwidth to check all
the other parts now.

- Arnaldo

Re: [PATCH] perf tests switch-tracking: Fix timestamp comparison

[Arnaldo Carvalho de Melo]

On Mon, Mar 31, 2025 at 06:27:59PM +0100, Leo Yan wrote:
> The test might fail on the Arm64 platform with the error:
> 
>   perf test -vvv "Track with sched_switch"
>   Missing sched_switch events
> 
> The issue is caused by incorrect handling of timestamp comparisons. The
> comparison result, a signed 64-bit value, was being directly cast to an
> int, leading to incorrect sorting for sched events.
> 
> Fix this by explicitly returning 0, 1, or -1 based on whether the result
> is zero, positive, or negative.
> 
> Fixes: d44bc5582972 ("perf tests: Add a test for tracking with sched_switch")
> Signed-off-by: Leo Yan <leo.yan@arm.com>

How can I reproduce this?

Testing on a rpi5, 64-bit debian, this test passes:

root@raspberrypi:~# uname -a
Linux raspberrypi 6.12.25+rpt-rpi-2712 #1 SMP PREEMPT Debian 1:6.12.25-1+rpt1 (2025-04-30) aarch64 GNU/Linux
root@raspberrypi:~# perf test -vvv "Track with sched_switch" |& tail -50
cycles event
cycles event
cycles event
cycles event
cycles event
cycles event
cycles event
cycles event
cycles event
cycles event
cycles event
cycles event
cycles event
cycles event
cycles event
cycles event
cycles event
cycles event
cycles event
cycles event
cycles event
cycles event
cycles event
cycles event
sched_switch: cpu: 3 prev_tid 208750 next_tid 0
sched_switch: cpu: 2 prev_tid 0 next_tid 195941
sched_switch: cpu: 2 prev_tid 195941 next_tid 208748
sched_switch: cpu: 2 prev_tid 208748 next_tid 0
sched_switch: cpu: 3 prev_tid 0 next_tid 208750
sched_switch: cpu: 3 prev_tid 208750 next_tid 34
sched_switch: cpu: 3 prev_tid 34 next_tid 0
sched_switch: cpu: 0 prev_tid 0 next_tid 208750
sched_switch: cpu: 0 prev_tid 208750 next_tid 21
sched_switch: cpu: 0 prev_tid 21 next_tid 0
sched_switch: cpu: 1 prev_tid 0 next_tid 208750
sched_switch: cpu: 1 prev_tid 208750 next_tid 24
sched_switch: cpu: 1 prev_tid 24 next_tid 0
sched_switch: cpu: 2 prev_tid 0 next_tid 208750
sched_switch: cpu: 2 prev_tid 208750 next_tid 29
sched_switch: cpu: 3 prev_tid 0 next_tid 208750
sched_switch: cpu: 2 prev_tid 29 next_tid 0
sched_switch: cpu: 3 prev_tid 208750 next_tid 34
sched_switch: cpu: 3 prev_tid 34 next_tid 0
sched_switch: cpu: 0 prev_tid 0 next_tid 208750
sched_switch: cpu: 1 prev_tid 0 next_tid 208750
sched_switch: cpu: 2 prev_tid 0 next_tid 208750
sched_switch: cpu: 3 prev_tid 0 next_tid 208750
507 events recorded
---- end(0) ----
109: Track with sched_switch                                         : Ok
root@raspberrypi:~#

Re: [PATCH] perf tests switch-tracking: Fix timestamp comparison

[Arnaldo Carvalho de Melo]

On Thu, May 22, 2025 at 10:55:46PM -0300, Arnaldo Carvalho de Melo wrote:
> On Mon, Mar 31, 2025 at 06:27:59PM +0100, Leo Yan wrote:
> > The test might fail on the Arm64 platform with the error:
> > 
> >   perf test -vvv "Track with sched_switch"
> >   Missing sched_switch events
> > 
> > The issue is caused by incorrect handling of timestamp comparisons. The
> > comparison result, a signed 64-bit value, was being directly cast to an
> > int, leading to incorrect sorting for sched events.
> > 
> > Fix this by explicitly returning 0, 1, or -1 based on whether the result
> > is zero, positive, or negative.
> > 
> > Fixes: d44bc5582972 ("perf tests: Add a test for tracking with sched_switch")
> > Signed-off-by: Leo Yan <leo.yan@arm.com>
> 
> How can I reproduce this?
> 
> Testing on a rpi5, 64-bit debian, this test passes:
> 
> root@raspberrypi:~# uname -a
> Linux raspberrypi 6.12.25+rpt-rpi-2712 #1 SMP PREEMPT Debian 1:6.12.25-1+rpt1 (2025-04-30) aarch64 GNU/Linux
> root@raspberrypi:~# perf test -vvv "Track with sched_switch" |& tail -50
> cycles event
> cycles event
> cycles event
> cycles event
> cycles event
> cycles event
> cycles event
> cycles event
> cycles event
> cycles event
> cycles event
> cycles event
> cycles event
> cycles event
> cycles event
> cycles event
> cycles event
> cycles event
> cycles event
> cycles event
> cycles event
> cycles event
> cycles event
> cycles event
> sched_switch: cpu: 3 prev_tid 208750 next_tid 0
> sched_switch: cpu: 2 prev_tid 0 next_tid 195941
> sched_switch: cpu: 2 prev_tid 195941 next_tid 208748
> sched_switch: cpu: 2 prev_tid 208748 next_tid 0
> sched_switch: cpu: 3 prev_tid 0 next_tid 208750
> sched_switch: cpu: 3 prev_tid 208750 next_tid 34
> sched_switch: cpu: 3 prev_tid 34 next_tid 0
> sched_switch: cpu: 0 prev_tid 0 next_tid 208750
> sched_switch: cpu: 0 prev_tid 208750 next_tid 21
> sched_switch: cpu: 0 prev_tid 21 next_tid 0
> sched_switch: cpu: 1 prev_tid 0 next_tid 208750
> sched_switch: cpu: 1 prev_tid 208750 next_tid 24
> sched_switch: cpu: 1 prev_tid 24 next_tid 0
> sched_switch: cpu: 2 prev_tid 0 next_tid 208750
> sched_switch: cpu: 2 prev_tid 208750 next_tid 29
> sched_switch: cpu: 3 prev_tid 0 next_tid 208750
> sched_switch: cpu: 2 prev_tid 29 next_tid 0
> sched_switch: cpu: 3 prev_tid 208750 next_tid 34
> sched_switch: cpu: 3 prev_tid 34 next_tid 0
> sched_switch: cpu: 0 prev_tid 0 next_tid 208750
> sched_switch: cpu: 1 prev_tid 0 next_tid 208750
> sched_switch: cpu: 2 prev_tid 0 next_tid 208750
> sched_switch: cpu: 3 prev_tid 0 next_tid 208750
> 507 events recorded
> ---- end(0) ----
> 109: Track with sched_switch                                         : Ok
> root@raspberrypi:~#

Further info:

acme@raspberrypi:~/git/perf-tools-next $ readelf -wi ~/bin/perf | head -40
Contents of the .debug_info section:

  Compilation Unit @ offset 0:
   Length:        0x79b (32-bit)
   Version:       5
   Unit Type:     DW_UT_compile (1)
   Abbrev Offset: 0
   Pointer Size:  8
 <0><c>: Abbrev Number: 22 (DW_TAG_compile_unit)
    <d>   DW_AT_producer    : (indirect string, offset: 0x9401): GNU C99 12.2.0 -mlittle-endian -mabi=lp64 -ggdb3 -std=gnu99 -fPIC -fasynchronous-unwind-tables
    <11>   DW_AT_language    : 12	(ANSI C99)
    <12>   DW_AT_name        : (indirect line string, offset: 0): fd/array.c
    <16>   DW_AT_comp_dir    : (indirect line string, offset: 0xb): /home/acme/git/perf-tools-next/tools/lib/api
    <1a>   DW_AT_low_pc      : 0xf80a0
    <22>   DW_AT_high_pc     : 0x650
    <2a>   DW_AT_stmt_list   : 0
    <2e>   DW_AT_macros      : 0
 <1><32>: Abbrev Number: 9 (DW_TAG_typedef)
    <33>   DW_AT_name        : (indirect string, offset: 0x44a1): size_t
    <37>   DW_AT_decl_file   : 2
    <38>   DW_AT_decl_line   : 214
    <39>   DW_AT_decl_column : 23
    <3a>   DW_AT_type        : <0x3e>
 <1><3e>: Abbrev Number: 5 (DW_TAG_base_type)
    <3f>   DW_AT_byte_size   : 8
    <40>   DW_AT_encoding    : 7	(unsigned)
    <41>   DW_AT_name        : (indirect string, offset: 0x137b): long unsigned int
 <1><45>: Abbrev Number: 23 (DW_TAG_pointer_type)
    <46>   DW_AT_byte_size   : 8
 <1><47>: Abbrev Number: 24 (DW_TAG_base_type)
    <48>   DW_AT_byte_size   : 4
    <49>   DW_AT_encoding    : 5	(signed)
    <4a>   DW_AT_name        : int
 <1><4e>: Abbrev Number: 5 (DW_TAG_base_type)
    <4f>   DW_AT_byte_size   : 1
    <50>   DW_AT_encoding    : 8	(unsigned char)
    <51>   DW_AT_name        : (indirect string, offset: 0x18e6): unsigned char
 <1><55>: Abbrev Number: 5 (DW_TAG_base_type)
    <56>   DW_AT_byte_size   : 2
    <57>   DW_AT_encoding    : 7	(unsigned)
acme@raspberrypi:~/git/perf-tools-next $

Re: [PATCH] perf tests switch-tracking: Fix timestamp comparison

[Leo Yan]

On Thu, May 22, 2025 at 10:57:41PM -0300, Arnaldo Carvalho de Melo wrote:
> On Thu, May 22, 2025 at 10:55:46PM -0300, Arnaldo Carvalho de Melo wrote:
> > On Mon, Mar 31, 2025 at 06:27:59PM +0100, Leo Yan wrote:
> > > The test might fail on the Arm64 platform with the error:
> > > 
> > >   perf test -vvv "Track with sched_switch"
> > >   Missing sched_switch events
> > > 
> > > The issue is caused by incorrect handling of timestamp comparisons. The
> > > comparison result, a signed 64-bit value, was being directly cast to an
> > > int, leading to incorrect sorting for sched events.
> > > 
> > > Fix this by explicitly returning 0, 1, or -1 based on whether the result
> > > is zero, positive, or negative.
> > > 
> > > Fixes: d44bc5582972 ("perf tests: Add a test for tracking with sched_switch")
> > > Signed-off-by: Leo Yan <leo.yan@arm.com>
> > 
> > How can I reproduce this?
> > 
> > Testing on a rpi5, 64-bit debian, this test passes:

Sorry that I did not give precise info for reproducing the failure.
The case does not fail everytime, usually I can trigger the failure
after run 20 ~ 30 times:

# while true; do perf test "Track with sched_switch"; done
106: Track with sched_switch                                         : Ok
106: Track with sched_switch                                         : Ok
106: Track with sched_switch                                         : Ok
106: Track with sched_switch                                         : Ok
106: Track with sched_switch                                         : Ok
106: Track with sched_switch                                         : Ok
106: Track with sched_switch                                         : Ok
106: Track with sched_switch                                         : Ok
106: Track with sched_switch                                         : Ok
106: Track with sched_switch                                         : Ok
106: Track with sched_switch                                         : Ok
106: Track with sched_switch                                         : Ok
106: Track with sched_switch                                         : Ok
106: Track with sched_switch                                         : Ok
106: Track with sched_switch                                         : FAILED!
106: Track with sched_switch                                         : Ok
106: Track with sched_switch                                         : Ok
106: Track with sched_switch                                         : Ok
106: Track with sched_switch                                         : Ok
106: Track with sched_switch                                         : Ok
106: Track with sched_switch                                         : Ok
106: Track with sched_switch                                         : Ok
106: Track with sched_switch                                         : Ok
106: Track with sched_switch                                         : FAILED!
106: Track with sched_switch                                         : Ok
106: Track with sched_switch                                         : Ok

I used cross compiler to build Perf tool on my host machine and tested on
Debian / Juno board.  Generally, I think this issue is not very specific
to GCC versions.  As both internal CI and my local env can reproduce the
issue.

Please let me know if need any more info.  Thanks!


---8<---

My Host Build compiler:

# aarch64-linux-gnu-gcc --version
aarch64-linux-gnu-gcc (Ubuntu 13.3.0-6ubuntu2~24.04) 13.3.0

Juno Board:

# lsb_release -a
No LSB modules are available.
Distributor ID: Debian
Description:    Debian GNU/Linux 12 (bookworm)
Release:        12
Codename:       bookworm

Re: [PATCH] perf tests switch-tracking: Fix timestamp comparison

[Ian Rogers]

On Fri, May 23, 2025 at 1:10 AM Leo Yan <leo.yan@arm.com> wrote:
>
> On Thu, May 22, 2025 at 10:57:41PM -0300, Arnaldo Carvalho de Melo wrote:
> > On Thu, May 22, 2025 at 10:55:46PM -0300, Arnaldo Carvalho de Melo wrote:
> > > On Mon, Mar 31, 2025 at 06:27:59PM +0100, Leo Yan wrote:
> > > > The test might fail on the Arm64 platform with the error:
> > > >
> > > >   perf test -vvv "Track with sched_switch"
> > > >   Missing sched_switch events
> > > >
> > > > The issue is caused by incorrect handling of timestamp comparisons. The
> > > > comparison result, a signed 64-bit value, was being directly cast to an
> > > > int, leading to incorrect sorting for sched events.
> > > >
> > > > Fix this by explicitly returning 0, 1, or -1 based on whether the result
> > > > is zero, positive, or negative.
> > > >
> > > > Fixes: d44bc5582972 ("perf tests: Add a test for tracking with sched_switch")
> > > > Signed-off-by: Leo Yan <leo.yan@arm.com>
> > >
> > > How can I reproduce this?
> > >
> > > Testing on a rpi5, 64-bit debian, this test passes:
>
> Sorry that I did not give precise info for reproducing the failure.
> The case does not fail everytime, usually I can trigger the failure
> after run 20 ~ 30 times:
>
> # while true; do perf test "Track with sched_switch"; done
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : FAILED!
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : FAILED!
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok

Just to advertise the "-r" or "--runs-per-test" option:
```
$ perf test -v -r 100 "Track with sched_switch"
```
The test is exclusive so you won't get any parallelism from this, but
you could in other cases.

Thanks,
Ian

> I used cross compiler to build Perf tool on my host machine and tested on
> Debian / Juno board.  Generally, I think this issue is not very specific
> to GCC versions.  As both internal CI and my local env can reproduce the
> issue.
>
> Please let me know if need any more info.  Thanks!
>
>
> ---8<---
>
> My Host Build compiler:
>
> # aarch64-linux-gnu-gcc --version
> aarch64-linux-gnu-gcc (Ubuntu 13.3.0-6ubuntu2~24.04) 13.3.0
>
> Juno Board:
>
> # lsb_release -a
> No LSB modules are available.
> Distributor ID: Debian
> Description:    Debian GNU/Linux 12 (bookworm)
> Release:        12
> Codename:       bookworm

Re: [PATCH] perf tests switch-tracking: Fix timestamp comparison

[Arnaldo Carvalho de Melo]

On Fri, May 23, 2025 at 09:10:36AM +0100, Leo Yan wrote:
> On Thu, May 22, 2025 at 10:57:41PM -0300, Arnaldo Carvalho de Melo wrote:
> > On Thu, May 22, 2025 at 10:55:46PM -0300, Arnaldo Carvalho de Melo wrote:
> > > On Mon, Mar 31, 2025 at 06:27:59PM +0100, Leo Yan wrote:
> > > > The test might fail on the Arm64 platform with the error:

> > > >   perf test -vvv "Track with sched_switch"
> > > >   Missing sched_switch events

> > > > The issue is caused by incorrect handling of timestamp comparisons. The
> > > > comparison result, a signed 64-bit value, was being directly cast to an
> > > > int, leading to incorrect sorting for sched events.

> > > > Fix this by explicitly returning 0, 1, or -1 based on whether the result
> > > > is zero, positive, or negative.

> > > > Fixes: d44bc5582972 ("perf tests: Add a test for tracking with sched_switch")
> > > > Signed-off-by: Leo Yan <leo.yan@arm.com>

> > > How can I reproduce this?

> > > Testing on a rpi5, 64-bit debian, this test passes:
> 
> Sorry that I did not give precise info for reproducing the failure.
> The case does not fail everytime, usually I can trigger the failure
> after run 20 ~ 30 times:
> 
> # while true; do perf test "Track with sched_switch"; done
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : FAILED!
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : FAILED!
> 106: Track with sched_switch                                         : Ok
> 106: Track with sched_switch                                         : Ok
 
> I used cross compiler to build Perf tool on my host machine and tested on
> Debian / Juno board.  Generally, I think this issue is not very specific
> to GCC versions.  As both internal CI and my local env can reproduce the
> issue.
 
> Please let me know if need any more info.  Thanks!
 
> ---8<---
 
> My Host Build compiler:
 
> # aarch64-linux-gnu-gcc --version
> aarch64-linux-gnu-gcc (Ubuntu 13.3.0-6ubuntu2~24.04) 13.3.0
 
> Juno Board:
 
> # lsb_release -a
> No LSB modules are available.
> Distributor ID: Debian
> Description:    Debian GNU/Linux 12 (bookworm)
> Release:        12
> Codename:       bookworm

Thanks for the extra info, I'll add it to the commit log message, and
perhaps we could make this test exclusive and use stress-ng to generate
some background noise in the form of a good number of processes, see:

root@x1:~# stress-ng --switch $(($(nproc) * 2)) --timeout 30s & for a in $(seq 50) ; do perf test switch ; done
[1] 1773322
stress-ng: info:  [1773322] setting to a 30 secs run per stressor
 77: Track with sched_switch                          : Running (1 active)
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : Running (1 active)
stress-ng: info:  [1773322] skipped: 0
stress-ng: info:  [1773322] passed: 24: switch (24)
stress-ng: info:  [1773322] failed: 0
stress-ng: info:  [1773322] metrics untrustworthy: 0
 77: Track with sched_switch                          : FAILED!
[1]+  Done                    stress-ng --switch $(($(nproc) * 2)) --timeout 30s
 77: Track with sched_switch                          : Ok
 77: Track with sched_switch                          : Ok
 77: Track with sched_switch                          : Ok
 77: Track with sched_switch                          : Ok
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : Ok
 77: Track with sched_switch                          : Ok
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : FAILED!
 77: Track with sched_switch                          : Ok
root@x1:~#

Now with your patch it also fails, so its for another reason:

--- start ---
test child forked, pid 1777071
Using CPUID GenuineIntel-6-BA-3
mmap size 528384B
45221 events recorded
Missing comm events
---- end(-1) ----
113: Track with sched_switch                                         : FAILED!

Lots of short lived processes makes it fail as well :-\

Oh well...

I was just trying to improve this test case so that we would show it
failing before your patch and passing after it, but I ran out of time
:-\

Your patch is correct, so I'll probably just add your comments and go
with it.

- Arnaldo

Re: [PATCH] perf tests switch-tracking: Fix timestamp comparison

[Leo Yan]

On Fri, May 23, 2025 at 02:12:58PM -0300, Arnaldo Carvalho de Melo wrote:

[...]

> Thanks for the extra info, I'll add it to the commit log message, and
> perhaps we could make this test exclusive and use stress-ng to generate
> some background noise in the form of a good number of processes, see:
> 
> root@x1:~# stress-ng --switch $(($(nproc) * 2)) --timeout 30s & for a in $(seq 50) ; do perf test switch ; done

Thanks for sharing the test command.

> Now with your patch it also fails, so its for another reason:
> 
> --- start ---
> test child forked, pid 1777071
> Using CPUID GenuineIntel-6-BA-3
> mmap size 528384B
> 45221 events recorded
> Missing comm events
> ---- end(-1) ----
> 113: Track with sched_switch                                         : FAILED!
> 
> Lots of short lived processes makes it fail as well :-\

I searched internal CI record, we also occasionally saw the error:

  Missing cycles events

I will find time to check if anything in test can be improved.  Seems
to me, the test is fragile if system has background activities.

> Your patch is correct, so I'll probably just add your comments and go
> with it.

Thanks!  Also thanks Ian's suggestion for the iteration command.

Leo

Re: [PATCH] perf tests switch-tracking: Fix timestamp comparison

[Arnaldo Carvalho de Melo]

On Tue, May 27, 2025 at 10:49:24AM +0100, Leo Yan wrote:
> On Fri, May 23, 2025 at 02:12:58PM -0300, Arnaldo Carvalho de Melo wrote:
 
> [...]
 
> > Thanks for the extra info, I'll add it to the commit log message, and
> > perhaps we could make this test exclusive and use stress-ng to generate
> > some background noise in the form of a good number of processes, see:

> > root@x1:~# stress-ng --switch $(($(nproc) * 2)) --timeout 30s & for a in $(seq 50) ; do perf test switch ; done
 
> Thanks for sharing the test command.
> 
> > Now with your patch it also fails, so its for another reason:

> > --- start ---
> > test child forked, pid 1777071
> > Using CPUID GenuineIntel-6-BA-3
> > mmap size 528384B
> > 45221 events recorded
> > Missing comm events
> > ---- end(-1) ----
> > 113: Track with sched_switch                                         : FAILED!
> > 
> > Lots of short lived processes makes it fail as well :-\
 
> I searched internal CI record, we also occasionally saw the error:
 
>   Missing cycles events
 
> I will find time to check if anything in test can be improved.  Seems
> to me, the test is fragile if system has background activities.

Great!
 
> > Your patch is correct, so I'll probably just add your comments and go
> > with it.
 
> Thanks!  Also thanks Ian's suggestion for the iteration command.

You're welcome, thanks for your work on improving perf, really
appreciated!

- Arnaldo