From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
	id S1752930AbbC3NWV (ORCPT <rfc822;w@1wt.eu>);
	Mon, 30 Mar 2015 09:22:21 -0400
Received: from casper.infradead.org ([85.118.1.10]:33304 "EHLO
	casper.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1751806AbbC3NWT (ORCPT
	<rfc822;linux-kernel@vger.kernel.org>);
	Mon, 30 Mar 2015 09:22:19 -0400
Date: Mon, 30 Mar 2015 10:22:26 -0300
From: Arnaldo Carvalho de Melo <arnaldo.melo@gmail.com>
To: Jiri Olsa <jolsa@redhat.com>
Cc: David Ahern <dsahern@gmail.com>, Jiri Olsa <jolsa@kernel.org>,
        Namhyung Kim <namhyung@kernel.org>,
        Stephane Eranian <eranian@google.com>,
        LKML <linux-kernel@vger.kernel.org>
Subject: Re: [BUG] segfault in perf-top -- thread refcnt
Message-ID: <20150330132226.GD24063@kernel.org>
References: <551593EA.2030201@gmail.com>
 <20150327201126.GM21510@kernel.org>
 <5515B9E6.5020007@gmail.com>
 <20150330080737.GD1413@krava>
 <20150330102220.GE1413@krava>
 <20150330112108.GG1413@krava>
 <20150330114907.GH1413@krava>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20150330114907.GH1413@krava>
X-Url: http://acmel.wordpress.com
User-Agent: Mutt/1.5.23 (2014-03-12)
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Em Mon, Mar 30, 2015 at 01:49:07PM +0200, Jiri Olsa escreveu:
> On Mon, Mar 30, 2015 at 01:21:08PM +0200, Jiri Olsa wrote:
> > > 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?
 
> that was the reason.. we do this separately.. not in thread__put..
> is there a reason for this? ;-)
 
> testing attached patch..
 
>  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);

You can't do that, look at the definition of struct thread:

struct thread {
        union {
                struct rb_node   rb_node;
                struct list_head node;
        };

I.e. a thread can't be at the same time in the rbtree (live threads) and
on the list (dead threads).

When the thread dies (PERF_RECORD_EXIT) we transition it from the live
threads rb_tree to the dead_threads list, where it will wait till the
last hist_entry holding a reference to it to call thread__put(), when we
will want to remove it _only_ from the dead_threads _list_.

- Arnaldo