| Message ID | 20230203154727.1142151-1-richard.purdie@linuxfoundation.org |
|---|---|
| State | Accepted, archived |
| Commit | d2c27ae5c0d06363d2f0a2a8eb4e8e492df58444 |
| Headers | show |
| Series | perf: Fix 6.1 kernel reproducibility issue | expand |
On Fri, Feb 3, 2023 at 10:47 AM Richard Purdie <richard.purdie@linuxfoundation.org> wrote: > > The pmu-events.c file is generated by a python script making os.scandir() > calls. The return value is "order on disk" which can cary between machines. > > Add in a sed to fix the perf source to sort this data which makes > the pmu-events.c file deterministic. Looks good to me. The perf recipe is the great collector of sed manipulations :) > > We should try and upstream this change but we'll need to sed for varying > kernel versions. We should also try and get the perf source being added > to the perf-devsrc package so when failures like this occur, diffoscope > is much more helpful! I can do this, if you haven't started on it. I can't say that i know exactly why it isn't already there, but it can't be that hard to figure out :) Bruce > > Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org> > --- > meta/recipes-kernel/perf/perf.bb | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/meta/recipes-kernel/perf/perf.bb b/meta/recipes-kernel/perf/perf.bb > index c1b0bd22d82..1dff39a17e4 100644 > --- a/meta/recipes-kernel/perf/perf.bb > +++ b/meta/recipes-kernel/perf/perf.bb > @@ -276,6 +276,10 @@ do_configure:prepend () { > sed -i -e "s,$target,$replacement1$replacement2$replacement3,g" \ > "${S}/tools/perf/pmu-events/Build" > fi > + if [ -e "${S}/tools/perf/pmu-events/jevents.py" ]; then > + sed -i -e "s#os.scandir(path)#sorted(os.scandir(path), key=lambda e: e.name)#g" \ > + "${S}/tools/perf/pmu-events/jevents.py" > + fi > # end reproducibility substitutions > > # We need to ensure the --sysroot option in CC is preserved > -- > 2.37.2 >
On Fri, 2023-02-03 at 11:02 -0500, Bruce Ashfield wrote: > On Fri, Feb 3, 2023 at 10:47 AM Richard Purdie > <richard.purdie@linuxfoundation.org> wrote: > > > > The pmu-events.c file is generated by a python script making os.scandir() > > calls. The return value is "order on disk" which can cary between machines. > > > > Add in a sed to fix the perf source to sort this data which makes > > the pmu-events.c file deterministic. > > Looks good to me. The perf recipe is the great collector of sed manipulations :) > > > > > We should try and upstream this change but we'll need to sed for varying > > kernel versions. We should also try and get the perf source being added > > to the perf-devsrc package so when failures like this occur, diffoscope > > is much more helpful! > > I can do this, if you haven't started on it. I can't say that i know > exactly why it > isn't already there, but it can't be that hard to figure out :) I haven't looked at submission upstream. I did have a quick look at the sources issue and realised: diff --git a/meta/recipes-kernel/perf/perf.bb b/meta/recipes-kernel/perf/perf.bb index 1dff39a17e4..0a3179f18be 100644 --- a/meta/recipes-kernel/perf/perf.bb +++ b/meta/recipes-kernel/perf/perf.bb @@ -361,5 +361,5 @@ FILES:${PN}-python = " \ FILES:${PN}-perl = "${libexecdir}/perf-core/scripts/perl" -INHIBIT_PACKAGE_DEBUG_SPLIT="1" +#INHIBIT_PACKAGE_DEBUG_SPLIT="1" DEBUG_OPTIMIZATION:append = " -Wno-error=maybe-uninitialized" which makes them appear. Does anyone remember why we have that and if we can remove it? :) Cheers, Richard
On Fri, Feb 3, 2023 at 11:34 AM Richard Purdie <richard.purdie@linuxfoundation.org> wrote: > > On Fri, 2023-02-03 at 11:02 -0500, Bruce Ashfield wrote: > > On Fri, Feb 3, 2023 at 10:47 AM Richard Purdie > > <richard.purdie@linuxfoundation.org> wrote: > > > > > > The pmu-events.c file is generated by a python script making os.scandir() > > > calls. The return value is "order on disk" which can cary between machines. > > > > > > Add in a sed to fix the perf source to sort this data which makes > > > the pmu-events.c file deterministic. > > > > Looks good to me. The perf recipe is the great collector of sed manipulations :) > > > > > > > > We should try and upstream this change but we'll need to sed for varying > > > kernel versions. We should also try and get the perf source being added > > > to the perf-devsrc package so when failures like this occur, diffoscope > > > is much more helpful! > > > > I can do this, if you haven't started on it. I can't say that i know > > exactly why it > > isn't already there, but it can't be that hard to figure out :) > > I haven't looked at submission upstream. I did have a quick look at the > sources issue and realised: > > diff --git a/meta/recipes-kernel/perf/perf.bb b/meta/recipes-kernel/perf/perf.bb > index 1dff39a17e4..0a3179f18be 100644 > --- a/meta/recipes-kernel/perf/perf.bb > +++ b/meta/recipes-kernel/perf/perf.bb > @@ -361,5 +361,5 @@ FILES:${PN}-python = " \ > FILES:${PN}-perl = "${libexecdir}/perf-core/scripts/perl" > > > -INHIBIT_PACKAGE_DEBUG_SPLIT="1" > +#INHIBIT_PACKAGE_DEBUG_SPLIT="1" > DEBUG_OPTIMIZATION:append = " -Wno-error=maybe-uninitialized" > > > which makes them appear. Does anyone remember why we have that and if > we can remove it? :) I can't recall why we had that in place. It would predate us taking a copy of the sources for the build, so it may have been related to that. Assuming nothing pops up as a new breakage ... I have no technical recollection of why it can't be changed. Bruce > > Cheers, > > Richard
On Fri, 2023-02-03 at 11:39 -0500, Bruce Ashfield wrote: > On Fri, Feb 3, 2023 at 11:34 AM Richard Purdie > <richard.purdie@linuxfoundation.org> wrote: > > > > On Fri, 2023-02-03 at 11:02 -0500, Bruce Ashfield wrote: > > > On Fri, Feb 3, 2023 at 10:47 AM Richard Purdie > > > <richard.purdie@linuxfoundation.org> wrote: > > > > > > > > The pmu-events.c file is generated by a python script making os.scandir() > > > > calls. The return value is "order on disk" which can cary between machines. > > > > > > > > Add in a sed to fix the perf source to sort this data which makes > > > > the pmu-events.c file deterministic. > > > > > > Looks good to me. The perf recipe is the great collector of sed manipulations :) > > > > > > > > > > > We should try and upstream this change but we'll need to sed for varying > > > > kernel versions. We should also try and get the perf source being added > > > > to the perf-devsrc package so when failures like this occur, diffoscope > > > > is much more helpful! > > > > > > I can do this, if you haven't started on it. I can't say that i know > > > exactly why it > > > isn't already there, but it can't be that hard to figure out :) > > > > I haven't looked at submission upstream. I did have a quick look at the > > sources issue and realised: > > > > diff --git a/meta/recipes-kernel/perf/perf.bb b/meta/recipes-kernel/perf/perf.bb > > index 1dff39a17e4..0a3179f18be 100644 > > --- a/meta/recipes-kernel/perf/perf.bb > > +++ b/meta/recipes-kernel/perf/perf.bb > > @@ -361,5 +361,5 @@ FILES:${PN}-python = " \ > > FILES:${PN}-perl = "${libexecdir}/perf-core/scripts/perl" > > > > > > -INHIBIT_PACKAGE_DEBUG_SPLIT="1" > > +#INHIBIT_PACKAGE_DEBUG_SPLIT="1" > > DEBUG_OPTIMIZATION:append = " -Wno-error=maybe-uninitialized" > > > > > > which makes them appear. Does anyone remember why we have that and if > > we can remove it? :) > > I can't recall why we had that in place. > > It would predate us taking a copy of the sources for the build, so it > may have been related to that. > > Assuming nothing pops up as a new breakage ... I have no technical > recollection of why it can't be changed. It was added by you in 2014! :) https://git.yoctoproject.org/poky/commit/meta/recipes-kernel/perf?id=08808c9f404d01c497b92e7d34a3bb0c0159a258 I think the world has changed and at least my local build didn't fail with that when removing it. I'll try testing a wider patch on the autobuilder and see if we get any errors. If we do, we should probably resolve them some other way. Getting more usable diffoscope output would be a huge win given how often perf breaks. Cheers, Richard
diff --git a/meta/recipes-kernel/perf/perf.bb b/meta/recipes-kernel/perf/perf.bb index c1b0bd22d82..1dff39a17e4 100644 --- a/meta/recipes-kernel/perf/perf.bb +++ b/meta/recipes-kernel/perf/perf.bb @@ -276,6 +276,10 @@ do_configure:prepend () { sed -i -e "s,$target,$replacement1$replacement2$replacement3,g" \ "${S}/tools/perf/pmu-events/Build" fi + if [ -e "${S}/tools/perf/pmu-events/jevents.py" ]; then + sed -i -e "s#os.scandir(path)#sorted(os.scandir(path), key=lambda e: e.name)#g" \ + "${S}/tools/perf/pmu-events/jevents.py" + fi # end reproducibility substitutions # We need to ensure the --sysroot option in CC is preserved
The pmu-events.c file is generated by a python script making os.scandir() calls. The return value is "order on disk" which can cary between machines. Add in a sed to fix the perf source to sort this data which makes the pmu-events.c file deterministic. We should try and upstream this change but we'll need to sed for varying kernel versions. We should also try and get the perf source being added to the perf-devsrc package so when failures like this occur, diffoscope is much more helpful! Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org> --- meta/recipes-kernel/perf/perf.bb | 4 ++++ 1 file changed, 4 insertions(+)