2022-11-16 23:44:21

by Namhyung Kim

[permalink] [raw]
Subject: [PATCH 05/12] perf test: Add 'leafloop' test workload

The leafloop workload is to run an infinite loop in the test_leaf
function. This is needed for the ARM fp callgraph test to verify if it
gets the correct callchains.

$ perf test -w leafloop

Signed-off-by: Namhyung Kim <[email protected]>
---
tools/perf/tests/builtin-test.c | 1 +
tools/perf/tests/tests.h | 1 +
tools/perf/tests/workloads/Build | 3 +++
tools/perf/tests/workloads/leafloop.c | 34 +++++++++++++++++++++++++++
4 files changed, 39 insertions(+)
create mode 100644 tools/perf/tests/workloads/leafloop.c

diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
index 161f38476e77..0ed5ac452f6e 100644
--- a/tools/perf/tests/builtin-test.c
+++ b/tools/perf/tests/builtin-test.c
@@ -121,6 +121,7 @@ static struct test_suite **tests[] = {
static struct test_workload *workloads[] = {
&workload__noploop,
&workload__thloop,
+ &workload__leafloop,
};

static int num_subtests(const struct test_suite *t)
diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
index e6edfeeadaeb..86804dd6452b 100644
--- a/tools/perf/tests/tests.h
+++ b/tools/perf/tests/tests.h
@@ -202,5 +202,6 @@ struct test_workload workload__##work = { \
/* The list of test workloads */
DECLARE_WORKLOAD(noploop);
DECLARE_WORKLOAD(thloop);
+DECLARE_WORKLOAD(leafloop);

#endif /* TESTS_H */
diff --git a/tools/perf/tests/workloads/Build b/tools/perf/tests/workloads/Build
index b8964b1099c0..631596bdb2b3 100644
--- a/tools/perf/tests/workloads/Build
+++ b/tools/perf/tests/workloads/Build
@@ -2,3 +2,6 @@

perf-y += noploop.o
perf-y += thloop.o
+perf-y += leafloop.o
+
+CFLAGS_leafloop.o = -g -O0 -fno-inline -fno-omit-frame-pointer
diff --git a/tools/perf/tests/workloads/leafloop.c b/tools/perf/tests/workloads/leafloop.c
new file mode 100644
index 000000000000..1bf5cc97649b
--- /dev/null
+++ b/tools/perf/tests/workloads/leafloop.c
@@ -0,0 +1,34 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#include <stdlib.h>
+#include <linux/compiler.h>
+#include "../tests.h"
+
+/* We want to check these symbols in perf script */
+noinline void leaf(volatile int b);
+noinline void parent(volatile int b);
+
+static volatile int a;
+
+noinline void leaf(volatile int b)
+{
+ for (;;)
+ a += b;
+}
+
+noinline void parent(volatile int b)
+{
+ leaf(b);
+}
+
+static int leafloop(int argc, const char **argv)
+{
+ int c = 1;
+
+ if (argc > 0)
+ c = atoi(argv[0]);
+
+ parent(c);
+ return 0;
+}
+
+DEFINE_WORKLOAD(leafloop);
--
2.38.1.584.g0f3c55d4c2-goog



2022-11-17 16:26:26

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 05/12] perf test: Add 'leafloop' test workload

Em Wed, Nov 16, 2022 at 03:38:47PM -0800, Namhyung Kim escreveu:
> The leafloop workload is to run an infinite loop in the test_leaf
> function. This is needed for the ARM fp callgraph test to verify if it
> gets the correct callchains.
>
> $ perf test -w leafloop

On fedora:36

In file included from /usr/include/bits/libc-header-start.h:33,
from /usr/include/stdlib.h:26,
from tests/workloads/leafloop.c:2:
/usr/include/features.h:412:4: error: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Werror=cpp]
412 | # warning _FORTIFY_SOURCE requires compiling with optimization (-O)
| ^~~~~~~
cc1: all warnings being treated as errors
make[5]: *** [/home/acme/git/perf/tools/build/Makefile.build:96: /tmp/build/perf/tests/workloads/leafloop.o] Error 1
make[5]: *** Waiting for unfinished jobs....

