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

[Nils Carlson]

Mathieu Desnoyers wrote:
> * 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! :)
>
>   
Sounds good. I'm going to try to keep my work at quite a high level, not 
digging much lower than the trace level. My goal is to cleanup tracectl 
a bit more.

David: Has there been any progress on the session daemon? I could maybe 
lend a hand there? I'd be happy to help in drafting an api if you want.

/Nils

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