Re: [PATCH 19/20] perf tools: Reference count struct thread
From: Namhyung Kim
Date: Tue Mar 03 2015 - 08:42:59 EST
Hi Arnaldo,
On Tue, Mar 03, 2015 at 12:26:08AM -0300, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
>
> 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.
[SNIP]
> 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);
[SNIP]
> 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);
> + }
> +}
I think we need to protect refcnt from concurrent accesses from
multiple threads. Not to mention my multi-thread work, perf top
already uses two threads.
For perf top case, hist_entry__new() will be called from main thread
and hist_entry__delete() might be called from display thread.
Thanks,
Namhyung
> +
> 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;
> --
> 1.9.3
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/