From 7ad0a0cb283d4004cf9274df8eec4e3530864fc7 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Tue, 15 Nov 2011 20:24:41 -0500 Subject: [PATCH] Fix multi-session UST handling combined with shm fd teardown shm fd is used as unique identifier for the channel when passing the stream fds to the consumer. However, closing the fd reuses the same identifier for the next round, thus getting the same consumer channel/streams as the previous one. This causes multi-session tracing to only work for the first session, not the following ones. So instead of doing a channel lookup on add channel and a stream lookup upon add stream, we "steal" the identifier. We still lookup the channel identifier upon stream add though: this means the sessiond needs to keep the channel shm fd open until it has finished sending all stream fds to the consumer. For kernel consumer, the update operation still does a stream id lookup. Signed-off-by: Mathieu Desnoyers --- liblttng-consumer/lttng-consumer.c | 43 ++++++++++++++++-------- liblttng-ustconsumer/lttng-ustconsumer.c | 5 ++- lttng-sessiond/ust-consumer.c | 7 ++-- 3 files changed, 38 insertions(+), 17 deletions(-) diff --git a/liblttng-consumer/lttng-consumer.c b/liblttng-consumer/lttng-consumer.c index c7c7b7a7e..293d5f808 100644 --- a/liblttng-consumer/lttng-consumer.c +++ b/liblttng-consumer/lttng-consumer.c @@ -63,6 +63,9 @@ static struct lttng_consumer_stream *consumer_find_stream(int key) { struct lttng_consumer_stream *iter; + /* Negative keys are lookup failures */ + if (key < 0) + return NULL; cds_list_for_each_entry(iter, &consumer_data.stream_list.head, list) { if (iter->key == key) { DBG("Found stream key %d", key); @@ -72,10 +75,22 @@ static struct lttng_consumer_stream *consumer_find_stream(int key) return NULL; } +static void consumer_steal_stream_key(int key) +{ + struct lttng_consumer_stream *stream; + + stream = consumer_find_stream(key); + if (stream) + stream->key = -1; +} + static struct lttng_consumer_channel *consumer_find_channel(int key) { struct lttng_consumer_channel *iter; + /* Negative keys are lookup failures */ + if (key < 0) + return NULL; cds_list_for_each_entry(iter, &consumer_data.channel_list.head, list) { if (iter->key == key) { DBG("Found channel key %d", key); @@ -85,6 +100,15 @@ static struct lttng_consumer_channel *consumer_find_channel(int key) return NULL; } +static void consumer_steal_channel_key(int key) +{ + struct lttng_consumer_channel *channel; + + channel = consumer_find_channel(key); + if (channel) + channel->key = -1; +} + /* * Remove a stream from the global list protected by a mutex. This * function is also responsible for freeing its data structures. @@ -211,11 +235,8 @@ int consumer_add_stream(struct lttng_consumer_stream *stream) int ret = 0; pthread_mutex_lock(&consumer_data.lock); - /* Check if already exist */ - if (consumer_find_stream(stream->key)) { - ret = -1; - goto end; - } + /* Steal stream identifier, for UST */ + consumer_steal_stream_key(stream->key); cds_list_add(&stream->list, &consumer_data.stream_list.head); consumer_data.stream_count++; consumer_data.need_update = 1; @@ -350,18 +371,12 @@ end: */ int consumer_add_channel(struct lttng_consumer_channel *channel) { - int ret = 0; - pthread_mutex_lock(&consumer_data.lock); - /* Check if already exist */ - if (consumer_find_channel(channel->key)) { - ret = -1; - goto end; - } + /* Steal channel identifier, for UST */ + consumer_steal_channel_key(channel->key); cds_list_add(&channel->list, &consumer_data.channel_list.head); -end: pthread_mutex_unlock(&consumer_data.lock); - return ret; + return 0; } /* diff --git a/liblttng-ustconsumer/lttng-ustconsumer.c b/liblttng-ustconsumer/lttng-ustconsumer.c index ed4a27c9d..f938d3548 100644 --- a/liblttng-ustconsumer/lttng-ustconsumer.c +++ b/liblttng-ustconsumer/lttng-ustconsumer.c @@ -209,7 +209,7 @@ int lttng_ustconsumer_recv_cmd(struct lttng_consumer_local_data *ctx, DBG("consumer_add_stream %s (%d,%d)", msg.u.stream.path_name, fds[0], fds[1]); assert(msg.u.stream.output == LTTNG_EVENT_MMAP); - new_stream = consumer_allocate_stream(msg.u.stream.channel_key, + new_stream = consumer_allocate_stream(msg.u.channel.channel_key, msg.u.stream.stream_key, fds[0], fds[1], msg.u.stream.state, @@ -234,6 +234,8 @@ int lttng_ustconsumer_recv_cmd(struct lttng_consumer_local_data *ctx, } case LTTNG_CONSUMER_UPDATE_STREAM: { + return -ENOSYS; +#if 0 if (ctx->on_update_stream != NULL) { ret = ctx->on_update_stream(msg.u.stream.stream_key, msg.u.stream.state); if (ret == 0) { @@ -245,6 +247,7 @@ int lttng_ustconsumer_recv_cmd(struct lttng_consumer_local_data *ctx, consumer_change_stream_state(msg.u.stream.stream_key, msg.u.stream.state); } +#endif break; } default: diff --git a/lttng-sessiond/ust-consumer.c b/lttng-sessiond/ust-consumer.c index df569368a..46558417e 100644 --- a/lttng-sessiond/ust-consumer.c +++ b/lttng-sessiond/ust-consumer.c @@ -46,8 +46,11 @@ static int send_channel_streams(int sock, lum.cmd_type = LTTNG_CONSUMER_ADD_CHANNEL; /* - * We need to keep shm_fd open to make sure this key stays unique within - * the session daemon. + * We need to keep shm_fd open while we transfer the stream file + * descriptors to make sure this key stays unique within the + * session daemon. We can free the channel shm_fd without + * problem after we finished sending stream fds for that + * channel. */ lum.u.channel.channel_key = uchan->obj->shm_fd; lum.u.channel.max_sb_size = uchan->attr.subbuf_size; -- 2.34.1