Re: [PATCH] perf/x86/ibs: Set data_src.mem_lvl_num as well

[Ravi Bangoria]

Hi Namhyung,

> @@ -748,12 +750,14 @@ static void perf_ibs_get_mem_lvl(union ibs_op_data2 *op_data2,
>  	if (ibs_caps & IBS_CAPS_ZEN4) {
>  		if (ibs_data_src == IBS_DATA_SRC_EXT_LOC_CACHE) {
>  			data_src->mem_lvl = PERF_MEM_LVL_L3 | PERF_MEM_LVL_HIT;
> +			data_src->mem_lvl_num = PERF_MEM_LVLNUM_L3;
>  			return;
>  		}
>  	} else {
>  		if (ibs_data_src == IBS_DATA_SRC_LOC_CACHE) {
>  			data_src->mem_lvl = PERF_MEM_LVL_L3 | PERF_MEM_LVL_REM_CCE1 |
>  					    PERF_MEM_LVL_HIT;
> +			data_src->mem_lvl_num = PERF_MEM_LVLNUM_L3;

mem_lvl_num does not have option to set multiple sources. Setting just
PERF_MEM_LVLNUM_L3 is bit misleading here. Documentation (PPR 55898 Rev
0.70 - Oct 14, 2022) says:

 "data returned from shared L3, other L2 on same CCX or other core's
  cache trough same node."

As per my knowledge, "shared L3" and "other L2 on same CCX" has similar
latency. But request need to go through DF for "other core's cache trough
same node" which incurs higher latency. Thus, setting both is important.
This was one of the reason to not use mem_lvl_num in IBS code.

2nd reason was, perf c2c (c2c_decode_stats()) does not use mem_lvl_num.

3rd reason was, perf mem sorting logic (sort__lvl_cmp()) does not consider
mem_lvl_num.

4th one was, if I set both mem_lvl and mem_lvl_num, like what other archs
do, `perf mem report` prints both, which is kind of ugly:

          464029  N/A
          340728  L1 or L1 hit
            8312  LFB/MAB or LFB/MAB hit
            7901  L2 or L2 hit
             123  L3 or Remote Cache (1 hop) or L3 hit

Without mem_lvl_num it's much cleaner:

          330057  N/A
          229646  L1 hit
            5842  L2 hit
            5726  LFB/MAB hit
              78  L3 or Remote Cache (1 hop) hit

I think we should clean this before applying this patch? Other option is
to add bpf filter support for mem_lvl. What do you think?

Thanks,
Ravi