Re: [PATCH 1/3] perf events: fix WARN_ON_ONCE for 64-bit raw data, SW events

[Peter Zijlstra <peterz@infradead.org>]

On Fri, 2011-02-18 at 07:33 -0700, David Ahern wrote:
> 
> On 02/18/11 04:00, Peter Zijlstra wrote:
> > On Thu, 2011-02-17 at 22:53 -0700, David Ahern wrote:
> >> This check does not work out for 64-bit counter:
> >>
> >>     sizeof(u32) + sizeof(u64) = 4 + 8 = 12
> >>
> >> which does not pass the WARN_ON_ONCE test. I'm guessing (hoping
> >> really) that the intent is that the size of the raw data is a
> >> multiple of 4 bytes, not 8.
> > 
> > No the data must come in 8bytes strides.
> > 
> 
> Then the sizeof the size field needs to be moved to u64; right now it is
> u32:

That's a false conclusion.

> struct perf_raw_record {
>     u32             size;
>     void                *data;
> };

> So size should be changed to u64 rather than modifying the WARN_ON_ONCE?

No.

> Another gotcha is in perf_output_sample there is:
>             struct {
>                 u32 size;
>                 u32 data;
>             } raw = {
>                 .size = sizeof(u32),
>                 .data = 0,
>             };
> 
> While that meets the 8-byte stride I was concerned that changing the
> size field to u64 breaks ABI.

It would.

No, what you need to do is provide data that is sized such that it
matches the 8b stride adding padding where needed.

Anyway, I don't think you need RAW at all.