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 <[email protected]>
---
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 */
+#define PERF_MEM_LVLNUM_IO 0x0a /* I/O */
#define PERF_MEM_LVLNUM_ANY_CACHE 0x0b /* Any cache */
#define PERF_MEM_LVLNUM_LFB 0x0c /* LFB */
#define PERF_MEM_LVLNUM_RAM 0x0d /* RAM */
--
2.31.1
The following commit has been merged into the perf/core branch of tip:
Commit-ID: ee3e88dfec23153d0675b5d00522297b9adf657c
Gitweb: https://git.kernel.org/tip/ee3e88dfec23153d0675b5d00522297b9adf657c
Author: Ravi Bangoria <[email protected]>
AuthorDate: Wed, 28 Sep 2022 15:27:51 +05:30
Committer: Peter Zijlstra <[email protected]>
CommitterDate: Thu, 29 Sep 2022 12:20:54 +02:00
perf/mem: Introduce PERF_MEM_LVLNUM_{EXTN_MEM|IO}
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 <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
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 e639c74..4ae3c24 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 */
+#define PERF_MEM_LVLNUM_IO 0x0a /* I/O */
#define PERF_MEM_LVLNUM_ANY_CACHE 0x0b /* Any cache */
#define PERF_MEM_LVLNUM_LFB 0x0c /* LFB */
#define PERF_MEM_LVLNUM_RAM 0x0d /* RAM */
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 <[email protected]>
> ---
> 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?
Thanks,
Kajol Jain
> +#define PERF_MEM_LVLNUM_IO 0x0a /* I/O */
> #define PERF_MEM_LVLNUM_ANY_CACHE 0x0b /* Any cache */
> #define PERF_MEM_LVLNUM_LFB 0x0c /* LFB */
> #define PERF_MEM_LVLNUM_RAM 0x0d /* RAM */
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 <[email protected]>
>> ---
>> 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 */
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
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 <[email protected]>
>>> ---
>>> 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
On 30-Sep-22 7:47 PM, Liang, Kan wrote:
>
>
> 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 <[email protected]>
>>>> ---
>>>> 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.
Ok. For now, I think below is good enough? Later we can introduce new
variable to provide type of cxl device.
From 5deb2055e2b5b0a61403f2d5f4e5a784b14a65e3 Mon Sep 17 00:00:00 2001
From: Ravi Bangoria <[email protected]>
Date: Sat, 1 Oct 2022 11:37:05 +0530
Subject: [PATCH] perf/mem: Rename PERF_MEM_LVLNUM_EXTN_MEM to
PERF_MEM_LVLNUM_CXL
PERF_MEM_LVLNUM_EXTN_MEM was introduced to cover CXL devices but it's
bit ambiguous name and also not generic enough to cover cxl.cache and
cxl.io devices. Rename it to PERF_MEM_LVLNUM_CXL to be more specific.
Signed-off-by: Ravi Bangoria <[email protected]>
---
arch/x86/events/amd/ibs.c | 2 +-
include/uapi/linux/perf_event.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
index 3271735f0070..4cb710efbdd9 100644
--- a/arch/x86/events/amd/ibs.c
+++ b/arch/x86/events/amd/ibs.c
@@ -801,7 +801,7 @@ static void perf_ibs_get_mem_lvl(union ibs_op_data2 *op_data2,
/* Extension Memory */
if (ibs_caps & IBS_CAPS_ZEN4 &&
ibs_data_src == IBS_DATA_SRC_EXT_EXT_MEM) {
- data_src->mem_lvl_num = PERF_MEM_LVLNUM_EXTN_MEM;
+ data_src->mem_lvl_num = PERF_MEM_LVLNUM_CXL;
if (op_data2->rmt_node) {
data_src->mem_remote = PERF_MEM_REMOTE_REMOTE;
/* IBS doesn't provide Remote socket detail */
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index 85be78e0e7f6..eb1090604d53 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -1337,7 +1337,7 @@ union perf_mem_data_src {
#define PERF_MEM_LVLNUM_L3 0x03 /* L3 */
#define PERF_MEM_LVLNUM_L4 0x04 /* L4 */
/* 5-0x8 available */
-#define PERF_MEM_LVLNUM_EXTN_MEM 0x09 /* Extension memory */
+#define PERF_MEM_LVLNUM_CXL 0x09 /* CXL */
#define PERF_MEM_LVLNUM_IO 0x0a /* I/O */
#define PERF_MEM_LVLNUM_ANY_CACHE 0x0b /* Any cache */
#define PERF_MEM_LVLNUM_LFB 0x0c /* LFB */
--
2.31.1
On 2022-10-01 2:37 a.m., Ravi Bangoria wrote:
> On 30-Sep-22 7:47 PM, Liang, Kan wrote:
>>
>>
>> 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 <[email protected]>
>>>>> ---
>>>>> 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.
>
> Ok. For now, I think below is good enough? Later we can introduce new
> variable to provide type of cxl device.
>
>
> From 5deb2055e2b5b0a61403f2d5f4e5a784b14a65e3 Mon Sep 17 00:00:00 2001
> From: Ravi Bangoria <[email protected]>
> Date: Sat, 1 Oct 2022 11:37:05 +0530
> Subject: [PATCH] perf/mem: Rename PERF_MEM_LVLNUM_EXTN_MEM to
> PERF_MEM_LVLNUM_CXL
>
> PERF_MEM_LVLNUM_EXTN_MEM was introduced to cover CXL devices but it's
> bit ambiguous name and also not generic enough to cover cxl.cache and
> cxl.io devices. Rename it to PERF_MEM_LVLNUM_CXL to be more specific.
Looks good to me.
Reviewed-by: Kan Liang <[email protected]>
Thanks,
Kan
>
> Signed-off-by: Ravi Bangoria <[email protected]>
> ---
> arch/x86/events/amd/ibs.c | 2 +-
> include/uapi/linux/perf_event.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
> index 3271735f0070..4cb710efbdd9 100644
> --- a/arch/x86/events/amd/ibs.c
> +++ b/arch/x86/events/amd/ibs.c
> @@ -801,7 +801,7 @@ static void perf_ibs_get_mem_lvl(union ibs_op_data2 *op_data2,
> /* Extension Memory */
> if (ibs_caps & IBS_CAPS_ZEN4 &&
> ibs_data_src == IBS_DATA_SRC_EXT_EXT_MEM) {
> - data_src->mem_lvl_num = PERF_MEM_LVLNUM_EXTN_MEM;
> + data_src->mem_lvl_num = PERF_MEM_LVLNUM_CXL;
> if (op_data2->rmt_node) {
> data_src->mem_remote = PERF_MEM_REMOTE_REMOTE;
> /* IBS doesn't provide Remote socket detail */
> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> index 85be78e0e7f6..eb1090604d53 100644
> --- a/include/uapi/linux/perf_event.h
> +++ b/include/uapi/linux/perf_event.h
> @@ -1337,7 +1337,7 @@ union perf_mem_data_src {
> #define PERF_MEM_LVLNUM_L3 0x03 /* L3 */
> #define PERF_MEM_LVLNUM_L4 0x04 /* L4 */
> /* 5-0x8 available */
> -#define PERF_MEM_LVLNUM_EXTN_MEM 0x09 /* Extension memory */
> +#define PERF_MEM_LVLNUM_CXL 0x09 /* CXL */
> #define PERF_MEM_LVLNUM_IO 0x0a /* I/O */
> #define PERF_MEM_LVLNUM_ANY_CACHE 0x0b /* Any cache */
> #define PERF_MEM_LVLNUM_LFB 0x0c /* LFB */
On 03-Oct-22 6:45 PM, Liang, Kan wrote:
>
>
> On 2022-10-01 2:37 a.m., Ravi Bangoria wrote:
>> On 30-Sep-22 7:47 PM, Liang, Kan wrote:
>>>
>>>
>>> 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 <[email protected]>
>>>>>> ---
>>>>>> 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.
>>
>> Ok. For now, I think below is good enough? Later we can introduce new
>> variable to provide type of cxl device.
>>
>>
>> From 5deb2055e2b5b0a61403f2d5f4e5a784b14a65e3 Mon Sep 17 00:00:00 2001
>> From: Ravi Bangoria <[email protected]>
>> Date: Sat, 1 Oct 2022 11:37:05 +0530
>> Subject: [PATCH] perf/mem: Rename PERF_MEM_LVLNUM_EXTN_MEM to
>> PERF_MEM_LVLNUM_CXL
>>
>> PERF_MEM_LVLNUM_EXTN_MEM was introduced to cover CXL devices but it's
>> bit ambiguous name and also not generic enough to cover cxl.cache and
>> cxl.io devices. Rename it to PERF_MEM_LVLNUM_CXL to be more specific.
>
> Looks good to me.
>
> Reviewed-by: Kan Liang <[email protected]>
Thanks Kan.
Peter, can you please include this patch along with the series?
Arnaldo, I'll respin tool side of patches with this change.
Thanks,
Ravi
Em Thu, Oct 06, 2022 at 05:08:55PM +0530, Ravi Bangoria escreveu:
> On 03-Oct-22 6:45 PM, Liang, Kan wrote:
> >> PERF_MEM_LVLNUM_EXTN_MEM was introduced to cover CXL devices but it's
> >> bit ambiguous name and also not generic enough to cover cxl.cache and
> >> cxl.io devices. Rename it to PERF_MEM_LVLNUM_CXL to be more specific.
> > Looks good to me.
> > Reviewed-by: Kan Liang <[email protected]>
> Thanks Kan.
> Peter, can you please include this patch along with the series?
> Arnaldo, I'll respin tool side of patches with this change.
Its already upstream, so please go on from there, ok?
I'm now processing perf patches after getting lost in pahole land for a
bit :-)
- Arnaldo
> Its already upstream, so please go on from there, ok?
Right. Only PERF_MEM_LVLNUM_EXTN_MEM -> PERF_MEM_LVLNUM_CXL kernel side
patch is pending. Tool side already uses PERF_MEM_LVLNUM_CXL macro.
Peter, let me know if you want me to resend.
Thanks,
Ravi
On Sat, Oct 01, 2022 at 12:07:40PM +0530, Ravi Bangoria wrote:
> From 5deb2055e2b5b0a61403f2d5f4e5a784b14a65e3 Mon Sep 17 00:00:00 2001
> From: Ravi Bangoria <[email protected]>
> Date: Sat, 1 Oct 2022 11:37:05 +0530
> Subject: [PATCH] perf/mem: Rename PERF_MEM_LVLNUM_EXTN_MEM to
> PERF_MEM_LVLNUM_CXL
>
> PERF_MEM_LVLNUM_EXTN_MEM was introduced to cover CXL devices but it's
> bit ambiguous name and also not generic enough to cover cxl.cache and
> cxl.io devices. Rename it to PERF_MEM_LVLNUM_CXL to be more specific.
>
> Signed-off-by: Ravi Bangoria <[email protected]>
Thanks!
> ---
> arch/x86/events/amd/ibs.c | 2 +-
> include/uapi/linux/perf_event.h | 2 +-
> 2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
> index 3271735f0070..4cb710efbdd9 100644
> --- a/arch/x86/events/amd/ibs.c
> +++ b/arch/x86/events/amd/ibs.c
> @@ -801,7 +801,7 @@ static void perf_ibs_get_mem_lvl(union ibs_op_data2 *op_data2,
> /* Extension Memory */
> if (ibs_caps & IBS_CAPS_ZEN4 &&
> ibs_data_src == IBS_DATA_SRC_EXT_EXT_MEM) {
> - data_src->mem_lvl_num = PERF_MEM_LVLNUM_EXTN_MEM;
> + data_src->mem_lvl_num = PERF_MEM_LVLNUM_CXL;
> if (op_data2->rmt_node) {
> data_src->mem_remote = PERF_MEM_REMOTE_REMOTE;
> /* IBS doesn't provide Remote socket detail */
> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
> index 85be78e0e7f6..eb1090604d53 100644
> --- a/include/uapi/linux/perf_event.h
> +++ b/include/uapi/linux/perf_event.h
> @@ -1337,7 +1337,7 @@ union perf_mem_data_src {
> #define PERF_MEM_LVLNUM_L3 0x03 /* L3 */
> #define PERF_MEM_LVLNUM_L4 0x04 /* L4 */
> /* 5-0x8 available */
> -#define PERF_MEM_LVLNUM_EXTN_MEM 0x09 /* Extension memory */
> +#define PERF_MEM_LVLNUM_CXL 0x09 /* CXL */
> #define PERF_MEM_LVLNUM_IO 0x0a /* I/O */
> #define PERF_MEM_LVLNUM_ANY_CACHE 0x0b /* Any cache */
> #define PERF_MEM_LVLNUM_LFB 0x0c /* LFB */
> --
> 2.31.1
The following commit has been merged into the perf/urgent branch of tip:
Commit-ID: cb6c18b5a41622c7a439508f7421f8766a91cb87
Gitweb: https://git.kernel.org/tip/cb6c18b5a41622c7a439508f7421f8766a91cb87
Author: Ravi Bangoria <[email protected]>
AuthorDate: Sat, 01 Oct 2022 11:37:05 +05:30
Committer: Peter Zijlstra <[email protected]>
CommitterDate: Thu, 27 Oct 2022 10:27:32 +02:00
perf/mem: Rename PERF_MEM_LVLNUM_EXTN_MEM to PERF_MEM_LVLNUM_CXL
PERF_MEM_LVLNUM_EXTN_MEM was introduced to cover CXL devices but it's
bit ambiguous name and also not generic enough to cover cxl.cache and
cxl.io devices. Rename it to PERF_MEM_LVLNUM_CXL to be more specific.
Signed-off-by: Ravi Bangoria <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
---
arch/x86/events/amd/ibs.c | 2 +-
include/uapi/linux/perf_event.h | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/events/amd/ibs.c b/arch/x86/events/amd/ibs.c
index 3271735..4cb710e 100644
--- a/arch/x86/events/amd/ibs.c
+++ b/arch/x86/events/amd/ibs.c
@@ -801,7 +801,7 @@ static void perf_ibs_get_mem_lvl(union ibs_op_data2 *op_data2,
/* Extension Memory */
if (ibs_caps & IBS_CAPS_ZEN4 &&
ibs_data_src == IBS_DATA_SRC_EXT_EXT_MEM) {
- data_src->mem_lvl_num = PERF_MEM_LVLNUM_EXTN_MEM;
+ data_src->mem_lvl_num = PERF_MEM_LVLNUM_CXL;
if (op_data2->rmt_node) {
data_src->mem_remote = PERF_MEM_REMOTE_REMOTE;
/* IBS doesn't provide Remote socket detail */
diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index 85be78e..ccb7f5d 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -1337,7 +1337,7 @@ union perf_mem_data_src {
#define PERF_MEM_LVLNUM_L3 0x03 /* L3 */
#define PERF_MEM_LVLNUM_L4 0x04 /* L4 */
/* 5-0x8 available */
-#define PERF_MEM_LVLNUM_EXTN_MEM 0x09 /* Extension memory */
+#define PERF_MEM_LVLNUM_CXL 0x09 /* CXL */
#define PERF_MEM_LVLNUM_IO 0x0a /* I/O */
#define PERF_MEM_LVLNUM_ANY_CACHE 0x0b /* Any cache */
#define PERF_MEM_LVLNUM_LFB 0x0c /* LFB */