Multiple fixes for enable/disable UST support

Makes the enable/disable command inside the main.c uses the event.c
functions. For this, the disable tracepoint functions (single and all)
for UST are added to event.c

Also fix the goto error path in event.c for disable/enable functions
where the UST event was removed from the hashtable if the event already
exist on the tracer side.

Fix the wrong hashtable being used for events lookup in ust_app.c for
enable/disable event for UST global domain.

Adds the disable event per PID function call in ust_app.c

At this commit, the "lttng disable-event -a -u" command segfault
lttng-ust at commit 5cd29ace704432c7a421f7749ef8591e30e1894c.

Signed-off-by: David Goulet <david.goulet@polymtl.ca>

diff --git a/lttng-sessiond/event.c b/lttng-sessiond/event.c
index 4e6ac03bf19f58a4761b63c973ba6bac6bf25a24..c8274c6bcf8a9100aed41422ff9db96734c01cf9 100644 (file)
--- a/lttng-sessiond/event.c
+++ b/lttng-sessiond/event.c
@@ -245,6 +245,12 @@ end:
        return ret;
 }
 
+/*
+ * ============================
+ * UST : The Ultimate Frontier!
+ * ============================
+ */
+
 /*
  * Enable all UST tracepoints for a channel from a UST session.
  */
