[ltt-dev] [UST PATCH] libust: New transport mutex v2

[Mathieu Desnoyers]

* Nils Carlson (nils.carlson at ericsson.com) wrote:
> Nils Carlson wrote:
>> Changes since v1:
>> 	Add a nesting comment
>>
>> Make the locking around the transport list more intelligent, give
>> it its own mutex.
>>
>> Signed-off-by: Nils Carlson <nils.carlson at ericsson.com>
>>   
> and merged. I'll continue trying to fix things here... I don't need fine  
> grained locking. But I think its reasonable that we can lock on a  
> per-trace basis eventually at least, so multiple different traces can be  
> modified simultaneously. We'll see how it goes though. No hurry.

OK. One thing to keep is mind is that the current UST versions have
strong cross-data-structure serialization dependencies between traces
and event ID assignment, which requires to serialize many operations all
across the place (e.g. start/stop traces and marker ID compaction). So I
would be really cautious about making the locking too fine-grained in
that area. (own lock for transport looks fine though)

This is all going to be cleaned up by the upcoming refactoring, because
we'll assign event IDs on a per-session and per-stream basis. Hurray! :)

Mathieu

>
> /Nils
>> ---
>>  libust/tracer.c |   14 +++++++++-----
>>  1 files changed, 9 insertions(+), 5 deletions(-)
>>
>> diff --git a/libust/tracer.c b/libust/tracer.c
>> index 100dec0..ffcc2e7 100644
>> --- a/libust/tracer.c
>> +++ b/libust/tracer.c
>> @@ -182,7 +182,8 @@ static enum ltt_channels get_channel_type_from_name(const char *name)
>>  //ust// }
>>   static CDS_LIST_HEAD(ltt_transport_list);
>> -
>> +/* transport mutex, nests inside traces mutex (ltt_lock_traces) */
>> +static DEFINE_MUTEX(ltt_transport_mutex);
>>  /**
>>   * ltt_transport_register - LTT transport registration
>>   * @transport: transport structure
>> @@ -204,9 +205,9 @@ void ltt_transport_register(struct ltt_transport *transport)
>>  	 */
>>  //ust//	vmalloc_sync_all();
>>  -	ltt_lock_traces();
>> +	pthread_mutex_lock(&ltt_transport_mutex);
>>  	cds_list_add_tail(&transport->node, &ltt_transport_list);
>> -	ltt_unlock_traces();
>> +	pthread_mutex_unlock(&ltt_transport_mutex);
>>  }
>>   /**
>> @@ -215,9 +216,9 @@ void ltt_transport_register(struct ltt_transport *transport)
>>   */
>>  void ltt_transport_unregister(struct ltt_transport *transport)
>>  {
>> -	ltt_lock_traces();
>> +	pthread_mutex_lock(&ltt_transport_mutex);
>>  	cds_list_del(&transport->node);
>> -	ltt_unlock_traces();
>> +	pthread_mutex_unlock(&ltt_transport_mutex);
>>  }
>>   static inline int is_channel_overwrite(enum ltt_channels chan,
>> @@ -458,12 +459,15 @@ int ltt_trace_set_type(const char *trace_name, const char *trace_type)
>>  		goto traces_error;
>>  	}
>>  +	pthread_mutex_lock(&ltt_transport_mutex);
>>  	cds_list_for_each_entry(tran_iter, &ltt_transport_list, node) {
>>  		if (!strcmp(tran_iter->name, trace_type)) {
>>  			transport = tran_iter;
>>  			break;
>>  		}
>>  	}
>> +	pthread_mutex_unlock(&ltt_transport_mutex);
>> +
>>  	if (!transport) {
>>  		ERR("Transport %s is not present", trace_type);
>>  		err = -EINVAL;
>>   
>
>
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>

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