Re: [BUG] segfault in perf-top -- thread refcnt

[Arnaldo Carvalho de Melo]

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
--
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/