perf mem: Refactor perf_mem__lvl_scnprintf() to process 'union perf_mem_data_src...
authorRavi Bangoria <ravi.bangoria@amd.com>
Fri, 7 Apr 2023 11:24:57 +0000 (16:54 +0530)
committerArnaldo Carvalho de Melo <acme@redhat.com>
Mon, 10 Apr 2023 22:27:00 +0000 (19:27 -0300)
Interpretation of 'union perf_mem_data_src' by perf_mem__lvl_scnprintf()
is non-intuitive. For ex, it ignores 'mem_lvl' when 'mem_hops' is set
but considers it otherwise. It prints both 'mem_lvl_num' and 'mem_lvl'
when 'mem_hops' is not set.

Refactor this function such that it behaves more intuitively: Use new
API 'mem_lvl_num'|'mem_remote'|'mem_hops' if 'mem_lvl_num' contains
value other than PERF_MEM_LVLNUM_NA. Otherwise, fallback to old API
'mem_lvl'.  Since new API has no way to indicate MISS, use it from old
api, otherwise don't club old and new APIs while parsing as well as
printing.

Before:

  $ sudo ./perf mem report -F sample,mem --stdio
  #      Samples  Memory access
  # ............  ........................
  #
          250097  N/A
          188907  L1 hit
            4116  L2 hit
            3496  Remote Cache (1 hop) hit
            3271  Remote Cache (2 hops) hit
             873  L3 hit
             598  Local RAM hit
             438  Remote RAM (1 hop) hit
               1  Uncached hit

After:

  $ sudo ./perf mem report -F sample,mem --stdio
  #      Samples  Memory access
  # ............  .......................................
  #
          255517  N/A
          189989  L1 hit
            4541  L2 hit
            3363  Remote core, same node Any cache hit
            3336  Remote node, same socket Any cache hit
            1275  L3 hit
             743  RAM hit
             545  Remote node, same socket RAM hit
               4  Uncached hit

Signed-off-by: Ravi Bangoria <ravi.bangoria@amd.com>
Acked-by: Namhyung Kim <namhyung@kernel.org>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Ananth Narayan <ananth.narayan@amd.com>
Cc: Ian Rogers <irogers@google.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Kajol Jain <kjain@linux.ibm.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Leo Yan <leo.yan@linaro.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Sandipan Das <sandipan.das@amd.com>
Cc: Santosh Shukla <santosh.shukla@amd.com>
Cc: Stephane Eranian <eranian@google.com>
Link: https://lore.kernel.org/r/20230407112459.548-5-ravi.bangoria@amd.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
tools/perf/util/mem-events.c

index 3a7c72b..ed1ee4b 100644 (file)
@@ -344,66 +344,71 @@ static int perf_mem__op_scnprintf(char *out, size_t sz, struct mem_info *mem_inf
 
 int perf_mem__lvl_scnprintf(char *out, size_t sz, struct mem_info *mem_info)
 {
-       size_t i, l = 0;
-       u64 m =  PERF_MEM_LVL_NA;
-       u64 hit, miss;
+       union perf_mem_data_src data_src;
        int printed = 0;
-
-       if (mem_info)
-               m  = mem_info->data_src.mem_lvl;
+       size_t l = 0;
+       size_t i;
+       int lvl;
+       char hit_miss[5] = {0};
 
        sz -= 1; /* -1 for null termination */
        out[0] = '\0';
 
-       hit = m & PERF_MEM_LVL_HIT;
-       miss = m & PERF_MEM_LVL_MISS;
+       if (!mem_info)
+               goto na;
 
-       /* already taken care of */
-       m &= ~(PERF_MEM_LVL_HIT|PERF_MEM_LVL_MISS);
+       data_src = mem_info->data_src;
 
-       if (mem_info && mem_info->data_src.mem_remote) {
-               strcat(out, "Remote ");
-               l += 7;
-       }
+       if (data_src.mem_lvl & PERF_MEM_LVL_HIT)
+               memcpy(hit_miss, "hit", 3);
+       else if (data_src.mem_lvl & PERF_MEM_LVL_MISS)
+               memcpy(hit_miss, "miss", 4);
 
-       /*
-        * Incase mem_hops field is set, we can skip printing data source via
-        * PERF_MEM_LVL namespace.
-        */
-       if (mem_info && mem_info->data_src.mem_hops) {
-               l += scnprintf(out + l, sz - l, "%s ", mem_hops[mem_info->data_src.mem_hops]);
-       } else {
-               for (i = 0; m && i < ARRAY_SIZE(mem_lvl); i++, m >>= 1) {
-                       if (!(m & 0x1))
-                               continue;
-                       if (printed++) {
-                               strcat(out, " or ");
-                               l += 4;
-                       }
-                       l += scnprintf(out + l, sz - l, mem_lvl[i]);
+       lvl = data_src.mem_lvl_num;
+       if (lvl && lvl != PERF_MEM_LVLNUM_NA) {
+               if (data_src.mem_remote) {
+                       strcat(out, "Remote ");
+                       l += 7;
                }
+
+               if (data_src.mem_hops)
+                       l += scnprintf(out + l, sz - l, "%s ", mem_hops[data_src.mem_hops]);
+
+               if (mem_lvlnum[lvl])
+                       l += scnprintf(out + l, sz - l, mem_lvlnum[lvl]);
+               else
+                       l += scnprintf(out + l, sz - l, "L%d", lvl);
+
+               l += scnprintf(out + l, sz - l, " %s", hit_miss);
+               return l;
        }
 
-       if (mem_info && mem_info->data_src.mem_lvl_num) {
-               int lvl = mem_info->data_src.mem_lvl_num;
+       lvl = data_src.mem_lvl;
+       if (!lvl)
+               goto na;
+
+       lvl &= ~(PERF_MEM_LVL_NA | PERF_MEM_LVL_HIT | PERF_MEM_LVL_MISS);
+       if (!lvl)
+               goto na;
+
+       for (i = 0; lvl && i < ARRAY_SIZE(mem_lvl); i++, lvl >>= 1) {
+               if (!(lvl & 0x1))
+                       continue;
                if (printed++) {
                        strcat(out, " or ");
                        l += 4;
                }
-               if (mem_lvlnum[lvl])
-                       l += scnprintf(out + l, sz - l, mem_lvlnum[lvl]);
-               else
-                       l += scnprintf(out + l, sz - l, "L%d", lvl);
+               l += scnprintf(out + l, sz - l, mem_lvl[i]);
        }
 
-       if (l == 0)
-               l += scnprintf(out + l, sz - l, "N/A");
-       if (hit)
-               l += scnprintf(out + l, sz - l, " hit");
-       if (miss)
-               l += scnprintf(out + l, sz - l, " miss");
+       if (printed) {
+               l += scnprintf(out + l, sz - l, " %s", hit_miss);
+               return l;
+       }
 
-       return l;
+na:
+       strcat(out, "N/A");
+       return 3;
 }
 
 static const char * const snoop_access[] = {