Hi Arnaldo,

On Tue, Mar 03, 2015 at 12:26:08AM -0300, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <[email protected]>
> 
> 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 [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to