[PATCH v3 6/6] perf lock contention: Filter events before creating lock_stat

[Ian Rogers]

In report_lock_contention_begin_event(), lock_stat_findnew() was
called before checking filters.nr_types (-Y), filters.nr_addrs (-L),
and match_callstack_filter() (-S).

When a filtered-out lock event (such as a mutex contention when
running with -Y spinlock) shared the same aggregation key as a
subsequent matching event (for example, contention on the mutex's
internal wait_lock spinlock from the same caller), the first event
created the lock_stat entry with the filtered-out lock's flags and the
second event reused that entry, causing non-spinlock entries to appear
in -Y spinlock output.

Check the type, address, and callstack filters before looking up or
creating the lock_stat entry.

Fixes: b4a7eff93c39 ("perf lock contention: Add -Y/--type-filter option")
Signed-off-by: Ian Rogers <irog...@google.com>
---
 tools/perf/builtin-lock.c | 61 +++++++++++++++++++++------------------
 1 file changed, 33 insertions(+), 28 deletions(-)

diff --git a/tools/perf/builtin-lock.c b/tools/perf/builtin-lock.c
index feaf1cfd08cf..a031709a2cc2 100644
--- a/tools/perf/builtin-lock.c
+++ b/tools/perf/builtin-lock.c
@@ -975,6 +975,7 @@ static int report_lock_contention_begin_event(struct 
perf_sample *sample)
        struct machine *machine = &session->machines.host;
        struct map *kmap;
        struct symbol *sym;
+       u64 *callstack = NULL;
 
        ret = get_key_by_aggr_mode(&key, addr, sample);
        if (ret < 0)
@@ -1010,33 +1011,6 @@ static int report_lock_contention_begin_event(struct 
perf_sample *sample)
                }
        }
 
-       ls = lock_stat_find(key);
-       if (!ls) {
-               char buf[128];
-               const char *name = "";
-
-               switch (aggr_mode) {
-               case LOCK_AGGR_ADDR:
-                       sym = machine__find_kernel_symbol(machine, key, &kmap);
-                       if (sym)
-                               name = sym->name;
-                       break;
-               case LOCK_AGGR_CALLER:
-                       name = buf;
-                       if (lock_contention_caller(sample, buf, sizeof(buf)) < 
0)
-                               name = "Unknown";
-                       break;
-               case LOCK_AGGR_CGROUP:
-               case LOCK_AGGR_TASK:
-               default:
-                       break;
-               }
-
-               ls = lock_stat_findnew(key, name, flags);
-               if (!ls)
-                       return -ENOMEM;
-       }
-
        if (filters.nr_types) {
                bool found = false;
 
@@ -1066,7 +1040,7 @@ static int report_lock_contention_begin_event(struct 
perf_sample *sample)
        }
 
        if (needs_callstack()) {
-               u64 *callstack = get_callstack(sample, max_stack_depth);
+               callstack = get_callstack(sample, max_stack_depth);
                if (callstack == NULL)
                        return 0;
 
@@ -1074,7 +1048,38 @@ static int report_lock_contention_begin_event(struct 
perf_sample *sample)
                        free(callstack);
                        return 0;
                }
+       }
+
+       ls = lock_stat_find(key);
+       if (!ls) {
+               char buf[128];
+               const char *name = "";
+
+               switch (aggr_mode) {
+               case LOCK_AGGR_ADDR:
+                       sym = machine__find_kernel_symbol(machine, key, &kmap);
+                       if (sym)
+                               name = sym->name;
+                       break;
+               case LOCK_AGGR_CALLER:
+                       name = buf;
+                       if (lock_contention_caller(sample, buf, sizeof(buf)) < 
0)
+                               name = "Unknown";
+                       break;
+               case LOCK_AGGR_CGROUP:
+               case LOCK_AGGR_TASK:
+               default:
+                       break;
+               }
+
+               ls = lock_stat_findnew(key, name, flags);
+               if (!ls) {
+                       free(callstack);
+                       return -ENOMEM;
+               }
+       }
 
+       if (callstack) {
                if (ls->callstack == NULL)
                        ls->callstack = callstack;
                else
-- 
2.56.0.212.g129ede20d9-goog