From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756635AbbCCNmz (ORCPT ); Tue, 3 Mar 2015 08:42:55 -0500 Received: from mail-pd0-f180.google.com ([209.85.192.180]:42531 "EHLO mail-pd0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756198AbbCCNmy (ORCPT ); Tue, 3 Mar 2015 08:42:54 -0500 Date: Tue, 3 Mar 2015 22:42:14 +0900 From: Namhyung Kim To: Arnaldo Carvalho de Melo Cc: Ingo Molnar , linux-kernel@vger.kernel.org, Arnaldo Carvalho de Melo , Adrian Hunter , Borislav Petkov , David Ahern , Don Zickus , Frederic Weisbecker , Jiri Olsa , Stephane Eranian Subject: Re: [PATCH 19/20] perf tools: Reference count struct thread Message-ID: <20150303134214.GB27046@danjae> References: <1425353169-21436-1-git-send-email-acme@kernel.org> <1425353169-21436-20-git-send-email-acme@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <1425353169-21436-20-git-send-email-acme@kernel.org> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Arnaldo, On Tue, Mar 03, 2015 at 12:26:08AM -0300, Arnaldo Carvalho de Melo wrote: > From: Arnaldo Carvalho de Melo > > 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 >