Modify default kernel channel size/number

Too small kernel channels (4kB) leads to unwelcome delay at trace
indexing and adds buffer switch overhead. Use a default closer to the
0.x LTTng for buffer size:

256kB subbuffer
4 subbuffers/buffer
for 1MB per channel per CPU in normal tracing mode, and 1MB + 256kB per
channel per CPU in overwrite mode.

We currently plan to still use 4kB * 8 sub-buffers for UST because we
allocate these buffers per process, which makes memory consumption more
of a concern.

The metadata channel is kept small (4kB), but we keep a number of
subbuffer of 8 for now, as the behavior between "start" and spawning the
consumer is in the wrong order: we will need to spawn the consumer
before start is performed, because "start" returns blocks for 10 seconds
and returns an error if the buffers are too small to write metadata (and
they are not consumed).

Also fix a memory leak in init_default_channel.

process_client_msg() now uses a check / alloc / failure for channel
lookup rather than an endless retry, which could have led to denial of
service in case of internal error.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

diff --git a/include/lttng-share.h b/include/lttng-share.h
index 9d3a259a6160b883f556137da29a5ae54fc325d8..d076ef3ade5d172d2ef63b9f34168e2b2701a817 100644 (file)
--- a/include/lttng-share.h
+++ b/include/lttng-share.h
@@ -28,20 +28,30 @@ typedef uint64_t u64;
 typedef __s64 s64;
 
 /* Default channel attributes */
-#define DEFAULT_CHANNEL_NAME                "channel0"
-#define DEFAULT_CHANNEL_OVERWRITE           0       /* usec */
+#define DEFAULT_CHANNEL_NAME                   "channel0"
+#define DEFAULT_CHANNEL_OVERWRITE              0       /* usec */
 /* DEFAULT_CHANNEL_SUBBUF_SIZE must always be a power of 2 */
-#define DEFAULT_CHANNEL_SUBBUF_SIZE         4096    /* bytes */
+#define DEFAULT_CHANNEL_SUBBUF_SIZE            4096    /* bytes */
 /* DEFAULT_CHANNEL_SUBBUF_NUM must always be a power of 2 */
-#define DEFAULT_CHANNEL_SUBBUF_NUM          8
-#define DEFAULT_CHANNEL_SWITCH_TIMER        0       /* usec */
-#define DEFAULT_CHANNEL_READ_TIMER          200     /* usec */
-/* See lttng-kernel.h enum lttng_kernel_output for channel output */
-#define DEFAULT_KERNEL_CHANNEL_OUTPUT       LTTNG_EVENT_SPLICE
+#define DEFAULT_CHANNEL_SUBBUF_NUM             8
+#define DEFAULT_CHANNEL_SWITCH_TIMER           0       /* usec */
+#define DEFAULT_CHANNEL_READ_TIMER             200     /* usec */
+#define DEFAULT_CHANNEL_OUTPUT                 LTTNG_EVENT_MMAP
+
+#define DEFAULT_METADATA_SUBBUF_SIZE           4096
+#define DEFAULT_METADATA_SUBBUF_NUM            8
+//TODO: keeping value to 8 currently because consumer is only spawned after
+//start, so it cannot empty the metadata buffer at trace start.
+//#define DEFAULT_METADATA_SUBBUF_NUM          2
 
-/* == NOT IMPLEMENTED ==
-#define DEFAULT_UST_CHANNEL_OUTPUT          LTTNG_UST_MMAP
-*/
+/* Kernel has different defaults */
+
+/* DEFAULT_KERNEL_CHANNEL_SUBBUF_SIZE must always be a power of 2 */
+#define DEFAULT_KERNEL_CHANNEL_SUBBUF_SIZE     262144    /* bytes */
+/* DEFAULT_KERNEL_CHANNEL_SUBBUF_NUM must always be a power of 2 */
+#define DEFAULT_KERNEL_CHANNEL_SUBBUF_NUM      4
+/* See lttng-kernel.h enum lttng_kernel_output for channel output */
+#define DEFAULT_KERNEL_CHANNEL_OUTPUT          LTTNG_EVENT_SPLICE
 
 /*
  * lttng user-space instrumentation type

diff --git a/ltt-sessiond/main.c b/ltt-sessiond/main.c
index 652798d348c2f05ab6f45ff5020abeccfd036329..470ab18188b162b38a2f98bfef1b94fb07eec0aa 100644 (file)
--- a/ltt-sessiond/main.c
+++ b/ltt-sessiond/main.c
@@ -1241,7 +1241,8 @@ static int notify_kernel_pollfd(void)
 /*
  * Allocate a channel structure and fill it.
  */
