summary |
shortlog |
log |
commit | commitdiff |
tree
raw |
patch |
inline | side by side (from parent 1:
8192bd8)
We currently use the channel FD number opened by the session daemon to
reference a channel in the consumer. This can lead to races where the
session daemon destroys a channel and recreates one with the same FD
number before the consumer has time to cleanup everything on its side,
so all the commands in between that use that FD number has a key may end
up working on the wrong objects.
This fix introduces a free running counter as the channel key, so this
decouples the channel key in the consumer from the channel FD in the
session daemon. This fixes the race observed in stress tests.
Signed-off-by: Julien Desfossez <jdesfossez@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
- ret = consumer_get_discarded_events(session->id, kchan->fd,
+ ret = consumer_get_discarded_events(session->id, kchan->key,
session->kernel_session->consumer,
discarded_events);
if (ret < 0) {
goto end;
}
session->kernel_session->consumer,
discarded_events);
if (ret < 0) {
goto end;
}
- ret = consumer_get_lost_packets(session->id, kchan->fd,
+ ret = consumer_get_lost_packets(session->id, kchan->key,
session->kernel_session->consumer,
lost_packets);
if (ret < 0) {
session->kernel_session->consumer,
lost_packets);
if (ret < 0) {
#include <stdlib.h>
#include <sys/stat.h>
#include <unistd.h>
#include <stdlib.h>
#include <sys/stat.h>
#include <unistd.h>
#include <common/common.h>
#include <common/defaults.h>
#include <common/common.h>
#include <common/defaults.h>
/* Prep channel message structure */
consumer_init_channel_comm_msg(&lkm,
LTTNG_CONSUMER_ADD_CHANNEL,
/* Prep channel message structure */
consumer_init_channel_comm_msg(&lkm,
LTTNG_CONSUMER_ADD_CHANNEL,
ksession->id,
pathname,
ksession->uid,
ksession->id,
pathname,
ksession->uid,
status = notification_thread_command_add_channel(
notification_thread_handle, session->name,
ksession->uid, ksession->gid,
status = notification_thread_command_add_channel(
notification_thread_handle, session->name,
ksession->uid, ksession->gid,
- channel->channel->name, channel->fd,
+ channel->channel->name, channel->key,
LTTNG_DOMAIN_KERNEL,
channel->channel->attr.subbuf_size * channel->channel->attr.num_subbuf);
rcu_read_unlock();
LTTNG_DOMAIN_KERNEL,
channel->channel->attr.subbuf_size * channel->channel->attr.num_subbuf);
rcu_read_unlock();
/* Prep stream consumer message */
consumer_init_stream_comm_msg(&lkm,
LTTNG_CONSUMER_ADD_STREAM,
/* Prep stream consumer message */
consumer_init_stream_comm_msg(&lkm,
LTTNG_CONSUMER_ADD_STREAM,
stream->fd,
stream->cpu);
stream->fd,
stream->cpu);
* Inform the relay that all the streams for the
* channel were sent.
*/
* Inform the relay that all the streams for the
* channel were sent.
*/
- ret = kernel_consumer_streams_sent(sock, session, chan->fd);
+ ret = kernel_consumer_streams_sent(sock, session, chan->key);
if (ret < 0) {
goto error;
}
if (ret < 0) {
goto error;
}
assert(channel);
assert(socket);
assert(channel);
assert(socket);
- DBG("Sending kernel consumer destroy channel key %d", channel->fd);
+ DBG("Sending kernel consumer destroy channel key %" PRIu64, channel->key);
memset(&msg, 0, sizeof(msg));
msg.cmd_type = LTTNG_CONSUMER_DESTROY_CHANNEL;
memset(&msg, 0, sizeof(msg));
msg.cmd_type = LTTNG_CONSUMER_DESTROY_CHANNEL;
- msg.u.destroy_channel.key = channel->fd;
+ msg.u.destroy_channel.key = channel->key;
pthread_mutex_lock(socket->lock);
health_code_update();
pthread_mutex_lock(socket->lock);
health_code_update();
#include "kern-modules.h"
#include "utils.h"
#include "kern-modules.h"
#include "utils.h"
+/*
+ * Key used to reference a channel between the sessiond and the consumer. This
+ * is only read and updated with the session_list lock held.
+ */
+static uint64_t next_kernel_channel_key;
+
/*
* Add context on a kernel channel.
*
/*
* Add context on a kernel channel.
*
cds_list_add(&lkc->list, &session->channel_list.head);
session->channel_count++;
lkc->session = session;
cds_list_add(&lkc->list, &session->channel_list.head);
session->channel_count++;
lkc->session = session;
+ lkc->key = ++next_kernel_channel_key;
- DBG("Kernel channel %s created (fd: %d)", lkc->channel->name, lkc->fd);
+ DBG("Kernel channel %s created (fd: %d, key: %" PRIu64 ")",
+ lkc->channel->name, lkc->fd, lkc->key);
- DBG("Kernel channel %s disabled (fd: %d)", chan->channel->name, chan->fd);
+ DBG("Kernel channel %s disabled (fd: %d, key: %" PRIu64 ")",
+ chan->channel->name, chan->fd, chan->key);
- DBG("Kernel channel %s enabled (fd: %d)", chan->channel->name, chan->fd);
+ DBG("Kernel channel %s enabled (fd: %d, key: %" PRIu64 ")",
+ chan->channel->name, chan->fd, chan->key);
&& channel->published_to_notification_thread) {
status = notification_thread_command_remove_channel(
notification_thread_handle,
&& channel->published_to_notification_thread) {
status = notification_thread_command_remove_channel(
notification_thread_handle,
- channel->fd, LTTNG_DOMAIN_KERNEL);
+ channel->key, LTTNG_DOMAIN_KERNEL);
assert(status == LTTNG_OK);
}
free(channel->channel->attr.extended.ptr);
assert(status == LTTNG_OK);
}
free(channel->channel->attr.extended.ptr);
/* Kernel channel */
struct ltt_kernel_channel {
int fd;
/* Kernel channel */
struct ltt_kernel_channel {
int fd;
+ uint64_t key; /* Key to reference this channel with the consumer. */
int enabled;
unsigned int stream_count;
unsigned int event_count;
int enabled;
unsigned int stream_count;
unsigned int event_count;