Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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>
Link: https://lore.kernel.org/r/20250529192206.971199-1-irogers@google.com
Signed-off-by: Namhyung Kim <namhyung@kernel.org>

authored by

Ian Rogers and committed by
Namhyung Kim
ae075693 11904107

+30 -9
+2
tools/perf/util/comm.c
··· 24 24 static void comm_strs__remove_if_last(struct comm_str *cs); 25 25 26 26 static void comm_strs__init(void) 27 + NO_THREAD_SAFETY_ANALYSIS /* Inherently single threaded due to pthread_once. */ 27 28 { 28 29 init_rwsem(&_comm_strs.lock); 29 30 _comm_strs.capacity = 16; ··· 120 119 } 121 120 122 121 static struct comm_str *__comm_strs__find(struct comm_strs *comm_strs, const char *str) 122 + SHARED_LOCKS_REQUIRED(comm_strs->lock) 123 123 { 124 124 struct comm_str **result; 125 125
+22 -4
tools/perf/util/thread.c
··· 41 41 } 42 42 43 43 struct thread *thread__new(pid_t pid, pid_t tid) 44 + NO_THREAD_SAFETY_ANALYSIS /* Allocation/creation is inherently single threaded. */ 44 45 { 45 46 RC_STRUCT(thread) *_thread = zalloc(sizeof(*_thread)); 46 47 struct thread *thread; ··· 201 200 return ret; 202 201 } 203 202 204 - struct comm *thread__comm(struct thread *thread) 203 + static struct comm *__thread__comm(struct thread *thread) 204 + SHARED_LOCKS_REQUIRED(thread__comm_lock(thread)) 205 205 { 206 206 if (list_empty(thread__comm_list(thread))) 207 207 return NULL; ··· 210 208 return list_first_entry(thread__comm_list(thread), struct comm, list); 211 209 } 212 210 211 + struct comm *thread__comm(struct thread *thread) 212 + { 213 + struct comm *res = NULL; 214 + 215 + down_read(thread__comm_lock(thread)); 216 + res = __thread__comm(thread); 217 + up_read(thread__comm_lock(thread)); 218 + return res; 219 + } 220 + 213 221 struct comm *thread__exec_comm(struct thread *thread) 214 222 { 215 223 struct comm *comm, *last = NULL, *second_last = NULL; 216 224 225 + down_read(thread__comm_lock(thread)); 217 226 list_for_each_entry(comm, thread__comm_list(thread), list) { 218 - if (comm->exec) 227 + if (comm->exec) { 228 + up_read(thread__comm_lock(thread)); 219 229 return comm; 230 + } 220 231 second_last = last; 221 232 last = comm; 222 233 } 234 + up_read(thread__comm_lock(thread)); 223 235 224 236 /* 225 237 * 'last' with no start time might be the parent's comm of a synthesized ··· 249 233 250 234 static int ____thread__set_comm(struct thread *thread, const char *str, 251 235 u64 timestamp, bool exec) 236 + EXCLUSIVE_LOCKS_REQUIRED(thread__comm_lock(thread)) 252 237 { 253 - struct comm *new, *curr = thread__comm(thread); 238 + struct comm *new, *curr = __thread__comm(thread); 254 239 255 240 /* Override the default :tid entry */ 256 241 if (!thread__comm_set(thread)) { ··· 302 285 } 303 286 304 287 static const char *__thread__comm_str(struct thread *thread) 288 + SHARED_LOCKS_REQUIRED(thread__comm_lock(thread)) 305 289 { 306 - const struct comm *comm = thread__comm(thread); 290 + const struct comm *comm = __thread__comm(thread); 307 291 308 292 if (!comm) 309 293 return NULL;
+6 -5
tools/perf/util/thread.h
··· 236 236 return &RC_CHK_ACCESS(thread)->namespaces_lock; 237 237 } 238 238 239 - static inline struct list_head *thread__comm_list(struct thread *thread) 240 - { 241 - return &RC_CHK_ACCESS(thread)->comm_list; 242 - } 243 - 244 239 static inline struct rw_semaphore *thread__comm_lock(struct thread *thread) 245 240 { 246 241 return &RC_CHK_ACCESS(thread)->comm_lock; 242 + } 243 + 244 + static inline struct list_head *thread__comm_list(struct thread *thread) 245 + SHARED_LOCKS_REQUIRED(thread__comm_lock(thread)) 246 + { 247 + return &RC_CHK_ACCESS(thread)->comm_list; 247 248 } 248 249 249 250 static inline u64 thread__db_id(const struct thread *thread)