[lttng-dev] [PATCH lttng-tools] UST periodical metadata flush

[David Goulet]

Mathieu Desnoyers:
>>>>>>   * For a given application and session, push metadata to consumer. The session
>>>>>>   * lock MUST be acquired here before calling this.
>>>>>> + * Either sock or consumer is required : if sock is NULL, the default
>>>>>> + * socket to send the metadata is retrieved from consumer, if sock
>>>>>> + * is not NULL we use it to send the metadata.
>>>>>>   *
>>>>>>   * Return 0 on success else a negative error.
>>>>>>   */
>>>>>>  static int push_metadata(struct ust_registry_session *registry,
>>>>>>  		struct consumer_output *consumer)
>>>>>>  {
>>>>>> -	int ret;
>>>>>> -	char *metadata_str = NULL;
>>>>>> -	size_t len, offset;
>>>>>> +	int ret_val;
>>>>>> +	ssize_t ret;
>>>>>>  	struct consumer_socket *socket;
>>>>>>  
>>>>>>  	assert(registry);
>>>>>> @@ -391,7 +447,7 @@ static int push_metadata(struct ust_registry_session *registry,
>>>>>>  	 * no start has been done previously.
>>>>>>  	 */
>>>>>>  	if (!registry->metadata_key) {
>>>>>> -		ret = 0;
>>>>>> +		ret_val = 0;
>>>>>>  		goto error_rcu_unlock;
>>>>>>  	}
>>>>>>  
>>>>>> @@ -399,7 +455,7 @@ static int push_metadata(struct ust_registry_session *registry,
>>>>>>  	socket = consumer_find_socket_by_bitness(registry->bits_per_long,
>>>>>>  			consumer);
>>>>>>  	if (!socket) {
>>>>>> -		ret = -1;
>>>>>> +		ret_val = -1;
>>>>>>  		goto error_rcu_unlock;
>>>>>>  	}
>>>>>>  
>>>>>> @@ -414,54 +470,19 @@ static int push_metadata(struct ust_registry_session *registry,
>>>>>>  	 * ability to reorder the metadata it receives.
>>>>>>  	 */
>>>>>>  	pthread_mutex_lock(socket->lock);
>>>>>> -	pthread_mutex_lock(&registry->lock);
>>>
>>> another question: We don't seem to need nesting of the registry lock
>>> inside the socket lock anymore on the sessiond side. We probably have
>>> some comments to update around those locks elsewhere ?
>>
>> Well not sure here. This function has been split in two where this one
>> finds the socket and lock it then calls ust_app_push_metadata that
>> *locks* the registry. So there is still nesting needed.
>> ust_app_push_metadata() *MUST* be called with the socket lock (it is
>> documented). So to that extent, the registry lock still nest in the
>> socket lock.
> 
> Why do we need to lock the socket lock insite push_metadata() ?
> 
> We could probably take both locks one after the other (not nested)
> within ust_app_push_metadata(), given that we now allow out-of-order
> metadata segments ?

The problem is with the new way we request metadata coming from the
consumer. We need to lock the socket through the receive command up to
the push metadata call (ust_app_push_metadata). We have now two call
sites that use this function so the split was needed where one function
finds the socket object, lock it and calls the push metadata. (see
ust_consumer_metadata_request() in sessiond/ust-consumer.c for the
second call site).

You see a better way of doing it ?

David

> 
> Thanks,
> 
> Mathieu
>