Re: [PATCH 01/12] perf_events: add generic taken branch sampling support

[Stephane Eranian]

On Fri, Oct 7, 2011 at 12:32 PM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
> On Fri, 2011-10-07 at 12:28 +0200, Stephane Eranian wrote:
>> On Thu, Oct 6, 2011 at 6:57 PM, Peter Zijlstra <peterz@xxxxxxxxxxxxx> wrote:
>> > On Thu, 2011-10-06 at 16:49 +0200, Stephane Eranian wrote:
>> >> Âstruct perf_branch_entry {
>> >> Â Â Â Â __u64 Â Â Â Â Â Â Â Â Â Â Â Â Â from;
>> >> Â Â Â Â __u64 Â Â Â Â Â Â Â Â Â Â Â Â Â to;
>> >> + Â Â Â struct {
>> >> + Â Â Â Â Â Â Â __u64 Â Â Â Â Â Â Â Â Â mispred:1, Â/* target mispredicted */
>> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â predicted:1,/* target predicted */
>> >> + Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â reserved:62;
>> >> + Â Â Â };
>> >> Â};
>> >
>> > Why that anonymous structure?
>> >
>> The interface can return more than source/destination, it can also
>> return prediction infos.
>> Prediction is boolean, thus we only need a couple of bits. The reason
>> there are two bits
>> and not just one is to disambiguate between: predicted correctly and
>> 'prediction reporting
>> unsupported'. For instance, prediction is also supported since
>> Nehalem, Core2/Atom do
>> not have it.
>
> Right, I got that.
>
>> But maybe you're just commenting of the anonymous vs. named struct for
>> that?
>
> I don't see the need for any struct, why can't the bitfield live in
> perf_branch_entry proper?
>
>> It is just for convenience. Isn't that the same argument for the
>> anonymous bitfield
>> in struct perf_event_attr?
>
> But that isn't wrapped in a structure either is it..
>
> I guess I'm asking, what's wrong with:
>
> struct perf_branch_entry {
> Â Â Â Â__u64 Â Â Â Â Â from;
> Â Â Â Â__u64 Â Â Â Â Â to;
> Â Â Â Â__u64 Â Â Â Â Â mispred:1,
> Â Â Â Â Â Â Â Â Â Â Â Âpredicted:1,
> Â Â Â Â Â Â Â Â Â Â Â Âreserved:62;
> };
>
Ok, I think I missed your original point about the anonymous
struct. What you're suggesting above is certainly fine with me.
I am not just used to bitfields mixed with other fields in a struct.
I'll fix that.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/