From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752752AbbC3KWZ (ORCPT ); Mon, 30 Mar 2015 06:22:25 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42951 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752400AbbC3KWX (ORCPT ); Mon, 30 Mar 2015 06:22:23 -0400 Date: Mon, 30 Mar 2015 12:22:20 +0200 From: Jiri Olsa To: David Ahern Cc: Arnaldo Carvalho de Melo , Jiri Olsa , Namhyung Kim , Stephane Eranian , LKML Subject: Re: [BUG] segfault in perf-top -- thread refcnt Message-ID: <20150330102220.GE1413@krava> References: <551593EA.2030201@gmail.com> <20150327201126.GM21510@kernel.org> <5515B9E6.5020007@gmail.com> <20150330080737.GD1413@krava> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20150330080737.GD1413@krava> 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 On Mon, Mar 30, 2015 at 10:07:37AM +0200, Jiri Olsa wrote: SNIP > > > > 2 things: > > 1. let run for a long time. go about using the server. do lots of builds, > > etc. it takes time > > > > 2. use a box with a LOT of cpus (1024 in my case) > > > > Make sure ulimit is set to get the core. > > reproduced under 24 cpu box with kernel build (make -j25) > running on background.. will try to look closer > > perf: Segmentation fault > -------- backtrace -------- > ./perf[0x4fd79b] > /lib64/libc.so.6(+0x358f0)[0x7f9cbff528f0] > ./perf(thread__put+0x5b)[0x4b1a7b] > ./perf(hists__delete_entries+0x70)[0x4c8670] > ./perf[0x436a88] > ./perf[0x4fa73d] > ./perf(perf_evlist__tui_browse_hists+0x97)[0x4fc437] > ./perf[0x4381d0] > /lib64/libpthread.so.0(+0x7ee5)[0x7f9cc1ff2ee5] > /lib64/libc.so.6(clone+0x6d)[0x7f9cc0011b8d] > [0x0] looks like race among __machine__findnew_thread and thread__put over the machine->threads rb_tree insert/removal is there a reason why thread__put does not erase itself from machine->threads? I'm trying attached patch.. so far so gut ;-) jirka --- diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index e335330..7e6abc7 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -30,6 +30,7 @@ int machine__init(struct machine *machine, const char *root_dir, pid_t pid) dsos__init(&machine->kernel_dsos); machine->threads = RB_ROOT; + pthread_mutex_init(&machine->threads_lock, NULL); INIT_LIST_HEAD(&machine->dead_threads); machine->last_match = NULL; @@ -380,10 +381,13 @@ static struct thread *__machine__findnew_thread(struct machine *machine, if (!create) return NULL; - th = thread__new(pid, tid); + th = thread__new(machine, pid, tid); if (th != NULL) { + + pthread_mutex_lock(&machine->threads_lock); rb_link_node(&th->rb_node, parent, p); rb_insert_color(&th->rb_node, &machine->threads); + pthread_mutex_unlock(&machine->threads_lock); /* * We have to initialize map_groups separately @@ -394,8 +398,10 @@ static struct thread *__machine__findnew_thread(struct machine *machine, * leader and that would screwed the rb tree. */ if (thread__init_map_groups(th, machine)) { + pthread_mutex_lock(&machine->threads_lock); rb_erase(&th->rb_node, &machine->threads); thread__delete(th); + pthread_mutex_unlock(&machine->threads_lock); return NULL; } /* diff --git a/tools/perf/util/machine.h b/tools/perf/util/machine.h index e2faf3b..e3468d6 100644 --- a/tools/perf/util/machine.h +++ b/tools/perf/util/machine.h @@ -30,6 +30,7 @@ struct machine { bool comm_exec; char *root_dir; struct rb_root threads; + pthread_mutex_t threads_lock; struct list_head dead_threads; struct thread *last_match; struct vdso_info *vdso_info; diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c index 1c8fbc9..4592fc4 100644 --- a/tools/perf/util/thread.c +++ b/tools/perf/util/thread.c @@ -26,7 +26,7 @@ int thread__init_map_groups(struct thread *thread, struct machine *machine) return thread->mg ? 0 : -1; } -struct thread *thread__new(pid_t pid, pid_t tid) +struct thread *thread__new(struct machine *machine, pid_t pid, pid_t tid) { char *comm_str; struct comm *comm; @@ -38,6 +38,7 @@ struct thread *thread__new(pid_t pid, pid_t tid) thread->ppid = -1; thread->cpu = -1; INIT_LIST_HEAD(&thread->comm_list); + thread->machine = machine; if (unwind__prepare_access(thread) < 0) goto err_thread; @@ -91,7 +92,14 @@ struct thread *thread__get(struct thread *thread) void thread__put(struct thread *thread) { if (thread && --thread->refcnt == 0) { + struct machine *machine = thread->machine; + list_del_init(&thread->node); + + pthread_mutex_lock(&machine->threads_lock); + rb_erase(&thread->rb_node, &machine->threads); + pthread_mutex_unlock(&machine->threads_lock); + thread__delete(thread); } } diff --git a/tools/perf/util/thread.h b/tools/perf/util/thread.h index 9b8a54d..df6fb69 100644 --- a/tools/perf/util/thread.h +++ b/tools/perf/util/thread.h @@ -31,12 +31,13 @@ struct thread { void *priv; struct thread_stack *ts; + struct machine *machine; }; struct machine; struct comm; -struct thread *thread__new(pid_t pid, pid_t tid); +struct thread *thread__new(struct machine *machine, pid_t pid, pid_t tid); int thread__init_map_groups(struct thread *thread, struct machine *machine); void thread__delete(struct thread *thread);