diff mbox

selftests/powerpc: Remove -flto from common CFLAGS

Message ID 1456727395-27386-1-git-send-email-sjitindarsingh@gmail.com (mailing list archive)
State Accepted
Headers show

Commit Message

Suraj Jitindar Singh Feb. 29, 2016, 6:29 a.m. UTC
LTO can cause GCC to inline some functions which have attributes set. The
act of inlining the functions can lead to GCC forgetting about the
attributes which leads to incorrect tests.
Notable example being: __attribute__((__target__("no-vsx")))
LTO can also interact strangely with custom assembly functions and cause
tests to intermittently fail.

Both these cases are hard to detect and require manual inspection of
binaries which is unlikely to happen for all tests. Furthermore, LTO
optimisations are not necessary for selftests and correctness is paramount
and as such it is best to disable LTO.

LTO can be enabled on a per test basis.

A pseries_le_defconfig kernel on a POWER8 was used to determine that the
same subset of selftests pass and fail with and without -flto in the
common Makefile.

These tests always fail:
selftests: per_event_excludes [FAIL]
selftests: event_attributes_test [FAIL]
selftests: ebb_vs_cpu_event_test [FAIL]
selftests: cpu_event_vs_ebb_test [FAIL]
selftests: cpu_event_pinned_vs_ebb_test [FAIL]
selftests: ipc_unmuxed [FAIL]
And the remaining tests PASS.

Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
---
 tools/testing/selftests/powerpc/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Michael Ellerman Feb. 29, 2016, 11:10 a.m. UTC | #1
Hi Suraj,

On Mon, 2016-29-02 at 06:29:55 UTC, Suraj Jitindar Singh wrote:
> LTO can cause GCC to inline some functions which have attributes set. The

You should define what LTO is the first time you use it.

> act of inlining the functions can lead to GCC forgetting about the
> attributes which leads to incorrect tests.
> Notable example being: __attribute__((__target__("no-vsx")))

That is probably a GCC bug, but we still need to work around it for now.

> LTO can also interact strangely with custom assembly functions and cause
> tests to intermittently fail.

That's probably Cyril writing bad asm :)

> Both these cases are hard to detect and require manual inspection of
> binaries which is unlikely to happen for all tests. Furthermore, LTO
> optimisations are not necessary for selftests and correctness is paramount
> and as such it is best to disable LTO.
> 
> LTO can be enabled on a per test basis.
> 
> A pseries_le_defconfig kernel on a POWER8 was used to determine that the
> same subset of selftests pass and fail with and without -flto in the
> common Makefile.
> 
> These tests always fail:
> selftests: per_event_excludes [FAIL]
> selftests: event_attributes_test [FAIL]
> selftests: ebb_vs_cpu_event_test [FAIL]
> selftests: cpu_event_vs_ebb_test [FAIL]
> selftests: cpu_event_pinned_vs_ebb_test [FAIL]

They shouldn't :)

Are you running as root? Bare metal or guest?

> selftests: ipc_unmuxed [FAIL]

That one is expected.

cheers
Cyril Bur Feb. 29, 2016, 11:08 p.m. UTC | #2
On Mon, 29 Feb 2016 22:10:13 +1100 (AEDT)
Michael Ellerman <mpe@ellerman.id.au> wrote:

> Hi Suraj,
> 
> On Mon, 2016-29-02 at 06:29:55 UTC, Suraj Jitindar Singh wrote:
> > LTO can cause GCC to inline some functions which have attributes set. The  
> 
> You should define what LTO is the first time you use it.
> 
> > act of inlining the functions can lead to GCC forgetting about the
> > attributes which leads to incorrect tests.
> > Notable example being: __attribute__((__target__("no-vsx")))  
> 
> That is probably a GCC bug, but we still need to work around it for now.

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70010

> 
> > LTO can also interact strangely with custom assembly functions and cause
> > tests to intermittently fail.  
> 
> That's probably Cyril writing bad asm :)

No doubt.