I'll try removing the _FORTIFY_SOURCE

> Signed-off-by: Namhyung Kim <[email protected]>
> ---
> tools/perf/tests/builtin-test.c | 1 +
> tools/perf/tests/tests.h | 1 +
> tools/perf/tests/workloads/Build | 3 +++
> tools/perf/tests/workloads/leafloop.c | 34 +++++++++++++++++++++++++++
> 4 files changed, 39 insertions(+)
> create mode 100644 tools/perf/tests/workloads/leafloop.c
>
> diff --git a/tools/perf/tests/builtin-test.c b/tools/perf/tests/builtin-test.c
> index 161f38476e77..0ed5ac452f6e 100644
> --- a/tools/perf/tests/builtin-test.c
> +++ b/tools/perf/tests/builtin-test.c
> @@ -121,6 +121,7 @@ static struct test_suite **tests[] = {
> static struct test_workload *workloads[] = {
> &workload__noploop,
> &workload__thloop,
> + &workload__leafloop,
> };
>
> static int num_subtests(const struct test_suite *t)
> diff --git a/tools/perf/tests/tests.h b/tools/perf/tests/tests.h
> index e6edfeeadaeb..86804dd6452b 100644
> --- a/tools/perf/tests/tests.h
> +++ b/tools/perf/tests/tests.h
> @@ -202,5 +202,6 @@ struct test_workload workload__##work = { \
> /* The list of test workloads */
> DECLARE_WORKLOAD(noploop);
> DECLARE_WORKLOAD(thloop);
> +DECLARE_WORKLOAD(leafloop);
>
> #endif /* TESTS_H */
> diff --git a/tools/perf/tests/workloads/Build b/tools/perf/tests/workloads/Build
> index b8964b1099c0..631596bdb2b3 100644
> --- a/tools/perf/tests/workloads/Build
> +++ b/tools/perf/tests/workloads/Build
> @@ -2,3 +2,6 @@
>
> perf-y += noploop.o
> perf-y += thloop.o
> +perf-y += leafloop.o
> +
> +CFLAGS_leafloop.o = -g -O0 -fno-inline -fno-omit-frame-pointer
> diff --git a/tools/perf/tests/workloads/leafloop.c b/tools/perf/tests/workloads/leafloop.c
> new file mode 100644
> index 000000000000..1bf5cc97649b
> --- /dev/null
> +++ b/tools/perf/tests/workloads/leafloop.c
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#include <stdlib.h>
> +#include <linux/compiler.h>
> +#include "../tests.h"
> +
> +/* We want to check these symbols in perf script */
> +noinline void leaf(volatile int b);
> +noinline void parent(volatile int b);
> +
> +static volatile int a;
> +
> +noinline void leaf(volatile int b)
> +{
> + for (;;)
> + a += b;
> +}
> +
> +noinline void parent(volatile int b)
> +{
> + leaf(b);
> +}
> +
> +static int leafloop(int argc, const char **argv)
> +{
> + int c = 1;
> +
> + if (argc > 0)
> + c = atoi(argv[0]);
> +
> + parent(c);
> + return 0;
> +}
> +
> +DEFINE_WORKLOAD(leafloop);
> --
> 2.38.1.584.g0f3c55d4c2-goog

--

- Arnaldo

2022-11-17 17:19:47

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 05/12] perf test: Add 'leafloop' test workload

Em Thu, Nov 17, 2022 at 01:06:16PM -0300, Arnaldo Carvalho de Melo escreveu:
> Em Wed, Nov 16, 2022 at 03:38:47PM -0800, Namhyung Kim escreveu:
> > The leafloop workload is to run an infinite loop in the test_leaf
> > function. This is needed for the ARM fp callgraph test to verify if it
> > gets the correct callchains.
> >
> > $ perf test -w leafloop
>
> On fedora:36
>
> In file included from /usr/include/bits/libc-header-start.h:33,
> from /usr/include/stdlib.h:26,
> from tests/workloads/leafloop.c:2:
> /usr/include/features.h:412:4: error: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Werror=cpp]
> 412 | # warning _FORTIFY_SOURCE requires compiling with optimization (-O)
> | ^~~~~~~
> cc1: all warnings being treated as errors
> make[5]: *** [/home/acme/git/perf/tools/build/Makefile.build:96: /tmp/build/perf/tests/workloads/leafloop.o] Error 1
> make[5]: *** Waiting for unfinished jobs....
>
> I'll try removing the _FORTIFY_SOURCE

