Fix: sessiond: don't allocate buffers and files for inactive sessions
authorMathieu Desnoyers <mathieu.desnoyers@efficios.com>
Mon, 19 Nov 2018 21:13:58 +0000 (16:13 -0500)
committerJérémie Galarneau <jeremie.galarneau@efficios.com>
Wed, 12 Dec 2018 19:23:25 +0000 (14:23 -0500)
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 <mathieu.desnoyers@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
src/bin/lttng-sessiond/ust-app.c

index f88df7406ea567049321b6c508fdc5b249a4ff35..0b0a62532125e6bc7ecd4a72728f1b8e997a18ba 100644 (file)
@@ -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);
 
        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 */
        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:
        rcu_read_unlock();
 
 error:
+end:
        return ret;
 }
 
        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);
 
        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 */
        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:
        rcu_read_unlock();
 
 error:
+end:
        return ret;
 }
 
        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);
 
                        "%s for session id %" PRIu64,
                        uevent->attr.name, uchan->name, usess->id);
 
+       if (!usess->active) {
+               goto end;
+       }
+
        rcu_read_lock();
 
        /* For all registered applications */
        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();
        }
 
        rcu_read_unlock();
-
+end:
        return ret;
 }
 
        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);
 
        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 */
        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();
 
 error_rcu_unlock:
        rcu_read_unlock();
+end:
        return ret;
 }
 
        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);
 
        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
        /*
         * 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();
 
 error:
        rcu_read_unlock();
+end:
        return ret;
 }
 
        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);
 
        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 */
        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();
        }
 
        rcu_read_unlock();
-
+end:
        return ret;
 }
 
        return ret;
 }
 
@@ -5012,6 +5040,10 @@ int ust_app_start_trace_all(struct ltt_ust_session *usess)
         */
        (void) ust_app_clear_quiescent_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) {
        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;
        }
                /* App session already created. */
                goto end;
        }
+       if (!usess->active) {
+               goto end;
+       }
        assert(ua_sess);
 
        pthread_mutex_lock(&ua_sess->lock);
        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);
 
 
        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;
 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 (!app->compatible) {
                return;
        }
-
+       if (!usess->active) {
+               return;
+       }
        if (trace_ust_pid_tracker_lookup(usess, app->pid)) {
                ust_app_global_create(usess, app);
        } else {
        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;
 
        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);
        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;
 
        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) {
        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();
        }
 
        rcu_read_unlock();
+end:
        return ret;
 }
 
        return ret;
 }
 
This page took 0.030278 seconds and 4 git commands to generate.