| Message ID | MN0PR21MB34844532CF2CD395DE99B07E912AA@MN0PR21MB3484.namprd21.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | Collect both user and kernel events for autofdo tests and autoprofiledbootstrap | expand |
On Fri, Jun 30, 2023 at 7:28 AM Eugene Rozenfeld via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > When we collect just user events for autofdo with lbr we get some events where branch > sources are kernel addresses and branch targets are user addresses. Without kernel MMAP > events create_gcov can't make sense of kernel addresses. Currently create_gcov fails if > it can't map at least 95% of events. We sometimes get below this threshold with just > user events. The change is to collect both user events and kernel events. Does this require elevated privileges? Can we instead "fix" create_gcov here? > Tested on x86_64-pc-linux-gnu. > > ChangeLog: > > * Makefile.in: Collect both kernel and user events for autofdo > * Makefile.tpl: Collect both kernel and user events for autofdo > > gcc/testsuite/ChangeLog: > > * lib/target-supports.exp: Collect both kernel and user events for autofdo > --- > Makefile.in | 2 +- > Makefile.tpl | 2 +- > gcc/testsuite/lib/target-supports.exp | 2 +- > 3 files changed, 3 insertions(+), 3 deletions(-) > > diff --git a/Makefile.in b/Makefile.in > index f19a9db621e..04307ca561b 100644 > --- a/Makefile.in > +++ b/Makefile.in > @@ -404,7 +404,7 @@ MAKEINFO = @MAKEINFO@ > EXPECT = @EXPECT@ > RUNTEST = @RUNTEST@ > > -AUTO_PROFILE = gcc-auto-profile -c 10000000 > +AUTO_PROFILE = gcc-auto-profile --all -c 10000000 > > # This just becomes part of the MAKEINFO definition passed down to > # sub-makes. It lets flags be given on the command line while still > diff --git a/Makefile.tpl b/Makefile.tpl > index 3a5b7ed3c92..d0fe7e2fb77 100644 > --- a/Makefile.tpl > +++ b/Makefile.tpl > @@ -407,7 +407,7 @@ MAKEINFO = @MAKEINFO@ > EXPECT = @EXPECT@ > RUNTEST = @RUNTEST@ > > -AUTO_PROFILE = gcc-auto-profile -c 10000000 > +AUTO_PROFILE = gcc-auto-profile --all -c 10000000 > > # This just becomes part of the MAKEINFO definition passed down to > # sub-makes. It lets flags be given on the command line while still > diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp > index 4d04df2a709..b16853d76df 100644 > --- a/gcc/testsuite/lib/target-supports.exp > +++ b/gcc/testsuite/lib/target-supports.exp > @@ -704,7 +704,7 @@ proc check_effective_target_keeps_null_pointer_checks { } { > # this allows parallelism of 16 and higher of parallel gcc-auto-profile > proc profopt-perf-wrapper { } { > global srcdir > - return "$srcdir/../config/i386/gcc-auto-profile -m8 " > + return "$srcdir/../config/i386/gcc-auto-profile --all -m8 " > } > > # Return true if profiling is supported on the target. > -- > 2.25.1 >
Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > On Fri, Jun 30, 2023 at 7:28 AM Eugene Rozenfeld via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: >> >> When we collect just user events for autofdo with lbr we get some events where branch >> sources are kernel addresses and branch targets are user addresses. Without kernel MMAP >> events create_gcov can't make sense of kernel addresses. Currently create_gcov fails if >> it can't map at least 95% of events. We sometimes get below this threshold with just >> user events. The change is to collect both user events and kernel events. > > Does this require elevated privileges? Can we instead "fix" create_gcov here? Right, requiring privileges for this is going to be a no-go for a lot of builders. In a distro context, for example, it means we can't consider autofdo at all.
I don't run this with elevated privileges but I set /proc/sys/kernel/kptr_restrict to 0. Setting that does require elevated privileges. If that's not acceptable, the only fix I can think of is to make that event mapping threshold percentage a parameter to create_gcov and pass something low enough. 80% instead of the current threshold of 95% should work, although it's a bit fragile. Eugene -----Original Message----- From: Sam James <sam@gentoo.org> Sent: Friday, June 30, 2023 1:59 AM To: Richard Biener <richard.guenther@gmail.com> Cc: Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com>; gcc-patches@gcc.gnu.org Subject: [EXTERNAL] Re: [PATCH] Collect both user and kernel events for autofdo tests and autoprofiledbootstrap [You don't often get email from sam@gentoo.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > On Fri, Jun 30, 2023 at 7:28 AM Eugene Rozenfeld via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: >> >> When we collect just user events for autofdo with lbr we get some >> events where branch sources are kernel addresses and branch targets >> are user addresses. Without kernel MMAP events create_gcov can't make >> sense of kernel addresses. Currently create_gcov fails if it can't >> map at least 95% of events. We sometimes get below this threshold with just user events. The change is to collect both user events and kernel events. > > Does this require elevated privileges? Can we instead "fix" create_gcov here? Right, requiring privileges for this is going to be a no-go for a lot of builders. In a distro context, for example, it means we can't consider autofdo at all.
I also set /proc/sys/kernel/perf_event_paranoid to 1 instead of the default 2. -----Original Message----- From: Gcc-patches <gcc-patches-bounces+erozen=microsoft.com@gcc.gnu.org> On Behalf Of Eugene Rozenfeld via Gcc-patches Sent: Friday, June 30, 2023 2:44 PM To: Sam James <sam@gentoo.org>; Richard Biener <richard.guenther@gmail.com> Cc: gcc-patches@gcc.gnu.org Subject: RE: [EXTERNAL] Re: [PATCH] Collect both user and kernel events for autofdo tests and autoprofiledbootstrap I don't run this with elevated privileges but I set /proc/sys/kernel/kptr_restrict to 0. Setting that does require elevated privileges. If that's not acceptable, the only fix I can think of is to make that event mapping threshold percentage a parameter to create_gcov and pass something low enough. 80% instead of the current threshold of 95% should work, although it's a bit fragile. Eugene -----Original Message----- From: Sam James <sam@gentoo.org> Sent: Friday, June 30, 2023 1:59 AM To: Richard Biener <richard.guenther@gmail.com> Cc: Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com>; gcc-patches@gcc.gnu.org Subject: [EXTERNAL] Re: [PATCH] Collect both user and kernel events for autofdo tests and autoprofiledbootstrap [You don't often get email from sam@gentoo.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > On Fri, Jun 30, 2023 at 7:28 AM Eugene Rozenfeld via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: >> >> When we collect just user events for autofdo with lbr we get some >> events where branch sources are kernel addresses and branch targets >> are user addresses. Without kernel MMAP events create_gcov can't make >> sense of kernel addresses. Currently create_gcov fails if it can't >> map at least 95% of events. We sometimes get below this threshold with just user events. The change is to collect both user events and kernel events. > > Does this require elevated privileges? Can we instead "fix" create_gcov here? Right, requiring privileges for this is going to be a no-go for a lot of builders. In a distro context, for example, it means we can't consider autofdo at all.
On Sat, Jul 1, 2023 at 12:05 AM Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com> wrote: > > I also set /proc/sys/kernel/perf_event_paranoid to 1 instead of the default 2. Does the perf attempt fail when the privileges are not adjusted and you specify --all? I see it adds /uk as flags, when I do > perf record -e instructions//uk ./a.out it doesn't complain in any way with > cat /proc/sys/kernel/kptr_restrict 1 > cat /proc/sys/kernel/perf_event_paranoid 2 so in case the 'kernel' side is simply ignored when profiling there isn't permitted/possible then I guess the patch is OK? Can you confirm? Thanks, Richard. > -----Original Message----- > From: Gcc-patches <gcc-patches-bounces+erozen=microsoft.com@gcc.gnu.org> On Behalf Of Eugene Rozenfeld via Gcc-patches > Sent: Friday, June 30, 2023 2:44 PM > To: Sam James <sam@gentoo.org>; Richard Biener <richard.guenther@gmail.com> > Cc: gcc-patches@gcc.gnu.org > Subject: RE: [EXTERNAL] Re: [PATCH] Collect both user and kernel events for autofdo tests and autoprofiledbootstrap > > I don't run this with elevated privileges but I set /proc/sys/kernel/kptr_restrict to 0. Setting that does require elevated privileges. > > If that's not acceptable, the only fix I can think of is to make that event mapping threshold percentage a parameter to create_gcov and pass something low enough. 80% instead of the current threshold of 95% should work, although it's a bit fragile. > > Eugene > > -----Original Message----- > From: Sam James <sam@gentoo.org> > Sent: Friday, June 30, 2023 1:59 AM > To: Richard Biener <richard.guenther@gmail.com> > Cc: Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com>; gcc-patches@gcc.gnu.org > Subject: [EXTERNAL] Re: [PATCH] Collect both user and kernel events for autofdo tests and autoprofiledbootstrap > > [You don't often get email from sam@gentoo.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] > > Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > > > On Fri, Jun 30, 2023 at 7:28 AM Eugene Rozenfeld via Gcc-patches > > <gcc-patches@gcc.gnu.org> wrote: > >> > >> When we collect just user events for autofdo with lbr we get some > >> events where branch sources are kernel addresses and branch targets > >> are user addresses. Without kernel MMAP events create_gcov can't make > >> sense of kernel addresses. Currently create_gcov fails if it can't > >> map at least 95% of events. We sometimes get below this threshold with just user events. The change is to collect both user events and kernel events. > > > > Does this require elevated privileges? Can we instead "fix" create_gcov here? > > Right, requiring privileges for this is going to be a no-go for a lot of builders. In a distro context, for example, it means we can't consider autofdo at all.
There is no warning and perf /uk succeeds when kptr_restrict is set to 1 and perf_event_paranoid set to 2. However, create_gcov may fail since it won't be able to understand kernel addresses and it requires at least 95% of events to be successfully mapped.
If I set both kptr_restrict and perf_event_paranoid to 1, then I do get warnings from perf (but it still succeeds and exits with a 0 code). And, of course create_gcov will also fail to map some events since it won't understand kernel addresses.
WARNING: Kernel address maps (/proc/{kallsyms,modules}) are restricted,
check /proc/sys/kernel/kptr_restrict and /proc/sys/kernel/perf_event_paranoid.
Samples in kernel functions may not be resolved if a suitable vmlinux
file is not found in the buildid cache or in the vmlinux path.
Samples in kernel modules won't be resolved at all.
If some relocation was applied (e.g. kexec) symbols may be misresolved
even with a suitable vmlinux or kallsyms file.
Couldn't record kernel reference relocation symbol
Symbol resolution may be skewed if relocation was used (e.g. kexec).
Check /proc/kallsyms permission or run as root.
[ perf record: Woken up 2 times to write data ]
[ perf record: Captured and wrote 0.037 MB /home/erozen/gcc1_objdir/gcc/testsuite/gcc/indir-call-prof.perf.data (86 samples) ]
Eugene
-----Original Message-----
From: Richard Biener <richard.guenther@gmail.com>
Sent: Monday, July 3, 2023 12:47 AM
To: Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com>
Cc: Sam James <sam@gentoo.org>; gcc-patches@gcc.gnu.org
Subject: Re: [EXTERNAL] Re: [PATCH] Collect both user and kernel events for autofdo tests and autoprofiledbootstrap
On Sat, Jul 1, 2023 at 12:05 AM Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com> wrote:
>
> I also set /proc/sys/kernel/perf_event_paranoid to 1 instead of the default 2.
Does the perf attempt fail when the privileges are not adjusted and you specify --all? I see it adds /uk as flags, when I do
> perf record -e instructions//uk ./a.out
it doesn't complain in any way with
> cat /proc/sys/kernel/kptr_restrict
1
> cat /proc/sys/kernel/perf_event_paranoid
2
so in case the 'kernel' side is simply ignored when profiling there isn't permitted/possible then I guess the patch is OK?
Can you confirm?
Thanks,
Richard.
> -----Original Message-----
> From: Gcc-patches
> <gcc-patches-bounces+erozen=microsoft.com@gcc.gnu.org> On Behalf Of
> Eugene Rozenfeld via Gcc-patches
> Sent: Friday, June 30, 2023 2:44 PM
> To: Sam James <sam@gentoo.org>; Richard Biener
> <richard.guenther@gmail.com>
> Cc: gcc-patches@gcc.gnu.org
> Subject: RE: [EXTERNAL] Re: [PATCH] Collect both user and kernel
> events for autofdo tests and autoprofiledbootstrap
>
> I don't run this with elevated privileges but I set /proc/sys/kernel/kptr_restrict to 0. Setting that does require elevated privileges.
>
> If that's not acceptable, the only fix I can think of is to make that event mapping threshold percentage a parameter to create_gcov and pass something low enough. 80% instead of the current threshold of 95% should work, although it's a bit fragile.
>
> Eugene
>
> -----Original Message-----
> From: Sam James <sam@gentoo.org>
> Sent: Friday, June 30, 2023 1:59 AM
> To: Richard Biener <richard.guenther@gmail.com>
> Cc: Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com>;
> gcc-patches@gcc.gnu.org
> Subject: [EXTERNAL] Re: [PATCH] Collect both user and kernel events
> for autofdo tests and autoprofiledbootstrap
>
> [You don't often get email from sam@gentoo.org. Learn why this is
> important at https://aka.ms/LearnAboutSenderIdentification ]
>
> Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>
> > On Fri, Jun 30, 2023 at 7:28 AM Eugene Rozenfeld via Gcc-patches
> > <gcc-patches@gcc.gnu.org> wrote:
> >>
> >> When we collect just user events for autofdo with lbr we get some
> >> events where branch sources are kernel addresses and branch targets
> >> are user addresses. Without kernel MMAP events create_gcov can't
> >> make sense of kernel addresses. Currently create_gcov fails if it
> >> can't map at least 95% of events. We sometimes get below this threshold with just user events. The change is to collect both user events and kernel events.
> >
> > Does this require elevated privileges? Can we instead "fix" create_gcov here?
>
> Right, requiring privileges for this is going to be a no-go for a lot of builders. In a distro context, for example, it means we can't consider autofdo at all.
On Wed, Jul 5, 2023 at 11:15 PM Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com> wrote: > > There is no warning and perf /uk succeeds when kptr_restrict is set to 1 and perf_event_paranoid set to 2. However, create_gcov may fail since it won't be able to understand kernel addresses and it requires at least 95% of events to be successfully mapped. OK, so I guess the patch is OK then given it can improve the situation in the right circumstances and doesn't hurt otherwise. Thanks, Richard. > If I set both kptr_restrict and perf_event_paranoid to 1, then I do get warnings from perf (but it still succeeds and exits with a 0 code). And, of course create_gcov will also fail to map some events since it won't understand kernel addresses. > > WARNING: Kernel address maps (/proc/{kallsyms,modules}) are restricted, > check /proc/sys/kernel/kptr_restrict and /proc/sys/kernel/perf_event_paranoid. > > Samples in kernel functions may not be resolved if a suitable vmlinux > file is not found in the buildid cache or in the vmlinux path. > > Samples in kernel modules won't be resolved at all. > > If some relocation was applied (e.g. kexec) symbols may be misresolved > even with a suitable vmlinux or kallsyms file. > > Couldn't record kernel reference relocation symbol > Symbol resolution may be skewed if relocation was used (e.g. kexec). > Check /proc/kallsyms permission or run as root. > [ perf record: Woken up 2 times to write data ] > [ perf record: Captured and wrote 0.037 MB /home/erozen/gcc1_objdir/gcc/testsuite/gcc/indir-call-prof.perf.data (86 samples) ] > > Eugene > > -----Original Message----- > From: Richard Biener <richard.guenther@gmail.com> > Sent: Monday, July 3, 2023 12:47 AM > To: Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com> > Cc: Sam James <sam@gentoo.org>; gcc-patches@gcc.gnu.org > Subject: Re: [EXTERNAL] Re: [PATCH] Collect both user and kernel events for autofdo tests and autoprofiledbootstrap > > On Sat, Jul 1, 2023 at 12:05 AM Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com> wrote: > > > > I also set /proc/sys/kernel/perf_event_paranoid to 1 instead of the default 2. > > Does the perf attempt fail when the privileges are not adjusted and you specify --all? I see it adds /uk as flags, when I do > > > perf record -e instructions//uk ./a.out > > it doesn't complain in any way with > > > cat /proc/sys/kernel/kptr_restrict > 1 > > cat /proc/sys/kernel/perf_event_paranoid > 2 > > so in case the 'kernel' side is simply ignored when profiling there isn't permitted/possible then I guess the patch is OK? > > Can you confirm? > > Thanks, > Richard. > > > -----Original Message----- > > From: Gcc-patches > > <gcc-patches-bounces+erozen=microsoft.com@gcc.gnu.org> On Behalf Of > > Eugene Rozenfeld via Gcc-patches > > Sent: Friday, June 30, 2023 2:44 PM > > To: Sam James <sam@gentoo.org>; Richard Biener > > <richard.guenther@gmail.com> > > Cc: gcc-patches@gcc.gnu.org > > Subject: RE: [EXTERNAL] Re: [PATCH] Collect both user and kernel > > events for autofdo tests and autoprofiledbootstrap > > > > I don't run this with elevated privileges but I set /proc/sys/kernel/kptr_restrict to 0. Setting that does require elevated privileges. > > > > If that's not acceptable, the only fix I can think of is to make that event mapping threshold percentage a parameter to create_gcov and pass something low enough. 80% instead of the current threshold of 95% should work, although it's a bit fragile. > > > > Eugene > > > > -----Original Message----- > > From: Sam James <sam@gentoo.org> > > Sent: Friday, June 30, 2023 1:59 AM > > To: Richard Biener <richard.guenther@gmail.com> > > Cc: Eugene Rozenfeld <Eugene.Rozenfeld@microsoft.com>; > > gcc-patches@gcc.gnu.org > > Subject: [EXTERNAL] Re: [PATCH] Collect both user and kernel events > > for autofdo tests and autoprofiledbootstrap > > > > [You don't often get email from sam@gentoo.org. Learn why this is > > important at https://aka.ms/LearnAboutSenderIdentification ] > > > > Richard Biener via Gcc-patches <gcc-patches@gcc.gnu.org> writes: > > > > > On Fri, Jun 30, 2023 at 7:28 AM Eugene Rozenfeld via Gcc-patches > > > <gcc-patches@gcc.gnu.org> wrote: > > >> > > >> When we collect just user events for autofdo with lbr we get some > > >> events where branch sources are kernel addresses and branch targets > > >> are user addresses. Without kernel MMAP events create_gcov can't > > >> make sense of kernel addresses. Currently create_gcov fails if it > > >> can't map at least 95% of events. We sometimes get below this threshold with just user events. The change is to collect both user events and kernel events. > > > > > > Does this require elevated privileges? Can we instead "fix" create_gcov here? > > > > Right, requiring privileges for this is going to be a no-go for a lot of builders. In a distro context, for example, it means we can't consider autofdo at all.
diff --git a/Makefile.in b/Makefile.in index f19a9db621e..04307ca561b 100644 --- a/Makefile.in +++ b/Makefile.in @@ -404,7 +404,7 @@ MAKEINFO = @MAKEINFO@ EXPECT = @EXPECT@ RUNTEST = @RUNTEST@ -AUTO_PROFILE = gcc-auto-profile -c 10000000 +AUTO_PROFILE = gcc-auto-profile --all -c 10000000 # This just becomes part of the MAKEINFO definition passed down to # sub-makes. It lets flags be given on the command line while still diff --git a/Makefile.tpl b/Makefile.tpl index 3a5b7ed3c92..d0fe7e2fb77 100644 --- a/Makefile.tpl +++ b/Makefile.tpl @@ -407,7 +407,7 @@ MAKEINFO = @MAKEINFO@ EXPECT = @EXPECT@ RUNTEST = @RUNTEST@ -AUTO_PROFILE = gcc-auto-profile -c 10000000 +AUTO_PROFILE = gcc-auto-profile --all -c 10000000 # This just becomes part of the MAKEINFO definition passed down to # sub-makes. It lets flags be given on the command line while still diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index 4d04df2a709..b16853d76df 100644 --- a/gcc/testsuite/lib/target-supports.exp +++ b/gcc/testsuite/lib/target-supports.exp @@ -704,7 +704,7 @@ proc check_effective_target_keeps_null_pointer_checks { } { # this allows parallelism of 16 and higher of parallel gcc-auto-profile proc profopt-perf-wrapper { } { global srcdir - return "$srcdir/../config/i386/gcc-auto-profile -m8 " + return "$srcdir/../config/i386/gcc-auto-profile --all -m8 " } # Return true if profiling is supported on the target.