[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250519224645.1810891-3-irogers@google.com>
Date: Mon, 19 May 2025 15:46:45 -0700
From: Ian Rogers <irogers@...gle.com>
To: langfei@...wei.com, Peter Zijlstra <peterz@...radead.org>,
Ingo Molnar <mingo@...hat.com>, Arnaldo Carvalho de Melo <acme@...nel.org>, Namhyung Kim <namhyung@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
Ian Rogers <irogers@...gle.com>, Adrian Hunter <adrian.hunter@...el.com>,
Kan Liang <kan.liang@...ux.intel.com>, Nathan Chancellor <nathan@...nel.org>,
Nick Desaulniers <nick.desaulniers+lkml@...il.com>, Bill Wendling <morbo@...gle.com>,
Justin Stitt <justinstitt@...gle.com>, James Clark <james.clark@...aro.org>,
Chaitanya S Prakash <chaitanyas.prakash@....com>, Athira Rajeev <atrajeev@...ux.vnet.ibm.com>,
Stephen Brennan <stephen.s.brennan@...cle.com>, Howard Chu <howardchu95@...il.com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org,
llvm@...ts.linux.dev
Subject: [PATCH v1 3/3] 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 <irogers@...gle.com>
---
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(-)
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)
--
2.49.0.1101.gccaa498523-goog
Powered by blists - more mailing lists