Works after I added this to datasym.c, leafloop.c and brstack.c:


diff --git a/tools/perf/tests/workloads/leafloop.c b/tools/perf/tests/workloads/leafloop.c
index 1bf5cc97649b0e23..5d72c001320e3013 100644
--- a/tools/perf/tests/workloads/leafloop.c
+++ b/tools/perf/tests/workloads/leafloop.c
@@ -1,4 +1,5 @@
/* SPDX-License-Identifier: GPL-2.0 */
+#undef _FORTIFY_SOURCE
#include <stdlib.h>
#include <linux/compiler.h>
#include "../tests.h"

2022-11-17 17:36:54

by Ian Rogers

[permalink] [raw]
Subject: Re: [PATCH 05/12] perf test: Add 'leafloop' test workload

On Thu, Nov 17, 2022 at 8:15 AM Arnaldo Carvalho de Melo
<[email protected]> wrote:
>
> Em Thu, Nov 17, 2022 at 01:06:16PM -0300, Arnaldo Carvalho de Melo escreveu:
> > Em Wed, Nov 16, 2022 at 03:38:47PM -0800, Namhyung Kim escreveu:
> > > The leafloop workload is to run an infinite loop in the test_leaf
> > > function. This is needed for the ARM fp callgraph test to verify if it
> > > gets the correct callchains.
> > >
> > > $ perf test -w leafloop
> >
> > On fedora:36
> >
> > In file included from /usr/include/bits/libc-header-start.h:33,
> > from /usr/include/stdlib.h:26,
> > from tests/workloads/leafloop.c:2:
> > /usr/include/features.h:412:4: error: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Werror=cpp]
> > 412 | # warning _FORTIFY_SOURCE requires compiling with optimization (-O)
> > | ^~~~~~~
> > cc1: all warnings being treated as errors
> > make[5]: *** [/home/acme/git/perf/tools/build/Makefile.build:96: /tmp/build/perf/tests/workloads/leafloop.o] Error 1
> > make[5]: *** Waiting for unfinished jobs....
> >
> > I'll try removing the _FORTIFY_SOURCE
>
> Works after I added this to datasym.c, leafloop.c and brstack.c:

Is there a reason we are compiling without -O ? Perhaps we can filter
setting _FORTIFY_SOURCE so that it depends on -O being enabled.

Thanks,
Ian

> diff --git a/tools/perf/tests/workloads/leafloop.c b/tools/perf/tests/workloads/leafloop.c
> index 1bf5cc97649b0e23..5d72c001320e3013 100644
> --- a/tools/perf/tests/workloads/leafloop.c
> +++ b/tools/perf/tests/workloads/leafloop.c
> @@ -1,4 +1,5 @@
> /* SPDX-License-Identifier: GPL-2.0 */
> +#undef _FORTIFY_SOURCE
> #include <stdlib.h>
> #include <linux/compiler.h>
> #include "../tests.h"

2022-11-17 17:52:48

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 05/12] perf test: Add 'leafloop' test workload

Em Thu, Nov 17, 2022 at 09:16:58AM -0800, Ian Rogers escreveu:
> On Thu, Nov 17, 2022 at 8:15 AM Arnaldo Carvalho de Melo
> <[email protected]> wrote:
> >
> > Em Thu, Nov 17, 2022 at 01:06:16PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > Em Wed, Nov 16, 2022 at 03:38:47PM -0800, Namhyung Kim escreveu:
> > > > The leafloop workload is to run an infinite loop in the test_leaf
> > > > function. This is needed for the ARM fp callgraph test to verify if it
> > > > gets the correct callchains.
> > > >
> > > > $ perf test -w leafloop
> > >
> > > On fedora:36
> > >
> > > In file included from /usr/include/bits/libc-header-start.h:33,
> > > from /usr/include/stdlib.h:26,
> > > from tests/workloads/leafloop.c:2:
> > > /usr/include/features.h:412:4: error: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Werror=cpp]
> > > 412 | # warning _FORTIFY_SOURCE requires compiling with optimization (-O)
> > > | ^~~~~~~
> > > cc1: all warnings being treated as errors
> > > make[5]: *** [/home/acme/git/perf/tools/build/Makefile.build:96: /tmp/build/perf/tests/workloads/leafloop.o] Error 1
> > > make[5]: *** Waiting for unfinished jobs....
> > >
> > > I'll try removing the _FORTIFY_SOURCE
> >
> > Works after I added this to datasym.c, leafloop.c and brstack.c:
>
> Is there a reason we are compiling without -O ? Perhaps we can filter

