[PATCH v2 01/26] perf thread: Remove notion of dead threads
Ian Rogers
irogers at google.com
Thu Jun 8 16:27:58 PDT 2023
The dead thread list is best effort. Threads live on it until the
reference count hits zero and they are removed. With correct reference
counting this should never happen. It is, however, part of the 'perf
sched' output that is now removed. If this is an issue we should
implement tracking of dead threads in a robust not best-effort way.
Signed-off-by: Ian Rogers <irogers at google.com>
---
tools/perf/builtin-sched.c | 23 +----------------------
tools/perf/util/cs-etm.c | 6 ------
tools/perf/util/intel-pt.c | 8 --------
tools/perf/util/machine.c | 32 +-------------------------------
tools/perf/util/thread.c | 25 +------------------------
tools/perf/util/thread.h | 11 +----------
6 files changed, 4 insertions(+), 101 deletions(-)
diff --git a/tools/perf/builtin-sched.c b/tools/perf/builtin-sched.c
index cc4ba506e119..3a30c2ac5b47 100644
--- a/tools/perf/builtin-sched.c
+++ b/tools/perf/builtin-sched.c
@@ -2760,7 +2760,7 @@ struct total_run_stats {
u64 total_run_time;
};
-static int __show_thread_runtime(struct thread *t, void *priv)
+static int show_thread_runtime(struct thread *t, void *priv)
{
struct total_run_stats *stats = priv;
struct thread_runtime *r;
@@ -2783,22 +2783,6 @@ static int __show_thread_runtime(struct thread *t, void *priv)
return 0;
}
-static int show_thread_runtime(struct thread *t, void *priv)
-{
- if (t->dead)
- return 0;
-
- return __show_thread_runtime(t, priv);
-}
-
-static int show_deadthread_runtime(struct thread *t, void *priv)
-{
- if (!t->dead)
- return 0;
-
- return __show_thread_runtime(t, priv);
-}
-
static size_t callchain__fprintf_folded(FILE *fp, struct callchain_node *node)
{
const char *sep = " <- ";
@@ -2890,11 +2874,6 @@ static void timehist_print_summary(struct perf_sched *sched,
if (!task_count)
printf("<no still running tasks>\n");
- printf("\nTerminated tasks:\n");
- machine__for_each_thread(m, show_deadthread_runtime, &totals);
- if (task_count == totals.task_count)
- printf("<no terminated tasks>\n");
-
/* CPU idle stats not tracked when samples were skipped */
if (sched->skipped_samples && !sched->idle_hist)
return;
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 91299cc56bf7..0f5be4ad24ba 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -3292,12 +3292,6 @@ int cs_etm__process_auxtrace_info_full(union perf_event *event,
goto err_free_queues;
}
- /*
- * Initialize list node so that at thread__zput() we can avoid
- * segmentation fault at list_del_init().
- */
- INIT_LIST_HEAD(&etm->unknown_thread->node);
-
err = thread__set_comm(etm->unknown_thread, "unknown", 0);
if (err)
goto err_delete_thread;
diff --git a/tools/perf/util/intel-pt.c b/tools/perf/util/intel-pt.c
index fe893c9bab3f..dde2ca77a005 100644
--- a/tools/perf/util/intel-pt.c
+++ b/tools/perf/util/intel-pt.c
@@ -4311,14 +4311,6 @@ int intel_pt_process_auxtrace_info(union perf_event *event,
goto err_free_queues;
}
- /*
- * Since this thread will not be kept in any rbtree not in a
- * list, initialize its list node so that at thread__put() the
- * current thread lifetime assumption is kept and we don't segfault
- * at list_del_init().
- */
- INIT_LIST_HEAD(&pt->unknown_thread->node);
-
err = thread__set_comm(pt->unknown_thread, "unknown", 0);
if (err)
goto err_delete_thread;
diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 9e02e19c1b7a..a1954ac85f59 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -241,17 +241,6 @@ void machine__exit(struct machine *machine)
for (i = 0; i < THREADS__TABLE_SIZE; i++) {
struct threads *threads = &machine->threads[i];
- struct thread *thread, *n;
- /*
- * Forget about the dead, at this point whatever threads were
- * left in the dead lists better have a reference count taken
- * by who is using them, and then, when they drop those references
- * and it finally hits zero, thread__put() will check and see that
- * its not in the dead threads list and will not try to remove it
- * from there, just calling thread__delete() straight away.
- */
- list_for_each_entry_safe(thread, n, &threads->dead, node)
- list_del_init(&thread->node);
exit_rwsem(&threads->lock);
}
@@ -2046,18 +2035,7 @@ static void __machine__remove_thread(struct machine *machine, struct thread *th,
rb_erase_cached(&th->rb_node, &threads->entries);
RB_CLEAR_NODE(&th->rb_node);
--threads->nr;
- /*
- * 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, &threads->dead);
- /*
- * We need to do the put here because if this is the last refcount,
- * then we will be touching the threads->dead head when removing the
- * thread.
- */
thread__put(th);
if (lock)
@@ -2145,10 +2123,8 @@ int machine__process_exit_event(struct machine *machine, union perf_event *event
if (dump_trace)
perf_event__fprintf_task(event, stdout);
- if (thread != NULL) {
- thread__exited(thread);
+ if (thread != NULL)
thread__put(thread);
- }
return 0;
}
@@ -3204,12 +3180,6 @@ int machine__for_each_thread(struct machine *machine,
if (rc != 0)
return rc;
}
-
- list_for_each_entry(thread, &threads->dead, node) {
- rc = fn(thread, priv);
- if (rc != 0)
- return rc;
- }
}
return rc;
}
diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c
index 4b5bdc277baa..d949bffc0ed6 100644
--- a/tools/perf/util/thread.c
+++ b/tools/perf/util/thread.c
@@ -125,31 +125,8 @@ struct thread *thread__get(struct thread *thread)
void thread__put(struct thread *thread)
{
- if (thread && refcount_dec_and_test(&thread->refcnt)) {
- /*
- * Remove it from the dead threads list, as last reference is
- * gone, if it is in a dead threads list.
- *
- * We may not be there anymore if say, the machine where it was
- * stored was already deleted, so we already removed it from
- * the dead threads and some other piece of code still keeps a
- * reference.
- *
- * This is what 'perf sched' does and finally drops it in
- * perf_sched__lat(), where it calls perf_sched__read_events(),
- * that processes the events by creating a session and deleting
- * it, which ends up destroying the list heads for the dead
- * threads, but before it does that it removes all threads from
- * it using list_del_init().
- *
- * So we need to check here if it is in a dead threads list and
- * if so, remove it before finally deleting the thread, to avoid
- * an use after free situation.
- */
- if (!list_empty(&thread->node))
- list_del_init(&thread->node);
+ if (thread && refcount_dec_and_test(&thread->refcnt))
thread__delete(thread);
- }
}
static struct namespaces *__thread__namespaces(const struct thread *thread)
diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h
index 395c626699a9..86737812e06b 100644
--- a/tools/perf/util/thread.h
+++ b/tools/perf/util/thread.h
@@ -30,10 +30,7 @@ struct lbr_stitch {
};
struct thread {
- union {
- struct rb_node rb_node;
- struct list_head node;
- };
+ struct rb_node rb_node;
struct maps *maps;
pid_t pid_; /* Not all tools update this */
pid_t tid;
@@ -43,7 +40,6 @@ struct thread {
refcount_t refcnt;
bool comm_set;
int comm_len;
- bool dead; /* if set thread has exited */
struct list_head namespaces_list;
struct rw_semaphore namespaces_lock;
struct list_head comm_list;
@@ -81,11 +77,6 @@ static inline void __thread__zput(struct thread **thread)
#define thread__zput(thread) __thread__zput(&thread)
-static inline void thread__exited(struct thread *thread)
-{
- thread->dead = true;
-}
-
struct namespaces *thread__namespaces(struct thread *thread);
int thread__set_namespaces(struct thread *thread, u64 timestamp,
struct perf_record_namespaces *event);
--
2.41.0.162.gfafddb0af9-goog
More information about the linux-arm-kernel
mailing list