Re: [PATCH v1 2/2] perf header: Support hybrid CPU_PMU_CAPS

[Jin, Yao]

Hi Jiri,

On 5/10/2021 9:11 PM, Jiri Olsa wrote:
> On Thu, May 06, 2021 at 10:43:39PM +0800, Jin, Yao wrote:
>
> SNIP
>
> > > > 'nr of rest pmus', we know that all pmus have been processed.
> > > >
> > > > Otherwise, we have to continue reading data file till we find something
> > > > incorrect and then finally drop the last read data.
> > >
> > > you have the size of the feature data right? I think we use
> > > it in other cases to check if there are more data
> >
> > The challenge for us is if we need to compatible with the old perf.data
> > which was generated by old perf tool.
> >
> > For the old perf.data, the layout in header is:
> >
> > nr of caps
> > caps string 1
> > caps string 2
> > ...
> > caps string N
> >
> > It doesn't carry with any other fields such as size of caps data.
> >
> > To be compatible with old perf.data, so I have to extend the layout to:
> >
> > nr of caps for pmu 1
> > caps string 1
> > caps string 2
> > ...
> > caps string N
> > name of pmu 1
> > nr of rest pmus
> >
> > nr of caps for pmu2
> > caps string 1
> > caps string 2
> > ...
> > caps string N
> > name of pmu 2
> > nr of rest pmus
> >
> > When the new perf tool detects the string such as "cpu_", it can know that
> > it's the pmu name field in new perf.data, otherwise it's the old perf.data.
>
> what if the cap string starts with 'cpu_' ?

I just assume the cap string would not have 'cpu_' string. Yes, I agree, that's not a very good 
solution.

> I think it might be better to create new feature that
> stores caps for multiple pmus in generic way

Yes, creating a new feature in header (such as "HYBRID_CPU_PMU_CAPS") is a better way.

> > If we add new field such as "size" to the layout, I'm afraid the new perf
> > tool can not process the old perf.data correctly.
> >
> > If we don't need to support old perf.data, that makes things easy.
>
> we need to

Yes, we need to. I'm now preparing v3.

Thanks
Jin Yao

> jirka