I assumed so as Namhyung added it, perhaps he is just carrying it from
the pre-existing shell tests?

I wonder its to have a predictable binary output that the test expects
when doing things like hardware tracing? As it come from the coresight
tests, IIRC.

- Arnaldo

> setting _FORTIFY_SOURCE so that it depends on -O being enabled.

> Thanks,
> Ian
>
> > diff --git a/tools/perf/tests/workloads/leafloop.c b/tools/perf/tests/workloads/leafloop.c
> > index 1bf5cc97649b0e23..5d72c001320e3013 100644
> > --- a/tools/perf/tests/workloads/leafloop.c
> > +++ b/tools/perf/tests/workloads/leafloop.c
> > @@ -1,4 +1,5 @@
> > /* SPDX-License-Identifier: GPL-2.0 */
> > +#undef _FORTIFY_SOURCE
> > #include <stdlib.h>
> > #include <linux/compiler.h>
> > #include "../tests.h"

--

- Arnaldo

2022-11-17 18:10:56

by Ian Rogers

[permalink] [raw]
Subject: Re: [PATCH 05/12] perf test: Add 'leafloop' test workload

On Thu, Nov 17, 2022 at 9:24 AM Arnaldo Carvalho de Melo
<[email protected]> wrote:
>
> Em Thu, Nov 17, 2022 at 09:16:58AM -0800, Ian Rogers escreveu:
> > On Thu, Nov 17, 2022 at 8:15 AM Arnaldo Carvalho de Melo
> > <[email protected]> wrote:
> > >
> > > Em Thu, Nov 17, 2022 at 01:06:16PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > > Em Wed, Nov 16, 2022 at 03:38:47PM -0800, Namhyung Kim escreveu:
> > > > > The leafloop workload is to run an infinite loop in the test_leaf
> > > > > function. This is needed for the ARM fp callgraph test to verify if it
> > > > > gets the correct callchains.
> > > > >
> > > > > $ perf test -w leafloop
> > > >
> > > > On fedora:36
> > > >
> > > > In file included from /usr/include/bits/libc-header-start.h:33,
> > > > from /usr/include/stdlib.h:26,
> > > > from tests/workloads/leafloop.c:2:
> > > > /usr/include/features.h:412:4: error: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Werror=cpp]
> > > > 412 | # warning _FORTIFY_SOURCE requires compiling with optimization (-O)
> > > > | ^~~~~~~
> > > > cc1: all warnings being treated as errors
> > > > make[5]: *** [/home/acme/git/perf/tools/build/Makefile.build:96: /tmp/build/perf/tests/workloads/leafloop.o] Error 1
> > > > make[5]: *** Waiting for unfinished jobs....
> > > >
> > > > I'll try removing the _FORTIFY_SOURCE
> > >
> > > Works after I added this to datasym.c, leafloop.c and brstack.c:
> >
> > Is there a reason we are compiling without -O ? Perhaps we can filter
>
> I assumed so as Namhyung added it, perhaps he is just carrying it from
> the pre-existing shell tests?
>
> I wonder its to have a predictable binary output that the test expects
> when doing things like hardware tracing? As it come from the coresight
> tests, IIRC.

Would the following in the Build be better:

```
# Undefine _FORTIFY_SOURCE as it doesn't work with -O0
CFLAGS_leafloop.o = -g -O0 -fno-inline -fno-omit-frame-pointer
-U_FORTIFY_SOURCE
```

We could also use make's `filter-out`. If we are disabling inlining
then there is also `-fno-optimize-sibling-calls` otherwise we can
still lose stack frames.

