Mutex wrapper, locking and memory leak fixes
Subsystems (3)
Ian Rogers
reviewerv4 cover Mutex wrapper, locking and memory leak fixes
When fixing a locking race and memory leak in:
https://lore.kernel.org/linux-perf-users/[email protected]/
It was requested that debug mutex code be separated out into its own files. This was, in part, done by Pavithra Gurushankar in:
https://lore.kernel.org/lkml/[email protected]/
These patches fix issues with the previous patches, add in the original dso->nsinfo fix and then build on our mutex wrapper with clang's -Wthread-safety analysis. The analysis found missing unlocks in builtin-sched.c which are fixed and -Wthread-safety is enabled by default when building with clang.
v4. Adds a comment for the trylock result, fixes the new line (missed
in v3) and removes two blank lines as suggested by Adrian Hunter.
v3. Adds a missing new line to the error messages and removes the
pshared argument to mutex_init by having two functions, mutex_init and mutex_init_pshared. These changes were suggested by Adrian Hunter.
v2. Breaks apart changes that s/pthread_mutex/mutex/g and the lock
annotations as requested by Arnaldo and Namhyung. A boolean is added to builtin-sched.c to terminate thread funcs rather than leaving them blocked on delted mutexes.
Ian Rogers (17):
perf bench: Update use of pthread mutex/cond
perf tests: Avoid pthread.h inclusion
perf hist: Update use of pthread mutex
perf bpf: Remove unused pthread.h include
perf lock: Remove unused pthread.h include
perf record: Update use of pthread mutex
perf sched: Update use of pthread mutex
perf ui: Update use of pthread mutex
perf mmap: Remove unnecessary pthread.h include
perf dso: Update use of pthread mutex
perf annotate: Update use of pthread mutex
perf top: Update use of pthread mutex
perf dso: Hold lock when accessing nsinfo
perf mutex: Add thread safety annotations
perf sched: Fixes for thread safety analysis
perf top: Fixes for thread safety analysis
perf build: Enable -Wthread-safety with clang
Pavithra Gurushankar (1):
perf mutex: Wrapped usage of mutex and cond
tools/perf/Makefile.config | 5 + tools/perf/bench/epoll-ctl.c | 33 +++--- tools/perf/bench/epoll-wait.c | 33 +++--- tools/perf/bench/futex-hash.c | 33 +++--- tools/perf/bench/futex-lock-pi.c | 33 +++--- tools/perf/bench/futex-requeue.c | 33 +++--- tools/perf/bench/futex-wake-parallel.c | 33 +++--- tools/perf/bench/futex-wake.c | 33 +++--- tools/perf/bench/numa.c | 93 ++++++---------- tools/perf/builtin-inject.c | 4 + tools/perf/builtin-lock.c | 1 - tools/perf/builtin-record.c | 13 ++- tools/perf/builtin-sched.c | 105 +++++++++--------- tools/perf/builtin-top.c | 45 ++++---- tools/perf/tests/mmap-basic.c | 2 - tools/perf/tests/openat-syscall-all-cpus.c | 2 +- tools/perf/tests/perf-record.c | 2 - tools/perf/ui/browser.c | 20 ++-- tools/perf/ui/browsers/annotate.c | 12 +-- tools/perf/ui/setup.c | 5 +- tools/perf/ui/tui/helpline.c | 5 +- tools/perf/ui/tui/progress.c | 8 +- tools/perf/ui/tui/setup.c | 8 +- tools/perf/ui/tui/util.c | 18 ++-- tools/perf/ui/ui.h | 4 +- tools/perf/util/Build | 1 + tools/perf/util/annotate.c | 15 +-- tools/perf/util/annotate.h | 4 +- tools/perf/util/bpf-event.h | 1 - tools/perf/util/build-id.c | 12 ++- tools/perf/util/dso.c | 19 ++-- tools/perf/util/dso.h | 4 +- tools/perf/util/hist.c | 6 +- tools/perf/util/hist.h | 4 +- tools/perf/util/map.c | 3 + tools/perf/util/mmap.h | 1 - tools/perf/util/mutex.c | 119 +++++++++++++++++++++ tools/perf/util/mutex.h | 108 +++++++++++++++++++ tools/perf/util/probe-event.c | 3 + tools/perf/util/symbol.c | 4 +- tools/perf/util/top.h | 5 +- 41 files changed, 569 insertions(+), 323 deletions(-) create mode 100644 tools/perf/util/mutex.c create mode 100644 tools/perf/util/mutex.h
Signature · 1 line
2.37.2.672.g94769d06f0-goog
Ian Rogers
reviewerPATCH v4 1/18 perf mutex: Wrapped usage of mutex and cond
From: Pavithra Gurushankar <[email protected]>
Added a new header file mutex.h that wraps the usage of pthread_mutex_t and pthread_cond_t. By abstracting these it is possible to introduce error checking.
--- tools/perf/util/Build | 1 + tools/perf/util/mutex.c | 117 ++++++++++++++++++++++++++++++++++++++++ tools/perf/util/mutex.h | 48 +++++++++++++++++ 3 files changed, 166 insertions(+) create mode 100644 tools/perf/util/mutex.c create mode 100644 tools/perf/util/mutex.h
tools/perf/util/Build +1 -0
@@ -143,6 +143,7 @@ perf-y += branch.o perf-y += mem2node.o perf-y += clockid.o perf-y += list_sort.o +perf-y += mutex.o perf-$(CONFIG_LIBBPF) += bpf-loader.o perf-$(CONFIG_LIBBPF) += bpf_map.o
tools/perf/util/mutex.c new file +117 -0
117 lines not on this page · raw message
tools/perf/util/mutex.h new file +48 -0
48 lines not on this page · raw message
Signature · 1 line
2.37.2.672.g94769d06f0-goog
Ian Rogers
reviewerPATCH v4 2/18 perf bench: Update use of pthread mutex/cond
In reply to Ian RogersSwitch to the use of mutex wrappers that provide better error checking.
--- tools/perf/bench/epoll-ctl.c | 33 ++++----- tools/perf/bench/epoll-wait.c | 33 ++++----- tools/perf/bench/futex-hash.c | 33 ++++----- tools/perf/bench/futex-lock-pi.c | 33 ++++----- tools/perf/bench/futex-requeue.c | 33 ++++----- tools/perf/bench/futex-wake-parallel.c | 33 ++++----- tools/perf/bench/futex-wake.c | 33 ++++----- tools/perf/bench/numa.c | 93 ++++++++++---------------- 8 files changed, 153 insertions(+), 171 deletions(-)
tools/perf/bench/epoll-ctl.c +17 -16
@@ -23,6 +23,7 @@#include <sys/eventfd.h> #include <perf/cpumap.h> +#include "../util/mutex.h" #include "../util/stat.h" #include <subcmd/parse-options.h> #include "bench.h"
@@ -58,10 +59,10 @@ static unsigned int nested = 0;/* amount of fds to monitor, per thread */ static unsigned int nfds =64 ; -static pthread_mutex_t thread_lock; +static struct mutex thread_lock;static unsigned int threads_starting;static struct stats all_stats[EPOLL_NR_OPS ]; -static pthread_cond_t thread_parent, thread_worker; +static struct cond thread_parent, thread_worker;struct worker {int tid;
@@ -174,12 +175,12 @@ static void *workerfn(void *arg)struct timespec ts = { .tv_sec =0 , .tv_nsec =250 }; -pthread_mutex_lock (&thread_lock); +mutex_lock (&thread_lock); threads_starting--;if (!threads_starting) -pthread_cond_signal (&thread_parent); -pthread_cond_wait (&thread_worker, &thread_lock); -pthread_mutex_unlock (&thread_lock); +cond_signal (&thread_parent); +cond_wait (&thread_worker, &thread_lock); +mutex_unlock (&thread_lock);/* Let 'em loose */ do {
@@ -367,9 +368,9 @@ int bench_epoll_ctl(int argc, const char **argv)for (i =0 ; i <EPOLL_NR_OPS ; i++)init_stats (&all_stats[i]); -pthread_mutex_init (&thread_lock,NULL ); -pthread_cond_init (&thread_parent,NULL ); -pthread_cond_init (&thread_worker,NULL ); +mutex_init (&thread_lock); +cond_init (&thread_parent); +cond_init (&thread_worker); threads_starting = nthreads;
@@ -377,11 +378,11 @@ int bench_epoll_ctl(int argc, const char **argv)do_threads (worker, cpu); -pthread_mutex_lock (&thread_lock); +mutex_lock (&thread_lock);while (threads_starting) -pthread_cond_wait (&thread_parent, &thread_lock); -pthread_cond_broadcast (&thread_worker); -pthread_mutex_unlock (&thread_lock); +cond_wait (&thread_parent, &thread_lock); +cond_broadcast (&thread_worker); +mutex_unlock (&thread_lock);sleep (nsecs);toggle_done (0 ,NULL ,NULL );
@@ -394,9 +395,9 @@ int bench_epoll_ctl(int argc, const char **argv) }/* cleanup & report results */ -pthread_cond_destroy (&thread_parent); -pthread_cond_destroy (&thread_worker); -pthread_mutex_destroy (&thread_lock); +cond_destroy (&thread_parent); +cond_destroy (&thread_worker); +mutex_destroy (&thread_lock);for (i =0 ; i < nthreads; i++) {unsigned long t[EPOLL_NR_OPS ];
tools/perf/bench/epoll-wait.c +17 -16
@@ -79,6 +79,7 @@#include <perf/cpumap.h> #include "../util/stat.h" +#include "../util/mutex.h" #include <subcmd/parse-options.h> #include "bench.h"
@@ -109,10 +110,10 @@ static bool multiq; /* use an epoll instance per thread *//* amount of fds to monitor, per thread */ static unsigned int nfds =64 ; -static pthread_mutex_t thread_lock; +static struct mutex thread_lock;static unsigned int threads_starting;static struct stats throughput_stats; -static pthread_cond_t thread_parent, thread_worker; +static struct cond thread_parent, thread_worker;struct worker {int tid;
@@ -189,12 +190,12 @@ static void *workerfn(void *arg)int to = nonblocking?0 :-1 ;int efd = multiq ? w->epollfd : epollfd; -pthread_mutex_lock (&thread_lock); +mutex_lock (&thread_lock); threads_starting--;if (!threads_starting) -pthread_cond_signal (&thread_parent); -pthread_cond_wait (&thread_worker, &thread_lock); -pthread_mutex_unlock (&thread_lock); +cond_signal (&thread_parent); +cond_wait (&thread_worker, &thread_lock); +mutex_unlock (&thread_lock); do { /*
@@ -485,9 +486,9 @@ int bench_epoll_wait(int argc, const char **argv) getpid(), nthreads, oneshot ? " (EPOLLONESHOT semantics)": "", nfds, nsecs);init_stats (&throughput_stats); -pthread_mutex_init (&thread_lock,NULL ); -pthread_cond_init (&thread_parent,NULL ); -pthread_cond_init (&thread_worker,NULL ); +mutex_init (&thread_lock); +cond_init (&thread_parent); +cond_init (&thread_worker); threads_starting = nthreads;
@@ -495,11 +496,11 @@ int bench_epoll_wait(int argc, const char **argv)do_threads (worker, cpu); -pthread_mutex_lock (&thread_lock); +mutex_lock (&thread_lock);while (threads_starting) -pthread_cond_wait (&thread_parent, &thread_lock); -pthread_cond_broadcast (&thread_worker); -pthread_mutex_unlock (&thread_lock); +cond_wait (&thread_parent, &thread_lock); +cond_broadcast (&thread_worker); +mutex_unlock (&thread_lock);/* * At this point the workers should be blocked waiting for read events
@@ -522,9 +523,9 @@ int bench_epoll_wait(int argc, const char **argv)err (EXIT_FAILURE ,"pthread_join" );/* cleanup & report results */ -pthread_cond_destroy (&thread_parent); -pthread_cond_destroy (&thread_worker); -pthread_mutex_destroy (&thread_lock); +cond_destroy (&thread_parent); +cond_destroy (&thread_worker); +mutex_destroy (&thread_lock);/* sort the array back before reporting */ if (randomize)
tools/perf/bench/futex-hash.c +17 -16
@@ -23,6 +23,7 @@#include <sys/mman.h> #include <perf/cpumap.h> +#include "../util/mutex.h" #include "../util/stat.h" #include <subcmd/parse-options.h> #include "bench.h"
@@ -34,10 +35,10 @@ static bool done = false;static int futex_flag =0 ;struct timeval bench__start, bench__end, bench__runtime; -static pthread_mutex_t thread_lock; +static struct mutex thread_lock;static unsigned int threads_starting;static struct stats throughput_stats; -static pthread_cond_t thread_parent, thread_worker; +static struct cond thread_parent, thread_worker;struct worker {int tid;
@@ -73,12 +74,12 @@ static void *workerfn(void *arg)unsigned int i;unsigned long ops = w->ops;/* avoid cacheline bouncing */ -pthread_mutex_lock (&thread_lock); +mutex_lock (&thread_lock); threads_starting--;if (!threads_starting) -pthread_cond_signal (&thread_parent); -pthread_cond_wait (&thread_worker, &thread_lock); -pthread_mutex_unlock (&thread_lock); +cond_signal (&thread_parent); +cond_wait (&thread_worker, &thread_lock); +mutex_unlock (&thread_lock); do { for (i = 0; i < params.nfutexes; i++, ops++) {
@@ -165,9 +166,9 @@ int bench_futex_hash(int argc, const char **argv) getpid(), params.nthreads, params.nfutexes, params.fshared ? "shared":"private", params.runtime);init_stats (&throughput_stats); -pthread_mutex_init (&thread_lock,NULL ); -pthread_cond_init (&thread_parent,NULL ); -pthread_cond_init (&thread_worker,NULL ); +mutex_init (&thread_lock); +cond_init (&thread_parent); +cond_init (&thread_worker); threads_starting = params.nthreads;pthread_attr_init (&thread_attr);
@@ -203,11 +204,11 @@ int bench_futex_hash(int argc, const char **argv)CPU_FREE (cpuset);pthread_attr_destroy (&thread_attr); -pthread_mutex_lock (&thread_lock); +mutex_lock (&thread_lock);while (threads_starting) -pthread_cond_wait (&thread_parent, &thread_lock); -pthread_cond_broadcast (&thread_worker); -pthread_mutex_unlock (&thread_lock); +cond_wait (&thread_parent, &thread_lock); +cond_broadcast (&thread_worker); +mutex_unlock (&thread_lock);sleep (params.runtime);toggle_done (0 ,NULL ,NULL );
@@ -219,9 +220,9 @@ int bench_futex_hash(int argc, const char **argv) }/* cleanup & report results */ -pthread_cond_destroy (&thread_parent); -pthread_cond_destroy (&thread_worker); -pthread_mutex_destroy (&thread_lock); +cond_destroy (&thread_parent); +cond_destroy (&thread_worker); +mutex_destroy (&thread_lock);for (i =0 ; i < params.nthreads; i++) { unsigned long t = bench__runtime.tv_sec > 0 ?
tools/perf/bench/futex-lock-pi.c +17 -16
@@ -8,6 +8,7 @@#include <pthread.h> #include <signal.h> +#include "../util/mutex.h" #include "../util/stat.h" #include <subcmd/parse-options.h> #include <linux/compiler.h>
@@ -34,10 +35,10 @@ static u_int32_t global_futex = 0;static struct worker *worker;static bool done = false;static int futex_flag =0 ; -static pthread_mutex_t thread_lock; +static struct mutex thread_lock;static unsigned int threads_starting;static struct stats throughput_stats; -static pthread_cond_t thread_parent, thread_worker; +static struct cond thread_parent, thread_worker;static struct bench_futex_parameters params = { .runtime =10 ,
@@ -83,12 +84,12 @@ static void *workerfn(void *arg)struct worker *w = (struct worker *) arg;unsigned long ops = w->ops; -pthread_mutex_lock (&thread_lock); +mutex_lock (&thread_lock); threads_starting--;if (!threads_starting) -pthread_cond_signal (&thread_parent); -pthread_cond_wait (&thread_worker, &thread_lock); -pthread_mutex_unlock (&thread_lock); +cond_signal (&thread_parent); +cond_wait (&thread_worker, &thread_lock); +mutex_unlock (&thread_lock); do { int ret;
@@ -197,9 +198,9 @@ int bench_futex_lock_pi(int argc, const char **argv) getpid(), params.nthreads, params.runtime);init_stats (&throughput_stats); -pthread_mutex_init (&thread_lock,NULL ); -pthread_cond_init (&thread_parent,NULL ); -pthread_cond_init (&thread_worker,NULL ); +mutex_init (&thread_lock); +cond_init (&thread_parent); +cond_init (&thread_worker); threads_starting = params.nthreads;pthread_attr_init (&thread_attr);
@@ -208,11 +209,11 @@ int bench_futex_lock_pi(int argc, const char **argv)create_threads (worker, thread_attr, cpu);pthread_attr_destroy (&thread_attr); -pthread_mutex_lock (&thread_lock); +mutex_lock (&thread_lock);while (threads_starting) -pthread_cond_wait (&thread_parent, &thread_lock); -pthread_cond_broadcast (&thread_worker); -pthread_mutex_unlock (&thread_lock); +cond_wait (&thread_parent, &thread_lock); +cond_broadcast (&thread_worker); +mutex_unlock (&thread_lock);sleep (params.runtime);toggle_done (0 ,NULL ,NULL );
@@ -224,9 +225,9 @@ int bench_futex_lock_pi(int argc, const char **argv) }/* cleanup & report results */ -pthread_cond_destroy (&thread_parent); -pthread_cond_destroy (&thread_worker); -pthread_mutex_destroy (&thread_lock); +cond_destroy (&thread_parent); +cond_destroy (&thread_worker); +mutex_destroy (&thread_lock);for (i =0 ; i < params.nthreads; i++) { unsigned long t = bench__runtime.tv_sec > 0 ?
tools/perf/bench/futex-requeue.c +17 -16
@@ -15,6 +15,7 @@#include <pthread.h> #include <signal.h> +#include "../util/mutex.h" #include "../util/stat.h" #include <subcmd/parse-options.h> #include <linux/compiler.h>
@@ -34,8 +35,8 @@ static u_int32_t futex1 = 0, futex2 = 0;static pthread_t *worker;static bool done = false; -static pthread_mutex_t thread_lock; -static pthread_cond_t thread_parent, thread_worker; +static struct mutex thread_lock; +static struct cond thread_parent, thread_worker;static struct stats requeuetime_stats, requeued_stats;static unsigned int threads_starting;static int futex_flag =0 ;
@@ -82,12 +83,12 @@ static void *workerfn(void *arg __maybe_unused) {int ret; -pthread_mutex_lock (&thread_lock); +mutex_lock (&thread_lock); threads_starting--;if (!threads_starting) -pthread_cond_signal (&thread_parent); -pthread_cond_wait (&thread_worker, &thread_lock); -pthread_mutex_unlock (&thread_lock); +cond_signal (&thread_parent); +cond_wait (&thread_worker, &thread_lock); +mutex_unlock (&thread_lock);while (1 ) {if (!params.pi) {
@@ -209,9 +210,9 @@ int bench_futex_requeue(int argc, const char **argv)init_stats (&requeued_stats);init_stats (&requeuetime_stats);pthread_attr_init (&thread_attr); -pthread_mutex_init (&thread_lock,NULL ); -pthread_cond_init (&thread_parent,NULL ); -pthread_cond_init (&thread_worker,NULL ); +mutex_init (&thread_lock); +cond_init (&thread_parent); +cond_init (&thread_worker);for (j =0 ; j < bench_repeat && !done; j++) {unsigned int nrequeued =0 , wakeups =0 ;
@@ -221,11 +222,11 @@ int bench_futex_requeue(int argc, const char **argv)block_threads (worker, thread_attr, cpu);/* make sure all threads are already blocked */ -pthread_mutex_lock (&thread_lock); +mutex_lock (&thread_lock);while (threads_starting) -pthread_cond_wait (&thread_parent, &thread_lock); -pthread_cond_broadcast (&thread_worker); -pthread_mutex_unlock (&thread_lock); +cond_wait (&thread_parent, &thread_lock); +cond_broadcast (&thread_worker); +mutex_unlock (&thread_lock);usleep (100000 );
@@ -297,9 +298,9 @@ int bench_futex_requeue(int argc, const char **argv) }/* cleanup & report results */ -pthread_cond_destroy (&thread_parent); -pthread_cond_destroy (&thread_worker); -pthread_mutex_destroy (&thread_lock); +cond_destroy (&thread_parent); +cond_destroy (&thread_worker); +mutex_destroy (&thread_lock);pthread_attr_destroy (&thread_attr);print_summary ();
tools/perf/bench/futex-wake-parallel.c +17 -16
@@ -10,6 +10,7 @@#include "bench.h" #include <linux/compiler.h> #include "../util/debug.h" +#include "../util/mutex.h" #ifndef HAVE_PTHREAD_BARRIER int bench_futex_wake_parallel (int argc__maybe_unused ,const char **argv__maybe_unused )
@@ -49,8 +50,8 @@ static u_int32_t futex = 0;static pthread_t *blocked_worker;static bool done = false; -static pthread_mutex_t thread_lock; -static pthread_cond_t thread_parent, thread_worker; +static struct mutex thread_lock; +static struct cond thread_parent, thread_worker;static pthread_barrier_t barrier;static struct stats waketime_stats, wakeup_stats;static unsigned int threads_starting;
@@ -125,12 +126,12 @@ static void wakeup_threads(struct thread_data *td, pthread_attr_t thread_attr)static void *blocked_workerfn (void *arg__maybe_unused ) { -pthread_mutex_lock (&thread_lock); +mutex_lock (&thread_lock); threads_starting--;if (!threads_starting) -pthread_cond_signal (&thread_parent); -pthread_cond_wait (&thread_worker, &thread_lock); -pthread_mutex_unlock (&thread_lock); +cond_signal (&thread_parent); +cond_wait (&thread_worker, &thread_lock); +mutex_unlock (&thread_lock);while (1 ) {/* handle spurious wakeups */ if (futex_wait (&futex,0 ,NULL , futex_flag) !=EINTR )
@@ -294,9 +295,9 @@ int bench_futex_wake_parallel(int argc, const char **argv)init_stats (&waketime_stats);pthread_attr_init (&thread_attr); -pthread_mutex_init (&thread_lock,NULL ); -pthread_cond_init (&thread_parent,NULL ); -pthread_cond_init (&thread_worker,NULL ); +mutex_init (&thread_lock); +cond_init (&thread_parent); +cond_init (&thread_worker);for (j =0 ; j < bench_repeat && !done; j++) { waking_worker =calloc (params.nwakes,sizeof (*waking_worker));
@@ -307,11 +308,11 @@ int bench_futex_wake_parallel(int argc, const char **argv)block_threads (blocked_worker, thread_attr, cpu);/* make sure all threads are already blocked */ -pthread_mutex_lock (&thread_lock); +mutex_lock (&thread_lock);while (threads_starting) -pthread_cond_wait (&thread_parent, &thread_lock); -pthread_cond_broadcast (&thread_worker); -pthread_mutex_unlock (&thread_lock); +cond_wait (&thread_parent, &thread_lock); +cond_broadcast (&thread_worker); +mutex_unlock (&thread_lock);usleep (100000 );
@@ -332,9 +333,9 @@ int bench_futex_wake_parallel(int argc, const char **argv) }/* cleanup & report results */ -pthread_cond_destroy (&thread_parent); -pthread_cond_destroy (&thread_worker); -pthread_mutex_destroy (&thread_lock); +cond_destroy (&thread_parent); +cond_destroy (&thread_worker); +mutex_destroy (&thread_lock);pthread_attr_destroy (&thread_attr);print_summary ();
tools/perf/bench/futex-wake.c +17 -16
@@ -14,6 +14,7 @@#include <pthread.h> #include <signal.h> +#include "../util/mutex.h" #include "../util/stat.h" #include <subcmd/parse-options.h> #include <linux/compiler.h>
@@ -34,8 +35,8 @@ static u_int32_t futex1 = 0;static pthread_t *worker;static bool done = false; -static pthread_mutex_t thread_lock; -static pthread_cond_t thread_parent, thread_worker; +static struct mutex thread_lock; +static struct cond thread_parent, thread_worker;static struct stats waketime_stats, wakeup_stats;static unsigned int threads_starting;static int futex_flag =0 ;
@@ -65,12 +66,12 @@ static const char * const bench_futex_wake_usage[] = {static void *workerfn (void *arg__maybe_unused ) { -pthread_mutex_lock (&thread_lock); +mutex_lock (&thread_lock); threads_starting--;if (!threads_starting) -pthread_cond_signal (&thread_parent); -pthread_cond_wait (&thread_worker, &thread_lock); -pthread_mutex_unlock (&thread_lock); +cond_signal (&thread_parent); +cond_wait (&thread_worker, &thread_lock); +mutex_unlock (&thread_lock);while (1 ) {if (futex_wait (&futex1,0 ,NULL , futex_flag) !=EINTR )
@@ -178,9 +179,9 @@ int bench_futex_wake(int argc, const char **argv)init_stats (&wakeup_stats);init_stats (&waketime_stats);pthread_attr_init (&thread_attr); -pthread_mutex_init (&thread_lock,NULL ); -pthread_cond_init (&thread_parent,NULL ); -pthread_cond_init (&thread_worker,NULL ); +mutex_init (&thread_lock); +cond_init (&thread_parent); +cond_init (&thread_worker);for (j =0 ; j < bench_repeat && !done; j++) {unsigned int nwoken =0 ;
@@ -190,11 +191,11 @@ int bench_futex_wake(int argc, const char **argv)block_threads (worker, thread_attr, cpu);/* make sure all threads are already blocked */ -pthread_mutex_lock (&thread_lock); +mutex_lock (&thread_lock);while (threads_starting) -pthread_cond_wait (&thread_parent, &thread_lock); -pthread_cond_broadcast (&thread_worker); -pthread_mutex_unlock (&thread_lock); +cond_wait (&thread_parent, &thread_lock); +cond_broadcast (&thread_worker); +mutex_unlock (&thread_lock);usleep (100000 );
@@ -224,9 +225,9 @@ int bench_futex_wake(int argc, const char **argv) }/* cleanup & report results */ -pthread_cond_destroy (&thread_parent); -pthread_cond_destroy (&thread_worker); -pthread_mutex_destroy (&thread_lock); +cond_destroy (&thread_parent); +cond_destroy (&thread_worker); +mutex_destroy (&thread_lock);pthread_attr_destroy (&thread_attr);print_summary ();
tools/perf/bench/numa.c +34 -59
201 lines not on this page · raw message
Signature · 1 line
2.37.2.672.g94769d06f0-goog
Ian Rogers
reviewerPATCH v4 3/18 perf tests: Avoid pthread.h inclusion
In reply to Ian Rogerspthread.h is being included for the side-effect of getting sched.h and macros like CPU_CLR. Switch to directly using sched.h, or if that is already present, just remove the pthread.h inclusion entirely.
--- tools/perf/tests/mmap-basic.c | 2 -- tools/perf/tests/openat-syscall-all-cpus.c | 2 +- tools/perf/tests/perf-record.c | 2 -- 3 files changed, 1 insertion(+), 5 deletions(-)
tools/perf/tests/mmap-basic.c +0 -2
@@ -1,8 +1,6 @@// SPDX-License-Identifier: GPL-2.0 #include <errno.h> #include <inttypes.h> -/* For the CLR_() macros */ -#include <pthread.h> #include <stdlib.h> #include <perf/cpumap.h>
tools/perf/tests/openat-syscall-all-cpus.c +1 -1
@@ -2,7 +2,7 @@#include <errno.h> #include <inttypes.h> /* For the CPU_* macros */ -#include <pthread.h> +#include <sched.h> #include <sys/types.h> #include <sys/stat.h>
tools/perf/tests/perf-record.c +0 -2
@@ -2,8 +2,6 @@#include <errno.h> #include <inttypes.h> #include <linux/string.h> -/* For the CLR_() macros */ -#include <pthread.h> #include <sched.h> #include <perf/mmap.h>
Signature · 1 line
2.37.2.672.g94769d06f0-goog
Ian Rogers
reviewerPATCH v4 4/18 perf hist: Update use of pthread mutex
In reply to Ian RogersSwitch to the use of mutex wrappers that provide better error checking.
--- tools/perf/builtin-top.c | 8 ++++---- tools/perf/util/hist.c | 6 +++--- tools/perf/util/hist.h | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-)
tools/perf/builtin-top.c +4 -4
@@ -220,7 +220,7 @@ static void perf_top__record_precise_ip(struct perf_top *top,* This function is now called with he->hists->lock held. * Release it before going to sleep. */ -pthread_mutex_unlock (&he->hists->lock); +mutex_unlock (&he->hists->lock);if (err == -ERANGE && !he->ms.map->erange_warned)ui__warn_map_erange (he->ms.map, sym, ip);
@@ -230,7 +230,7 @@ static void perf_top__record_precise_ip(struct perf_top *top,sleep (1 ); } -pthread_mutex_lock (&he->hists->lock); +mutex_lock (&he->hists->lock); } }
@@ -836,12 +836,12 @@ static void perf_event__process_sample(struct perf_tool *tool,else iter.ops = &hist_iter_normal; -pthread_mutex_lock (&hists->lock); +mutex_lock (&hists->lock);if (hist_entry_iter__add (&iter, &al, top->max_stack, top) <0 )pr_err ("Problem incrementing symbol period, skipping event\n" ); -pthread_mutex_unlock (&hists->lock); +mutex_unlock (&hists->lock); }addr_location__put (&al);
tools/perf/util/hist.c +3 -3
@@ -1622,13 +1622,13 @@ struct rb_root_cached *hists__get_rotate_entries_in(struct hists *hists) {struct rb_root_cached *root; -pthread_mutex_lock (&hists->lock); +mutex_lock (&hists->lock); root = hists->entries_in;if (++hists->entries_in > &hists->entries_in_array[1 ]) hists->entries_in = &hists->entries_in_array[0 ]; -pthread_mutex_unlock (&hists->lock); +mutex_unlock (&hists->lock);return root; }
@@ -2805,7 +2805,7 @@ int __hists__init(struct hists *hists, struct perf_hpp_list *hpp_list) hists->entries_in = &hists->entries_in_array[0 ]; hists->entries_collapsed =RB_ROOT_CACHED ; hists->entries =RB_ROOT_CACHED ; -pthread_mutex_init (&hists->lock,NULL ); +mutex_init (&hists->lock); hists->socket_filter =-1 ; hists->hpp_list = hpp_list;INIT_LIST_HEAD (&hists->hpp_formats);
tools/perf/util/hist.h +2 -2
@@ -4,10 +4,10 @@#include <linux/rbtree.h> #include <linux/types.h> -#include <pthread.h> #include "evsel.h" #include "color.h" #include "events_stats.h" +#include "mutex.h" struct hist_entry ;struct hist_entry_ops ;
@@ -98,7 +98,7 @@ struct hists {const struct dso *dso_filter;const char *uid_filter_str;const char *symbol_filter_str; -pthread_mutex_t lock; +struct mutex lock;struct hists_stats stats;u64 event_stream;u16 col_len[HISTC_NR_COLS ];
Signature · 1 line
2.37.2.672.g94769d06f0-goog
Ian Rogers
reviewerPATCH v4 5/18 perf bpf: Remove unused pthread.h include
In reply to Ian RogersNo pthread usage in bpf-event.h.
--- tools/perf/util/bpf-event.h | 1 - 1 file changed, 1 deletion(-)
tools/perf/util/bpf-event.h +0 -1
@@ -4,7 +4,6 @@#include <linux/compiler.h> #include <linux/rbtree.h> -#include <pthread.h> #include <api/fd/array.h> #include <stdio.h>
Signature · 1 line
2.37.2.672.g94769d06f0-goog
Ian Rogers
reviewerPATCH v4 6/18 perf lock: Remove unused pthread.h include
In reply to Ian RogersNo pthread usage in builtin-lock.c.
--- tools/perf/builtin-lock.c | 1 - 1 file changed, 1 deletion(-)
tools/perf/builtin-lock.c +0 -1
@@ -28,7 +28,6 @@#include <sys/types.h> #include <sys/prctl.h> #include <semaphore.h> -#include <pthread.h> #include <math.h> #include <limits.h>
Signature · 1 line
2.37.2.672.g94769d06f0-goog
Ian Rogers
reviewerPATCH v4 7/18 perf record: Update use of pthread mutex
In reply to Ian RogersSwitch to the use of mutex wrappers that provide better error checking for synth_lock.
--- tools/perf/builtin-record.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
tools/perf/builtin-record.c +9 -4
@@ -21,6 +21,7 @@#include "util/evsel.h" #include "util/debug.h" #include "util/mmap.h" +#include "util/mutex.h" #include "util/target.h" #include "util/session.h" #include "util/tool.h"
@@ -608,17 +609,18 @@ static int process_synthesized_event(struct perf_tool *tool,return record__write (rec,NULL , event, event->header.size); } +static struct mutex synth_lock; +static int process_locked_synthesized_event (struct perf_tool *tool,union perf_event *event,struct perf_sample *sample__maybe_unused ,struct machine *machine__maybe_unused ) { -static pthread_mutex_t synth_lock =PTHREAD_MUTEX_INITIALIZER ;int ret; -pthread_mutex_lock (&synth_lock); +mutex_lock (&synth_lock); ret =process_synthesized_event (tool, event, sample, machine); -pthread_mutex_unlock (&synth_lock); +mutex_unlock (&synth_lock);return ret; }
@@ -1917,6 +1919,7 @@ static int record__synthesize(struct record *rec, bool tail) }if (rec->opts.nr_threads_synthesize >1 ) { +mutex_init (&synth_lock);perf_set_multithreaded (); f = process_locked_synthesized_event; }
@@ -1930,8 +1933,10 @@ static int record__synthesize(struct record *rec, bool tail) rec->opts.nr_threads_synthesize); } -if (rec->opts.nr_threads_synthesize >1 ) +if (rec->opts.nr_threads_synthesize >1 ) {perf_set_singlethreaded (); +mutex_destroy (&synth_lock); + } out:return err;
Signature · 1 line
2.37.2.672.g94769d06f0-goog
Ian Rogers
reviewerPATCH v4 8/18 perf sched: Update use of pthread mutex
In reply to Ian RogersSwitch to the use of mutex wrappers that provide better error checking. Update cmd_sched so that we always explicitly destroy the mutexes.
--- tools/perf/builtin-sched.c | 67 ++++++++++++++++++-------------------- 1 file changed, 32 insertions(+), 35 deletions(-)
tools/perf/builtin-sched.c +32 -35
@@ -7,6 +7,7 @@#include "util/evlist.h" #include "util/evsel.h" #include "util/evsel_fprintf.h" +#include "util/mutex.h" #include "util/symbol.h" #include "util/thread.h" #include "util/header.h"
@@ -184,8 +185,8 @@ struct perf_sched {struct task_desc **pid_to_task;struct task_desc **tasks;const struct trace_sched_handler *tp_handler; -pthread_mutex_t start_work_mutex; -pthread_mutex_t work_done_wait_mutex; +struct mutex start_work_mutex; +struct mutex work_done_wait_mutex;int profile_cpu;/* * Track the current task - that way we can know whether there's any
@@ -635,10 +636,8 @@ static void *thread_func(void *ctx) again: ret =sem_post (&this_task->ready_for_work);BUG_ON (ret); - ret =pthread_mutex_lock (&sched->start_work_mutex); -BUG_ON (ret); - ret =pthread_mutex_unlock (&sched->start_work_mutex); -BUG_ON (ret); +mutex_lock (&sched->start_work_mutex); +mutex_unlock (&sched->start_work_mutex); cpu_usage_0 =get_cpu_usage_nsec_self (fd);
@@ -652,10 +651,8 @@ static void *thread_func(void *ctx) ret =sem_post (&this_task->work_done_sem);BUG_ON (ret); - ret =pthread_mutex_lock (&sched->work_done_wait_mutex); -BUG_ON (ret); - ret =pthread_mutex_unlock (&sched->work_done_wait_mutex); -BUG_ON (ret); +mutex_lock (&sched->work_done_wait_mutex); +mutex_unlock (&sched->work_done_wait_mutex); goto again; }
@@ -672,10 +669,8 @@ static void create_tasks(struct perf_sched *sched) err =pthread_attr_setstacksize (&attr, (size_t )max (16 *1024 , (int )PTHREAD_STACK_MIN ));BUG_ON (err); - err =pthread_mutex_lock (&sched->start_work_mutex); -BUG_ON (err); - err =pthread_mutex_lock (&sched->work_done_wait_mutex); -BUG_ON (err); +mutex_lock (&sched->start_work_mutex); +mutex_lock (&sched->work_done_wait_mutex);for (i =0 ; i < sched->nr_tasks; i++) {struct sched_thread_parms *parms =malloc (sizeof (*parms));BUG_ON (parms ==NULL );
@@ -699,7 +694,7 @@ static void wait_for_tasks(struct perf_sched *sched) sched->start_time =get_nsecs (); sched->cpu_usage =0 ; -pthread_mutex_unlock (&sched->work_done_wait_mutex); +mutex_unlock (&sched->work_done_wait_mutex);for (i =0 ; i < sched->nr_tasks; i++) { task = sched->tasks[i];
@@ -707,12 +702,11 @@ static void wait_for_tasks(struct perf_sched *sched)BUG_ON (ret);sem_init (&task->ready_for_work,0 ,0 ); } - ret =pthread_mutex_lock (&sched->work_done_wait_mutex); -BUG_ON (ret); +mutex_lock (&sched->work_done_wait_mutex); cpu_usage_0 =get_cpu_usage_nsec_parent (); -pthread_mutex_unlock (&sched->start_work_mutex); +mutex_unlock (&sched->start_work_mutex);for (i =0 ; i < sched->nr_tasks; i++) { task = sched->tasks[i];
@@ -734,8 +728,7 @@ static void wait_for_tasks(struct perf_sched *sched) sched->runavg_parent_cpu_usage = (sched->runavg_parent_cpu_usage * (sched->replay_repeat -1 ) + sched->parent_cpu_usage)/sched->replay_repeat; - ret =pthread_mutex_lock (&sched->start_work_mutex); -BUG_ON (ret); +mutex_lock (&sched->start_work_mutex);for (i =0 ; i < sched->nr_tasks; i++) { task = sched->tasks[i];
@@ -3430,8 +3423,6 @@ int cmd_sched(int argc, const char **argv) }, .cmp_pid = LIST_HEAD_INIT(sched.cmp_pid), .sort_list = LIST_HEAD_INIT(sched.sort_list), - .start_work_mutex = PTHREAD_MUTEX_INITIALIZER, - .work_done_wait_mutex = PTHREAD_MUTEX_INITIALIZER, .sort_order = default_sort_order, .replay_repeat = 10, .profile_cpu = -1,
@@ -3545,8 +3536,10 @@ int cmd_sched(int argc, const char **argv) .fork_event = replay_fork_event, };unsigned int i; -int ret; +int ret =0 ; +mutex_init (&sched.start_work_mutex); +mutex_init (&sched.work_done_wait_mutex);for (i =0 ; i <ARRAY_SIZE (sched.curr_pid); i++) sched.curr_pid[i] =-1 ;
@@ -3558,11 +3551,10 @@ int cmd_sched(int argc, const char **argv)/* * Aliased to 'perf script' for now: */ -if (!strcmp (argv[0 ],"script" )) -return cmd_script (argc, argv); - -if (strlen (argv[0 ]) >2 &&strstarts ("record" , argv[0 ])) { -return __cmd_record (argc, argv); +if (!strcmp (argv[0 ],"script" )) { + ret =cmd_script (argc, argv); + }else if (strlen (argv[0 ]) >2 &&strstarts ("record" , argv[0 ])) { + ret =__cmd_record (argc, argv); }else if (strlen (argv[0 ]) >2 &&strstarts ("latency" , argv[0 ])) { sched.tp_handler = &lat_ops;if (argc >1 ) {
@@ -3571,7 +3563,7 @@ int cmd_sched(int argc, const char **argv)usage_with_options (latency_usage, latency_options); }setup_sorting (&sched, latency_options, latency_usage); -return perf_sched__lat (&sched); + ret =perf_sched__lat (&sched); }else if (!strcmp (argv[0 ],"map" )) {if (argc) { argc =parse_options (argc, argv, map_options, map_usage,0 );
@@ -3580,7 +3572,7 @@ int cmd_sched(int argc, const char **argv) } sched.tp_handler = &map_ops;setup_sorting (&sched, latency_options, latency_usage); -return perf_sched__map (&sched); + ret =perf_sched__map (&sched); }else if (strlen (argv[0 ]) >2 &&strstarts ("replay" , argv[0 ])) { sched.tp_handler = &replay_ops;if (argc) {
@@ -3588,7 +3580,7 @@ int cmd_sched(int argc, const char **argv)if (argc)usage_with_options (replay_usage, replay_options); } -return perf_sched__replay (&sched); + ret =perf_sched__replay (&sched); }else if (!strcmp (argv[0 ],"timehist" )) {if (argc) { argc = parse_options(argc, argv, timehist_options,
@@ -3604,16 +3596,21 @@ int cmd_sched(int argc, const char **argv)parse_options_usage (NULL , timehist_options,"w" , true);if (sched.show_next)parse_options_usage (NULL , timehist_options,"n" , true); -return -EINVAL ; + ret = -EINVAL ; + goto out; } ret =symbol__validate_sym_arguments ();if (ret) -return ret; + goto out; -return perf_sched__timehist (&sched); + ret =perf_sched__timehist (&sched); }else {usage_with_options (sched_usage, sched_options); } -return 0 ; +out: +mutex_destroy (&sched.start_work_mutex); +mutex_destroy (&sched.work_done_wait_mutex); + +return ret; }
Signature · 1 line
2.37.2.672.g94769d06f0-goog
Ian Rogers
reviewerPATCH v4 9/18 perf ui: Update use of pthread mutex
In reply to Ian RogersSwitch to the use of mutex wrappers that provide better error checking.
--- tools/perf/ui/browser.c | 20 ++++++++++---------- tools/perf/ui/browsers/annotate.c | 2 +- tools/perf/ui/setup.c | 5 +++-- tools/perf/ui/tui/helpline.c | 5 ++--- tools/perf/ui/tui/progress.c | 8 ++++---- tools/perf/ui/tui/setup.c | 8 ++++---- tools/perf/ui/tui/util.c | 18 +++++++++--------- tools/perf/ui/ui.h | 4 ++-- 8 files changed, 35 insertions(+), 35 deletions(-)
tools/perf/ui/browser.c +10 -10
@@ -268,9 +268,9 @@ void __ui_browser__show_title(struct ui_browser *browser, const char *title)void ui_browser__show_title (struct ui_browser *browser,const char *title) { -pthread_mutex_lock (&ui__lock); +mutex_lock (&ui__lock);__ui_browser__show_title (browser, title); -pthread_mutex_unlock (&ui__lock); +mutex_unlock (&ui__lock); } int ui_browser__show(struct ui_browser *browser, const char *title,
@@ -284,7 +284,7 @@ int ui_browser__show(struct ui_browser *browser, const char *title, browser->refresh_dimensions (browser); -pthread_mutex_lock (&ui__lock); +mutex_lock (&ui__lock);__ui_browser__show_title (browser, title); browser->title = title;
@@ -295,16 +295,16 @@ int ui_browser__show(struct ui_browser *browser, const char *title,va_end (ap);if (err >0 )ui_helpline__push (browser->helpline); -pthread_mutex_unlock (&ui__lock); +mutex_unlock (&ui__lock);return err ?0 :-1 ; }void ui_browser__hide (struct ui_browser *browser) { -pthread_mutex_lock (&ui__lock); +mutex_lock (&ui__lock);ui_helpline__pop ();zfree (&browser->helpline); -pthread_mutex_unlock (&ui__lock); +mutex_unlock (&ui__lock); }static void ui_browser__scrollbar_set (struct ui_browser *browser)
@@ -352,9 +352,9 @@ static int __ui_browser__refresh(struct ui_browser *browser)int ui_browser__refresh (struct ui_browser *browser) { -pthread_mutex_lock (&ui__lock); +mutex_lock (&ui__lock);__ui_browser__refresh (browser); -pthread_mutex_unlock (&ui__lock); +mutex_unlock (&ui__lock);return 0 ; }
@@ -390,10 +390,10 @@ int ui_browser__run(struct ui_browser *browser, int delay_secs)while (1 ) {off_t offset; -pthread_mutex_lock (&ui__lock); +mutex_lock (&ui__lock); err =__ui_browser__refresh (browser);SLsmg_refresh (); -pthread_mutex_unlock (&ui__lock); +mutex_unlock (&ui__lock);if (err <0 )break ;
tools/perf/ui/browsers/annotate.c +1 -1
@@ -8,11 +8,11 @@#include "../../util/hist.h" #include "../../util/sort.h" #include "../../util/map.h" +#include "../../util/mutex.h" #include "../../util/symbol.h" #include "../../util/evsel.h" #include "../../util/evlist.h" #include <inttypes.h> -#include <pthread.h> #include <linux/kernel.h> #include <linux/string.h> #include <linux/zalloc.h>
tools/perf/ui/setup.c +3 -2
@@ -1,5 +1,4 @@// SPDX-License-Identifier: GPL-2.0 -#include <pthread.h> #include <dlfcn.h> #include <unistd.h>
@@ -8,7 +7,7 @@#include "../util/hist.h" #include "ui.h" -pthread_mutex_t ui__lock =PTHREAD_MUTEX_INITIALIZER ; +struct mutex ui__lock;void *perf_gtk_handle;int use_browser =-1 ;
@@ -76,6 +75,7 @@ int stdio__config_color(const struct option *opt __maybe_unused,void setup_browser (bool fallback_to_pager) { +mutex_init (&ui__lock);if (use_browser <2 && (!isatty (1 ) || dump_trace)) use_browser =0 ;
@@ -118,4 +118,5 @@ void exit_browser(bool wait_for_ok)default :break ; } +mutex_destroy (&ui__lock); }
tools/perf/ui/tui/helpline.c +2 -3
@@ -2,7 +2,6 @@#include <stdio.h> #include <stdlib.h> #include <string.h> -#include <pthread.h> #include <linux/kernel.h> #include <linux/string.h>
@@ -33,7 +32,7 @@ static int tui_helpline__show(const char *format, va_list ap)int ret;static int backlog; -pthread_mutex_lock (&ui__lock); +mutex_lock (&ui__lock); ret =vscnprintf (ui_helpline__last_msg + backlog,sizeof (ui_helpline__last_msg) - backlog, format, ap); backlog += ret;
@@ -45,7 +44,7 @@ static int tui_helpline__show(const char *format, va_list ap)SLsmg_refresh (); backlog =0 ; } -pthread_mutex_unlock (&ui__lock); +mutex_unlock (&ui__lock);return ret; }
tools/perf/ui/tui/progress.c +4 -4
@@ -45,7 +45,7 @@ static void tui_progress__update(struct ui_progress *p) }ui__refresh_dimensions (false); -pthread_mutex_lock (&ui__lock); +mutex_lock (&ui__lock); y = SLtt_Screen_Rows /2 -2 ;SLsmg_set_color (0 );SLsmg_draw_box (y,0 ,3 , SLtt_Screen_Cols);
@@ -56,7 +56,7 @@ static void tui_progress__update(struct ui_progress *p) bar = ((SLtt_Screen_Cols -2 ) * p->curr) / p->total;SLsmg_fill_region (y,1 ,1 , bar,' ' );SLsmg_refresh (); -pthread_mutex_unlock (&ui__lock); +mutex_unlock (&ui__lock); }static void tui_progress__finish (void )
@@ -67,12 +67,12 @@ static void tui_progress__finish(void)return ;ui__refresh_dimensions (false); -pthread_mutex_lock (&ui__lock); +mutex_lock (&ui__lock); y = SLtt_Screen_Rows /2 -2 ;SLsmg_set_color (0 );SLsmg_fill_region (y,0 ,3 , SLtt_Screen_Cols,' ' );SLsmg_refresh (); -pthread_mutex_unlock (&ui__lock); +mutex_unlock (&ui__lock); }static struct ui_progress_ops tui_progress__ops = {
tools/perf/ui/tui/setup.c +4 -4
@@ -29,10 +29,10 @@ void ui__refresh_dimensions(bool force) {if (force || ui__need_resize) { ui__need_resize =0 ; -pthread_mutex_lock (&ui__lock); +mutex_lock (&ui__lock);SLtt_get_screen_size ();SLsmg_reinit_smg (); -pthread_mutex_unlock (&ui__lock); +mutex_unlock (&ui__lock); } }
@@ -170,10 +170,10 @@ void ui__exit(bool wait_for_ok) "Press any key...", 0);SLtt_set_cursor_visibility (1 ); -if (!pthread_mutex_trylock (&ui__lock)) { +if (mutex_trylock (&ui__lock)) {SLsmg_refresh ();SLsmg_reset_smg (); -pthread_mutex_unlock (&ui__lock); +mutex_unlock (&ui__lock); }SLang_reset_tty ();perf_error__unregister (&perf_tui_eops);
tools/perf/ui/tui/util.c +9 -9
@@ -95,7 +95,7 @@ int ui_browser__input_window(const char *title, const char *text, char *input, t = sep +1 ; } -pthread_mutex_lock (&ui__lock); +mutex_lock (&ui__lock); max_len +=2 ; nr_lines +=8 ;
@@ -125,17 +125,17 @@ int ui_browser__input_window(const char *title, const char *text, char *input,SLsmg_write_nstring ((char *)exit_msg, max_len);SLsmg_refresh (); -pthread_mutex_unlock (&ui__lock); +mutex_unlock (&ui__lock); x +=2 ; len =0 ; key =ui__getch (delay_secs);while (key !=K_TIMER && key !=K_ENTER && key !=K_ESC ) { -pthread_mutex_lock (&ui__lock); +mutex_lock (&ui__lock);if (key ==K_BKSPC ) {if (len ==0 ) { -pthread_mutex_unlock (&ui__lock); +mutex_unlock (&ui__lock); goto next_key; }SLsmg_gotorc (y, x + --len);
@@ -147,7 +147,7 @@ int ui_browser__input_window(const char *title, const char *text, char *input, }SLsmg_refresh (); -pthread_mutex_unlock (&ui__lock); +mutex_unlock (&ui__lock);/* XXX more graceful overflow handling needed */ if (len ==sizeof (buf) -1 ) {
@@ -215,19 +215,19 @@ void __ui__info_window(const char *title, const char *text, const char *exit_msgvoid ui__info_window (const char *title,const char *text) { -pthread_mutex_lock (&ui__lock); +mutex_lock (&ui__lock);__ui__info_window (title, text,NULL );SLsmg_refresh (); -pthread_mutex_unlock (&ui__lock); +mutex_unlock (&ui__lock); }int ui__question_window (const char *title,const char *text,const char *exit_msg,int delay_secs) { -pthread_mutex_lock (&ui__lock); +mutex_lock (&ui__lock);__ui__info_window (title, text, exit_msg);SLsmg_refresh (); -pthread_mutex_unlock (&ui__lock); +mutex_unlock (&ui__lock);return ui__getch (delay_secs); }
tools/perf/ui/ui.h +2 -2
@@ -2,11 +2,11 @@#ifndef _PERF_UI_H_#define _PERF_UI_H_ 1 -#include <pthread.h> +#include "../util/mutex.h" #include <stdbool.h> #include <linux/compiler.h> -extern pthread_mutex_t ui__lock; +extern struct mutex ui__lock;extern void *perf_gtk_handle;extern int use_browser;
Signature · 1 line
2.37.2.672.g94769d06f0-goog
Ian Rogers
reviewerPATCH v4 10/18 perf mmap: Remove unnecessary pthread.h include
In reply to Ian RogersThe comment says it is for cpu_set_t which isn't used in the header.
--- tools/perf/util/mmap.h | 1 - 1 file changed, 1 deletion(-)
tools/perf/util/mmap.h +0 -1
@@ -9,7 +9,6 @@#include <linux/bitops.h> #include <perf/cpumap.h> #include <stdbool.h> -#include <pthread.h> // for cpu_set_t #ifdef HAVE_AIO_SUPPORT #include <aio.h> #endif
Signature · 1 line
2.37.2.672.g94769d06f0-goog
Ian Rogers
reviewerPATCH v4 11/18 perf dso: Update use of pthread mutex
In reply to Ian RogersSwitch to the use of mutex wrappers that provide better error checking.
--- tools/perf/util/dso.c | 12 ++++++------ tools/perf/util/dso.h | 4 ++-- tools/perf/util/symbol.c | 4 ++-- 3 files changed, 10 insertions(+), 10 deletions(-)
tools/perf/util/dso.c +6 -6
@@ -795,7 +795,7 @@ dso_cache__free(struct dso *dso)struct rb_root *root = &dso->data.cache;struct rb_node *next =rb_first (root); -pthread_mutex_lock (&dso->lock); +mutex_lock (&dso->lock);while (next) {struct dso_cache *cache;
@@ -804,7 +804,7 @@ dso_cache__free(struct dso *dso)rb_erase (&cache->rb_node, root);free (cache); } -pthread_mutex_unlock (&dso->lock); +mutex_unlock (&dso->lock); }static struct dso_cache *__dso_cache__find (struct dso *dso,u64 offset)
@@ -841,7 +841,7 @@ dso_cache__insert(struct dso *dso, struct dso_cache *new)struct dso_cache *cache;u64 offset = new->offset; -pthread_mutex_lock (&dso->lock); +mutex_lock (&dso->lock);while (*p !=NULL ) {u64 end;
@@ -862,7 +862,7 @@ dso_cache__insert(struct dso *dso, struct dso_cache *new) cache =NULL ; out: -pthread_mutex_unlock (&dso->lock); +mutex_unlock (&dso->lock);return cache; }
@@ -1297,7 +1297,7 @@ struct dso *dso__new_id(const char *name, struct dso_id *id) dso->root =NULL ;INIT_LIST_HEAD (&dso->node);INIT_LIST_HEAD (&dso->data.open_entry); -pthread_mutex_init (&dso->lock,NULL ); +mutex_init (&dso->lock);refcount_set (&dso->refcnt,1 ); }
@@ -1336,7 +1336,7 @@ void dso__delete(struct dso *dso)dso__free_a2l (dso);zfree (&dso->symsrc_filename);nsinfo__zput (dso->nsinfo); -pthread_mutex_destroy (&dso->lock); +mutex_destroy (&dso->lock);free (dso); }
tools/perf/util/dso.h +2 -2
@@ -2,7 +2,6 @@#ifndef __PERF_DSO#define __PERF_DSO -#include <pthread.h> #include <linux/refcount.h> #include <linux/types.h> #include <linux/rbtree.h>
@@ -11,6 +10,7 @@#include <stdio.h> #include <linux/bitops.h> #include "build-id.h" +#include "mutex.h" struct machine ;struct map ;
@@ -145,7 +145,7 @@ struct dso_cache {struct auxtrace_cache ;struct dso { -pthread_mutex_t lock; +struct mutex lock;struct list_head node;struct rb_node rb_node;/* rbtree node sorted by long name */ struct rb_root *root;/* root of rbtree that rb_node is in */
tools/perf/util/symbol.c +2 -2
@@ -1800,7 +1800,7 @@ int dso__load(struct dso *dso, struct map *map) }nsinfo__mountns_enter (dso->nsinfo, &nsc); -pthread_mutex_lock (&dso->lock); +mutex_lock (&dso->lock);/* check again under the dso->lock */ if (dso__loaded (dso)) {
@@ -1964,7 +1964,7 @@ int dso__load(struct dso *dso, struct map *map) ret =0 ; out:dso__set_loaded (dso); -pthread_mutex_unlock (&dso->lock); +mutex_unlock (&dso->lock);nsinfo__mountns_exit (&nsc);return ret;
Signature · 1 line
2.37.2.672.g94769d06f0-goog
Ian Rogers
reviewerPATCH v4 12/18 perf annotate: Update use of pthread mutex
In reply to Ian RogersSwitch to the use of mutex wrappers that provide better error checking.
--- tools/perf/builtin-top.c | 14 +++++++------- tools/perf/ui/browsers/annotate.c | 10 +++++----- tools/perf/util/annotate.c | 13 ++++++------- tools/perf/util/annotate.h | 4 ++-- 4 files changed, 20 insertions(+), 21 deletions(-)
tools/perf/builtin-top.c +7 -7
@@ -136,10 +136,10 @@ static int perf_top__parse_source(struct perf_top *top, struct hist_entry *he) } notes =symbol__annotation (sym); -pthread_mutex_lock (¬es->lock); +mutex_lock (¬es->lock);if (!symbol__hists (sym, top->evlist->core.nr_entries)) { -pthread_mutex_unlock (¬es->lock); +mutex_unlock (¬es->lock);pr_err ("Not enough memory for annotating '%s' symbol!\n" , sym->name);sleep (1 );
@@ -155,7 +155,7 @@ static int perf_top__parse_source(struct perf_top *top, struct hist_entry *he)pr_err ("Couldn't annotate %s: %s\n" , sym->name, msg); } -pthread_mutex_unlock (¬es->lock); +mutex_unlock (¬es->lock);return err; }
@@ -208,12 +208,12 @@ static void perf_top__record_precise_ip(struct perf_top *top, notes =symbol__annotation (sym); -if (pthread_mutex_trylock (¬es->lock)) +if (!mutex_trylock (¬es->lock))return ; err =hist_entry__inc_addr_samples (he, sample, evsel, ip); -pthread_mutex_unlock (¬es->lock); +mutex_unlock (¬es->lock);if (unlikely (err)) {/*
@@ -250,7 +250,7 @@ static void perf_top__show_details(struct perf_top *top) symbol = he->ms.sym; notes =symbol__annotation (symbol); -pthread_mutex_lock (¬es->lock); +mutex_lock (¬es->lock);symbol__calc_percent (symbol, evsel);
@@ -271,7 +271,7 @@ static void perf_top__show_details(struct perf_top *top)if (more !=0 )printf ("%d lines not displayed, maybe increase display entries [e]\n" , more); out_unlock: -pthread_mutex_unlock (¬es->lock); +mutex_unlock (¬es->lock); }static void perf_top__resort_hists (struct perf_top *t)
tools/perf/ui/browsers/annotate.c +5 -5
@@ -319,7 +319,7 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser, browser->entries =RB_ROOT ; -pthread_mutex_lock (¬es->lock); +mutex_lock (¬es->lock);symbol__calc_percent (sym, evsel);
@@ -348,7 +348,7 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser, }disasm_rb_tree__insert (browser, &pos->al); } -pthread_mutex_unlock (¬es->lock); +mutex_unlock (¬es->lock); browser->curr_hot =rb_last (&browser->entries); }
@@ -474,10 +474,10 @@ static bool annotate_browser__callq(struct annotate_browser *browser, } notes =symbol__annotation (dl->ops.target.sym); -pthread_mutex_lock (¬es->lock); +mutex_lock (¬es->lock);if (!symbol__hists (dl->ops.target.sym, evsel->evlist->core.nr_entries)) { -pthread_mutex_unlock (¬es->lock); +mutex_unlock (¬es->lock);ui__warning ("Not enough memory for annotating '%s' symbol!\n" , dl->ops.target.sym->name);return true;
@@ -486,7 +486,7 @@ static bool annotate_browser__callq(struct annotate_browser *browser, target_ms.maps = ms->maps; target_ms.map = ms->map; target_ms.sym = dl->ops.target.sym; -pthread_mutex_unlock (¬es->lock); +mutex_unlock (¬es->lock);symbol__tui_annotate (&target_ms, evsel, hbt, browser->opts);sym_title (ms->sym, ms->map, title,sizeof (title), browser->opts->percent_type);ui_browser__show_title (&browser->b, title);
tools/perf/util/annotate.c +6 -7
@@ -35,7 +35,6 @@#include "arch/common.h" #include "namespaces.h" #include <regex.h> -#include <pthread.h> #include <linux/bitops.h> #include <linux/kernel.h> #include <linux/string.h>
@@ -821,7 +820,7 @@ void symbol__annotate_zero_histograms(struct symbol *sym) {struct annotation *notes =symbol__annotation (sym); -pthread_mutex_lock (¬es->lock); +mutex_lock (¬es->lock);if (notes->src !=NULL ) {memset (notes->src->histograms,0 , notes->src->nr_histograms * notes->src->sizeof_sym_hist);
@@ -829,7 +828,7 @@ void symbol__annotate_zero_histograms(struct symbol *sym)memset (notes->src->cycles_hist,0 ,symbol__size (sym) *sizeof (struct cyc_hist )); } -pthread_mutex_unlock (¬es->lock); +mutex_unlock (¬es->lock); } static int __symbol__account_cycles(struct cyc_hist *ch,
@@ -1086,7 +1085,7 @@ void annotation__compute_ipc(struct annotation *notes, size_t size) notes->hit_insn =0 ; notes->cover_insn =0 ; -pthread_mutex_lock (¬es->lock); +mutex_lock (¬es->lock);for (offset = size -1 ; offset >=0 ; --offset) {struct cyc_hist *ch;
@@ -1105,7 +1104,7 @@ void annotation__compute_ipc(struct annotation *notes, size_t size) notes->have_cycles = true; } } -pthread_mutex_unlock (¬es->lock); +mutex_unlock (¬es->lock); } int addr_map_symbol__inc_samples(struct addr_map_symbol *ams, struct perf_sample *sample,
@@ -1258,13 +1257,13 @@ int disasm_line__scnprintf(struct disasm_line *dl, char *bf, size_t size, bool rvoid annotation__init (struct annotation *notes) { -pthread_mutex_init (¬es->lock,NULL ); +mutex_init (¬es->lock); }void annotation__exit (struct annotation *notes) {annotated_source__delete (notes->src); -pthread_mutex_destroy (¬es->lock); +mutex_destroy (¬es->lock); }static void annotation_line__add (struct annotation_line *al,struct list_head *head)
tools/perf/util/annotate.h +2 -2
@@ -8,9 +8,9 @@#include <linux/types.h> #include <linux/list.h> #include <linux/rbtree.h> -#include <pthread.h> #include <asm/bug.h> #include "symbol_conf.h" +#include "mutex.h" #include "spark.h" struct hist_browser_timer ;
@@ -273,7 +273,7 @@ struct annotated_source { };struct annotation { -pthread_mutex_t lock; +struct mutex lock;u64 max_coverage;u64 start;u64 hit_cycles;
Signature · 1 line
2.37.2.672.g94769d06f0-goog
Ian Rogers
reviewerPATCH v4 13/18 perf top: Update use of pthread mutex
In reply to Ian RogersSwitch to the use of mutex wrappers that provide better error checking.
--- tools/perf/builtin-top.c | 18 +++++++++--------- tools/perf/util/top.h | 5 +++-- 2 files changed, 12 insertions(+), 11 deletions(-)
tools/perf/builtin-top.c +9 -9
@@ -893,10 +893,10 @@ static void perf_top__mmap_read_idx(struct perf_top *top, int idx)perf_mmap__consume (&md->core);if (top->qe.rotate) { -pthread_mutex_lock (&top->qe.mutex); +mutex_lock (&top->qe.mutex); top->qe.rotate = false; -pthread_cond_signal (&top->qe.cond); -pthread_mutex_unlock (&top->qe.mutex); +cond_signal (&top->qe.cond); +mutex_unlock (&top->qe.mutex); } }
@@ -1100,10 +1100,10 @@ static void *process_thread(void *arg) out =rotate_queues (top); -pthread_mutex_lock (&top->qe.mutex); +mutex_lock (&top->qe.mutex); top->qe.rotate = true; -pthread_cond_wait (&top->qe.cond, &top->qe.mutex); -pthread_mutex_unlock (&top->qe.mutex); +cond_wait (&top->qe.cond, &top->qe.mutex); +mutex_unlock (&top->qe.mutex);if (ordered_events__flush (out,OE_FLUSH__TOP ))pr_err ("failed to process events\n" );
@@ -1217,8 +1217,8 @@ static void init_process_thread(struct perf_top *top)ordered_events__set_copy_on_queue (&top->qe.data[0 ], true);ordered_events__set_copy_on_queue (&top->qe.data[1 ], true); top->qe.in = &top->qe.data[0 ]; -pthread_mutex_init (&top->qe.mutex,NULL ); -pthread_cond_init (&top->qe.cond,NULL ); +mutex_init (&top->qe.mutex); +cond_init (&top->qe.cond); }static int __cmd_top (struct perf_top *top)
@@ -1349,7 +1349,7 @@ static int __cmd_top(struct perf_top *top) out_join:pthread_join (thread,NULL ); out_join_thread: -pthread_cond_signal (&top->qe.cond); +cond_signal (&top->qe.cond);pthread_join (thread_process,NULL );return ret; }
tools/perf/util/top.h +3 -2
@@ -5,6 +5,7 @@#include "tool.h" #include "evswitch.h" #include "annotate.h" +#include "mutex.h" #include "ordered-events.h" #include "record.h" #include <linux/types.h>
@@ -53,8 +54,8 @@ struct perf_top {struct ordered_events *in;struct ordered_events data[2 ];bool rotate; -pthread_mutex_t mutex; -pthread_cond_t cond; +struct mutex mutex; +struct cond cond; } qe; };
Signature · 1 line
2.37.2.672.g94769d06f0-goog
Ian Rogers
reviewerPATCH v4 14/18 perf dso: Hold lock when accessing nsinfo
In reply to Ian RogersThere may be threads racing to update dso->nsinfo:
https://lore.kernel.org/linux-perf-users/CAP-5=fWZH20L4kv-BwVtGLwR=Em3AOOT+Q4QGivvQuYn5AsPRg@mail.gmail.com/
Holding the dso->lock avoids use-after-free, memory leaks and other such bugs. Apply the fix in:
https://lore.kernel.org/linux-perf-users/[email protected]/
of there being a missing nsinfo__put now that the accesses are data race free. Fixes test "Lookup mmap thread" when compiled with address sanitizer.
--- tools/perf/builtin-inject.c | 4 ++++ tools/perf/util/annotate.c | 2 ++ tools/perf/util/build-id.c | 12 +++++++++--- tools/perf/util/dso.c | 7 ++++++- tools/perf/util/map.c | 3 +++ tools/perf/util/probe-event.c | 3 +++ tools/perf/util/symbol.c | 2 +- 7 files changed, 28 insertions(+), 5 deletions(-)
tools/perf/builtin-inject.c +4 -0
@@ -436,8 +436,10 @@ static struct dso *findnew_dso(int pid, int tid, const char *filename, }if (dso) { +mutex_lock (&dso->lock);nsinfo__put (dso->nsinfo); dso->nsinfo = nsi; +mutex_unlock (&dso->lock); }else nsinfo__put (nsi);
@@ -620,6 +622,7 @@ static int dso__read_build_id(struct dso *dso)if (dso->has_build_id)return 0 ; +mutex_lock (&dso->lock);nsinfo__mountns_enter (dso->nsinfo, &nsc);if (filename__read_build_id (dso->long_name, &dso->bid) >0 ) dso->has_build_id = true;
@@ -633,6 +636,7 @@ static int dso__read_build_id(struct dso *dso)free (new_name); }nsinfo__mountns_exit (&nsc); +mutex_unlock (&dso->lock);return dso->has_build_id ?0 :-1 ; }
tools/perf/util/annotate.c +2 -0
@@ -1697,6 +1697,7 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t fil*/ __symbol__join_symfs (filename, filename_size, dso->long_name); +mutex_lock (&dso->lock);if (access (filename,R_OK ) && errno ==ENOENT && dso->nsinfo) {char *new_name =filename_with_chroot (dso->nsinfo->pid, filename);
@@ -1705,6 +1706,7 @@ static int dso__disassemble_filename(struct dso *dso, char *filename, size_t filfree (new_name); } } +mutex_unlock (&dso->lock); }free (build_id_path);
tools/perf/util/build-id.c +9 -3
@@ -898,11 +898,15 @@ static int filename__read_build_id_ns(const char *filename,static bool dso__build_id_mismatch (struct dso *dso,const char *name) {struct build_id bid; +bool ret = false; -if (filename__read_build_id_ns (name, &bid, dso->nsinfo) <0 ) -return false; +mutex_lock (&dso->lock); +if (filename__read_build_id_ns (name, &bid, dso->nsinfo) >=0 ) + ret = !dso__build_id_equal (dso, &bid); -return !dso__build_id_equal (dso, &bid); +mutex_unlock (&dso->lock); + +return ret; } static int dso__cache_build_id(struct dso *dso, struct machine *machine,
@@ -941,8 +945,10 @@ static int dso__cache_build_id(struct dso *dso, struct machine *machine,if (!is_kallsyms &&dso__build_id_mismatch (dso, name)) goto out_free; +mutex_lock (&dso->lock); ret =build_id_cache__add_b (&dso->bid, name, dso->nsinfo, is_kallsyms, is_vdso, proper_name, root_dir); +mutex_unlock (&dso->lock); out_free:free (allocated_name);return ret;
tools/perf/util/dso.c +6 -1
@@ -501,6 +501,7 @@ static int __open_dso(struct dso *dso, struct machine *machine)if (!name)return -ENOMEM ; +mutex_lock (&dso->lock);if (machine) root_dir = machine->root_dir;
@@ -541,6 +542,7 @@ static int __open_dso(struct dso *dso, struct machine *machine)unlink (name); out: +mutex_unlock (&dso->lock);free (name);return fd; }
@@ -559,8 +561,11 @@ static int open_dso(struct dso *dso, struct machine *machine)int fd;struct nscookie nsc; -if (dso->binary_type !=DSO_BINARY_TYPE__BUILD_ID_CACHE ) +if (dso->binary_type !=DSO_BINARY_TYPE__BUILD_ID_CACHE ) { +mutex_lock (&dso->lock);nsinfo__mountns_enter (dso->nsinfo, &nsc); +mutex_unlock (&dso->lock); + } fd =__open_dso (dso, machine);if (dso->binary_type !=DSO_BINARY_TYPE__BUILD_ID_CACHE )nsinfo__mountns_exit (&nsc);
tools/perf/util/map.c +3 -0
@@ -181,7 +181,10 @@ struct map *map__new(struct machine *machine, u64 start, u64 len,if (!(prot &PROT_EXEC ))dso__set_loaded (dso); } +mutex_lock (&dso->lock); +nsinfo__put (dso->nsinfo); dso->nsinfo = nsi; +mutex_unlock (&dso->lock);if (build_id__is_defined (bid)) {dso__set_build_id (dso, bid);
tools/perf/util/probe-event.c +3 -0
@@ -29,6 +29,7 @@#include "color.h" #include "map.h" #include "maps.h" +#include "mutex.h" #include "symbol.h" #include <api/fs/fs.h> #include "trace-event.h" /* For __maybe_unused */
@@ -180,8 +181,10 @@ struct map *get_target_map(const char *target, struct nsinfo *nsi, bool user) map =dso__new_map (target);if (map && map->dso) { +mutex_lock (&map->dso->lock);nsinfo__put (map->dso->nsinfo); map->dso->nsinfo =nsinfo__get (nsi); +mutex_unlock (&map->dso->lock); }return map; }else {
tools/perf/util/symbol.c +1 -1
@@ -1791,6 +1791,7 @@ int dso__load(struct dso *dso, struct map *map)char newmapname[PATH_MAX ];const char *map_path = dso->long_name; +mutex_lock (&dso->lock); perfmap =strncmp (dso->name,"/tmp/perf-" ,10 ) ==0 ;if (perfmap) { if (dso->nsinfo && (dso__find_perf_map(newmapname,
@@ -1800,7 +1801,6 @@ int dso__load(struct dso *dso, struct map *map) }nsinfo__mountns_enter (dso->nsinfo, &nsc); -mutex_lock (&dso->lock);/* check again under the dso->lock */ if (dso__loaded (dso)) {
Signature · 1 line
2.37.2.672.g94769d06f0-goog
Ian Rogers
reviewerPATCH v4 15/18 perf mutex: Add thread safety annotations
In reply to Ian RogersAdd thread safety annotations to struct mutex so that when compiled with clang's -Wthread-safety warnings are generated for erroneous lock patterns. NO_THREAD_SAFETY_ANALYSIS is needed for mutex_lock/mutex_unlock as the analysis doesn't under pthread calls.
--- tools/perf/util/mutex.c | 2 ++ tools/perf/util/mutex.h | 70 ++++++++++++++++++++++++++++++++++++++--- 2 files changed, 67 insertions(+), 5 deletions(-)
tools/perf/util/mutex.c +2 -0
@@ -50,11 +50,13 @@ void mutex_destroy(struct mutex *mtx) }void mutex_lock (struct mutex *mtx) +NO_THREAD_SAFETY_ANALYSIS {CHECK_ERR (pthread_mutex_lock (&mtx->lock)); }void mutex_unlock (struct mutex *mtx) +NO_THREAD_SAFETY_ANALYSIS {CHECK_ERR (pthread_mutex_unlock (&mtx->lock)); }
tools/perf/util/mutex.h +65 -5
93 lines not on this page · raw message
Signature · 1 line
2.37.2.672.g94769d06f0-goog
Ian Rogers
reviewerPATCH v4 16/18 perf sched: Fixes for thread safety analysis
In reply to Ian RogersAdd annotations to describe lock behavior. Add unlocks so that mutexes aren't conditionally held on exit from perf_sched__replay. Add an exit variable so that thread_func can terminate, rather than leaving the threads blocked on mutexes.
--- tools/perf/builtin-sched.c | 46 ++++++++++++++++++++++++-------------- 1 file changed, 29 insertions(+), 17 deletions(-)
tools/perf/builtin-sched.c +29 -17
@@ -246,6 +246,7 @@ struct perf_sched {const char *time_str;struct perf_time_interval ptime;struct perf_time_interval hist_time; +volatile bool thread_funcs_exit; };/* per thread run time data */
@@ -633,31 +634,34 @@ static void *thread_func(void *ctx)prctl (PR_SET_NAME , comm2);if (fd <0 )return NULL ; -again: - ret =sem_post (&this_task->ready_for_work); -BUG_ON (ret); -mutex_lock (&sched->start_work_mutex); -mutex_unlock (&sched->start_work_mutex); - cpu_usage_0 =get_cpu_usage_nsec_self (fd); +while (!sched->thread_funcs_exit) { + ret =sem_post (&this_task->ready_for_work); +BUG_ON (ret); +mutex_lock (&sched->start_work_mutex); +mutex_unlock (&sched->start_work_mutex); -for (i =0 ; i < this_task->nr_events; i++) { - this_task->curr_event = i; -perf_sched__process_event (sched, this_task->atoms[i]); - } + cpu_usage_0 =get_cpu_usage_nsec_self (fd); - cpu_usage_1 =get_cpu_usage_nsec_self (fd); - this_task->cpu_usage = cpu_usage_1 - cpu_usage_0; - ret =sem_post (&this_task->work_done_sem); -BUG_ON (ret); +for (i =0 ; i < this_task->nr_events; i++) { + this_task->curr_event = i; +perf_sched__process_event (sched, this_task->atoms[i]); + } -mutex_lock (&sched->work_done_wait_mutex); -mutex_unlock (&sched->work_done_wait_mutex); + cpu_usage_1 =get_cpu_usage_nsec_self (fd); + this_task->cpu_usage = cpu_usage_1 - cpu_usage_0; + ret =sem_post (&this_task->work_done_sem); +BUG_ON (ret); - goto again; +mutex_lock (&sched->work_done_wait_mutex); +mutex_unlock (&sched->work_done_wait_mutex); + } +return NULL ; }static void create_tasks (struct perf_sched *sched) + EXCLUSIVE_LOCK_FUNCTION(sched->start_work_mutex) + EXCLUSIVE_LOCK_FUNCTION(sched->work_done_wait_mutex) {struct task_desc *task;pthread_attr_t attr;
@@ -687,6 +691,8 @@ static void create_tasks(struct perf_sched *sched) }static void wait_for_tasks (struct perf_sched *sched) + EXCLUSIVE_LOCKS_REQUIRED(sched->work_done_wait_mutex) + EXCLUSIVE_LOCKS_REQUIRED(sched->start_work_mutex) {u64 cpu_usage_0, cpu_usage_1;struct task_desc *task;
@@ -738,6 +744,8 @@ static void wait_for_tasks(struct perf_sched *sched) }static void run_one_test (struct perf_sched *sched) + EXCLUSIVE_LOCKS_REQUIRED(sched->work_done_wait_mutex) + EXCLUSIVE_LOCKS_REQUIRED(sched->start_work_mutex) {u64 T0 ,T1 , delta, avg_delta, fluct;
@@ -3309,11 +3317,15 @@ static int perf_sched__replay(struct perf_sched *sched)print_task_traces (sched);add_cross_task_wakeups (sched); + sched->thread_funcs_exit = false;create_tasks (sched);printf ("------------------------------------------------------------\n" );for (i =0 ; i < sched->replay_repeat; i++)run_one_test (sched); + sched->thread_funcs_exit = true; +mutex_unlock (&sched->start_work_mutex); +mutex_unlock (&sched->work_done_wait_mutex);return 0 ; }
Signature · 1 line
2.37.2.672.g94769d06f0-goog
Ian Rogers
reviewerPATCH v4 17/18 perf top: Fixes for thread safety analysis
In reply to Ian RogersAdd annotations to describe lock behavior.
--- tools/perf/builtin-top.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
tools/perf/builtin-top.c +3 -2
@@ -196,6 +196,7 @@ static void perf_top__record_precise_ip(struct perf_top *top,struct hist_entry *he, struct perf_sample *sample, struct evsel *evsel, u64 ip) + EXCLUSIVE_LOCKS_REQUIRED(he->hists->lock) { struct annotation *notes;struct symbol *sym = he->ms.sym;
@@ -724,13 +725,13 @@ static void *display_thread(void *arg)static int hist_iter__top_callback (struct hist_entry_iter *iter,struct addr_location *al,bool single,void *arg) + EXCLUSIVE_LOCKS_REQUIRED(iter->he->hists->lock) {struct perf_top *top = arg; -struct hist_entry *he = iter->he;struct evsel *evsel = iter->evsel;if (perf_hpp_list.sym && single) -perf_top__record_precise_ip (top, he, iter->sample, evsel, al->addr); +perf_top__record_precise_ip (top, iter->he, iter->sample, evsel, al->addr); hist__account_cycles(iter->sample->branch_stack, al, iter->sample, !(top->record_opts.branch_stack & PERF_SAMPLE_BRANCH_ANY),
Signature · 1 line
2.37.2.672.g94769d06f0-goog
Ian Rogers
reviewerPATCH v4 18/18 perf build: Enable -Wthread-safety with clang
In reply to Ian RogersIf building with clang then enable -Wthread-safety warnings.
--- tools/perf/Makefile.config | 5 +++++ 1 file changed, 5 insertions(+)
tools/perf/Makefile.config +5 -0
@@ -19,6 +19,11 @@ detected_var = $(shell echo "$(1)=$($(1))" >> $(OUTPUT).config-detected) CFLAGS := $(EXTRA_CFLAGS) $(filter-out -Wnested-externs,$(EXTRA_WARNINGS)) HOSTCFLAGS := $(filter-out -Wnested-externs,$(EXTRA_WARNINGS)) +# Enabled Wthread-safety analysis for clang builds. +ifeq ($(CC_NO_CLANG), 0) + CFLAGS += -Wthread-safety +endif + include $(srctree)/tools/scripts/Makefile.arch $(call detected_var,SRCARCH)
Signature · 1 line
2.37.2.672.g94769d06f0-goog
Quoted 43 lines · Ian Rogers
When fixing a locking race and memory leak in:
https://lore.kernel.org/linux-perf-users/[email protected]/It was requested that debug mutex code be separated out into its own files. This was, in part, done by Pavithra Gurushankar in:
https://lore.kernel.org/lkml/[email protected]/These patches fix issues with the previous patches, add in the original
dso->nsinfofix and then build on our mutex wrapper with clang's -Wthread-safety analysis. The analysis found missing unlocks inbuiltin-sched.cwhich are fixed and -Wthread-safety is enabled by default when building with clang.v4. Adds a comment for the trylock result, fixes the new line (missed
in v3) and removes two blank lines as suggested by Adrian Hunter.
v3. Adds a missing new line to the error messages and removes the
pshared argument tomutex_initby having two functions,mutex_initandmutex_init_pshared. These changes were suggested by Adrian Hunter.
v2. Breaks apart changes that s/pthread_mutex/mutex/g and the lock
annotations as requested by Arnaldo and Namhyung. A boolean is added tobuiltin-sched.cto terminate thread funcs rather than leaving them blocked on delted mutexes.Ian Rogers (17):
perf bench: Update use of pthread mutex/cond
perf tests: Avoidpthread.hinclusion
perf hist: Update use of pthread mutex
perf bpf: Remove unusedpthread.hinclude
perf lock: Remove unusedpthread.hinclude
perf record: Update use of pthread mutex
perf sched: Update use of pthread mutex
perf ui: Update use of pthread mutex
perf mmap: Remove unnecessarypthread.hinclude
perf dso: Update use of pthread mutex
perf annotate: Update use of pthread mutex
perf top: Update use of pthread mutex
perf dso: Hold lock when accessing nsinfo
perf mutex: Add thread safety annotations
perf sched: Fixes for thread safety analysis
perf top: Fixes for thread safety analysis
perf build: Enable -Wthread-safety with clang
Quoted 2 lines
Pavithra Gurushankar (1):
perf mutex: Wrapped usage of mutex and cond
For the patches 1-7 and 10-13
Thanks,
Namhyung
Quoted 49 lines
tools/perf/Makefile.config | 5 + tools/perf/bench/epoll-ctl.c | 33 +++--- tools/perf/bench/epoll-wait.c | 33 +++--- tools/perf/bench/futex-hash.c | 33 +++--- tools/perf/bench/futex-lock-pi.c | 33 +++--- tools/perf/bench/futex-requeue.c | 33 +++--- tools/perf/bench/futex-wake-parallel.c | 33 +++--- tools/perf/bench/futex-wake.c | 33 +++--- tools/perf/bench/numa.c | 93 ++++++---------- tools/perf/builtin-inject.c | 4 + tools/perf/builtin-lock.c | 1 - tools/perf/builtin-record.c | 13 ++- tools/perf/builtin-sched.c | 105 +++++++++--------- tools/perf/builtin-top.c | 45 ++++---- tools/perf/tests/mmap-basic.c | 2 - tools/perf/tests/openat-syscall-all-cpus.c | 2 +- tools/perf/tests/perf-record.c | 2 - tools/perf/ui/browser.c | 20 ++-- tools/perf/ui/browsers/annotate.c | 12 +-- tools/perf/ui/setup.c | 5 +- tools/perf/ui/tui/helpline.c | 5 +- tools/perf/ui/tui/progress.c | 8 +- tools/perf/ui/tui/setup.c | 8 +- tools/perf/ui/tui/util.c | 18 ++-- tools/perf/ui/ui.h | 4 +- tools/perf/util/Build | 1 + tools/perf/util/annotate.c | 15 +-- tools/perf/util/annotate.h | 4 +- tools/perf/util/bpf-event.h | 1 - tools/perf/util/build-id.c | 12 ++- tools/perf/util/dso.c | 19 ++-- tools/perf/util/dso.h | 4 +- tools/perf/util/hist.c | 6 +- tools/perf/util/hist.h | 4 +- tools/perf/util/map.c | 3 + tools/perf/util/mmap.h | 1 - tools/perf/util/mutex.c | 119 +++++++++++++++++++++ tools/perf/util/mutex.h | 108 +++++++++++++++++++ tools/perf/util/probe-event.c | 3 + tools/perf/util/symbol.c | 4 +- tools/perf/util/top.h | 5 +- 41 files changed, 569 insertions(+), 323 deletions(-) create mode 100644 tools/perf/util/mutex.c create mode 100644 tools/perf/util/mutex.h--
2.37.2.672.g94769d06f0-goog
Arnaldo Carvalho de Melo
maintainerEm Fri, Aug 26, 2022 at 12:08:06PM -0700, Namhyung Kim escreveu:
Quoted 44 lines · Ian Rogers
Quoted 43 lines
When fixing a locking race and memory leak in:
https://lore.kernel.org/linux-perf-users/[email protected]/It was requested that debug mutex code be separated out into its own files. This was, in part, done by Pavithra Gurushankar in:
https://lore.kernel.org/lkml/[email protected]/These patches fix issues with the previous patches, add in the original
dso->nsinfofix and then build on our mutex wrapper with clang's -Wthread-safety analysis. The analysis found missing unlocks inbuiltin-sched.cwhich are fixed and -Wthread-safety is enabled by default when building with clang.v4. Adds a comment for the trylock result, fixes the new line (missed
in v3) and removes two blank lines as suggested by Adrian Hunter.
v3. Adds a missing new line to the error messages and removes the
pshared argument tomutex_initby having two functions,mutex_initandmutex_init_pshared. These changes were suggested by Adrian Hunter.
v2. Breaks apart changes that s/pthread_mutex/mutex/g and the lock
annotations as requested by Arnaldo and Namhyung. A boolean is added tobuiltin-sched.cto terminate thread funcs rather than leaving them blocked on delted mutexes.Ian Rogers (17):
perf bench: Update use of pthread mutex/cond
perf tests: Avoidpthread.hinclusion
perf hist: Update use of pthread mutex
perf bpf: Remove unusedpthread.hinclude
perf lock: Remove unusedpthread.hinclude
perf record: Update use of pthread mutex
perf sched: Update use of pthread mutex
perf ui: Update use of pthread mutex
perf mmap: Remove unnecessarypthread.hinclude
perf dso: Update use of pthread mutex
perf annotate: Update use of pthread mutex
perf top: Update use of pthread mutex
perf dso: Hold lock when accessing nsinfo
perf mutex: Add thread safety annotations
perf sched: Fixes for thread safety analysis
perf top: Fixes for thread safety analysis
perf build: Enable -Wthread-safety with clang
Quoted 6 lines
Quoted 2 lines
Pavithra Gurushankar (1):
perf mutex: Wrapped usage of mutex and condFor the patches 1-7 and 10-13
Acked-by Namhyung Kim <[email protected]>
Thanks added it, will try to get what is done so far merged and then we can go on addressing the other issues brought up in this thread.
- Arnaldo