Re: [PATCH v3 01/15] perf/mem: Introduce PERF_MEM_LVLNUM_{EXTN_MEM|IO}

[Liang, Kan]

On 2022-09-30 8:50 a.m., Ravi Bangoria wrote:
> On 30-Sep-22 4:18 PM, kajoljain wrote:
>>
>>
>> On 9/28/22 15:27, Ravi Bangoria wrote:
>>> PERF_MEM_LVLNUM_EXTN_MEM which can be used to indicate accesses to
>>> extension memory like CXL etc. PERF_MEM_LVL_IO can be used for IO
>>> accesses but it can not distinguish between local and remote IO.
>>> Introduce new field PERF_MEM_LVLNUM_IO which can be clubbed with
>>> PERF_MEM_REMOTE_REMOTE to indicate Remote IO accesses.
>>>
>>> Signed-off-by: Ravi Bangoria <ravi.bangoria@xxxxxxx>
>>> ---
>>>  include/uapi/linux/perf_event.h | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
>>> index e639c74cf5fb..4ae3c249f675 100644
>>> --- a/include/uapi/linux/perf_event.h
>>> +++ b/include/uapi/linux/perf_event.h
>>> @@ -1336,7 +1336,9 @@ union perf_mem_data_src {
>>>  #define PERF_MEM_LVLNUM_L2	0x02 /* L2 */
>>>  #define PERF_MEM_LVLNUM_L3	0x03 /* L3 */
>>>  #define PERF_MEM_LVLNUM_L4	0x04 /* L4 */
>>> -/* 5-0xa available */
>>> +/* 5-0x8 available */
>>> +#define PERF_MEM_LVLNUM_EXTN_MEM 0x09 /* Extension memory */
>>
>> Hi Ravi,
>>     Here we are adding entry explicitly for accesses to Extension memory
>> like CXL. In future if we want to extend it for cache or other accesses
>> , we again need to add new entries.
>> Can we rather add single entry say PERF_MEM_LVLNUM_EXTN and further can
>> use reserved bits to specify memory/cache?
> 
> Is everybody okay with this:
> 
> #define PERF_MEM_LVLNUM_EXTN	0x09 /* CXL */

I think a generic name, PERF_MEM_LVLNUM_EXTN, only make sense, when it
wants to include all the types of the Extension memory, e.g., CXL, PMEM,
HBM, etc. Then we can set this bit and the corresponding CXL bits to
understand the real source. Is it the case here?

But if it's only for the CXL, I think it's better to use a dedicated
name, PERF_MEM_LVLNUM_CXL. (as we did for PMEM, PERF_MEM_LVLNUM_PMEM).
If so, I don't think we need the PERF_MEM_EXTN_CXL_ANY.

Thanks,
Kan

> 
> And a 3 bit variable to define what type of cxl that would be:
> 
> #define PERF_MEM_EXTN_CXL_ANY	0x1
> #define PERF_MEM_EXTN_CXL_MEM	0x2
> #define PERF_MEM_EXTN_CXL_CACHE	0x2
> #define PERF_MEM_EXTN_CXL_IO	0x3
> 
> Peter, Shall I send this as addon patch series or are you okay reverting
> current patches?
> 
> Thanks,
> Ravi