Thanks,
Ian

> - Arnaldo
>
> > setting _FORTIFY_SOURCE so that it depends on -O being enabled.
>
> > Thanks,
> > Ian
> >
> > > diff --git a/tools/perf/tests/workloads/leafloop.c b/tools/perf/tests/workloads/leafloop.c
> > > index 1bf5cc97649b0e23..5d72c001320e3013 100644
> > > --- a/tools/perf/tests/workloads/leafloop.c
> > > +++ b/tools/perf/tests/workloads/leafloop.c
> > > @@ -1,4 +1,5 @@
> > > /* SPDX-License-Identifier: GPL-2.0 */
> > > +#undef _FORTIFY_SOURCE
> > > #include <stdlib.h>
> > > #include <linux/compiler.h>
> > > #include "../tests.h"
>
> --
>
> - Arnaldo

2022-11-17 19:36:58

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH 05/12] perf test: Add 'leafloop' test workload

Hi,

On Thu, Nov 17, 2022 at 9:42 AM Ian Rogers <[email protected]> wrote:
>
> On Thu, Nov 17, 2022 at 9:24 AM Arnaldo Carvalho de Melo
> <[email protected]> wrote:
> >
> > Em Thu, Nov 17, 2022 at 09:16:58AM -0800, Ian Rogers escreveu:
> > > On Thu, Nov 17, 2022 at 8:15 AM Arnaldo Carvalho de Melo
> > > <[email protected]> wrote:
> > > >
> > > > Em Thu, Nov 17, 2022 at 01:06:16PM -0300, Arnaldo Carvalho de Melo escreveu:
> > > > > Em Wed, Nov 16, 2022 at 03:38:47PM -0800, Namhyung Kim escreveu:
> > > > > > The leafloop workload is to run an infinite loop in the test_leaf
> > > > > > function. This is needed for the ARM fp callgraph test to verify if it
> > > > > > gets the correct callchains.
> > > > > >
> > > > > > $ perf test -w leafloop
> > > > >
> > > > > On fedora:36
> > > > >
> > > > > In file included from /usr/include/bits/libc-header-start.h:33,
> > > > > from /usr/include/stdlib.h:26,
> > > > > from tests/workloads/leafloop.c:2:
> > > > > /usr/include/features.h:412:4: error: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Werror=cpp]
> > > > > 412 | # warning _FORTIFY_SOURCE requires compiling with optimization (-O)
> > > > > | ^~~~~~~
> > > > > cc1: all warnings being treated as errors
> > > > > make[5]: *** [/home/acme/git/perf/tools/build/Makefile.build:96: /tmp/build/perf/tests/workloads/leafloop.o] Error 1
> > > > > make[5]: *** Waiting for unfinished jobs....
> > > > >
> > > > > I'll try removing the _FORTIFY_SOURCE
> > > >
> > > > Works after I added this to datasym.c, leafloop.c and brstack.c:
> > >
> > > Is there a reason we are compiling without -O ? Perhaps we can filter
> >
> > I assumed so as Namhyung added it, perhaps he is just carrying it from
> > the pre-existing shell tests?

Exactly :)

> >
> > I wonder its to have a predictable binary output that the test expects
> > when doing things like hardware tracing? As it come from the coresight
> > tests, IIRC.

I think it just checks frame-pointer based callstacks on ARM to have the
precise results for leaves and their parents.


>
> Would the following in the Build be better:
>
> ```
> # Undefine _FORTIFY_SOURCE as it doesn't work with -O0
> CFLAGS_leafloop.o = -g -O0 -fno-inline -fno-omit-frame-pointer
> -U_FORTIFY_SOURCE
> ```
>
> We could also use make's `filter-out`. If we are disabling inlining
> then there is also `-fno-optimize-sibling-calls` otherwise we can
> still lose stack frames.

I wonder if it's enough to use -O0 as it's enabled from -O2.
Maybe we can get rid of -fno-inline as well.

German, did you have any concerns for those options?

Thanks,
Namhyung

2022-11-18 11:49:47

by James Clark

[permalink] [raw]
Subject: Re: [PATCH 05/12] perf test: Add 'leafloop' test workload



