Re: [RFC PATCH] perf: align raw sample data on 64-bit boundaries

[Frederic Weisbecker <fweisbec@gmail.com>]

On Wed, May 19, 2010 at 11:31:00AM +0200, Robert Richter wrote:
> On 19.05.10 03:39:10, Frederic Weisbecker wrote:
> 
> > > This changes the ABI and requires changes in the userland tools. For
> > > tools/perf this is at a single location in event.c only. This could
> > > also introduce some overhead on smaller architectures, but currently
> > > this is only used on x86 or for transferring raw tracepoint
> > > data.
> > 
> > 
> > No this is used on any architectures that event have a minimal support for
> > perf events.
> > 
> > I use tracepoint raw samples in sparc 64 for example (which has much more
> > than the minimal support).
> 
> Isn't here the same alignment problem on archs there unsigned long is
> 64 bit? Also, most samples I found have a size of multiples of 8
> bytes, so even on 32 bit archs there would be a padding of 4 bytes
> somethere in the sample.



Yeah there was an alignment problem in sparc 64 that I fixed in perf
tools lately. The fix is more a hack though, the real solution would
be to have this alignment thing fixed.

And yeah, probably most samples need padding.


 
> > I don't think we should do this. Ok it's true we've screwed up
> > something there but breaking the ABI is going to make the things
> > even worst I think.
> 
> I was not sure how hard an ABI breakage would be. I think the small
> number of users of raw samples is manageable, but I understand if you
> feel uncomfortable with it.


I don't know how many people use it. But I prefer not to take that
risk.



> 
> > I would feel better with a new PERF_SAMPLE_RAW_ALIGNED sample_type
> > and schedule the deprecation of PERF_SAMPLE_RAW for later but keep
> > it for some releases.
> 
> This could be an alternative. Though, it duplicates code paths and
> introduces ugly sample type checks. Another alternative would be to
> check the size value, if it is (n * sizeof(u64)) we could asume 64 bit
> alignment. But maybe this makes things much worse.
> 
> -Robert


It doesn't duplicate much code paths, we only have a few corner cases to
plug in. And more importantly, that would be temporary if we schedule the
older PERF_SAMPLE_RAW in, say, three releases from now.

This ensures an easy forward compatibility (older perf tools -> newer kernel).
But the backward compatibility is less easy (newer perf tools -> older kernel)
as it means we need to test dynamically if we have PERF_SAMPLE_RAW_ALIGNED,
otherwise we need to fall back to using the older one.