> 
> > Both these cases are hard to detect and require manual inspection of
> > binaries which is unlikely to happen for all tests. Furthermore, LTO
> > optimisations are not necessary for selftests and correctness is paramount
> > and as such it is best to disable LTO.
> > 
> > LTO can be enabled on a per test basis.
> > 
> > A pseries_le_defconfig kernel on a POWER8 was used to determine that the
> > same subset of selftests pass and fail with and without -flto in the
> > common Makefile.
> > 
> > These tests always fail:
> > selftests: per_event_excludes [FAIL]
> > selftests: event_attributes_test [FAIL]
> > selftests: ebb_vs_cpu_event_test [FAIL]
> > selftests: cpu_event_vs_ebb_test [FAIL]
> > selftests: cpu_event_pinned_vs_ebb_test [FAIL]  
> 
> They shouldn't :)
> 
> Are you running as root? Bare metal or guest?
> 

Interesting. I believe this was run baremetal as root. I'm going to test the
patch in qemu at root. Is there a list of expected failures in certain
situations?

> > selftests: ipc_unmuxed [FAIL]  
> 
> That one is expected.
> 
> cheers
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
Michael Ellerman Feb. 29, 2016, 11:35 p.m. UTC | #3
On Tue, 2016-03-01 at 10:08 +1100, Cyril Bur wrote:
> On Mon, 29 Feb 2016 22:10:13 +1100 (AEDT)
> Michael Ellerman <mpe@ellerman.id.au> wrote:
> > On Mon, 2016-29-02 at 06:29:55 UTC, Suraj Jitindar Singh wrote:
> > > Both these cases are hard to detect and require manual inspection of
> > > binaries which is unlikely to happen for all tests. Furthermore, LTO
> > > optimisations are not necessary for selftests and correctness is paramount
> > > and as such it is best to disable LTO.
> > >
> > > LTO can be enabled on a per test basis.
> > >
> > > A pseries_le_defconfig kernel on a POWER8 was used to determine that the
> > > same subset of selftests pass and fail with and without -flto in the
> > > common Makefile.
> > >
> > > These tests always fail:
> > > selftests: per_event_excludes [FAIL]
> > > selftests: event_attributes_test [FAIL]
> > > selftests: ebb_vs_cpu_event_test [FAIL]
> > > selftests: cpu_event_vs_ebb_test [FAIL]
> > > selftests: cpu_event_pinned_vs_ebb_test [FAIL]
> >
> > They shouldn't :)
> >
> > Are you running as root? Bare metal or guest?
>
> Interesting. I believe this was run baremetal as root. I'm going to test the
> patch in qemu at root. Is there a list of expected failures in certain
> situations?

Nope.

Most of those require the perf paranoid level to be set, see eg:

  tools/testing/selftests/powerpc/pmu/ebb/cpu_event_vs_ebb_test.c:        SKIP_IF(require_paranoia_below(1));

cheers
Cyril Bur March 1, 2016, 3:11 a.m. UTC | #4
On Mon, 29 Feb 2016 22:10:13 +1100 (AEDT)
Michael Ellerman <mpe@ellerman.id.au> wrote:

> Hi Suraj,
> 
> On Mon, 2016-29-02 at 06:29:55 UTC, Suraj Jitindar Singh wrote:
> > LTO can cause GCC to inline some functions which have attributes set. The  
> 
> You should define what LTO is the first time you use it.
> 
> > act of inlining the functions can lead to GCC forgetting about the
> > attributes which leads to incorrect tests.
> > Notable example being: __attribute__((__target__("no-vsx")))  
> 
> That is probably a GCC bug, but we still need to work around it for now.
> 
> > LTO can also interact strangely with custom assembly functions and cause
> > tests to intermittently fail.  
> 
> That's probably Cyril writing bad asm :)
> 
> > Both these cases are hard to detect and require manual inspection of
> > binaries which is unlikely to happen for all tests. Furthermore, LTO
> > optimisations are not necessary for selftests and correctness is paramount
> > and as such it is best to disable LTO.
> > 
> > LTO can be enabled on a per test basis.
> > 
> > A pseries_le_defconfig kernel on a POWER8 was used to determine that the
> > same subset of selftests pass and fail with and without -flto in the
> > common Makefile.
> > 
> > These tests always fail:
> > selftests: per_event_excludes [FAIL]
> > selftests: event_attributes_test [FAIL]
> > selftests: ebb_vs_cpu_event_test [FAIL]
> > selftests: cpu_event_vs_ebb_test [FAIL]
> > selftests: cpu_event_pinned_vs_ebb_test [FAIL]  
> 
> They shouldn't :)
> 
> Are you running as root? Bare metal or guest?

The answer here is that /proc/sys/kernel/perf_event_paranoid defaults to 1 but
0 is needed for these tests to not SKIP. The standard harness does not decern
between skips and fails, running each test in powerpc/ shows SKIP/FAIL.

