Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: ZxmTUo5/SdwjWQVzsUhN5M6ow+HFzDQU5bgzHYTHTNiSKefhzWMJyRUHcJr2xt73BJXBUKg3qP
 oPtDt+Hwiszg==
IronPort-SDR: rLv7jY/3kBuzYMTV3rx0id8R76ZofJs8fvOdYemXecoT5NHN0gChnNr9eiQiiL5ILvDVWdBjjK
 POTa+QcP0bdA==
Subject: Re: [PATCH v1 2/2] perf header: Support hybrid CPU_PMU_CAPS
To:     Jiri Olsa <jolsa@redhat.com>
Cc:     acme@kernel.org, jolsa@kernel.org, peterz@infradead.org,
        mingo@redhat.com, alexander.shishkin@linux.intel.com,
        Linux-kernel@vger.kernel.org, ak@linux.intel.com,
        kan.liang@intel.com, yao.jin@intel.com
References: <20210430074602.3028-1-yao.jin@linux.intel.com>
 <20210430074602.3028-2-yao.jin@linux.intel.com> <YJFjTCsk9dCd6QP7@krava>
 <c0bd3baa-3209-23f3-7058-c6908434de2d@linux.intel.com>
 <YJPtmyzTEe/IUID4@krava>
 <5cafb729-1dcd-ad4a-507b-1155c16a785e@linux.intel.com>
 <YJkxECvg6clImtOY@krava>
From:   "Jin, Yao" <yao.jin@linux.intel.com>
Message-ID: <249153c0-a560-aa78-97a9-039dee5be86b@linux.intel.com>
Date:   Tue, 11 May 2021 09:15:26 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.10.1
MIME-Version: 1.0
In-Reply-To: <YJkxECvg6clImtOY@krava>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

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
>