* David Goulet ([email protected]) wrote: > > > 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(®istry->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 ?
After discussing over phone, we will nest the registry lock inside the socket lock. Thanks, Mathieu > > David > > > > > Thanks, > > > > Mathieu > > -- Mathieu Desnoyers EfficiOS Inc. http://www.efficios.com _______________________________________________ lttng-dev mailing list [email protected] http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