On 17/11/2022 18:11, Namhyung Kim wrote:
> Hi,
>
> On Thu, Nov 17, 2022 at 9:42 AM Ian Rogers <[email protected]> wrote:
>>
>> On Thu, Nov 17, 2022 at 9:24 AM Arnaldo Carvalho de Melo
>> <[email protected]> wrote:
>>>
>>> Em Thu, Nov 17, 2022 at 09:16:58AM -0800, Ian Rogers escreveu:
>>>> On Thu, Nov 17, 2022 at 8:15 AM Arnaldo Carvalho de Melo
>>>> <[email protected]> wrote:
>>>>>
>>>>> Em Thu, Nov 17, 2022 at 01:06:16PM -0300, Arnaldo Carvalho de Melo escreveu:
>>>>>> Em Wed, Nov 16, 2022 at 03:38:47PM -0800, Namhyung Kim escreveu:
>>>>>>> The leafloop workload is to run an infinite loop in the test_leaf
>>>>>>> function. This is needed for the ARM fp callgraph test to verify if it
>>>>>>> gets the correct callchains.
>>>>>>>
>>>>>>> $ perf test -w leafloop
>>>>>>
>>>>>> On fedora:36
>>>>>>
>>>>>> In file included from /usr/include/bits/libc-header-start.h:33,
>>>>>> from /usr/include/stdlib.h:26,
>>>>>> from tests/workloads/leafloop.c:2:
>>>>>> /usr/include/features.h:412:4: error: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Werror=cpp]
>>>>>> 412 | # warning _FORTIFY_SOURCE requires compiling with optimization (-O)
>>>>>> | ^~~~~~~
>>>>>> cc1: all warnings being treated as errors
>>>>>> make[5]: *** [/home/acme/git/perf/tools/build/Makefile.build:96: /tmp/build/perf/tests/workloads/leafloop.o] Error 1
>>>>>> make[5]: *** Waiting for unfinished jobs....
>>>>>>
>>>>>> I'll try removing the _FORTIFY_SOURCE
>>>>>
>>>>> Works after I added this to datasym.c, leafloop.c and brstack.c:
>>>>
>>>> Is there a reason we are compiling without -O ? Perhaps we can filter
>>>
>>> I assumed so as Namhyung added it, perhaps he is just carrying it from
>>> the pre-existing shell tests?
>
> Exactly :)
>
>>>
>>> I wonder its to have a predictable binary output that the test expects
>>> when doing things like hardware tracing? As it come from the coresight
>>> tests, IIRC.
>
> I think it just checks frame-pointer based callstacks on ARM to have the
> precise results for leaves and their parents.
>
>
>>
>> Would the following in the Build be better:
>>
>> ```
>> # Undefine _FORTIFY_SOURCE as it doesn't work with -O0
>> CFLAGS_leafloop.o = -g -O0 -fno-inline -fno-omit-frame-pointer
>> -U_FORTIFY_SOURCE
>> ```
>>
>> We could also use make's `filter-out`. If we are disabling inlining
>> then there is also `-fno-optimize-sibling-calls` otherwise we can
>> still lose stack frames.
>
> I wonder if it's enough to use -O0 as it's enabled from -O2.
> Maybe we can get rid of -fno-inline as well.
>
> German, did you have any concerns for those options?
>

Is it possible to go with the -U_FORTIFY_SOURCE option? From looking at
the disassembly, changing -O and the other -f options makes quite a bit
of difference.

It's fairly important to that test because it's testing that the
combination of both frame pointer unwinding and dwarf unwinding result
in the complete stack.

If we change the options I'd have to go back and double check with
different compiler versions that it's still doing the right thing. For
example if a frame pointer is included for the last frame, then the
dwarf bit doesn't get tested.


> Thanks,
> Namhyung

2022-11-18 15:14:56

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH 05/12] perf test: Add 'leafloop' test workload

