Fix RCU-related hangs: incorrect lttng_ht_destroy use
[lttng-tools.git] / src / bin / lttng-sessiond / ust-registry.c
index 31d33df4c72c294875dd26cd165ba692dcbb7b86..7250dd12d751b0edcce93ffd4b886f8bdd7c7395 100644 (file)
  */
 #define _GNU_SOURCE
 #include <assert.h>
+#include <inttypes.h>
 
 #include <common/common.h>
+#include <common/hashtable/utils.h>
+#include <lttng/lttng.h>
+
 #include "ust-registry.h"
 
 /*
@@ -53,6 +57,19 @@ no_match:
        return 0;
 }
 
+static unsigned long ht_hash_event(void *_key, unsigned long seed)
+{
+       uint64_t xored_key;
+       struct ust_registry_event *key = _key;
+
+       assert(key);
+
+       xored_key = (uint64_t) (hash_key_str(key->name, seed) ^
+                       hash_key_str(key->signature, seed));
+
+       return hash_key_u64(&xored_key, seed);
+}
+
 /*
  * Allocate event and initialize it. This does NOT set a valid event id from a
  * registry.
@@ -82,7 +99,7 @@ static struct ust_registry_event *alloc_event(int session_objd,
                strncpy(event->name, name, sizeof(event->name));
                event->name[sizeof(event->name) - 1] = '\0';
        }
-       lttng_ht_node_init_str(&event->node, event->name);
+       cds_lfht_node_init(&event->node.node);
 
 error:
        return event;
@@ -110,8 +127,8 @@ static void destroy_event(struct ust_registry_event *event)
  */
 static void destroy_event_rcu(struct rcu_head *head)
 {
-       struct lttng_ht_node_str *node =
-               caa_container_of(head, struct lttng_ht_node_str, head);
+       struct lttng_ht_node_u64 *node =
+               caa_container_of(head, struct lttng_ht_node_u64, head);
        struct ust_registry_event *event =
                caa_container_of(node, struct ust_registry_event, node);
 
@@ -128,7 +145,7 @@ static void destroy_event_rcu(struct rcu_head *head)
 struct ust_registry_event *ust_registry_find_event(
                struct ust_registry_channel *chan, char *name, char *sig)
 {
-       struct lttng_ht_node_str *node;
+       struct lttng_ht_node_u64 *node;
        struct lttng_ht_iter iter;
        struct ust_registry_event *event = NULL;
        struct ust_registry_event key;
@@ -142,9 +159,9 @@ struct ust_registry_event *ust_registry_find_event(
        key.name[sizeof(key.name) - 1] = '\0';
        key.signature = sig;
 
-       cds_lfht_lookup(chan->ht->ht, chan->ht->hash_fct(name, lttng_ht_seed),
+       cds_lfht_lookup(chan->ht->ht, chan->ht->hash_fct(&key, lttng_ht_seed),
                        chan->ht->match_fct, &key, &iter.iter);
-       node = lttng_ht_iter_get_node_str(&iter);
+       node = lttng_ht_iter_get_node_u64(&iter);
        if (!node) {
                goto end;
        }
@@ -165,19 +182,20 @@ end:
  * Should be called with session registry mutex held.
  */
 int ust_registry_create_event(struct ust_registry_session *session,
-               struct ust_registry_channel *chan,
-               int session_objd, int channel_objd, char *name, char *sig,
-               size_t nr_fields, struct ustctl_field *fields, int loglevel,
-               char *model_emf_uri, uint32_t *event_id)
+               uint64_t chan_key, int session_objd, int channel_objd, char *name,
+               char *sig, size_t nr_fields, struct ustctl_field *fields, int loglevel,
+               char *model_emf_uri, int buffer_type, uint32_t *event_id_p)
 {
        int ret;
+       uint32_t event_id;
        struct cds_lfht_node *nptr;
        struct ust_registry_event *event = NULL;
+       struct ust_registry_channel *chan;
 
        assert(session);
-       assert(chan);
        assert(name);
        assert(sig);
+       assert(event_id_p);
 
        /*
         * This should not happen but since it comes from the UST tracer, an
@@ -188,54 +206,76 @@ int ust_registry_create_event(struct ust_registry_session *session,
                goto error;
        }
 
+       rcu_read_lock();
+
+       chan = ust_registry_channel_find(session, chan_key);
+       if (!chan) {
+               ret = -EINVAL;
+               goto error_unlock;
+       }
+
        /* Check if we've reached the maximum possible id. */
        if (ust_registry_is_max_id(chan->used_event_id)) {
                ret = -ENOENT;
-               goto error;
+               goto error_unlock;
        }
 
        event = alloc_event(session_objd, channel_objd, name, sig, nr_fields,
                        fields, loglevel, model_emf_uri);
        if (!event) {
                ret = -ENOMEM;
-               goto error;
+               goto error_unlock;
        }
 
-       event->id = ust_registry_get_next_event_id(chan);
-
        DBG3("UST registry creating event with event: %s, sig: %s, id: %u, "
-                       "chan_objd: %u, sess_objd: %u", event->name, event->signature,
-                       event->id, event->channel_objd, event->session_objd);
+                       "chan_objd: %u, sess_objd: %u, chan_id: %u", event->name,
+                       event->signature, event->id, event->channel_objd,
+                       event->session_objd, chan->chan_id);
 
-       rcu_read_lock();
        /*
         * This is an add unique with a custom match function for event. The node
         * are matched using the event name and signature.
         */
-       nptr = cds_lfht_add_unique(chan->ht->ht, chan->ht->hash_fct(event->node.key,
+       nptr = cds_lfht_add_unique(chan->ht->ht, chan->ht->hash_fct(event,
                                lttng_ht_seed), chan->ht->match_fct, event, &event->node.node);
        if (nptr != &event->node.node) {
-               ERR("UST registry create event add unique failed for event: %s, "
-                               "sig: %s, id: %u, chan_objd: %u, sess_objd: %u", event->name,
-                               event->signature, event->id, event->channel_objd,
-                               event->session_objd);
-               ret = -EINVAL;
-               goto error_unlock;
+               if (buffer_type == LTTNG_BUFFER_PER_UID) {
+                       /*
+                        * This is normal, we just have to send the event id of the
+                        * returned node and make sure we destroy the previously allocated
+                        * event object.
+                        */
+                       destroy_event(event);
+                       event = caa_container_of(nptr, struct ust_registry_event,
+                                       node.node);
+                       assert(event);
+                       event_id = event->id;
+               } else {
+                       ERR("UST registry create event add unique failed for event: %s, "
+                                       "sig: %s, id: %u, chan_objd: %u, sess_objd: %u",
+                                       event->name, event->signature, event->id,
+                                       event->channel_objd, event->session_objd);
+                       ret = -EINVAL;
+                       goto error_unlock;
+               }
+       } else {
+               /* Request next event id if the node was successfully added. */
+               event_id = event->id = ust_registry_get_next_event_id(chan);
        }
 
-       /* Set event id if user wants it. */
-       if (event_id) {
-               *event_id = event->id;
-       }
-       rcu_read_unlock();
+       *event_id_p = event_id;
 
-       /* Append to metadata */
-       ret = ust_metadata_event_statedump(session, chan, event);
-       if (ret) {
-               ERR("Error appending event metadata (errno = %d)", ret);
-               return ret;
+       if (!event->metadata_dumped) {
+               /* Append to metadata */
+               ret = ust_metadata_event_statedump(session, chan, event);
+               if (ret) {
+                       ERR("Error appending event metadata (errno = %d)", ret);
+                       rcu_read_unlock();
+                       return ret;
+               }
        }
 
+       rcu_read_unlock();
        return 0;
 
 error_unlock:
@@ -269,49 +309,158 @@ void ust_registry_destroy_event(struct ust_registry_channel *chan,
 }
 
 /*
- * Initialize registry with default values.
+ * We need to execute ht_destroy outside of RCU read-side critical
+ * section, so we postpone its execution using call_rcu. It is simpler
+ * than to change the semantic of the many callers of
+ * destroy_channel().
  */
-void ust_registry_channel_init(struct ust_registry_session *session,
-               struct ust_registry_channel *chan)
+static
+void destroy_channel_rcu(struct rcu_head *head)
 {
-       assert(chan);
-
-       memset(chan, 0, sizeof(struct ust_registry_channel));
-
-       chan->ht = lttng_ht_new(0, LTTNG_HT_TYPE_STRING);
-       assert(chan->ht);
+       struct ust_registry_channel *chan =
+               caa_container_of(head, struct ust_registry_channel, rcu_head);
 
-       /* Set custom match function. */
-       chan->ht->match_fct = ht_match_event;
+       lttng_ht_destroy(chan->ht);
+       free(chan);
 }
 
 /*
  * Destroy every element of the registry and free the memory. This does NOT
  * free the registry pointer since it might not have been allocated before so
  * it's the caller responsability.
- *
- * This MUST be called within a RCU read side lock section.
  */
-void ust_registry_channel_destroy(struct ust_registry_session *session,
-               struct ust_registry_channel *chan)
+static void destroy_channel(struct ust_registry_channel *chan)
 {
        struct lttng_ht_iter iter;
        struct ust_registry_event *event;
 
        assert(chan);
 
+       rcu_read_lock();
        /* Destroy all event associated with this registry. */
        cds_lfht_for_each_entry(chan->ht->ht, &iter.iter, event, node.node) {
                /* Delete the node from the ht and free it. */
                ust_registry_destroy_event(chan, event);
        }
-       lttng_ht_destroy(chan->ht);
+       rcu_read_unlock();
+       call_rcu(&chan->rcu_head, destroy_channel_rcu);
 }
 
 /*
  * Initialize registry with default values.
  */
-int ust_registry_session_init(struct ust_registry_session *session,
+int ust_registry_channel_add(struct ust_registry_session *session,
+               uint64_t key)
+{
+       int ret = 0;
+       struct ust_registry_channel *chan;
+
+       assert(session);
+
+       chan = zmalloc(sizeof(*chan));
+       if (!chan) {
+               PERROR("zmalloc ust registry channel");
+               ret = -ENOMEM;
+               goto error;
+       }
+
+       chan->ht = lttng_ht_new(0, LTTNG_HT_TYPE_STRING);
+       if (!chan->ht) {
+               ret = -ENOMEM;
+               goto error;
+       }
+
+       /* Set custom match function. */
+       chan->ht->match_fct = ht_match_event;
+       chan->ht->hash_fct = ht_hash_event;
+
+       /*
+        * Assign a channel ID right now since the event notification comes
+        * *before* the channel notify so the ID needs to be set at this point so
+        * the metadata can be dumped for that event.
+        */
+       if (ust_registry_is_max_id(session->used_channel_id)) {
+               ret = -1;
+               goto error;
+       }
+       chan->chan_id = ust_registry_get_next_chan_id(session);
+
+       rcu_read_lock();
+       lttng_ht_node_init_u64(&chan->node, key);
+       lttng_ht_add_unique_u64(session->channels, &chan->node);
+       rcu_read_unlock();
+
+error:
+       return ret;
+}
+
+/*
+ * Find a channel in the given registry. RCU read side lock MUST be acquired
+ * before calling this function and as long as the event reference is kept by
+ * the caller.
+ *
+ * On success, the pointer is returned else NULL.
+ */
+struct ust_registry_channel *ust_registry_channel_find(
+               struct ust_registry_session *session, uint64_t key)
+{
+       struct lttng_ht_node_u64 *node;
+       struct lttng_ht_iter iter;
+       struct ust_registry_channel *chan = NULL;
+
+       assert(session);
+       assert(session->channels);
+
+       DBG3("UST registry channel finding key %" PRIu64, key);
+
+       lttng_ht_lookup(session->channels, &key, &iter);
+       node = lttng_ht_iter_get_node_u64(&iter);
+       if (!node) {
+               goto end;
+       }
+       chan = caa_container_of(node, struct ust_registry_channel, node);
+
+end:
+       return chan;
+}
+
+/*
+ * Remove channel using key from registry and free memory.
+ */
+void ust_registry_channel_del_free(struct ust_registry_session *session,
+               uint64_t key)
+{
+       struct lttng_ht_iter iter;
+       struct ust_registry_channel *chan;
+       int ret;
+
+       assert(session);
+
+       rcu_read_lock();
+       chan = ust_registry_channel_find(session, key);
+       if (!chan) {
+               rcu_read_unlock();
+               goto end;
+       }
+
+       iter.iter.node = &chan->node.node;
+       ret = lttng_ht_del(session->channels, &iter);
+       assert(!ret);
+       rcu_read_unlock();
+       destroy_channel(chan);
+
+end:
+       return;
+}
+
+/*
+ * Initialize registry with default values and set the newly allocated session
+ * pointer to sessionp.
+ *
+ * Return 0 on success and sessionp is set or else return -1 and sessionp is
+ * kept untouched.
+ */
+int ust_registry_session_init(struct ust_registry_session **sessionp,
                struct ust_app *app,
                uint32_t bits_per_long,
                uint32_t uint8_t_alignment,
@@ -319,13 +468,20 @@ int ust_registry_session_init(struct ust_registry_session *session,
                uint32_t uint32_t_alignment,
                uint32_t uint64_t_alignment,
                uint32_t long_alignment,
-               int byte_order)
+               int byte_order,
+               uint32_t major,
+               uint32_t minor)
 {
        int ret;
+       struct ust_registry_session *session;
 
-       assert(session);
+       assert(sessionp);
 
-       memset(session, 0, sizeof(struct ust_registry_session));
+       session = zmalloc(sizeof(*session));
+       if (!session) {
+               PERROR("zmalloc ust registry session");
+               goto error;
+       }
 
        pthread_mutex_init(&session->lock, NULL);
        session->bits_per_long = bits_per_long;
@@ -336,6 +492,11 @@ int ust_registry_session_init(struct ust_registry_session *session,
        session->long_alignment = long_alignment;
        session->byte_order = byte_order;
 
+       session->channels = lttng_ht_new(0, LTTNG_HT_TYPE_U64);
+       if (!session->channels) {
+               goto error;
+       }
+
        ret = lttng_uuid_generate(session->uuid);
        if (ret) {
                ERR("Failed to generate UST uuid (errno = %d)", ret);
@@ -343,13 +504,15 @@ int ust_registry_session_init(struct ust_registry_session *session,
        }
 
        pthread_mutex_lock(&session->lock);
-       ret = ust_metadata_session_statedump(session, app);
+       ret = ust_metadata_session_statedump(session, app, major, minor);
        pthread_mutex_unlock(&session->lock);
        if (ret) {
                ERR("Failed to generate session metadata (errno = %d)", ret);
                goto error;
        }
 
+       *sessionp = session;
+
        return 0;
 
 error:
@@ -363,10 +526,23 @@ error:
 void ust_registry_session_destroy(struct ust_registry_session *reg)
 {
        int ret;
+       struct lttng_ht_iter iter;
+       struct ust_registry_channel *chan;
 
        /* On error, EBUSY can be returned if lock. Code flow error. */
        ret = pthread_mutex_destroy(&reg->lock);
        assert(!ret);
 
+       rcu_read_lock();
+       /* Destroy all event associated with this registry. */
+       cds_lfht_for_each_entry(reg->channels->ht, &iter.iter, chan, node.node) {
+               /* Delete the node from the ht and free it. */
+               ret = lttng_ht_del(reg->channels, &iter);
+               assert(!ret);
+               destroy_channel(chan);
+       }
+       rcu_read_unlock();
+
+       lttng_ht_destroy(reg->channels);
        free(reg->metadata);
 }
This page took 0.028367 seconds and 4 git commands to generate.