Messages in this thread | | | Date | Mon, 20 Jan 2020 21:24:45 +0100 | From | Peter Zijlstra <> | Subject | Re: [RESEND PATCH V5 1/2] perf/core: Add new branch sample type for HW index of raw branch records |
| |
On Mon, Jan 20, 2020 at 11:50:59AM -0500, Liang, Kan wrote: > > > On 1/20/2020 4:23 AM, Peter Zijlstra wrote: > > On Thu, Jan 16, 2020 at 07:57:56AM -0800, kan.liang@linux.intel.com wrote: > > > > > struct perf_branch_stack { > > > __u64 nr; > > > + __u64 hw_idx; > > > struct perf_branch_entry entries[0]; > > > }; > > > > The above and below order doesn't match. > > > > > @@ -849,7 +853,11 @@ enum perf_event_type { > > > * char data[size];}&& PERF_SAMPLE_RAW > > > * > > > * { u64 nr; > > > - * { u64 from, to, flags } lbr[nr];} && PERF_SAMPLE_BRANCH_STACK > > > + * { u64 from, to, flags } lbr[nr]; > > > + * > > > + * # only available if PERF_SAMPLE_BRANCH_HW_INDEX is set > > > + * u64 hw_idx; > > > + * } && PERF_SAMPLE_BRANCH_STACK > > > > That wants to be written as: > > > > { u64 nr; > > { u64 from, to, flags; } entries[nr]; > > { u64 hw_idx; } && PERF_SAMPLE_BRANCH_HW_INDEX > > } && PERF_SAMPLE_BRANCH_STACK > > > > But the big question is; why isn't it: > > > > { u64 nr; > > { u64 hw_idx; } && PERF_SAMPLE_BRANCH_HW_INDEX > > { u64 from, to, flags; } entries[nr]; > > } && PERF_SAMPLE_BRANCH_STACK > > > > to match the struct perf_branch_stack order. Having that variable sized > > entry in the middle just seems weird. > > > Usually, new data should be output to the end of a sample.
Because.... you want old tools to read new output?
> However, the entries[0] is sized entry, so I have to put the hw_idx before
entries[0] is only in the C thing, and in C you indeed have to put hw_idx before.
> entry. It makes the inconsistency. Sorry for the confusion caused.
n/p it's clear now I think.
| |