Messages in this thread | | | Date | Sat, 21 Mar 2009 10:56:52 +0100 | From | Ingo Molnar <> | Subject | Re: [PATCH] perfcounters: fix type/event_id layout on big-endian systems |
| |
* Paul Mackerras <paulus@samba.org> wrote:
> 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.
yeah - i pulled your fix, thanks Paul.
Maybe the best option is to get rid of the bitfields and use masks ...
Ingo
| |