From f3b623b8490af7a9b819cbcf2d99ab4597ece94b Mon Sep 17 00:00:00 2001 From: Arnaldo Carvalho de Melo Date: Mon, 2 Mar 2015 22:21:35 -0300 Subject: [PATCH] perf tools: Reference count struct thread We need to do that to stop accumulating entries in the dead_threads linked list, i.e. we were keeping references to threads in struct hists that continue to exist even after a thread exited and was removed from the machine threads rbtree. We still keep the dead_threads list, but just for debugging, allowing us to iterate at any given point over the threads that still are referenced by things like struct hist_entry. Cc: Adrian Hunter Cc: Borislav Petkov Cc: David Ahern Cc: Don Zickus Cc: Frederic Weisbecker Cc: Jiri Olsa Cc: Namhyung Kim Cc: Stephane Eranian Link: http://lkml.kernel.org/n/tip-3ejvfyed0r7ue61dkurzjux4@git.kernel.org Signed-off-by: Arnaldo Carvalho de Melo --- tools/perf/builtin-sched.c | 2 +- tools/perf/builtin-trace.c | 7 +++++- tools/perf/ui/browsers/hists.c | 6 ++--- tools/perf/util/build-id.c | 5 ++-- tools/perf/util/hist.c | 2 ++ tools/perf/util/hist.h | 2 +- tools/perf/util/machine.c | 44 ++++++++++++++++++---------------- tools/perf/util/machine.h | 1 - tools/perf/util/session.c | 6 ----- tools/perf/util/thread.c | 14 +++++++++++ tools/perf/util/thread.h | 13 ++++++++++ 11 files changed, 66 insertions(+), 36 deletions(-) diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c index 7ce296618717..e00e2eaf89da 100644 --- a/tools/perf/builtin-sched.c +++ b/tools/perf/builtin-sched.c @@ -831,7 +831,7 @@ static int thread_atoms_insert(struct perf_sched *sched, struct thread *thread) return -1; } - atoms->thread = thread; + atoms->thread = thread__get(thread); INIT_LIST_HEAD(&atoms->work_list); __thread_latency_insert(&sched->atom_root, atoms, &sched->cmp_pid); return 0; diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c index d95a8f4d988c..211614fba217 100644 --- a/tools/perf/builtin-trace.c +++ b/tools/perf/builtin-trace.c @@ -1741,7 +1741,10 @@ static int trace__sys_enter(struct trace *trace, struct perf_evsel *evsel, } else ttrace->entry_pending = true; - trace->current = thread; + if (trace->current != thread) { + thread__put(trace->current); + trace->current = thread__get(thread); + } return 0; } @@ -2274,6 +2277,8 @@ static int trace__run(struct trace *trace, int argc, const char **argv) } out_disable: + thread__zput(trace->current); + perf_evlist__disable(evlist); if (!err) { diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c index 788506eef567..ad312d91caed 100644 --- a/tools/perf/ui/browsers/hists.c +++ b/tools/perf/ui/browsers/hists.c @@ -1467,7 +1467,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events, perf_hpp__set_user_width(symbol_conf.col_width_list_str); while (1) { - const struct thread *thread = NULL; + struct thread *thread = NULL; const struct dso *dso = NULL; int choice = 0, annotate = -2, zoom_dso = -2, zoom_thread = -2, @@ -1754,13 +1754,13 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events, pstack__remove(fstack, &browser->hists->thread_filter); zoom_out_thread: ui_helpline__pop(); - browser->hists->thread_filter = NULL; + thread__zput(browser->hists->thread_filter); perf_hpp__set_elide(HISTC_THREAD, false); } else { ui_helpline__fpush("To zoom out press <- or -> + \"Zoom out of %s(%d) thread\"", thread->comm_set ? thread__comm_str(thread) : "", thread->tid); - browser->hists->thread_filter = thread; + browser->hists->thread_filter = thread__get(thread); perf_hpp__set_elide(HISTC_THREAD, false); pstack__push(fstack, &browser->hists->thread_filter); } diff --git a/tools/perf/util/build-id.c b/tools/perf/util/build-id.c index ffdc338df925..a19674666b4e 100644 --- a/tools/perf/util/build-id.c +++ b/tools/perf/util/build-id.c @@ -61,8 +61,9 @@ static int perf_event__exit_del_thread(struct perf_tool *tool __maybe_unused, if (thread) { rb_erase(&thread->rb_node, &machine->threads); - machine->last_match = NULL; - thread__delete(thread); + if (machine->last_match == thread) + thread__zput(machine->last_match); + thread__put(thread); } return 0; diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c index 70b48a65064c..95f5ab707b74 100644 --- a/tools/perf/util/hist.c +++ b/tools/perf/util/hist.c @@ -355,6 +355,7 @@ static struct hist_entry *hist_entry__new(struct hist_entry *template, callchain_init(he->callchain); INIT_LIST_HEAD(&he->pairs.node); + thread__get(he->thread); } return he; @@ -941,6 +942,7 @@ hist_entry__collapse(struct hist_entry *left, struct hist_entry *right) void hist_entry__delete(struct hist_entry *he) { + thread__zput(he->thread); zfree(&he->branch_info); zfree(&he->mem_info); zfree(&he->stat_acc); diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h index 2b690d028907..e988c9fcd1bc 100644 --- a/tools/perf/util/hist.h +++ b/tools/perf/util/hist.h @@ -60,7 +60,7 @@ struct hists { struct rb_root entries_collapsed; u64 nr_entries; u64 nr_non_filtered_entries; - const struct thread *thread_filter; + struct thread *thread_filter; const struct dso *dso_filter; const char *uid_filter_str; const char *symbol_filter_str; diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index 9e0f60a7e7b3..24f8c978cfd4 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -14,6 +14,8 @@ #include "unwind.h" #include "linux/hash.h" +static void machine__remove_thread(struct machine *machine, struct thread *th); + static void dsos__init(struct dsos *dsos) { INIT_LIST_HEAD(&dsos->head); @@ -89,16 +91,6 @@ static void dsos__delete(struct dsos *dsos) } } -void machine__delete_dead_threads(struct machine *machine) -{ - struct thread *n, *t; - - list_for_each_entry_safe(t, n, &machine->dead_threads, node) { - list_del(&t->node); - thread__delete(t); - } -} - void machine__delete_threads(struct machine *machine) { struct rb_node *nd = rb_first(&machine->threads); @@ -106,9 +98,8 @@ void machine__delete_threads(struct machine *machine) while (nd) { struct thread *t = rb_entry(nd, struct thread, rb_node); - rb_erase(&t->rb_node, &machine->threads); nd = rb_next(nd); - thread__delete(t); + machine__remove_thread(machine, t); } } @@ -361,9 +352,13 @@ static struct thread *__machine__findnew_thread(struct machine *machine, * the full rbtree: */ th = machine->last_match; - if (th && th->tid == tid) { - machine__update_thread_pid(machine, th, pid); - return th; + if (th != NULL) { + if (th->tid == tid) { + machine__update_thread_pid(machine, th, pid); + return th; + } + + thread__zput(machine->last_match); } while (*p != NULL) { @@ -371,7 +366,7 @@ static struct thread *__machine__findnew_thread(struct machine *machine, th = rb_entry(parent, struct thread, rb_node); if (th->tid == tid) { - machine->last_match = th; + machine->last_match = thread__get(th); machine__update_thread_pid(machine, th, pid); return th; } @@ -403,8 +398,11 @@ static struct thread *__machine__findnew_thread(struct machine *machine, thread__delete(th); return NULL; } - - machine->last_match = th; + /* + * It is now in the rbtree, get a ref + */ + thread__get(th); + machine->last_match = thread__get(th); } return th; @@ -1238,13 +1236,17 @@ int machine__process_mmap_event(struct machine *machine, union perf_event *event static void machine__remove_thread(struct machine *machine, struct thread *th) { - machine->last_match = NULL; + if (machine->last_match == th) + thread__zput(machine->last_match); + rb_erase(&th->rb_node, &machine->threads); /* - * We may have references to this thread, for instance in some hist_entry - * instances, so just move them to a separate list. + * Move it first to the dead_threads list, then drop the reference, + * if this is the last reference, then the thread__delete destructor + * will be called and we will remove it from the dead_threads list. */ list_add_tail(&th->node, &machine->dead_threads); + thread__put(th); } int machine__process_fork_event(struct machine *machine, union perf_event *event, diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h index e8b7779a0a3f..e2faf3b47e7b 100644 --- a/tools/perf/util/machine.h +++ b/tools/perf/util/machine.h @@ -118,7 +118,6 @@ void machines__set_comm_exec(struct machines *machines, bool comm_exec); struct machine *machine__new_host(void); int machine__init(struct machine *machine, const char *root_dir, pid_t pid); void machine__exit(struct machine *machine); -void machine__delete_dead_threads(struct machine *machine); void machine__delete_threads(struct machine *machine); void machine__delete(struct machine *machine); diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c index e4f166981ff0..ed4e5cf2bd9d 100644 --- a/tools/perf/util/session.c +++ b/tools/perf/util/session.c @@ -138,11 +138,6 @@ struct perf_session *perf_session__new(struct perf_data_file *file, return NULL; } -static void perf_session__delete_dead_threads(struct perf_session *session) -{ - machine__delete_dead_threads(&session->machines.host); -} - static void perf_session__delete_threads(struct perf_session *session) { machine__delete_threads(&session->machines.host); @@ -167,7 +162,6 @@ static void perf_session_env__delete(struct perf_session_env *env) void perf_session__delete(struct perf_session *session) { perf_session__destroy_kernel_maps(session); - perf_session__delete_dead_threads(session); perf_session__delete_threads(session); perf_session_env__delete(&session->header.env); machines__exit(&session->machines); diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c index 9ebc8b1f9be5..a5dbba95107f 100644 --- a/tools/perf/util/thread.c +++ b/tools/perf/util/thread.c @@ -82,6 +82,20 @@ void thread__delete(struct thread *thread) free(thread); } +struct thread *thread__get(struct thread *thread) +{ + ++thread->refcnt; + return thread; +} + +void thread__put(struct thread *thread) +{ + if (thread && --thread->refcnt == 0) { + list_del_init(&thread->node); + thread__delete(thread); + } +} + struct comm *thread__comm(const struct thread *thread) { if (list_empty(&thread->comm_list)) diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h index 160fd066a7d1..783b6688d2f7 100644 --- a/tools/perf/util/thread.h +++ b/tools/perf/util/thread.h @@ -20,6 +20,7 @@ struct thread { pid_t tid; pid_t ppid; int cpu; + int refcnt; char shortname[3]; bool comm_set; bool dead; /* if set thread has exited */ @@ -37,6 +38,18 @@ struct comm; struct thread *thread__new(pid_t pid, pid_t tid); int thread__init_map_groups(struct thread *thread, struct machine *machine); void thread__delete(struct thread *thread); + +struct thread *thread__get(struct thread *thread); +void thread__put(struct thread *thread); + +static inline void __thread__zput(struct thread **thread) +{ + thread__put(*thread); + *thread = NULL; +} + +#define thread__zput(thread) __thread__zput(&thread) + static inline void thread__exited(struct thread *thread) { thread->dead = true;