[lttng-dev] [PATCH lttng-tools 1/3] Fix: Add missing call rcu and read side lock

[mathieu.desnoyers@efficios.com (Mathieu Desnoyers)]

* David Goulet (dgoulet at efficios.com) wrote:
> Signed-off-by: David Goulet <dgoulet at efficios.com>
> ---
>  src/common/consumer.c |    8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/src/common/consumer.c b/src/common/consumer.c
> index f01eb5d..7c3762e 100644
> --- a/src/common/consumer.c
> +++ b/src/common/consumer.c
> @@ -688,7 +688,9 @@ void consumer_del_channel(struct lttng_consumer_channel *channel)
>  		}
>  	}
>  
> +	rcu_read_lock();
>  	call_rcu(&channel->node.head, consumer_free_channel);
> +	rcu_read_unlock();

this rcu read lock is useless.

>  end:
>  	pthread_mutex_unlock(&consumer_data.lock);
>  }
> @@ -1535,7 +1537,7 @@ static void destroy_stream_ht(struct lttng_ht *ht)
>  		ret = lttng_ht_del(ht, &iter);
>  		assert(!ret);
>  
> -		free(stream);
> +		call_rcu(&stream->node.head, consumer_free_stream);

Good point. While you are there, please remove the bogus comment at the
top of destroy_stream_ht(). It does not take into account that
lttng_ht_del can trigger a concurrent resize (performed by call rcu
worker threads).


>  	}
>  	rcu_read_unlock();
>  
> @@ -1635,7 +1637,9 @@ static void consumer_del_metadata_stream(struct lttng_consumer_stream *stream)
>  		consumer_del_channel(stream->chan);
>  	}
>  
> -	free(stream);
> +	rcu_read_lock();
> +	call_rcu(&stream->node.head, consumer_free_stream);
> +	rcu_read_unlock();

call_rcu makes sense here, but not rcu read lock.

Thanks,

Mathieu

>  }
>  
>  /*
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev at lists.lttng.org
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

-- 
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com