-static struct lttng_channel *init_default_channel(char *name)
+static struct lttng_channel *init_default_channel(enum lttng_domain_type domain_type,
+                                               char *name)
 {
        struct lttng_channel *chan;
 
@@ -1253,18 +1254,29 @@ static struct lttng_channel *init_default_channel(char *name)
 
        if (snprintf(chan->name, NAME_MAX, "%s", name) < 0) {
                perror("snprintf channel name");
-               return NULL;
+               goto error;
        }
 
        chan->attr.overwrite = DEFAULT_CHANNEL_OVERWRITE;
-       chan->attr.subbuf_size = DEFAULT_CHANNEL_SUBBUF_SIZE;
-       chan->attr.num_subbuf = DEFAULT_CHANNEL_SUBBUF_NUM;
        chan->attr.switch_timer_interval = DEFAULT_CHANNEL_SWITCH_TIMER;
        chan->attr.read_timer_interval = DEFAULT_CHANNEL_READ_TIMER;
-       chan->attr.output = DEFAULT_KERNEL_CHANNEL_OUTPUT;
 
-error:
+       switch (domain_type) {
+       case LTTNG_DOMAIN_KERNEL:
+               chan->attr.subbuf_size = DEFAULT_KERNEL_CHANNEL_SUBBUF_SIZE;
+               chan->attr.num_subbuf = DEFAULT_KERNEL_CHANNEL_SUBBUF_NUM;
+               chan->attr.output = DEFAULT_KERNEL_CHANNEL_OUTPUT;
+               break;
+               /* TODO: add UST */
+       default:
+               goto error;     /* Not implemented */
+       }
+
        return chan;
+
+error:
+       free(chan);
+       return NULL;
 }
 
 /*
@@ -1713,26 +1725,32 @@ static int process_client_msg(struct command_ctx *cmd_ctx)
 
                switch (cmd_ctx->lsm->domain.type) {
                case LTTNG_DOMAIN_KERNEL:
-                       do {
+                       kchan = get_kernel_channel_by_name(channel_name,
+                                       cmd_ctx->session->kernel_session);
+                       if (kchan == NULL) {
+                               DBG("Channel not found. Creating channel %s", channel_name);
+
+                               chan = init_default_channel(cmd_ctx->lsm->domain.type, channel_name);
+                               if (chan == NULL) {
+                                       ret = LTTCOMM_FATAL;
+                                       goto error;
+                               }
+
+                               ret = kernel_create_channel(cmd_ctx->session->kernel_session,
+                                               chan, cmd_ctx->session->kernel_session->trace_path);
+                               if (ret < 0) {
+                                       ret = LTTCOMM_KERN_CHAN_FAIL;
+                                       goto error;
+                               }
                                kchan = get_kernel_channel_by_name(channel_name,
                                                cmd_ctx->session->kernel_session);
                                if (kchan == NULL) {
-                                       DBG("Channel not found. Creating channel %s", channel_name);
-
-                                       chan = init_default_channel(channel_name);
-                                       if (chan == NULL) {
-                                               ret = LTTCOMM_FATAL;
-                                               goto error;
-                                       }
-
-                                       ret = kernel_create_channel(cmd_ctx->session->kernel_session,
-                                                       chan, cmd_ctx->session->kernel_session->trace_path);
-                                       if (ret < 0) {
-                                               ret = LTTCOMM_KERN_CHAN_FAIL;
-                                               goto error;
-                                       }
+                                       ERR("Channel %s not found after creation. Internal error, giving up.",
+                                               channel_name);
+                                       ret = LTTCOMM_FATAL;
+                                       goto error;
                                }
-                       } while (kchan == NULL);
+                       }
 
                        kevent = get_kernel_event_by_name(cmd_ctx->lsm->u.enable.event.name, kchan);
                        if (kevent == NULL) {
@@ -1785,26 +1803,32 @@ static int process_client_msg(struct command_ctx *cmd_ctx)
 
                switch (cmd_ctx->lsm->domain.type) {
                case LTTNG_DOMAIN_KERNEL:
-                       do {
+                       kchan = get_kernel_channel_by_name(channel_name,
+                                       cmd_ctx->session->kernel_session);
+                       if (kchan == NULL) {
+                               DBG("Channel not found. Creating channel %s", channel_name);
+
+                               chan = init_default_channel(cmd_ctx->lsm->domain.type, channel_name);
+                               if (chan == NULL) {
+                                       ret = LTTCOMM_FATAL;
+                                       goto error;
+                               }
+
+                               ret = kernel_create_channel(cmd_ctx->session->kernel_session,
+                                               chan, cmd_ctx->session->kernel_session->trace_path);
+                               if (ret < 0) {
+                                       ret = LTTCOMM_KERN_CHAN_FAIL;
+                                       goto error;
+                               }
                                kchan = get_kernel_channel_by_name(channel_name,
                                                cmd_ctx->session->kernel_session);
                                if (kchan == NULL) {
-                                       DBG("Channel not found. Creating channel %s", channel_name);
-
-                                       chan = init_default_channel(channel_name);
-                                       if (chan == NULL) {
-                                               ret = LTTCOMM_FATAL;
-                                               goto error;
-                                       }
-
-                                       ret = kernel_create_channel(cmd_ctx->session->kernel_session,
-                                                       chan, cmd_ctx->session->kernel_session->trace_path);
-                                       if (ret < 0) {
-                                               ret = LTTCOMM_KERN_CHAN_FAIL;
-                                               goto error;
-                                       }
+                                       ERR("Channel %s not found after creation. Internal error, giving up.",
+                                               channel_name);
+                                       ret = LTTCOMM_FATAL;
+                                       goto error;
                                }
-                       } while (kchan == NULL);
+                       }
 
                        /* For each event in the kernel session */
                        cds_list_for_each_entry(kevent, &kchan->events_list.head, list) {

diff --git a/ltt-sessiond/trace.c b/ltt-sessiond/trace.c
index 774321e59a7d085edc8a5f50a5eae54652e3c15e..621e4dd3f67d7fda38ac38cbab288666ed4a6460 100644 (file)
--- a/ltt-sessiond/trace.c
+++ b/ltt-sessiond/trace.c
@@ -247,8 +247,8 @@ struct ltt_kernel_metadata *trace_create_kernel_metadata(char *path)
 
        /* Set default attributes */
        chan->attr.overwrite = DEFAULT_CHANNEL_OVERWRITE;
-       chan->attr.subbuf_size = DEFAULT_CHANNEL_SUBBUF_SIZE;
-       chan->attr.num_subbuf = DEFAULT_CHANNEL_SUBBUF_NUM;
+       chan->attr.subbuf_size = DEFAULT_METADATA_SUBBUF_SIZE;
+       chan->attr.num_subbuf = DEFAULT_METADATA_SUBBUF_NUM;
        chan->attr.switch_timer_interval = DEFAULT_CHANNEL_SWITCH_TIMER;
        chan->attr.read_timer_interval = DEFAULT_CHANNEL_READ_TIMER;
        chan->attr.output = DEFAULT_KERNEL_CHANNEL_OUTPUT;

diff --git a/lttng/commands/enable_channels.c b/lttng/commands/enable_channels.c
index e49559e4f416cb390c26549397f78449ab4c5886..bd732f22f20d4e4979323c1c75a32f1e0c77dee9 100644 (file)
--- a/lttng/commands/enable_channels.c
+++ b/lttng/commands/enable_channels.c
@@ -162,12 +162,25 @@ error:
  */
 static void init_channel_config(void)
 {
-       chan.attr.overwrite = DEFAULT_CHANNEL_OVERWRITE;
-       chan.attr.subbuf_size = DEFAULT_CHANNEL_SUBBUF_SIZE;
-       chan.attr.num_subbuf = DEFAULT_CHANNEL_SUBBUF_NUM;
-       chan.attr.switch_timer_interval = DEFAULT_CHANNEL_SWITCH_TIMER;
-       chan.attr.read_timer_interval = DEFAULT_CHANNEL_READ_TIMER;
-       chan.attr.output = DEFAULT_KERNEL_CHANNEL_OUTPUT;
+       if (opt_kernel) {
+               /* kernel default */
+               chan.attr.overwrite = DEFAULT_CHANNEL_OVERWRITE;
+               chan.attr.switch_timer_interval = DEFAULT_CHANNEL_SWITCH_TIMER;
+               chan.attr.read_timer_interval = DEFAULT_CHANNEL_READ_TIMER;
+
+               chan.attr.subbuf_size = DEFAULT_KERNEL_CHANNEL_SUBBUF_SIZE;
+               chan.attr.num_subbuf = DEFAULT_KERNEL_CHANNEL_SUBBUF_NUM;
+               chan.attr.output = DEFAULT_KERNEL_CHANNEL_OUTPUT;
+       } else {
+               /* default behavior, used by UST. */
+               chan.attr.overwrite = DEFAULT_CHANNEL_OVERWRITE;
+               chan.attr.switch_timer_interval = DEFAULT_CHANNEL_SWITCH_TIMER;
+               chan.attr.read_timer_interval = DEFAULT_CHANNEL_READ_TIMER;
+
+               chan.attr.subbuf_size = DEFAULT_CHANNEL_SUBBUF_SIZE;
+               chan.attr.num_subbuf = DEFAULT_CHANNEL_SUBBUF_NUM;
+               chan.attr.output = DEFAULT_CHANNEL_OUTPUT;
+       }
 }
 
 /*

diff --git a/tests/test_kernel_data_trace.c b/tests/test_kernel_data_trace.c
index 7a1f076c6e354fa21f34d505342c29f0c9d99a12..2892ab236888d9cdc5ca787eba942469336c76d8 100644 (file)
--- a/tests/test_kernel_data_trace.c
+++ b/tests/test_kernel_data_trace.c
@@ -96,9 +96,9 @@ static void create_kernel_metadata(void)
        assert(kern->metadata->conf->attr.overwrite
                        == DEFAULT_CHANNEL_OVERWRITE);
        assert(kern->metadata->conf->attr.subbuf_size
-                       == DEFAULT_CHANNEL_SUBBUF_SIZE);
+                       == DEFAULT_METADATA_SUBBUF_SIZE);
        assert(kern->metadata->conf->attr.num_subbuf
-                       == DEFAULT_CHANNEL_SUBBUF_NUM);
+                       == DEFAULT_METADATA_SUBBUF_NUM);
        assert(kern->metadata->conf->attr.switch_timer_interval
                        == DEFAULT_CHANNEL_SWITCH_TIMER);
        assert(kern->metadata->conf->attr.read_timer_interval