I have run four kernels under QEMU/KVM on a fairly busy VM box and I have the
same result for all 4. All had /proc/sys/kernel/perf_event_paranoid set to 0
before running the tests. Each test was run by it's self not with the
run_kselftest.sh script which hides SKIPs.

pseries_defconfig (BE) qemu/KVM PATCHED
# grep FAIL *.out
cpu_event_pinned_vs_ebb_test.out:[FAIL] Test FAILED on line 87
ipc_unmuxed.out:[FAIL] Test FAILED on line 38
per_event_excludes.out:[FAIL] Test FAILED on line 95

pseries_defconfig (BE) qemu/KVM unpatched
# grep FAIL *.out
cpu_event_pinned_vs_ebb_test.out:[FAIL] Test FAILED on line 87
ipc_unmuxed.out:[FAIL] Test FAILED on line 38
per_event_excludes.out:[FAIL] Test FAILED on line 95

pseries_le_defconfig (LE) qemu/KVM PATCHED
# grep FAIL *.out
cpu_event_pinned_vs_ebb_test.out:[FAIL] Test FAILED on line 87
ipc_unmuxed.out:[FAIL] Test FAILED on line 38
per_event_excludes.out:[FAIL] Test FAILED on line 95

pseries_le_defconfig (LE) qemu/KVM unpatched
# grep FAIL *.out
cpu_event_pinned_vs_ebb_test.out:[FAIL] Test FAILED on line 87
ipc_unmuxed.out:[FAIL] Test FAILED on line 38
per_event_excludes.out:[FAIL] Test FAILED on line 95

There were no matches for grep SKIP *.out for any of the four.

This patch appears to not have affected any of the tests.

Reviewed-by: Cyril Bur <cyrilbur@gmail.com>

> 
> > selftests: ipc_unmuxed [FAIL]  
> 
> That one is expected.
> 
> cheers
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
Suraj Jitindar Singh March 1, 2016, 4:46 a.m. UTC | #5
On 01/03/16 14:11, Cyril Bur wrote:
> On Mon, 29 Feb 2016 22:10:13 +1100 (AEDT)
> Michael Ellerman <mpe@ellerman.id.au> wrote:
>
>> Hi Suraj,
>>
>> On Mon, 2016-29-02 at 06:29:55 UTC, Suraj Jitindar Singh wrote:
>>> LTO can cause GCC to inline some functions which have attributes set. The  
>> You should define what LTO is the first time you use it.
>>
>>> act of inlining the functions can lead to GCC forgetting about the
>>> attributes which leads to incorrect tests.
>>> Notable example being: __attribute__((__target__("no-vsx")))  
>> That is probably a GCC bug, but we still need to work around it for now.
>>
>>> LTO can also interact strangely with custom assembly functions and cause
>>> tests to intermittently fail.  
>> That's probably Cyril writing bad asm :)
>>
>>> Both these cases are hard to detect and require manual inspection of
>>> binaries which is unlikely to happen for all tests. Furthermore, LTO
>>> optimisations are not necessary for selftests and correctness is paramount
>>> and as such it is best to disable LTO.
>>>
>>> LTO can be enabled on a per test basis.
>>>
>>> A pseries_le_defconfig kernel on a POWER8 was used to determine that the
>>> same subset of selftests pass and fail with and without -flto in the
>>> common Makefile.
>>>
>>> These tests always fail:
>>> selftests: per_event_excludes [FAIL]
>>> selftests: event_attributes_test [FAIL]
>>> selftests: ebb_vs_cpu_event_test [FAIL]
>>> selftests: cpu_event_vs_ebb_test [FAIL]
>>> selftests: cpu_event_pinned_vs_ebb_test [FAIL]  
>> They shouldn't :)
>>
>> Are you running as root? Bare metal or guest?
> The answer here is that /proc/sys/kernel/perf_event_paranoid defaults to 1 but
> 0 is needed for these tests to not SKIP. The standard harness does not decern
> between skips and fails, running each test in powerpc/ shows SKIP/FAIL.
>
> I have run four kernels under QEMU/KVM on a fairly busy VM box and I have the
> same result for all 4. All had /proc/sys/kernel/perf_event_paranoid set to 0
> before running the tests. Each test was run by it's self not with the
> run_kselftest.sh script which hides SKIPs.
>
> pseries_defconfig (BE) qemu/KVM PATCHED
> # grep FAIL *.out
> cpu_event_pinned_vs_ebb_test.out:[FAIL] Test FAILED on line 87
> ipc_unmuxed.out:[FAIL] Test FAILED on line 38
> per_event_excludes.out:[FAIL] Test FAILED on line 95
>
> pseries_defconfig (BE) qemu/KVM unpatched
> # grep FAIL *.out
> cpu_event_pinned_vs_ebb_test.out:[FAIL] Test FAILED on line 87
> ipc_unmuxed.out:[FAIL] Test FAILED on line 38
> per_event_excludes.out:[FAIL] Test FAILED on line 95
>
> pseries_le_defconfig (LE) qemu/KVM PATCHED
> # grep FAIL *.out
> cpu_event_pinned_vs_ebb_test.out:[FAIL] Test FAILED on line 87
> ipc_unmuxed.out:[FAIL] Test FAILED on line 38
> per_event_excludes.out:[FAIL] Test FAILED on line 95
>
> pseries_le_defconfig (LE) qemu/KVM unpatched
> # grep FAIL *.out
> cpu_event_pinned_vs_ebb_test.out:[FAIL] Test FAILED on line 87
> ipc_unmuxed.out:[FAIL] Test FAILED on line 38
> per_event_excludes.out:[FAIL] Test FAILED on line 95
>
> There were no matches for grep SKIP *.out for any of the four.
>
> This patch appears to not have affected any of the tests.
>
> Reviewed-by: Cyril Bur <cyrilbur@gmail.com>
>
The same tests were run with /proc/sys/kernel/perf_event_paranoid set to 0 on an bare metal power8 system with the same results.