@@ -255,7 +261,7 @@ int event_ust_enable_all_tracepoints(struct ltt_ust_session *usess, int domain,
        size_t size;
        struct cds_lfht_iter iter;
        struct ltt_ust_event *uevent = NULL;
-       struct lttng_event *events;
+       struct lttng_event *events = NULL;
 
        switch (domain) {
        case LTTNG_DOMAIN_UST:
@@ -301,7 +307,7 @@ int event_ust_enable_all_tracepoints(struct ltt_ust_session *usess, int domain,
                        uevent = trace_ust_create_event(&events[i]);
                        if (uevent == NULL) {
                                ret = LTTCOMM_FATAL;
-                               goto error;
+                               goto error_destroy;
                        }
 
                        /* Create event for the specific PID */
@@ -310,10 +316,11 @@ int event_ust_enable_all_tracepoints(struct ltt_ust_session *usess, int domain,
                        if (ret < 0) {
                                if (ret == -EEXIST) {
                                        ret = LTTCOMM_UST_EVENT_EXIST;
+                                       goto error;
                                } else {
                                        ret = LTTCOMM_UST_ENABLE_FAIL;
+                                       goto error_destroy;
                                }
-                               goto error;
                        }
 
                        uevent->enabled = 1;
@@ -336,8 +343,11 @@ int event_ust_enable_all_tracepoints(struct ltt_ust_session *usess, int domain,
 
        return LTTCOMM_OK;
 
-error:
+error_destroy:
        trace_ust_destroy_event(uevent);
+
+error:
+       free(events);
        return ret;
 }
 
@@ -360,6 +370,12 @@ int event_ust_enable_tracepoint(struct ltt_ust_session *usess, int domain,
                to_create = 1;
        }
 
+       if (uevent->enabled) {
+               /* It's already enabled so everything is OK */
+               ret = LTTCOMM_OK;
+               goto end;
+       }
+
        switch (domain) {
        case LTTNG_DOMAIN_UST:
        {
@@ -374,14 +390,12 @@ int event_ust_enable_tracepoint(struct ltt_ust_session *usess, int domain,
                if (ret < 0) {
                        if (ret == -EEXIST) {
                                ret = LTTCOMM_UST_EVENT_EXIST;
+                               goto end;
                        } else {
                                ret = LTTCOMM_UST_ENABLE_FAIL;
+                               goto error;
                        }
-                       goto error;
                }
-
-               DBG("Event UST %s added to channel %s", uevent->attr.name,
-                               uchan->name);
                break;
        }
        case LTTNG_DOMAIN_UST_EXEC_NAME:
@@ -389,42 +403,140 @@ int event_ust_enable_tracepoint(struct ltt_ust_session *usess, int domain,
        case LTTNG_DOMAIN_UST_PID_FOLLOW_CHILDREN:
        default:
                ret = LTTCOMM_NOT_IMPLEMENTED;
-               goto error;
+               goto end;
        }
 
        uevent->enabled = 1;
        /* Add ltt ust event to channel */
-       rcu_read_lock();
-       hashtable_add_unique(uchan->events, &uevent->node);
-       rcu_read_unlock();
+       if (to_create) {
+               rcu_read_lock();
+               hashtable_add_unique(uchan->events, &uevent->node);
+               rcu_read_unlock();
+       }
 
-       return LTTCOMM_OK;
+       ret = LTTCOMM_OK;
+
+end:
+       DBG("Event UST %s %s in channel %s", uevent->attr.name,
+                       to_create ? "created" : "enabled", uchan->name);
+
+       return ret;
 
 error:
        trace_ust_destroy_event(uevent);
        return ret;
 }
 
-#ifdef DISABLE
-int event_ust_disable_tracepoint(struct ltt_ust_session *ustsession,
-               struct ltt_ust_channel *ustchan, char *event_name)
+/*
+ * Disable UST tracepoint of a channel from a UST session.
+ */
+int event_ust_disable_tracepoint(struct ltt_ust_session *usess, int domain,
+               struct ltt_ust_channel *uchan, char *event_name)
 {
        int ret;
-       struct ltt_ust_event *ustevent;
+       struct ltt_ust_event *uevent;
 
-       ustevent = trace_ust_find_event_by_name(ustchan->events, event_name);
-       if (ustevent == NULL) {
-               ret = LTTCOMM_NO_EVENT;
-               goto end;
+       uevent = trace_ust_find_event_by_name(uchan->events, event_name);
+       if (uevent == NULL) {
+               ret = LTTCOMM_UST_EVENT_NOT_FOUND;
+               goto error;
        }
-       //ret = ustctl_disable(ustsession->sock, ustevent->obj);
-       if (ret < 0) {
-               ret = LTTCOMM_UST_ENABLE_FAIL;
+
+       if (uevent->enabled == 0) {
+               /* It's already enabled so everything is OK */
+               ret = LTTCOMM_OK;
                goto end;
        }
-       ustevent->enabled = 0;
+
+       switch (domain) {
+       case LTTNG_DOMAIN_UST:
+               ret = ust_app_disable_event_glb(usess, uchan, uevent);
+               if (ret < 0 && ret != -EEXIST) {
+                       ret = LTTCOMM_UST_DISABLE_FAIL;
+                       goto error;
+               }
+               break;
+       case LTTNG_DOMAIN_UST_EXEC_NAME:
+       case LTTNG_DOMAIN_UST_PID:
+       case LTTNG_DOMAIN_UST_PID_FOLLOW_CHILDREN:
+       default:
+               ret = LTTCOMM_NOT_IMPLEMENTED;
+               goto error;
+       }
+
+       uevent->enabled = 0;
        ret = LTTCOMM_OK;
+
 end:
+       DBG2("Event UST %s disabled in channel %s", uevent->attr.name,
+                       uchan->name);
+
+error:
+       return ret;
+}
+
+/*
+ * Disable all UST tracepoints for a channel from a UST session.
+ */
+int event_ust_disable_all_tracepoints(struct ltt_ust_session *usess, int domain,
+               struct ltt_ust_channel *uchan)
+{
+       int ret, i;
+       size_t size;
+       struct cds_lfht_iter iter;
+       struct ltt_ust_event *uevent = NULL;
+       struct lttng_event *events = NULL;
+
+       switch (domain) {
+       case LTTNG_DOMAIN_UST:
+       {
+               /* Disabling existing events */
+               cds_lfht_for_each_entry(uchan->events, &iter, uevent, node) {
+                       if (uevent->enabled == 1) {
+                               ret = ust_app_disable_event_glb(usess, uchan, uevent);
+                               if (ret < 0) {
+                                       continue;
+                               }
+                               uevent->enabled = 0;
+                       }
+               }
+
+               /* Get all UST available events */
+               size = ust_app_list_events(&events);
+               if (size < 0) {
+                       ret = LTTCOMM_UST_LIST_FAIL;
+                       goto error;
+               }
+
+               for (i = 0; i < size; i++) {
+                       uevent = trace_ust_find_event_by_name(uchan->events,
+                                       events[i].name);
+                       if (uevent != NULL && uevent->enabled == 1) {
+                               ret = ust_app_disable_event_pid(usess, uchan, uevent,
+                                               events[i].pid);
+                               if (ret < 0 && ret != -EEXIST) {
+                                       ret = LTTCOMM_UST_DISABLE_FAIL;
+                                       goto error;
+                               }
+                               uevent->enabled = 0;
+                               continue;
+                       }
+               }
+
+               free(events);
+               break;
+       }
+       case LTTNG_DOMAIN_UST_EXEC_NAME:
+       case LTTNG_DOMAIN_UST_PID:
+       case LTTNG_DOMAIN_UST_PID_FOLLOW_CHILDREN:
+       default:
+               ret = LTTCOMM_NOT_IMPLEMENTED;
+               goto error;
+       }
+
+       return LTTCOMM_OK;
+
+error:
+       free(events);
        return ret;
 }
-#endif

diff --git a/lttng-sessiond/event.h b/lttng-sessiond/event.h
index 2eafeccaeac75c372c9635f3bf7b9c2728ed7728..9ea0e462157f46127d1a3ad729e633cb68e0de0b 100644 (file)
--- a/lttng-sessiond/event.h
+++ b/lttng-sessiond/event.h
@@ -42,9 +42,11 @@ int event_kernel_enable_all(struct ltt_kernel_session *ksession,
 
 int event_ust_enable_tracepoint(struct ltt_ust_session *usess, int domain,
                struct ltt_ust_channel *uchan, struct lttng_event *event);
-int event_ust_disable_tracepoint(struct ltt_ust_session *ustsession,
-               struct ltt_ust_channel *ustchan, char *event_name);
+int event_ust_disable_tracepoint(struct ltt_ust_session *usess, int domain,
+               struct ltt_ust_channel *uchan, char *event_name);
 int event_ust_enable_all_tracepoints(struct ltt_ust_session *usess, int domain,
                struct ltt_ust_channel *uchan);
+int event_ust_disable_all_tracepoints(struct ltt_ust_session *usess, int domain,
+               struct ltt_ust_channel *uchan);
 
 #endif /* _LTT_EVENT_H */

diff --git a/lttng-sessiond/main.c b/lttng-sessiond/main.c
index dda9168bb48222c3f9b5081baaadc6f4eb8cc48b..822c1df7de1d86b5fefcc42d2f5a99e5564202d8 100644 (file)
--- a/lttng-sessiond/main.c
+++ b/lttng-sessiond/main.c
@@ -2409,9 +2409,8 @@ static int cmd_disable_event(struct ltt_session *session, int domain,
        }
        case LTTNG_DOMAIN_UST:
        {
-               struct ltt_ust_session *usess;
                struct ltt_ust_channel *uchan;
-               struct ltt_ust_event *uevent;
+               struct ltt_ust_session *usess;
 
                usess = session->ust_session;
 
@@ -2422,23 +2421,13 @@ static int cmd_disable_event(struct ltt_session *session, int domain,
                        goto error;
                }
 
-               uevent = trace_ust_find_event_by_name(uchan->events, event_name);
-               if (uevent == NULL) {
-                       ret = LTTCOMM_UST_EVENT_NOT_FOUND;
-                       goto error;
-               }
-
-               ret = ust_app_disable_event_glb(usess, uchan, uevent);
-               if (ret < 0) {
-                       ret = LTTCOMM_UST_DISABLE_FAIL;
+               ret = event_ust_disable_tracepoint(usess, domain, uchan, event_name);
+               if (ret != LTTCOMM_OK) {
                        goto error;
                }
 
-               uevent->enabled = 0;
-
-               DBG2("Disable UST event %s in channel %s completed", event_name,
+               DBG3("Disable UST event %s in channel %s completed", event_name,
                                channel_name);
-
                break;
        }
        case LTTNG_DOMAIN_UST_EXEC_NAME:
@@ -2499,13 +2488,12 @@ static int cmd_disable_event_all(struct ltt_session *session, int domain,
                        goto error;
                }
 
-               ret = ust_app_disable_all_event_glb(usess, uchan);
-               if (ret < 0) {
-                       ret = LTTCOMM_UST_DISABLE_FAIL;
+               ret = event_ust_disable_all_tracepoints(usess, domain, uchan);
+               if (ret != 0) {
                        goto error;
                }
 
-               DBG2("Disable all UST event in channel %s completed", channel_name);
+               DBG3("Disable all UST events in channel %s completed", channel_name);
 
                break;
        }

diff --git a/lttng-sessiond/trace-ust.c b/lttng-sessiond/trace-ust.c
index 903a4430393641eccdd9b4367cd65b1bdbf4ff82..8fd144b6ba7fc77aa5052a18cdb7aa1e727f6397 100644 (file)
--- a/lttng-sessiond/trace-ust.c
+++ b/lttng-sessiond/trace-ust.c
@@ -63,7 +63,7 @@ struct ltt_ust_event *trace_ust_find_event_by_name(struct cds_lfht *ht,
        struct cds_lfht_iter iter;
 
        rcu_read_lock();
-       node = hashtable_lookup(ht, (void *) name, strlen(name), &iter);
+       node = hashtable_lookup(ht, (void *)name, strlen(name), &iter);
        if (node == NULL) {
                rcu_read_unlock();
                goto error;

diff --git a/lttng-sessiond/ust-app.c b/lttng-sessiond/ust-app.c
index 904fcc3b947b1195e52e34e59e8051667b4a2331..4f65553c8385d770c6f9c9a1e517a21e8ba0c06c 100644 (file)
--- a/lttng-sessiond/ust-app.c
+++ b/lttng-sessiond/ust-app.c
@@ -996,8 +996,7 @@ error:
  * Disable on the tracer side a ust app event for the session and channel.
  */
 static int disable_ust_app_event(struct ust_app_session *ua_sess,
-               struct ust_app_channel *ua_chan, struct ust_app_event *ua_event,
-               struct ust_app *app)
+               struct ust_app_event *ua_event, struct ust_app *app)
 {
        int ret;
 
@@ -1146,6 +1145,9 @@ int create_ust_app_event(struct ust_app_session *ua_sess,
 
        hashtable_add_unique(ua_chan->events, &ua_event->node);
 
+       DBG2("UST app create event %s for PID %d completed",
+                       ua_event->name, app->key.pid);
+
 end:
 error:
        return ret;
@@ -1593,7 +1595,7 @@ int ust_app_disable_event_glb(struct ltt_ust_session *usess,
                }
                ua_event = caa_container_of(ua_event_node, struct ust_app_event, node);
 
-               ret = disable_ust_app_event(ua_sess, ua_chan, ua_event, app);
+               ret = disable_ust_app_event(ua_sess, ua_event, app);
                if (ret < 0) {
                        /* XXX: Report error someday... */
                        continue;
@@ -1641,7 +1643,7 @@ int ust_app_disable_all_event_glb(struct ltt_ust_session *usess,
 
                /* Disable each events of channel */
                cds_lfht_for_each_entry(ua_chan->events, &uiter, ua_event, node) {
-                       ret = disable_ust_app_event(ua_sess, ua_chan, ua_event, app);
+                       ret = disable_ust_app_event(ua_sess, ua_event, app);
                        if (ret < 0) {
                                /* XXX: Report error someday... */
                                continue;
@@ -1741,24 +1743,23 @@ int ust_app_enable_event_glb(struct ltt_ust_session *usess,
 
                ua_chan = caa_container_of(ua_chan_node, struct ust_app_channel, node);
 
-               ua_event_node = hashtable_lookup(ua_sess->channels,
+               ua_event_node = hashtable_lookup(ua_chan->events,
                                (void*)uevent->attr.name, strlen(uevent->attr.name), &uiter);
                if (ua_event_node == NULL) {
-                       DBG3("UST app enable event %s not found. Skipping app",
-                                       uevent->attr.name);
+                       DBG3("UST app enable event %s not found for app PID %d."
+                                       "Skipping app", uevent->attr.name, app->key.pid);
                        continue;
                }
                ua_event = caa_container_of(ua_event_node, struct ust_app_event, node);
 
                ret = enable_ust_app_event(ua_sess, ua_event, app);
                if (ret < 0) {
-                       /* XXX: Report error someday... */
-                       continue;
+                       goto error;
                }
        }
 
+error:
        rcu_read_unlock();
-
        return ret;
 }
 
@@ -2291,7 +2292,7 @@ int ust_app_enable_event_pid(struct ltt_ust_session *usess,
 
        ua_chan = caa_container_of(ua_chan_node, struct ust_app_channel, node);
 
-       ua_event_node = hashtable_lookup(ua_sess->channels,
+       ua_event_node = hashtable_lookup(ua_chan->events,
                        (void*)uevent->attr.name, strlen(uevent->attr.name), &iter);
        if (ua_event_node == NULL) {
                ret = create_ust_app_event(ua_sess, ua_chan, uevent, app);
@@ -2311,3 +2312,59 @@ error:
        rcu_read_unlock();
        return ret;
 }
+
+/*
+ * Disable event for a channel from a UST session for a specific PID.
+ */
+int ust_app_disable_event_pid(struct ltt_ust_session *usess,
+               struct ltt_ust_channel *uchan, struct ltt_ust_event *uevent, pid_t pid)
+{
+       int ret = 0;
+       struct cds_lfht_iter iter;
+       struct cds_lfht_node *ua_chan_node, *ua_event_node;
+       struct ust_app *app;
+       struct ust_app_session *ua_sess;
+       struct ust_app_channel *ua_chan;
+       struct ust_app_event *ua_event;
+
+       DBG("UST app disabling event %s for PID %d", uevent->attr.name, pid);
+
+       rcu_read_lock();
+
+       app = ust_app_find_by_pid(pid);
+       if (app == NULL) {
+               ERR("UST app disable event per PID %d not found", pid);
+               ret = -1;
+               goto error;
+       }
+
+       ua_sess = lookup_session_by_app(usess, app);
+       /* If ua_sess is NULL, there is a code flow error */
+       assert(ua_sess);
+
+       /* Lookup channel in the ust app session */
+       ua_chan_node = hashtable_lookup(ua_sess->channels, (void *)uchan->name,
+                       strlen(uchan->name), &iter);
+       if (ua_chan_node == NULL) {
+               /* Channel does not exist, skip disabling */
+               goto error;
+       }
+       ua_chan = caa_container_of(ua_chan_node, struct ust_app_channel, node);
+
+       ua_event_node = hashtable_lookup(ua_chan->events,
+                       (void*)uevent->attr.name, strlen(uevent->attr.name), &iter);
+       if (ua_event_node == NULL) {
+               /* Event does not exist, skip disabling */
+               goto error;
+       }
+       ua_event = caa_container_of(ua_event_node, struct ust_app_event, node);
+
+       ret = disable_ust_app_event(ua_sess, ua_event, app);
+       if (ret < 0) {
+               goto error;
+       }
+
+error:
+       rcu_read_unlock();
+       return ret;
+}

diff --git a/lttng-sessiond/ust-app.h b/lttng-sessiond/ust-app.h
index 9fb4df1a70b8c72d35b92859e0bb46d42d5a1ef7..a5ae56a21398d1f7f6c482a3347acf0bc96da930 100644 (file)
--- a/lttng-sessiond/ust-app.h
+++ b/lttng-sessiond/ust-app.h
@@ -129,6 +129,9 @@ int ust_app_create_channel_glb(struct ltt_ust_session *usess,
                struct ltt_ust_channel *uchan);
 int ust_app_create_event_glb(struct ltt_ust_session *usess,
                struct ltt_ust_channel *uchan, struct ltt_ust_event *uevent);
+int ust_app_disable_event_pid(struct ltt_ust_session *usess,
+               struct ltt_ust_channel *uchan, struct ltt_ust_event *uevent,
+               pid_t pid);
 int ust_app_enable_event_pid(struct ltt_ust_session *usess,
                struct ltt_ust_channel *uchan, struct ltt_ust_event *uevent,
                pid_t pid);
@@ -298,6 +301,13 @@ int ust_app_enable_event_pid(struct ltt_ust_session *usess,
 {
        return 0;
 }
+static inline
+int ust_app_disable_event_pid(struct ltt_ust_session *usess,
+               struct ltt_ust_channel *uchan, struct ltt_ust_event *uevent,
+               pid_t pid)
+{
+       return 0;
+}
 
 #endif /* HAVE_LIBLTTNG_UST_CTL */