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@google.com>
Cc: Adrian Hunter <adrian.hunter@intel.com>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Athira Rajeev <atrajeev@linux.vnet.ibm.com>
Cc: Bill Wendling <morbo@google.com>
Cc: Chaitanya S Prakash <chaitanyas.prakash@arm.com>
Cc: Fei Lang <langfei@huawei.com>
Cc: Howard Chu <howardchu95@gmail.com>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: James Clark <james.clark@linaro.org>
Cc: Jiri Olsa <jolsa@kernel.org>
Cc: Justin Stitt <justinstitt@google.com>
Cc: Kan Liang <kan.liang@linux.intel.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Namhyung Kim <namhyung@kernel.org>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Nick Desaulniers <nick.desaulniers+lkml@gmail.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephen Brennan <stephen.s.brennan@oracle.com>
Link: https://lore.kernel.org/r/20250519224645.1810891-3-irogers@google.com
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>

diff --git a/tools/perf/util/comm.c b/tools/perf/util/comm.c
index 8aa456d7c2cd2d74f575e61594098cb571199370..9880247a2c3364cbefef3fc3e99d40885eb3c9f1 100644 (file)
--- 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 415c0e5d1e751a47b16ae0b622aae2e0bac1f362..c202b98b36c292157b47e79136d66ef6d17f9467 100644 (file)
--- 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 cd574a896418ac9497a8544c8417c3aa6fa86ae6..56e08c8ae005e82b4a934505ea25f27fab9bae67 100644 (file)
--- 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)