On Sun, 2 May 2021 at 20:39, Eric W. Biederman <ebiederm@xxxxxxxxxxxx> wrote: > > Marco Elver <elver@xxxxxxxxxx> writes: > > > On Sat, 1 May 2021 at 01:44, Eric W. Biederman <ebiederm@xxxxxxxxxxxx> wrote: > >> > >> Don't abuse si_errno and deliver all of the perf data in si_perf. > >> > >> Signed-off-by: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx> > >> --- > > > > Thank you for the fix, this looks cleaner. > > > > Just note that this patch needs to include updates to > > tools/testing/selftests/perf_events. This should do it: > >> sed -i 's/si_perf/si_perf.data/g; s/si_errno/si_perf.type/g' tools/testing/selftests/perf_events/*.c > > > > Subject: s/perf_data/perf data/ ? > > > > For uapi, need to switch to __u32, see below. > > Good point. > > The one thing that this doesn't do is give you a 64bit field > on 32bit architectures. > > On 32bit builds the layout is: > > int si_signo; > int si_errno; > int si_code; > void __user *_addr; > > So I believe if the first 3 fields were moved into the _sifields union > si_perf could define a 64bit field as it's first member and it would not > break anything else. > > Given that the data field is 64bit that seems desirable. Yes, it's quite unfortunate -- it was __u64 at first, but then we noticed it broke 32-bit architectures like arm: https://lore.kernel.org/linux-arch/20210422191823.79012-1-elver@xxxxxxxxxx/ Thanks, -- Marco
