From 0498a00cbc6b5b40609016d0c5e695cd50efc01f Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Mon, 19 Nov 2018 16:13:58 -0500 Subject: [PATCH] Fix: sessiond: don't allocate buffers and files for inactive sessions MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit When tracing is inactive (before start/after stop), the current behavior is to track all applications registered as UST data producers and allocate buffers and files. However, we guarantee that the trace is readable (invariant) after a "stop" command has waited for data pending to complete. Unfortunately, tracking additional applications (and adding their files) after tracing is stopped (for each pid in per-pid buffers, for new uid in per-uid buffers) does not respect this guarantee. Fix this by *not* allocating channels, events, contexts when tracing is inactive, but rather allocate those lazily just before tracing starts. One reason why this was not originally done was to ensure we could have a fast start command. There are however other ways to achieve this in the future that will respect the stop invariant guarantees. Signed-off-by: Mathieu Desnoyers Signed-off-by: Jérémie Galarneau --- src/bin/lttng-sessiond/ust-app.c | 63 +++++++++++++++++++++++++++----- 1 file changed, 53 insertions(+), 10 deletions(-) diff --git a/src/bin/lttng-sessiond/ust-app.c b/src/bin/lttng-sessiond/ust-app.c index f88df7406..0b0a62532 100644 --- a/src/bin/lttng-sessiond/ust-app.c +++ b/src/bin/lttng-sessiond/ust-app.c @@ -4012,6 +4012,10 @@ int ust_app_disable_channel_glb(struct ltt_ust_session *usess, DBG2("UST app disabling channel %s from global domain for session id %" PRIu64, uchan->name, usess->id); + if (!usess->active) { + goto end; + } + rcu_read_lock(); /* For every registered applications */ @@ -4050,6 +4054,7 @@ int ust_app_disable_channel_glb(struct ltt_ust_session *usess, rcu_read_unlock(); error: +end: return ret; } @@ -4073,6 +4078,10 @@ int ust_app_enable_channel_glb(struct ltt_ust_session *usess, DBG2("UST app enabling channel %s to global domain for session id %" PRIu64, uchan->name, usess->id); + if (!usess->active) { + goto end; + } + rcu_read_lock(); /* For every registered applications */ @@ -4100,6 +4109,7 @@ int ust_app_enable_channel_glb(struct ltt_ust_session *usess, rcu_read_unlock(); error: +end: return ret; } @@ -4121,6 +4131,10 @@ int ust_app_disable_event_glb(struct ltt_ust_session *usess, "%s for session id %" PRIu64, uevent->attr.name, uchan->name, usess->id); + if (!usess->active) { + goto end; + } + rcu_read_lock(); /* For all registered applications */ @@ -4165,7 +4179,7 @@ int ust_app_disable_event_glb(struct ltt_ust_session *usess, } rcu_read_unlock(); - +end: return ret; } @@ -4187,6 +4201,10 @@ int ust_app_create_channel_glb(struct ltt_ust_session *usess, DBG2("UST app adding channel %s to UST domain for session id %" PRIu64, uchan->name, usess->id); + if (!usess->active) { + goto end; + } + rcu_read_lock(); /* For every registered applications */ @@ -4266,6 +4284,7 @@ int ust_app_create_channel_glb(struct ltt_ust_session *usess, error_rcu_unlock: rcu_read_unlock(); +end: return ret; } @@ -4286,6 +4305,10 @@ int ust_app_enable_event_glb(struct ltt_ust_session *usess, DBG("UST app enabling event %s for all apps for session id %" PRIu64, uevent->attr.name, usess->id); + if (!usess->active) { + goto end; + } + /* * NOTE: At this point, this function is called only if the session and * channel passed are already created for all apps. and enabled on the @@ -4351,6 +4374,7 @@ int ust_app_enable_event_glb(struct ltt_ust_session *usess, error: rcu_read_unlock(); +end: return ret; } @@ -4371,6 +4395,10 @@ int ust_app_create_event_glb(struct ltt_ust_session *usess, DBG("UST app creating event %s for all apps for session id %" PRIu64, uevent->attr.name, usess->id); + if (!usess->active) { + goto end; + } + rcu_read_lock(); /* For all registered applications */ @@ -4417,7 +4445,7 @@ int ust_app_create_event_glb(struct ltt_ust_session *usess, } rcu_read_unlock(); - +end: return ret; } @@ -5012,6 +5040,10 @@ int ust_app_start_trace_all(struct ltt_ust_session *usess) */ (void) ust_app_clear_quiescent_session(usess); + cds_lfht_for_each_entry(ust_app_ht->ht, &iter.iter, app, pid_n.node) { + ust_app_global_update(usess, app); + } + cds_lfht_for_each_entry(ust_app_ht->ht, &iter.iter, app, pid_n.node) { ret = ust_app_start_trace(usess, app); if (ret < 0) { @@ -5106,6 +5138,9 @@ void ust_app_global_create(struct ltt_ust_session *usess, struct ust_app *app) /* App session already created. */ goto end; } + if (!usess->active) { + goto end; + } assert(ua_sess); pthread_mutex_lock(&ua_sess->lock); @@ -5159,14 +5194,12 @@ void ust_app_global_create(struct ltt_ust_session *usess, struct ust_app *app) pthread_mutex_unlock(&ua_sess->lock); - if (usess->active) { - ret = ust_app_start_trace(usess, app); - if (ret < 0) { - goto error; - } - - DBG2("UST trace started for app pid %d", app->pid); + ret = ust_app_start_trace(usess, app); + if (ret < 0) { + goto error; } + + DBG2("UST trace started for app pid %d", app->pid); end: /* Everything went well at this point. */ return; @@ -5208,7 +5241,9 @@ void ust_app_global_update(struct ltt_ust_session *usess, struct ust_app *app) if (!app->compatible) { return; } - + if (!usess->active) { + return; + } if (trace_ust_pid_tracker_lookup(usess, app->pid)) { ust_app_global_create(usess, app); } else { @@ -5224,6 +5259,9 @@ void ust_app_global_update_all(struct ltt_ust_session *usess) struct lttng_ht_iter iter; struct ust_app *app; + if (!usess->active) { + return; + } rcu_read_lock(); cds_lfht_for_each_entry(ust_app_ht->ht, &iter.iter, app, pid_n.node) { ust_app_global_update(usess, app); @@ -5244,6 +5282,10 @@ int ust_app_add_ctx_channel_glb(struct ltt_ust_session *usess, struct ust_app_session *ua_sess; struct ust_app *app; + if (!usess->active) { + goto end; + } + rcu_read_lock(); cds_lfht_for_each_entry(ust_app_ht->ht, &iter.iter, app, pid_n.node) { @@ -5283,6 +5325,7 @@ int ust_app_add_ctx_channel_glb(struct ltt_ust_session *usess, } rcu_read_unlock(); +end: return ret; } -- 2.34.1