Em Fri, Nov 18, 2022 at 11:32:43AM +0000, James Clark escreveu:
>
>
> On 17/11/2022 18:11, Namhyung Kim wrote:
> > Hi,
> >
> > On Thu, Nov 17, 2022 at 9:42 AM Ian Rogers <[email protected]> wrote:
> >>
> >> On Thu, Nov 17, 2022 at 9:24 AM Arnaldo Carvalho de Melo
> >> <[email protected]> wrote:
> >>>
> >>> Em Thu, Nov 17, 2022 at 09:16:58AM -0800, Ian Rogers escreveu:
> >>>> On Thu, Nov 17, 2022 at 8:15 AM Arnaldo Carvalho de Melo
> >>>> <[email protected]> wrote:
> >>>>>
> >>>>> Em Thu, Nov 17, 2022 at 01:06:16PM -0300, Arnaldo Carvalho de Melo escreveu:
> >>>>>> Em Wed, Nov 16, 2022 at 03:38:47PM -0800, Namhyung Kim escreveu:
> >>>>>>> The leafloop workload is to run an infinite loop in the test_leaf
> >>>>>>> function. This is needed for the ARM fp callgraph test to verify if it
> >>>>>>> gets the correct callchains.
> >>>>>>>
> >>>>>>> $ perf test -w leafloop
> >>>>>>
> >>>>>> On fedora:36
> >>>>>>
> >>>>>> In file included from /usr/include/bits/libc-header-start.h:33,
> >>>>>> from /usr/include/stdlib.h:26,
> >>>>>> from tests/workloads/leafloop.c:2:
> >>>>>> /usr/include/features.h:412:4: error: #warning _FORTIFY_SOURCE requires compiling with optimization (-O) [-Werror=cpp]
> >>>>>> 412 | # warning _FORTIFY_SOURCE requires compiling with optimization (-O)
> >>>>>> | ^~~~~~~
> >>>>>> cc1: all warnings being treated as errors
> >>>>>> make[5]: *** [/home/acme/git/perf/tools/build/Makefile.build:96: /tmp/build/perf/tests/workloads/leafloop.o] Error 1
> >>>>>> make[5]: *** Waiting for unfinished jobs....
> >>>>>>
> >>>>>> I'll try removing the _FORTIFY_SOURCE
> >>>>>
> >>>>> Works after I added this to datasym.c, leafloop.c and brstack.c:
> >>>>
> >>>> Is there a reason we are compiling without -O ? Perhaps we can filter
> >>>
> >>> I assumed so as Namhyung added it, perhaps he is just carrying it from
> >>> the pre-existing shell tests?
> >
> > Exactly :)
> >
> >>>
> >>> I wonder its to have a predictable binary output that the test expects
> >>> when doing things like hardware tracing? As it come from the coresight
> >>> tests, IIRC.
> >
> > I think it just checks frame-pointer based callstacks on ARM to have the
> > precise results for leaves and their parents.
> >
> >
> >>
> >> Would the following in the Build be better:
> >>
> >> ```
> >> # Undefine _FORTIFY_SOURCE as it doesn't work with -O0
> >> CFLAGS_leafloop.o = -g -O0 -fno-inline -fno-omit-frame-pointer
> >> -U_FORTIFY_SOURCE
> >> ```
> >>
> >> We could also use make's `filter-out`. If we are disabling inlining
> >> then there is also `-fno-optimize-sibling-calls` otherwise we can
> >> still lose stack frames.
> >
> > I wonder if it's enough to use -O0 as it's enabled from -O2.
> > Maybe we can get rid of -fno-inline as well.
> >
> > German, did you have any concerns for those options?
> >
>
> Is it possible to go with the -U_FORTIFY_SOURCE option? From looking at
> the disassembly, changing -O and the other -f options makes quite a bit
> of difference.

I thought about doing it as a -U_FORTIFY_SOURCE but ended up doing it in
each test as I thought that way to be more robust, i.e. the way the
makefiles get that per-object CFLAGS and add to the global one could
flip and then this would break again.

But if people prefer it in the per-object file Build rule, np.

- Arnaldo

> It's fairly important to that test because it's testing that the
> combination of both frame pointer unwinding and dwarf unwinding result
> in the complete stack.
>
> If we change the options I'd have to go back and double check with
> different compiler versions that it's still doing the right thing. For
> example if a frame pointer is included for the last frame, then the
> dwarf bit doesn't get tested.
>
>
> > Thanks,
> > Namhyung