pseries_le_defconfig (LE) PATCHED
# grep FAIL results_patched
[FAIL] Test FAILED on line 95
selftests: per_event_excludes [FAIL]
[FAIL] Test FAILED on line 87
selftests: cpu_event_pinned_vs_ebb_test [FAIL]
selftests: ipc_unmuxed [FAIL]

pseries_le_defconfig (LE) unpatched
# grep FAIL results_unpatched
[FAIL] Test FAILED on line 95
selftests: per_event_excludes [FAIL]
[FAIL] Test FAILED on line 87
selftests: cpu_event_pinned_vs_ebb_test [FAIL]
selftests: ipc_unmuxed [FAIL]

Thus there was no change in the test results between the patched and unpatched systems.

>>> selftests: ipc_unmuxed [FAIL]  
>> That one is expected.
>>
>> cheers
>> _______________________________________________
>> Linuxppc-dev mailing list
>> Linuxppc-dev@lists.ozlabs.org
>> https://lists.ozlabs.org/listinfo/linuxppc-dev
Michael Ellerman March 3, 2016, 10:58 a.m. UTC | #6
On Mon, 2016-29-02 at 06:29:55 UTC, Suraj Jitindar Singh wrote:
> LTO can cause GCC to inline some functions which have attributes set. The
> act of inlining the functions can lead to GCC forgetting about the
> attributes which leads to incorrect tests.
> Notable example being: __attribute__((__target__("no-vsx")))
> LTO can also interact strangely with custom assembly functions and cause
> tests to intermittently fail.
...
> 
> Signed-off-by: Suraj Jitindar Singh <sjitindarsingh@gmail.com>
> Reviewed-by: Cyril Bur <cyrilbur@gmail.com>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/a4cf0a2e1d3b98cac5fd5ba928

cheers
diff mbox

Patch

diff --git a/tools/testing/selftests/powerpc/Makefile b/tools/testing/selftests/powerpc/Makefile
index 0c2706b..7925a96 100644
--- a/tools/testing/selftests/powerpc/Makefile
+++ b/tools/testing/selftests/powerpc/Makefile
@@ -8,7 +8,7 @@  ifeq ($(ARCH),powerpc)
 
 GIT_VERSION = $(shell git describe --always --long --dirty || echo "unknown")
 
-CFLAGS := -Wall -O2 -flto -Wall -Werror -DGIT_VERSION='"$(GIT_VERSION)"' -I$(CURDIR) $(CFLAGS)
+CFLAGS := -Wall -O2 -Wall -Werror -DGIT_VERSION='"$(GIT_VERSION)"' -I$(CURDIR) $(CFLAGS)
 
 export CFLAGS