From 68808f4e0bbb3adccff72ff9dab6ec9f3a9e6866 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Wed, 9 Jul 2014 15:24:04 -0400 Subject: [PATCH] Fix: use biggest subbuffer size for snapshot max-size Instead of using the output max size divided by the total amount of streams in the session, we find the largest subbuffer in the session's channels. Using that value, we multiply it by the total amount of streams which gives us the lower limit of the snapshot size. That is enough to make sure that we can take the snapshot or not. Once done, the max stream size possible used for the snapshot record is the largest subbuffer size in the session. This is to make sure that every channel can extract the same amount of data which ensure fairness between each channel in the session. Fixes #783 Acked-by: Julien Desfossez Signed-off-by: David Goulet --- doc/man/lttng.1 | 4 ++ include/lttng/lttng-error.h | 2 +- src/bin/lttng-sessiond/cmd.c | 105 ++++++++++++++++++++++++++---- src/bin/lttng-sessiond/kernel.c | 18 +---- src/bin/lttng-sessiond/kernel.h | 2 +- src/bin/lttng-sessiond/ust-app.c | 53 +++------------ src/bin/lttng-sessiond/ust-app.h | 4 +- src/bin/lttng/commands/snapshot.c | 5 ++ src/common/error.c | 1 + 9 files changed, 116 insertions(+), 78 deletions(-) diff --git a/doc/man/lttng.1 b/doc/man/lttng.1 index 3c7bbb44b..d4fba1187 100644 --- a/doc/man/lttng.1 +++ b/doc/man/lttng.1 @@ -880,6 +880,10 @@ Name of the snapshot's output. Maximum size in bytes of the snapshot. The maxium size does not include the metadata file. Human readable format is accepted: {+k,+M,+G}. For instance, \-\-max-size 5M + +The minimum size of a snapshot is computed by multiplying the total amount of +streams in the session by the largest subbuffer size. This is to ensure +fairness between channels when extracting data. .TP .BR "\-C, \-\-ctrl-url URL" Set control path URL. (Must use -D also) diff --git a/include/lttng/lttng-error.h b/include/lttng/lttng-error.h index f4f3a8344..e18fb00e8 100644 --- a/include/lttng/lttng-error.h +++ b/include/lttng/lttng-error.h @@ -110,7 +110,7 @@ enum lttng_error_code { LTTNG_ERR_LOAD_INVALID_CONFIG = 87, /* Invalid session configuration */ LTTNG_ERR_LOAD_IO_FAIL = 88, /* IO error while reading a session configuration */ LTTNG_ERR_LOAD_SESSION_NOENT = 89, /* Session file not found */ - /* 90 */ + LTTNG_ERR_MAX_SIZE_INVALID = 90, /* Snapshot max size is invalid. */ /* 91 */ /* 92 */ /* 93 */ diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c index fff4b917c..044e9eefe 100644 --- a/src/bin/lttng-sessiond/cmd.c +++ b/src/bin/lttng-sessiond/cmd.c @@ -2832,7 +2832,7 @@ error: */ static int record_kernel_snapshot(struct ltt_kernel_session *ksess, struct snapshot_output *output, struct ltt_session *session, - int wait, int nb_streams) + int wait, uint64_t max_stream_size) { int ret; @@ -2863,7 +2863,7 @@ static int record_kernel_snapshot(struct ltt_kernel_session *ksess, goto error_snapshot; } - ret = kernel_snapshot_record(ksess, output, wait, nb_streams); + ret = kernel_snapshot_record(ksess, output, wait, max_stream_size); if (ret != LTTNG_OK) { goto error_snapshot; } @@ -2884,7 +2884,7 @@ error: */ static int record_ust_snapshot(struct ltt_ust_session *usess, struct snapshot_output *output, struct ltt_session *session, - int wait, int nb_streams) + int wait, uint64_t max_stream_size) { int ret; @@ -2915,7 +2915,7 @@ static int record_ust_snapshot(struct ltt_ust_session *usess, goto error_snapshot; } - ret = ust_app_snapshot_record(usess, output, wait, nb_streams); + ret = ust_app_snapshot_record(usess, output, wait, max_stream_size); if (ret < 0) { switch (-ret) { case EINVAL: @@ -2940,11 +2940,51 @@ error: return ret; } +/* + * Return the biggest subbuffer size of all channels in the given session. + */ +static uint64_t get_session_max_subbuf_size(struct ltt_session *session) +{ + uint64_t max_size = 0; + + assert(session); + + if (session->kernel_session) { + struct ltt_kernel_channel *chan; + struct ltt_kernel_session *ksess = session->kernel_session; + + /* + * For each channel, add to the max size the size of each subbuffer + * multiplied by their sized. + */ + cds_list_for_each_entry(chan, &ksess->channel_list.head, list) { + if (chan->channel->attr.subbuf_size > max_size) { + max_size = chan->channel->attr.subbuf_size; + } + } + } + + if (session->ust_session) { + struct lttng_ht_iter iter; + struct ltt_ust_channel *uchan; + struct ltt_ust_session *usess = session->ust_session; + + cds_lfht_for_each_entry(usess->domain_global.channels->ht, &iter.iter, + uchan, node.node) { + if (uchan->attr.subbuf_size > max_size) { + max_size = uchan->attr.subbuf_size; + } + } + } + + return max_size; +} + /* * Returns the total number of streams for a session or a negative value * on error. */ -static unsigned int get_total_nb_stream(struct ltt_session *session) +static unsigned int get_session_nb_streams(struct ltt_session *session) { unsigned int total_streams = 0; @@ -2978,6 +3018,7 @@ int cmd_snapshot_record(struct ltt_session *session, unsigned int use_tmp_output = 0; struct snapshot_output tmp_output; unsigned int nb_streams, snapshot_success = 0; + uint64_t session_max_size = 0, max_stream_size = 0; assert(session); @@ -3017,17 +3058,43 @@ int cmd_snapshot_record(struct ltt_session *session, } /* - * Get the total number of stream of that session which is used by the - * maximum size of the snapshot feature. + * Get the session maximum size for a snapshot meaning it will compute the + * size of all streams from all domain. + */ + max_stream_size = get_session_max_subbuf_size(session); + + nb_streams = get_session_nb_streams(session); + if (nb_streams) { + /* + * The maximum size of the snapshot is the number of streams multiplied + * by the biggest subbuf size of all channels in a session which is the + * maximum stream size available for each stream. The session max size + * is now checked against the snapshot max size value given by the user + * and if lower, an error is returned. + */ + session_max_size = max_stream_size * nb_streams; + } + + DBG3("Snapshot max size is %" PRIu64 " for max stream size of %" PRIu64, + session_max_size, max_stream_size); + + /* + * If we use a temporary output, check right away if the max size fits else + * for each output the max size will be checked. */ - nb_streams = get_total_nb_stream(session); + if (use_tmp_output && + (tmp_output.max_size != 0 && + tmp_output.max_size < session_max_size)) { + ret = LTTNG_ERR_MAX_SIZE_INVALID; + goto error; + } if (session->kernel_session) { struct ltt_kernel_session *ksess = session->kernel_session; if (use_tmp_output) { ret = record_kernel_snapshot(ksess, &tmp_output, session, - wait, nb_streams); + wait, max_stream_size); if (ret != LTTNG_OK) { goto error; } @@ -3051,6 +3118,13 @@ int cmd_snapshot_record(struct ltt_session *session, tmp_output.max_size = output->max_size; } + if (tmp_output.max_size != 0 && + tmp_output.max_size < session_max_size) { + rcu_read_unlock(); + ret = LTTNG_ERR_MAX_SIZE_INVALID; + goto error; + } + /* Use temporary name. */ if (*output->name != '\0') { strncpy(tmp_output.name, output->name, @@ -3060,7 +3134,7 @@ int cmd_snapshot_record(struct ltt_session *session, tmp_output.nb_snapshot = session->snapshot.nb_snapshot; ret = record_kernel_snapshot(ksess, &tmp_output, - session, wait, nb_streams); + session, wait, max_stream_size); if (ret != LTTNG_OK) { rcu_read_unlock(); goto error; @@ -3076,7 +3150,7 @@ int cmd_snapshot_record(struct ltt_session *session, if (use_tmp_output) { ret = record_ust_snapshot(usess, &tmp_output, session, - wait, nb_streams); + wait, max_stream_size); if (ret != LTTNG_OK) { goto error; } @@ -3100,6 +3174,13 @@ int cmd_snapshot_record(struct ltt_session *session, tmp_output.max_size = output->max_size; } + if (tmp_output.max_size != 0 && + tmp_output.max_size < session_max_size) { + rcu_read_unlock(); + ret = LTTNG_ERR_MAX_SIZE_INVALID; + goto error; + } + /* Use temporary name. */ if (*output->name != '\0') { strncpy(tmp_output.name, output->name, @@ -3109,7 +3190,7 @@ int cmd_snapshot_record(struct ltt_session *session, tmp_output.nb_snapshot = session->snapshot.nb_snapshot; ret = record_ust_snapshot(usess, &tmp_output, session, - wait, nb_streams); + wait, max_stream_size); if (ret != LTTNG_OK) { rcu_read_unlock(); goto error; diff --git a/src/bin/lttng-sessiond/kernel.c b/src/bin/lttng-sessiond/kernel.c index b997a4aea..1fae30f0a 100644 --- a/src/bin/lttng-sessiond/kernel.c +++ b/src/bin/lttng-sessiond/kernel.c @@ -823,13 +823,12 @@ void kernel_destroy_channel(struct ltt_kernel_channel *kchan) * Return 0 on success or else return a LTTNG_ERR code. */ int kernel_snapshot_record(struct ltt_kernel_session *ksess, - struct snapshot_output *output, int wait, unsigned int nb_streams) + struct snapshot_output *output, int wait, uint64_t max_size_per_stream) { int err, ret, saved_metadata_fd; struct consumer_socket *socket; struct lttng_ht_iter iter; struct ltt_kernel_metadata *saved_metadata; - uint64_t max_size_per_stream = 0; assert(ksess); assert(ksess->consumer); @@ -855,10 +854,6 @@ int kernel_snapshot_record(struct ltt_kernel_session *ksess, goto error_open_stream; } - if (output->max_size > 0 && nb_streams > 0) { - max_size_per_stream = output->max_size / nb_streams; - } - /* Send metadata to consumer and snapshot everything. */ cds_lfht_for_each_entry(ksess->consumer->socks->ht, &iter.iter, socket, node.node) { @@ -885,17 +880,6 @@ int kernel_snapshot_record(struct ltt_kernel_session *ksess, /* For each channel, ask the consumer to snapshot it. */ cds_list_for_each_entry(chan, &ksess->channel_list.head, list) { - if (max_size_per_stream && - chan->channel->attr.subbuf_size > max_size_per_stream) { - ret = LTTNG_ERR_INVALID; - DBG3("Kernel snapshot record maximum stream size %" PRIu64 - " is smaller than subbuffer size of %" PRIu64, - max_size_per_stream, chan->channel->attr.subbuf_size); - (void) kernel_consumer_destroy_metadata(socket, - ksess->metadata); - goto error_consumer; - } - pthread_mutex_lock(socket->lock); ret = consumer_snapshot_channel(socket, chan->fd, output, 0, ksess->uid, ksess->gid, diff --git a/src/bin/lttng-sessiond/kernel.h b/src/bin/lttng-sessiond/kernel.h index e79478012..681301fc2 100644 --- a/src/bin/lttng-sessiond/kernel.h +++ b/src/bin/lttng-sessiond/kernel.h @@ -55,7 +55,7 @@ int kernel_validate_version(int tracer_fd); void kernel_destroy_session(struct ltt_kernel_session *ksess); void kernel_destroy_channel(struct ltt_kernel_channel *kchan); int kernel_snapshot_record(struct ltt_kernel_session *ksess, - struct snapshot_output *output, int wait, unsigned int nb_streams); + struct snapshot_output *output, int wait, uint64_t max_stream_size); int init_kernel_workarounds(void); diff --git a/src/bin/lttng-sessiond/ust-app.c b/src/bin/lttng-sessiond/ust-app.c index fc744fc02..4a7fadaf5 100644 --- a/src/bin/lttng-sessiond/ust-app.c +++ b/src/bin/lttng-sessiond/ust-app.c @@ -4911,28 +4911,19 @@ void ust_app_destroy(struct ust_app *app) * Return 0 on success or else a negative value. */ int ust_app_snapshot_record(struct ltt_ust_session *usess, - struct snapshot_output *output, int wait, unsigned int nb_streams) + struct snapshot_output *output, int wait, uint64_t max_stream_size) { int ret = 0; unsigned int snapshot_done = 0; struct lttng_ht_iter iter; struct ust_app *app; char pathname[PATH_MAX]; - uint64_t max_stream_size = 0; assert(usess); assert(output); rcu_read_lock(); - /* - * Compute the maximum size of a single stream if a max size is asked by - * the caller. - */ - if (output->max_size > 0 && nb_streams > 0) { - max_stream_size = output->max_size / nb_streams; - } - switch (usess->buffer_type) { case LTTNG_BUFFER_PER_UID: { @@ -4962,30 +4953,16 @@ int ust_app_snapshot_record(struct ltt_ust_session *usess, /* Add the UST default trace dir to path. */ cds_lfht_for_each_entry(reg->registry->channels->ht, &iter.iter, reg_chan, node.node) { - - /* - * Make sure the maximum stream size is not lower than the - * subbuffer size or else it's an error since we won't be able to - * snapshot anything. - */ - if (max_stream_size && - reg_chan->subbuf_size > max_stream_size) { - ret = -EINVAL; - DBG3("UST app snapshot record maximum stream size %" PRIu64 - " is smaller than subbuffer size of %zu", - max_stream_size, reg_chan->subbuf_size); - goto error; - } - ret = consumer_snapshot_channel(socket, reg_chan->consumer_key, output, 0, - usess->uid, usess->gid, pathname, wait, + ret = consumer_snapshot_channel(socket, reg_chan->consumer_key, + output, 0, usess->uid, usess->gid, pathname, wait, max_stream_size); if (ret < 0) { goto error; } } - ret = consumer_snapshot_channel(socket, reg->registry->reg.ust->metadata_key, output, - 1, usess->uid, usess->gid, pathname, wait, - max_stream_size); + ret = consumer_snapshot_channel(socket, + reg->registry->reg.ust->metadata_key, output, 1, + usess->uid, usess->gid, pathname, wait, max_stream_size); if (ret < 0) { goto error; } @@ -5027,22 +5004,8 @@ int ust_app_snapshot_record(struct ltt_ust_session *usess, cds_lfht_for_each_entry(ua_sess->channels->ht, &chan_iter.iter, ua_chan, node.node) { - /* - * Make sure the maximum stream size is not lower than the - * subbuffer size or else it's an error since we won't be able to - * snapshot anything. - */ - if (max_stream_size && - ua_chan->attr.subbuf_size > max_stream_size) { - ret = -EINVAL; - DBG3("UST app snapshot record maximum stream size %" PRIu64 - " is smaller than subbuffer size of %" PRIu64, - max_stream_size, ua_chan->attr.subbuf_size); - goto error; - } - - ret = consumer_snapshot_channel(socket, ua_chan->key, output, 0, - ua_sess->euid, ua_sess->egid, pathname, wait, + ret = consumer_snapshot_channel(socket, ua_chan->key, output, + 0, ua_sess->euid, ua_sess->egid, pathname, wait, max_stream_size); if (ret < 0) { goto error; diff --git a/src/bin/lttng-sessiond/ust-app.h b/src/bin/lttng-sessiond/ust-app.h index 2fd3b56d9..f83e857cf 100644 --- a/src/bin/lttng-sessiond/ust-app.h +++ b/src/bin/lttng-sessiond/ust-app.h @@ -325,7 +325,7 @@ ssize_t ust_app_push_metadata(struct ust_registry_session *registry, struct consumer_socket *socket, int send_zero_data); void ust_app_destroy(struct ust_app *app); int ust_app_snapshot_record(struct ltt_ust_session *usess, - struct snapshot_output *output, int wait, unsigned int nb_streams); + struct snapshot_output *output, int wait, uint64_t max_stream_size); unsigned int ust_app_get_nb_stream(struct ltt_ust_session *usess); struct ust_app *ust_app_find_by_sock(int sock); @@ -510,7 +510,7 @@ void ust_app_destroy(struct ust_app *app) } static inline int ust_app_snapshot_record(struct ltt_ust_session *usess, - struct snapshot_output *output, int wait, unsigned int nb_stream) + struct snapshot_output *output, int wait, uint64_t max_stream_size) { return 0; } diff --git a/src/bin/lttng/commands/snapshot.c b/src/bin/lttng/commands/snapshot.c index 94deb0cfa..2d250afc3 100644 --- a/src/bin/lttng/commands/snapshot.c +++ b/src/bin/lttng/commands/snapshot.c @@ -372,6 +372,11 @@ static int record(const char *url) ret = lttng_snapshot_record(current_session_name, output, 0); if (ret < 0) { + if (ret == -LTTNG_ERR_MAX_SIZE_INVALID) { + ERR("The minimum size of a snapshot is computed by multiplying " + "the total amount of streams with the largest subbuffer " + "in the session."); + } goto error; } diff --git a/src/common/error.c b/src/common/error.c index 5abb7453a..26dc06798 100644 --- a/src/common/error.c +++ b/src/common/error.c @@ -160,6 +160,7 @@ static const char *error_string_array[] = { [ ERROR_INDEX(LTTNG_ERR_LOAD_INVALID_CONFIG) ] = "Invalid session configuration", [ ERROR_INDEX(LTTNG_ERR_LOAD_IO_FAIL) ] = "IO error while reading a session configuration", [ ERROR_INDEX(LTTNG_ERR_LOAD_SESSION_NOENT) ] = "Session file not found", + [ ERROR_INDEX(LTTNG_ERR_MAX_SIZE_INVALID) ] = "Snapshot max size is invalid", /* Last element */ [ ERROR_INDEX(LTTNG_ERR_NR) ] = "Unknown error code" -- 2.34.1