Fix: add a kernel context list to the channel

The internal state of the session daemon was recording only one context
per channel thus overwriting the previous one if multiple context were
added causing a memory leak.

This commit adds a list inside a kernel channel which keeps track of all
context added. It also fixes the save command that now saves all of
them.

Fixes #205

Signed-off-by: David Goulet <dgoulet@efficios.com>

diff --git a/src/bin/lttng-sessiond/context.c b/src/bin/lttng-sessiond/context.c
index 1b87df2e90ab03a3d95192a5afc102fc21404397..10352d026952ec3361acccf6aa07ac97bab4b34f 100644 (file)
--- a/src/bin/lttng-sessiond/context.c
+++ b/src/bin/lttng-sessiond/context.c
@@ -34,7 +34,7 @@
  * Add kernel context to all channel.
  */
 static int add_kctx_all_channels(struct ltt_kernel_session *ksession,
-               struct lttng_kernel_context *kctx)
+               struct ltt_kernel_context *kctx)
 {
        int ret;
        struct ltt_kernel_channel *kchan;
@@ -62,7 +62,7 @@ error:
 /*
  * Add kernel context to a specific channel.
  */
-static int add_kctx_to_channel(struct lttng_kernel_context *kctx,
+static int add_kctx_to_channel(struct ltt_kernel_context *kctx,
                struct ltt_kernel_channel *kchan)
 {
        int ret;
@@ -149,60 +149,66 @@ int context_kernel_add(struct ltt_kernel_session *ksession,
 {
        int ret;
        struct ltt_kernel_channel *kchan;
-       struct lttng_kernel_context kctx;
+       struct ltt_kernel_context *kctx;
 
        assert(ksession);
        assert(ctx);
        assert(channel_name);
 
+       kctx = trace_kernel_create_context(NULL);
+       if (!kctx) {
+               ret = LTTNG_ERR_NOMEM;
+               goto error;
+       }
+
        /* Setup kernel context structure */
        switch (ctx->ctx) {
        case LTTNG_EVENT_CONTEXT_PID:
-               kctx.ctx = LTTNG_KERNEL_CONTEXT_PID;
+               kctx->ctx.ctx = LTTNG_KERNEL_CONTEXT_PID;
                break;
        case LTTNG_EVENT_CONTEXT_PROCNAME:
-               kctx.ctx = LTTNG_KERNEL_CONTEXT_PROCNAME;
+               kctx->ctx.ctx = LTTNG_KERNEL_CONTEXT_PROCNAME;
                break;
        case LTTNG_EVENT_CONTEXT_PRIO:
-               kctx.ctx = LTTNG_KERNEL_CONTEXT_PRIO;
+               kctx->ctx.ctx = LTTNG_KERNEL_CONTEXT_PRIO;
                break;
        case LTTNG_EVENT_CONTEXT_NICE:
-               kctx.ctx = LTTNG_KERNEL_CONTEXT_NICE;
+               kctx->ctx.ctx = LTTNG_KERNEL_CONTEXT_NICE;
                break;
        case LTTNG_EVENT_CONTEXT_VPID:
-               kctx.ctx = LTTNG_KERNEL_CONTEXT_VPID;
+               kctx->ctx.ctx = LTTNG_KERNEL_CONTEXT_VPID;
                break;
        case LTTNG_EVENT_CONTEXT_TID:
-               kctx.ctx = LTTNG_KERNEL_CONTEXT_TID;
+               kctx->ctx.ctx = LTTNG_KERNEL_CONTEXT_TID;
                break;
        case LTTNG_EVENT_CONTEXT_VTID:
-               kctx.ctx = LTTNG_KERNEL_CONTEXT_VTID;
+               kctx->ctx.ctx = LTTNG_KERNEL_CONTEXT_VTID;
                break;
        case LTTNG_EVENT_CONTEXT_PPID:
-               kctx.ctx = LTTNG_KERNEL_CONTEXT_PPID;
+               kctx->ctx.ctx = LTTNG_KERNEL_CONTEXT_PPID;
                break;
        case LTTNG_EVENT_CONTEXT_VPPID:
-               kctx.ctx = LTTNG_KERNEL_CONTEXT_VPPID;
+               kctx->ctx.ctx = LTTNG_KERNEL_CONTEXT_VPPID;
                break;
        case LTTNG_EVENT_CONTEXT_HOSTNAME:
-               kctx.ctx = LTTNG_KERNEL_CONTEXT_HOSTNAME;
+               kctx->ctx.ctx = LTTNG_KERNEL_CONTEXT_HOSTNAME;
                break;
        case LTTNG_EVENT_CONTEXT_PERF_CPU_COUNTER:
        case LTTNG_EVENT_CONTEXT_PERF_COUNTER:
-               kctx.ctx = LTTNG_KERNEL_CONTEXT_PERF_CPU_COUNTER;
+               kctx->ctx.ctx = LTTNG_KERNEL_CONTEXT_PERF_CPU_COUNTER;
                break;
        default:
                return LTTNG_ERR_KERN_CONTEXT_FAIL;
        }
 
-       kctx.u.perf_counter.type = ctx->u.perf_counter.type;
-       kctx.u.perf_counter.config = ctx->u.perf_counter.config;
-       strncpy(kctx.u.perf_counter.name, ctx->u.perf_counter.name,
+       kctx->ctx.u.perf_counter.type = ctx->u.perf_counter.type;
+       kctx->ctx.u.perf_counter.config = ctx->u.perf_counter.config;
+       strncpy(kctx->ctx.u.perf_counter.name, ctx->u.perf_counter.name,
                        LTTNG_SYMBOL_NAME_LEN);
-       kctx.u.perf_counter.name[LTTNG_SYMBOL_NAME_LEN - 1] = '\0';
+       kctx->ctx.u.perf_counter.name[LTTNG_SYMBOL_NAME_LEN - 1] = '\0';
 
        if (*channel_name == '\0') {
-               ret = add_kctx_all_channels(ksession, &kctx);
+               ret = add_kctx_all_channels(ksession, kctx);
                if (ret != LTTNG_OK) {
                        goto error;
                }
@@ -214,7 +220,7 @@ int context_kernel_add(struct ltt_kernel_session *ksession,
                        goto error;
                }
 
-               ret = add_kctx_to_channel(&kctx, kchan);
+               ret = add_kctx_to_channel(kctx, kchan);
                if (ret != LTTNG_OK) {
                        goto error;
                }

diff --git a/src/bin/lttng-sessiond/kernel.c b/src/bin/lttng-sessiond/kernel.c
index 44a71fc595de1427682bf84b19991bf0725da1bd..b997a4aeab6b7ac0aac7208fa82bacd6e964ce28 100644 (file)
--- a/src/bin/lttng-sessiond/kernel.c
+++ b/src/bin/lttng-sessiond/kernel.c
@@ -37,7 +37,7 @@
  * Add context on a kernel channel.
  */
 int kernel_add_channel_context(struct ltt_kernel_channel *chan,
-               struct lttng_kernel_context *ctx)
+               struct ltt_kernel_context *ctx)
 {
        int ret;
 
@@ -45,7 +45,7 @@ int kernel_add_channel_context(struct ltt_kernel_channel *chan,
        assert(ctx);
 
        DBG("Adding context to channel %s", chan->channel->name);
-       ret = kernctl_add_context(chan->fd, ctx);
+       ret = kernctl_add_context(chan->fd, &ctx->ctx);
        if (ret < 0) {
                if (errno != EEXIST) {
                        PERROR("add context ioctl");
@@ -56,13 +56,7 @@ int kernel_add_channel_context(struct ltt_kernel_channel *chan,
                goto error;
        }
 
-       chan->ctx = zmalloc(sizeof(struct lttng_kernel_context));
-       if (chan->ctx == NULL) {
-               PERROR("zmalloc event context");
-               goto error;
-       }
-
-       memcpy(chan->ctx, ctx, sizeof(struct lttng_kernel_context));
+       cds_list_add_tail(&ctx->list, &chan->ctx_list);
 
        return 0;
 

diff --git a/src/bin/lttng-sessiond/kernel.h b/src/bin/lttng-sessiond/kernel.h
index 78d12aba3ddc0bd68bd0699181f9457eca898075..e7947801257557befe55b32de144c81c2d2c415c 100644 (file)
--- a/src/bin/lttng-sessiond/kernel.h
+++ b/src/bin/lttng-sessiond/kernel.h
@@ -32,7 +32,7 @@
 #define KERNEL_EVENT_INIT_LIST_SIZE 64
 
 int kernel_add_channel_context(struct ltt_kernel_channel *chan,
-               struct lttng_kernel_context *ctx);
+               struct ltt_kernel_context *ctx);
 int kernel_create_session(struct ltt_session *session, int tracer_fd);
 int kernel_create_channel(struct ltt_kernel_session *session,
                struct lttng_channel *chan);

diff --git a/src/bin/lttng-sessiond/save.c b/src/bin/lttng-sessiond/save.c
index 8afdbb5002bcb9a7e0bf8ef1d3871ed6e1c7d300..684c4edbf9c80d5dd3089dde71c6b130772efad8 100644 (file)
--- a/src/bin/lttng-sessiond/save.c
+++ b/src/bin/lttng-sessiond/save.c
@@ -670,12 +670,6 @@ int save_kernel_context(struct config_writer *writer,
                goto end;
        }
 
-       ret = config_writer_open_element(writer, config_element_contexts);
-       if (ret) {
-               ret = LTTNG_ERR_SAVE_IO_FAIL;
-               goto end;
-       }
-
        ret = config_writer_open_element(writer, config_element_context);
        if (ret) {
                ret = LTTNG_ERR_SAVE_IO_FAIL;
@@ -741,6 +735,30 @@ int save_kernel_context(struct config_writer *writer,
                goto end;
        }
 
+end:
+       return ret;
+}
+
+static
+int save_kernel_contexts(struct config_writer *writer,
+               struct ltt_kernel_channel *kchan)
+{
+       int ret;
+       struct ltt_kernel_context *ctx;
+
+       ret = config_writer_open_element(writer, config_element_contexts);
+       if (ret) {
+               ret = LTTNG_ERR_SAVE_IO_FAIL;
+               goto end;
+       }
+
+       cds_list_for_each_entry(ctx, &kchan->ctx_list, list) {
+               ret = save_kernel_context(writer, &ctx->ctx);
+               if (ret) {
+                       goto end;
+               }
+       }
+
        /* /contexts */
        ret = config_writer_close_element(writer);
        if (ret) {
@@ -848,7 +866,7 @@ int save_kernel_channel(struct config_writer *writer,
                goto end;
        }
 
-       ret = save_kernel_context(writer, kchan->ctx);
+       ret = save_kernel_contexts(writer, kchan);
        if (ret) {
                goto end;
        }

diff --git a/src/bin/lttng-sessiond/trace-kernel.c b/src/bin/lttng-sessiond/trace-kernel.c
index 363d010a1a2aed47213e51ae240ed3a00fd5d460..8e074fd91b0dcbe829f8c053d32ac6e92766d32c 100644 (file)
--- a/src/bin/lttng-sessiond/trace-kernel.c
+++ b/src/bin/lttng-sessiond/trace-kernel.c
@@ -165,10 +165,10 @@ struct ltt_kernel_channel *trace_kernel_create_channel(
        lkc->stream_count = 0;
        lkc->event_count = 0;
        lkc->enabled = 1;
-       lkc->ctx = NULL;
        /* Init linked list */
        CDS_INIT_LIST_HEAD(&lkc->events_list.head);
        CDS_INIT_LIST_HEAD(&lkc->stream_list.head);
+       CDS_INIT_LIST_HEAD(&lkc->ctx_list);
 
        return lkc;
 
@@ -176,6 +176,30 @@ error:
        return NULL;
 }
 
+/*
+ * Allocate and init a kernel context object.
+ *
+ * Return the allocated object or NULL on error.
+ */
+struct ltt_kernel_context *trace_kernel_create_context(
+               struct lttng_kernel_context *ctx)
+{
+       struct ltt_kernel_context *kctx;
+
+       kctx = zmalloc(sizeof(*kctx));
+       if (!kctx) {
+               PERROR("zmalloc kernel context");
+               goto error;
+       }
+
+       if (ctx) {
+               memcpy(&kctx->ctx, ctx, sizeof(kctx->ctx));
+       }
+
+error:
+       return kctx;
+}
+
 /*
  * Allocate and initialize a kernel event. Set name and event type.
  *
@@ -375,6 +399,17 @@ void trace_kernel_destroy_event(struct ltt_kernel_event *event)
        free(event);
 }
 
+/*
+ * Cleanup kernel context structure.
+ */
+void trace_kernel_destroy_context(struct ltt_kernel_context *ctx)
+{
+       assert(ctx);
+
+       cds_list_del(&ctx->list);
+       free(ctx);
+}
+
 /*
  * Cleanup kernel channel structure.
  */
@@ -382,6 +417,7 @@ void trace_kernel_destroy_channel(struct ltt_kernel_channel *channel)
 {
        struct ltt_kernel_stream *stream, *stmp;
        struct ltt_kernel_event *event, *etmp;
+       struct ltt_kernel_context *ctx, *ctmp;
        int ret;
 
        assert(channel);
@@ -405,11 +441,15 @@ void trace_kernel_destroy_channel(struct ltt_kernel_channel *channel)
                trace_kernel_destroy_event(event);
        }
 
+       /* For each context in the channel list */
+       cds_list_for_each_entry_safe(ctx, ctmp, &channel->ctx_list, list) {
+               trace_kernel_destroy_context(ctx);
+       }
+
        /* Remove from channel list */
        cds_list_del(&channel->list);
 
        free(channel->channel);
-       free(channel->ctx);
        free(channel);
 }
 

diff --git a/src/bin/lttng-sessiond/trace-kernel.h b/src/bin/lttng-sessiond/trace-kernel.h
index f576e3a60c6dd76a63104dc4f8803911ec4538b9..db91b2ed24cd57191e92a32376f0bc94e6b47482 100644 (file)
--- a/src/bin/lttng-sessiond/trace-kernel.h
+++ b/src/bin/lttng-sessiond/trace-kernel.h
@@ -42,6 +42,11 @@ struct ltt_kernel_channel_list {
        struct cds_list_head head;
 };
 
+struct ltt_kernel_context {
+       struct lttng_kernel_context ctx;
+       struct cds_list_head list;
+};
+
 /* Kernel event */
 struct ltt_kernel_event {
        int fd;
@@ -56,11 +61,7 @@ struct ltt_kernel_channel {
        int enabled;
        unsigned int stream_count;
        unsigned int event_count;
-       /*
-        * TODO: need internal representation to support more than a
-        * single context.
-        */
-       struct lttng_kernel_context *ctx;
+       struct cds_list_head ctx_list;
        struct lttng_channel *channel;
        struct ltt_kernel_event_list events_list;
        struct ltt_kernel_stream_list stream_list;
@@ -135,6 +136,8 @@ struct ltt_kernel_event *trace_kernel_create_event(struct lttng_event *ev);
 struct ltt_kernel_metadata *trace_kernel_create_metadata(void);
 struct ltt_kernel_stream *trace_kernel_create_stream(const char *name,
                unsigned int count);
+struct ltt_kernel_context *trace_kernel_create_context(
+               struct lttng_kernel_context *ctx);
 
 /*
  * Destroy functions free() the data structure and remove from linked list if
@@ -145,5 +148,6 @@ void trace_kernel_destroy_metadata(struct ltt_kernel_metadata *metadata);
 void trace_kernel_destroy_channel(struct ltt_kernel_channel *channel);
 void trace_kernel_destroy_event(struct ltt_kernel_event *event);
 void trace_kernel_destroy_stream(struct ltt_kernel_stream *stream);
+void trace_kernel_destroy_context(struct ltt_kernel_context *ctx);
 
 #endif /* _LTT_TRACE_KERNEL_H */

diff --git a/tests/unit/test_kernel_data.c b/tests/unit/test_kernel_data.c
index e2182e9262bfccf82c94c3dc5a2fcbe854fb30d2..5b82672d73206d5045dfd35a8d9c097b29238857 100644 (file)
--- a/tests/unit/test_kernel_data.c
+++ b/tests/unit/test_kernel_data.c
@@ -120,7 +120,6 @@ static void test_create_kernel_channel(void)
        ok(chan->fd == -1 &&
           chan->enabled == 1 &&
           chan->stream_count == 0 &&
-          chan->ctx == NULL &&
           chan->channel->attr.overwrite  == attr.attr.overwrite,
           "Validate kernel channel");