Messages in this thread | | | Date | Sat, 21 Mar 2009 20:52:45 +1100 | From | Paul Mackerras <> | Subject | Re: [PATCH] perfcounters: fix type/event_id layout on big-endian systems |
| |
Ingo Molnar writes:
> hm, this ifdef really looks ugly. How about just changing event_id > to 64 bits and having a separate u32 type field? The size impact is > minimal, the cleanliness win is significant :-)
We wanted to get a complete and unique identifier for the event into 64 bits so that we could put it into the ring buffer for PERF_RECORD_GROUP and have it take up only one 8-byte slot and yet identify uniquely which counter's value follows it. I don't know that that is absolutely necessary but it sounds like a nice property.
We could easily go back to a 1-bit raw field and have the type be either a 64-bit raw value or an 8-bit type plus 32 or 56-bit event_id.
Or we could keep the current layout but use explicit shifts and masks rather than bitfields.
I don't know the current C rules concerning unions very well, but I have the impression that writing to one member of a union and reading another is undefined behaviour, which is another strike against the current code if true...
Anyway, the point is that the current code doesn't compile on powerpc and wouldn't work properly even if it did, so we need to do something.
Paul.
| |