* David Goulet ([email protected]) wrote:
> Signed-off-by: David Goulet <[email protected]>
> ---
> src/common/consumer.c | 66
> +++++++++++++++++++++++---
> src/common/kernel-consumer/kernel-consumer.c | 19 ++------
> src/common/ust-consumer/ust-consumer.c | 19 ++------
> 3 files changed, 66 insertions(+), 38 deletions(-)
>
> diff --git a/src/common/consumer.c b/src/common/consumer.c
> index e61a227..cf0e715 100644
> --- a/src/common/consumer.c
> +++ b/src/common/consumer.c
> @@ -2657,6 +2657,33 @@ error:
> }
>
> /*
> + * Try to lock the stream mutex.
> + *
> + * On success, 1 is returned else 0 indicating that the mutex is NOT lock.
> + */
> +static int stream_try_lock(struct lttng_consumer_stream *stream)
> +{
> + int ret;
> +
> + assert(stream);
> +
> + /*
> + * Try to lock the stream mutex. On failure, we know that the stream is
> + * being used else where hence there is data still being extracted.
> + */
> + ret = pthread_mutex_trylock(&stream->lock);
> + if (ret == EBUSY) {
> + ret = 0;
> + goto end;
> + }
what are we doing with the other errors ?
> +
> + ret = 1;
> +
> +end:
> + return ret;
> +}
> +
> +/*
> * Check if for a given session id there is still data needed to be extract
> * from the buffers.
> *
> @@ -2696,17 +2723,43 @@ int consumer_data_available(uint64_t id)
> ht->hash_fct((void *)((unsigned long) id), 0x42UL),
> ht->match_fct, (void *)((unsigned long) id),
> &iter.iter, stream, node_session_id.node) {
> - /* Check the stream for data. */
> - ret = data_available(stream);
> - if (ret == 0) {
> + /* If this call fails, the stream is being used hence data
> pending. */
> + ret = stream_try_lock(stream);
> + if (!ret) {
> goto data_not_available;
> }
>
> + /*
> + * A removed node from the hash table indicates that the stream
> has
> + * been deleted thus having a guarantee that the buffers are
> closed
> + * on the consumer side. However, data can still be transmitted
> + * over the network so don't skip the relayd check.
> + */
> + ret = cds_lfht_is_node_deleted(&stream->node.node);
> + if (!ret) {
> + /* Check the stream if there is data in the buffers. */
> + ret = data_available(stream);
> + if (ret == 0) {
> + pthread_mutex_unlock(&stream->lock);
> + goto data_not_available;
> + }
> + }
> +
> + /* Relayd check */
> if (stream->net_seq_idx != -1) {
> relayd = consumer_find_relayd(stream->net_seq_idx);
> - assert(relayd);
> + if (!relayd) {
> + /*
> + * At this point, if the relayd object is not
> available for the
> + * given stream, it is because the relayd is
> being cleanup so
cleanup -> cleaned up
> + * every stream associated with it (for a
> session id value) are
> + * or wil be marked for deletion hence not
> having data pending
hence not having -> hence do not have
wil -> will
> + * anymore.
> + */
> + pthread_mutex_unlock(&stream->lock);
> + goto data_not_available;
> + }
>
> - pthread_mutex_lock(&stream->lock);
> pthread_mutex_lock(&relayd->ctrl_sock_mutex);
> if (stream->metadata_flag) {
> ret =
> relayd_quiescent_control(&relayd->control_sock);
> @@ -2715,11 +2768,12 @@ int consumer_data_available(uint64_t id)
> stream->relayd_stream_id,
> stream->next_net_seq_num);
> }
> pthread_mutex_unlock(&relayd->ctrl_sock_mutex);
> - pthread_mutex_unlock(&stream->lock);
> if (ret == 0) {
> + pthread_mutex_unlock(&stream->lock);
dependency (lock nesting order) between
pthread_mutex_unlock(&relayd->ctrl_sock_mutex);
pthread_mutex_unlock(&stream->lock);
should be documented.
Thanks,
Mathieu
> goto data_not_available;
> }
> }
> + pthread_mutex_unlock(&stream->lock);
> }
>
> /*
> diff --git a/src/common/kernel-consumer/kernel-consumer.c
> b/src/common/kernel-consumer/kernel-consumer.c
> index 249df8a..196deee 100644
> --- a/src/common/kernel-consumer/kernel-consumer.c
> +++ b/src/common/kernel-consumer/kernel-consumer.c
> @@ -485,7 +485,8 @@ error:
>
> /*
> * Check if data is still being extracted from the buffers for a specific
> - * stream. Consumer data lock MUST be acquired before calling this function.
> + * stream. Consumer data lock MUST be acquired before calling this function
> + * and the stream lock.
> *
> * Return 0 if the traced data are still getting read else 1 meaning that the
> * data is available for trace viewer reading.
> @@ -496,31 +497,17 @@ int lttng_kconsumer_data_available(struct
> lttng_consumer_stream *stream)
>
> assert(stream);
>
> - /*
> - * Try to lock the stream mutex. On failure, we know that the stream is
> - * being used else where hence there is data still being extracted.
> - */
> - ret = pthread_mutex_trylock(&stream->lock);
> - if (ret == EBUSY) {
> - /* Data not available */
> - ret = 0;
> - goto end;
> - }
> - /* The stream is now locked so we can do our ustctl calls */
> -
> ret = kernctl_get_next_subbuf(stream->wait_fd);
> if (ret == 0) {
> /* There is still data so let's put back this subbuffer. */
> ret = kernctl_put_subbuf(stream->wait_fd);
> assert(ret == 0);
> - goto end_unlock;
> + goto end;
> }
>
> /* Data is available to be read for this stream. */
> ret = 1;
>
> -end_unlock:
> - pthread_mutex_unlock(&stream->lock);
> end:
> return ret;
> }
> diff --git a/src/common/ust-consumer/ust-consumer.c
> b/src/common/ust-consumer/ust-consumer.c
> index e8e3f93..4d3671a 100644
> --- a/src/common/ust-consumer/ust-consumer.c
> +++ b/src/common/ust-consumer/ust-consumer.c
> @@ -526,7 +526,8 @@ error:
>
> /*
> * Check if data is still being extracted from the buffers for a specific
> - * stream. Consumer data lock MUST be acquired before calling this function.
> + * stream. Consumer data lock MUST be acquired before calling this function
> + * and the stream lock.
> *
> * Return 0 if the traced data are still getting read else 1 meaning that the
> * data is available for trace viewer reading.
> @@ -539,31 +540,17 @@ int lttng_ustconsumer_data_available(struct
> lttng_consumer_stream *stream)
>
> DBG("UST consumer checking data availability");
>
> - /*
> - * Try to lock the stream mutex. On failure, we know that the stream is
> - * being used else where hence there is data still being extracted.
> - */
> - ret = pthread_mutex_trylock(&stream->lock);
> - if (ret == EBUSY) {
> - /* Data not available */
> - ret = 0;
> - goto end;
> - }
> - /* The stream is now locked so we can do our ustctl calls */
> -
> ret = ustctl_get_next_subbuf(stream->chan->handle, stream->buf);
> if (ret == 0) {
> /* There is still data so let's put back this subbuffer. */
> ret = ustctl_put_subbuf(stream->chan->handle, stream->buf);
> assert(ret == 0);
> - goto end_unlock;
> + goto end;
> }
>
> /* Data is available to be read for this stream. */
> ret = 1;
>
> -end_unlock:
> - pthread_mutex_unlock(&stream->lock);
> end:
> return ret;
> }
> --
> 1.7.10.4
>
>
> _______________________________________________
> lttng-dev mailing list
> [email protected]
> http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
--
Mathieu Desnoyers
Operating System Efficiency R&D Consultant
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
[email protected]
http://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
