[lkml]   [2010]   [May]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
SubjectRe: [RFC PATCH] perf: align raw sample data on 64-bit boundaries
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

> > 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.

 \ /
  Last update: 2010-05-24 23:27    [W:0.079 / U:3.108 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site