From 7900938850645ed41770bddba524416f8c84dc2d Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Tue, 1 Apr 2025 13:27:15 -0700 Subject: perf trace: Add missing thread__put() in thread__e_machine() Add missing thread__put() of the found parent thread in thread__e_machine(). Reviewed-by: Howard Chu Signed-off-by: Ian Rogers Cc: Adrian Hunter Cc: Alexander Shishkin Cc: Ingo Molnar Cc: Jiri Olsa Cc: Kan Liang Cc: Mark Rutland Cc: Namhyung Kim Cc: Peter Zijlstra Link: https://lore.kernel.org/r/20250401202715.3493567-1-irogers@google.com [ split from a larger patch ] Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/thread.c | 1 + 1 file changed, 1 insertion(+) (limited to 'tools/perf/util/thread.c') diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c index 89585f53c1d5..415c0e5d1e75 100644 --- a/tools/perf/util/thread.c +++ b/tools/perf/util/thread.c @@ -471,6 +471,7 @@ uint16_t thread__e_machine(struct thread *thread, struct machine *machine) if (parent) { e_machine = thread__e_machine(parent, machine); + thread__put(parent); thread__set_e_machine(thread, e_machine); return e_machine; } -- cgit v1.2.3 From 8f454c95817d15ee529d58389612ea4b34f5ffb3 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Mon, 19 May 2025 15:46:45 -0700 Subject: perf thread: Ensure comm_lock held for comm_list Add thread safety annotations for comm_list and add locking for two instances where the list is accessed without the lock held (in contradiction to ____thread__set_comm()). Signed-off-by: Ian Rogers Cc: Adrian Hunter Cc: Alexander Shishkin Cc: Athira Rajeev Cc: Bill Wendling Cc: Chaitanya S Prakash Cc: Fei Lang Cc: Howard Chu Cc: Ingo Molnar Cc: James Clark Cc: Jiri Olsa Cc: Justin Stitt Cc: Kan Liang Cc: Mark Rutland Cc: Namhyung Kim Cc: Nathan Chancellor Cc: Nick Desaulniers Cc: Peter Zijlstra Cc: Stephen Brennan Link: https://lore.kernel.org/r/20250519224645.1810891-3-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/comm.c | 2 ++ tools/perf/util/thread.c | 17 +++++++++++++---- tools/perf/util/thread.h | 9 +++++---- 3 files changed, 20 insertions(+), 8 deletions(-) (limited to 'tools/perf/util/thread.c') diff --git a/tools/perf/util/comm.c b/tools/perf/util/comm.c index 8aa456d7c2cd..9880247a2c33 100644 --- a/tools/perf/util/comm.c +++ b/tools/perf/util/comm.c @@ -24,6 +24,7 @@ static struct comm_strs { static void comm_strs__remove_if_last(struct comm_str *cs); static void comm_strs__init(void) + NO_THREAD_SAFETY_ANALYSIS /* Inherently single threaded due to pthread_once. */ { init_rwsem(&_comm_strs.lock); _comm_strs.capacity = 16; @@ -119,6 +120,7 @@ static void comm_strs__remove_if_last(struct comm_str *cs) } static struct comm_str *__comm_strs__find(struct comm_strs *comm_strs, const char *str) + SHARED_LOCKS_REQUIRED(comm_strs->lock) { struct comm_str **result; diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c index 415c0e5d1e75..c202b98b36c2 100644 --- a/tools/perf/util/thread.c +++ b/tools/perf/util/thread.c @@ -41,6 +41,7 @@ int thread__init_maps(struct thread *thread, struct machine *machine) } struct thread *thread__new(pid_t pid, pid_t tid) + NO_THREAD_SAFETY_ANALYSIS /* Allocation/creation is inherently single threaded. */ { RC_STRUCT(thread) *_thread = zalloc(sizeof(*_thread)); struct thread *thread; @@ -202,22 +203,29 @@ int thread__set_namespaces(struct thread *thread, u64 timestamp, struct comm *thread__comm(struct thread *thread) { - if (list_empty(thread__comm_list(thread))) - return NULL; + struct comm *res = NULL; - return list_first_entry(thread__comm_list(thread), struct comm, list); + down_read(thread__comm_lock(thread)); + if (!list_empty(thread__comm_list(thread))) + res = list_first_entry(thread__comm_list(thread), struct comm, list); + up_read(thread__comm_lock(thread)); + return res; } struct comm *thread__exec_comm(struct thread *thread) { struct comm *comm, *last = NULL, *second_last = NULL; + down_read(thread__comm_lock(thread)); list_for_each_entry(comm, thread__comm_list(thread), list) { - if (comm->exec) + if (comm->exec) { + up_read(thread__comm_lock(thread)); return comm; + } second_last = last; last = comm; } + up_read(thread__comm_lock(thread)); /* * 'last' with no start time might be the parent's comm of a synthesized @@ -233,6 +241,7 @@ struct comm *thread__exec_comm(struct thread *thread) static int ____thread__set_comm(struct thread *thread, const char *str, u64 timestamp, bool exec) + EXCLUSIVE_LOCKS_REQUIRED(thread__comm_lock(thread)) { struct comm *new, *curr = thread__comm(thread); diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h index cd574a896418..56e08c8ae005 100644 --- a/tools/perf/util/thread.h +++ b/tools/perf/util/thread.h @@ -236,14 +236,15 @@ static inline struct rw_semaphore *thread__namespaces_lock(struct thread *thread return &RC_CHK_ACCESS(thread)->namespaces_lock; } -static inline struct list_head *thread__comm_list(struct thread *thread) +static inline struct rw_semaphore *thread__comm_lock(struct thread *thread) { - return &RC_CHK_ACCESS(thread)->comm_list; + return &RC_CHK_ACCESS(thread)->comm_lock; } -static inline struct rw_semaphore *thread__comm_lock(struct thread *thread) +static inline struct list_head *thread__comm_list(struct thread *thread) + SHARED_LOCKS_REQUIRED(thread__comm_lock(thread)) { - return &RC_CHK_ACCESS(thread)->comm_lock; + return &RC_CHK_ACCESS(thread)->comm_list; } static inline u64 thread__db_id(const struct thread *thread) -- cgit v1.2.3 From 24bcc31fc75b760e7813e1c8257a7439e045e8ad Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Wed, 28 May 2025 12:53:08 -0300 Subject: Revert "perf thread: Ensure comm_lock held for comm_list" This reverts commit 8f454c95817d15ee529d58389612ea4b34f5ffb3. 'perf top' is freezing on exit sometimes, bisected to this one, revert. Cc: Adrian Hunter Cc: Alexander Shishkin Cc: Athira Rajeev Cc: Bill Wendling Cc: Chaitanya S Prakash Cc: Fei Lang Cc: Howard Chu Cc: Ian Rogers Cc: Ingo Molnar Cc: James Clark Cc: Jiri Olsa Cc: Justin Stitt Cc: Kan Liang Cc: Mark Rutland Cc: Namhyung Kim Cc: Nathan Chancellor Cc: Nick Desaulniers Cc: Peter Zijlstra Cc: Stephen Brennan Link: https://lore.kernel.org/r/aDcyvvOKZkRYbjul@x1 Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/comm.c | 2 -- tools/perf/util/thread.c | 17 ++++------------- tools/perf/util/thread.h | 9 ++++----- 3 files changed, 8 insertions(+), 20 deletions(-) (limited to 'tools/perf/util/thread.c') diff --git a/tools/perf/util/comm.c b/tools/perf/util/comm.c index 9880247a2c33..8aa456d7c2cd 100644 --- a/tools/perf/util/comm.c +++ b/tools/perf/util/comm.c @@ -24,7 +24,6 @@ static struct comm_strs { static void comm_strs__remove_if_last(struct comm_str *cs); static void comm_strs__init(void) - NO_THREAD_SAFETY_ANALYSIS /* Inherently single threaded due to pthread_once. */ { init_rwsem(&_comm_strs.lock); _comm_strs.capacity = 16; @@ -120,7 +119,6 @@ static void comm_strs__remove_if_last(struct comm_str *cs) } static struct comm_str *__comm_strs__find(struct comm_strs *comm_strs, const char *str) - SHARED_LOCKS_REQUIRED(comm_strs->lock) { struct comm_str **result; diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c index c202b98b36c2..415c0e5d1e75 100644 --- a/tools/perf/util/thread.c +++ b/tools/perf/util/thread.c @@ -41,7 +41,6 @@ int thread__init_maps(struct thread *thread, struct machine *machine) } struct thread *thread__new(pid_t pid, pid_t tid) - NO_THREAD_SAFETY_ANALYSIS /* Allocation/creation is inherently single threaded. */ { RC_STRUCT(thread) *_thread = zalloc(sizeof(*_thread)); struct thread *thread; @@ -203,29 +202,22 @@ int thread__set_namespaces(struct thread *thread, u64 timestamp, struct comm *thread__comm(struct thread *thread) { - struct comm *res = NULL; + if (list_empty(thread__comm_list(thread))) + return NULL; - down_read(thread__comm_lock(thread)); - if (!list_empty(thread__comm_list(thread))) - res = list_first_entry(thread__comm_list(thread), struct comm, list); - up_read(thread__comm_lock(thread)); - return res; + return list_first_entry(thread__comm_list(thread), struct comm, list); } struct comm *thread__exec_comm(struct thread *thread) { struct comm *comm, *last = NULL, *second_last = NULL; - down_read(thread__comm_lock(thread)); list_for_each_entry(comm, thread__comm_list(thread), list) { - if (comm->exec) { - up_read(thread__comm_lock(thread)); + if (comm->exec) return comm; - } second_last = last; last = comm; } - up_read(thread__comm_lock(thread)); /* * 'last' with no start time might be the parent's comm of a synthesized @@ -241,7 +233,6 @@ struct comm *thread__exec_comm(struct thread *thread) static int ____thread__set_comm(struct thread *thread, const char *str, u64 timestamp, bool exec) - EXCLUSIVE_LOCKS_REQUIRED(thread__comm_lock(thread)) { struct comm *new, *curr = thread__comm(thread); diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h index 56e08c8ae005..cd574a896418 100644 --- a/tools/perf/util/thread.h +++ b/tools/perf/util/thread.h @@ -236,15 +236,14 @@ static inline struct rw_semaphore *thread__namespaces_lock(struct thread *thread return &RC_CHK_ACCESS(thread)->namespaces_lock; } -static inline struct rw_semaphore *thread__comm_lock(struct thread *thread) +static inline struct list_head *thread__comm_list(struct thread *thread) { - return &RC_CHK_ACCESS(thread)->comm_lock; + return &RC_CHK_ACCESS(thread)->comm_list; } -static inline struct list_head *thread__comm_list(struct thread *thread) - SHARED_LOCKS_REQUIRED(thread__comm_lock(thread)) +static inline struct rw_semaphore *thread__comm_lock(struct thread *thread) { - return &RC_CHK_ACCESS(thread)->comm_list; + return &RC_CHK_ACCESS(thread)->comm_lock; } static inline u64 thread__db_id(const struct thread *thread) -- cgit v1.2.3 From a913ef6fd883c05bd6538ed21ee1e773f0d750b7 Mon Sep 17 00:00:00 2001 From: Ian Rogers Date: Wed, 28 May 2025 21:39:37 -0700 Subject: perf callchain: Always populate the addr_location map when adding IP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dropping symbols also meant the callchain maps wasn't populated, but the callchain map is needed to find the DSO. Plumb the symbols option better, falling back to thread__find_map() rather than thread__find_symbol() when symbols are disabled. Fixes: 02b2705017d2e5ad ("perf callchain: Allow symbols to be optional when resolving a callchain") Signed-off-by: Ian Rogers Cc: Adrian Hunter Cc: Alexander Shishkin Cc: Andi Kleen Cc: Athira Rajeev Cc: Ben Gainey Cc: Chaitanya S Prakash Cc: Charlie Jenkins Cc: Chun-Tse Shao Cc: Colin Ian King Cc: Dmitriy Vyukov Cc: Dr. David Alan Gilbert Cc: Graham Woodward Cc: Howard Chu Cc: Ilkka Koskinen Cc: Ingo Molnar Cc: James Clark Cc: Jiri Olsa Cc: John Garry Cc: Kajol Jain Cc: Kan Liang Cc: Krzysztof Łopatowski Cc: Leo Yan Cc: Levi Yun Cc: Li Huafei Cc: linux-arm-kernel@lists.infradead.org Cc: Mark Rutland Cc: Martin Liška Cc: Masami Hiramatsu Cc: Matt Fleming Cc: Mike Leach Cc: Namhyung Kim Cc: Peter Zijlstra Cc: Ravi Bangoria Cc: Song Liu Cc: Steinar H. Gunderson Cc: Stephen Brennan Cc: Steve Clevenger Cc: Thomas Falcon Cc: Veronika Molnarova Cc: Weilin Wang Cc: Will Deacon Cc: Yicong Yang Cc: Yujie Liu Cc: Zhongqiu Han Cc: Zixian Cai Link: https://lore.kernel.org/r/20250529044000.759937-2-irogers@google.com Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/util/machine.c | 6 ++++-- tools/perf/util/thread.c | 8 ++++++-- tools/perf/util/thread.h | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) (limited to 'tools/perf/util/thread.c') diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index c5e28d15323f..7ec12c207970 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -2011,7 +2011,7 @@ static void ip__resolve_ams(struct thread *thread, * Thus, we have to try consecutively until we find a match * or else, the symbol is unknown */ - thread__find_cpumode_addr_location(thread, ip, &al); + thread__find_cpumode_addr_location(thread, ip, /*symbols=*/true, &al); ams->addr = ip; ams->al_addr = al.addr; @@ -2113,7 +2113,7 @@ static int add_callchain_ip(struct thread *thread, al.sym = NULL; al.srcline = NULL; if (!cpumode) { - thread__find_cpumode_addr_location(thread, ip, &al); + thread__find_cpumode_addr_location(thread, ip, symbols, &al); } else { if (ip >= PERF_CONTEXT_MAX) { switch (ip) { @@ -2141,6 +2141,8 @@ static int add_callchain_ip(struct thread *thread, } if (symbols) thread__find_symbol(thread, *cpumode, ip, &al); + else + thread__find_map(thread, *cpumode, ip, &al); } if (al.sym != NULL) { diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c index 415c0e5d1e75..ffb48cc2103f 100644 --- a/tools/perf/util/thread.c +++ b/tools/perf/util/thread.c @@ -410,7 +410,7 @@ int thread__fork(struct thread *thread, struct thread *parent, u64 timestamp, bo } void thread__find_cpumode_addr_location(struct thread *thread, u64 addr, - struct addr_location *al) + bool symbols, struct addr_location *al) { size_t i; const u8 cpumodes[] = { @@ -421,7 +421,11 @@ void thread__find_cpumode_addr_location(struct thread *thread, u64 addr, }; for (i = 0; i < ARRAY_SIZE(cpumodes); i++) { - thread__find_symbol(thread, cpumodes[i], addr, al); + if (symbols) + thread__find_symbol(thread, cpumodes[i], addr, al); + else + thread__find_map(thread, cpumodes[i], addr, al); + if (al->map) break; } diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h index cd574a896418..2b90bbed7a61 100644 --- a/tools/perf/util/thread.h +++ b/tools/perf/util/thread.h @@ -126,7 +126,7 @@ struct symbol *thread__find_symbol_fb(struct thread *thread, u8 cpumode, u64 addr, struct addr_location *al); void thread__find_cpumode_addr_location(struct thread *thread, u64 addr, - struct addr_location *al); + bool symbols, struct addr_location *al); int thread__memcpy(struct thread *thread, struct machine *machine, void *buf, u64 ip, int len, bool *is64bit); -- cgit v1.2.3