Centralize locking with ust_lock, fork handling
authorMathieu Desnoyers <mathieu.desnoyers@efficios.com>
Fri, 26 Aug 2011 19:46:41 +0000 (15:46 -0400)
committerMathieu Desnoyers <mathieu.desnoyers@efficios.com>
Fri, 26 Aug 2011 19:46:41 +0000 (15:46 -0400)
A single lock is held across forks (ust_lock), which synchronizes _all_
operations performed on UST structures.

- cleanup/reinit UST after fork in child.
- ensure to unlock URCU-bp mutex before calling synchronize_rcu again.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
libust/ltt-events.c
libust/ltt-probes.c
libust/ltt-tracer-core.h
libust/lttng-ust-abi.c
libust/lttng-ust-comm.c
libust/tracepoint.c

index 0c6ef373079b417bc44a66e10e0f2dd0638706bb..325fe1d06df8e54d537ba03468306dfe359fea17 100644 (file)
@@ -34,16 +34,17 @@ typedef u32 uint32_t;
 
 /*
  * The sessions mutex is the centralized mutex across UST tracing
- * control and probe registration.
+ * control and probe registration. All operations within this file are
+ * called by the communication thread, under ust_lock protection.
  */
 static DEFINE_MUTEX(sessions_mutex);
 
-void lock_ust(void)
+void ust_lock(void)
 {
        pthread_mutex_lock(&sessions_mutex);
 }
 
-void unlock_ust(void)
+void ust_unlock(void)
 {
        pthread_mutex_unlock(&sessions_mutex);
 }
@@ -162,12 +163,10 @@ struct ltt_session *ltt_session_create(void)
        session = zmalloc(sizeof(struct ltt_session));
        if (!session)
                return NULL;
-       pthread_mutex_lock(&sessions_mutex);
        CDS_INIT_LIST_HEAD(&session->chan);
        CDS_INIT_LIST_HEAD(&session->events);
        uuid_generate(session->uuid);
        cds_list_add(&session->list, &sessions);
-       pthread_mutex_unlock(&sessions_mutex);
        return session;
 }
 
@@ -177,7 +176,6 @@ void ltt_session_destroy(struct ltt_session *session)
        struct ltt_event *event, *tmpevent;
        int ret;
 
-       pthread_mutex_lock(&sessions_mutex);
        CMM_ACCESS_ONCE(session->active) = 0;
        cds_list_for_each_entry(event, &session->events, list) {
                ret = _ltt_event_unregister(event);
@@ -189,7 +187,6 @@ void ltt_session_destroy(struct ltt_session *session)
        cds_list_for_each_entry_safe(chan, tmpchan, &session->chan, list)
                _ltt_channel_destroy(chan);
        cds_list_del(&session->list);
-       pthread_mutex_unlock(&sessions_mutex);
        free(session);
 }
 
@@ -198,7 +195,6 @@ int ltt_session_enable(struct ltt_session *session)
        int ret = 0;
        struct ltt_channel *chan;
 
-       pthread_mutex_lock(&sessions_mutex);
        if (session->active) {
                ret = -EBUSY;
                goto end;
@@ -223,7 +219,6 @@ int ltt_session_enable(struct ltt_session *session)
        if (ret)
                CMM_ACCESS_ONCE(session->active) = 0;
 end:
-       pthread_mutex_unlock(&sessions_mutex);
        return ret;
 }
 
@@ -231,14 +226,12 @@ int ltt_session_disable(struct ltt_session *session)
 {
        int ret = 0;
 
-       pthread_mutex_lock(&sessions_mutex);
        if (!session->active) {
                ret = -EBUSY;
                goto end;
        }
        CMM_ACCESS_ONCE(session->active) = 0;
 end:
-       pthread_mutex_unlock(&sessions_mutex);
        return ret;
 }
 
@@ -311,7 +304,6 @@ struct ltt_channel *ltt_channel_create(struct ltt_session *session,
        struct ltt_channel *chan;
        struct ltt_transport *transport;
 
-       pthread_mutex_lock(&sessions_mutex);
        if (session->been_active)
                goto active;    /* Refuse to add channel to active session */
        transport = ltt_transport_find(transport_name);
@@ -338,7 +330,6 @@ struct ltt_channel *ltt_channel_create(struct ltt_session *session,
        chan->enabled = 1;
        chan->ops = &transport->ops;
        cds_list_add(&chan->list, &session->chan);
-       pthread_mutex_unlock(&sessions_mutex);
        return chan;
 
 create_error:
@@ -346,7 +337,6 @@ create_error:
 nomem:
 notransport:
 active:
-       pthread_mutex_unlock(&sessions_mutex);
        return NULL;
 }
 
@@ -372,7 +362,6 @@ struct ltt_event *ltt_event_create(struct ltt_channel *chan,
        struct ltt_event *event;
        int ret;
 
-       pthread_mutex_lock(&sessions_mutex);
        if (chan->used_event_id == -1UL)
                goto full;
        /*
@@ -426,7 +415,6 @@ struct ltt_event *ltt_event_create(struct ltt_channel *chan,
                        goto statedump_error;
        }
        cds_list_add(&event->list, &chan->session->events);
-       pthread_mutex_unlock(&sessions_mutex);
        return event;
 
 statedump_error:
@@ -442,7 +430,6 @@ register_error:
 cache_error:
 exist:
 full:
-       pthread_mutex_unlock(&sessions_mutex);
        return NULL;
 }
 
@@ -494,7 +481,7 @@ void _ltt_event_destroy(struct ltt_event *event)
 
 /*
  * We have exclusive access to our metadata buffer (protected by the
- * sessions_mutex), so we can do racy operations such as looking for
+ * ust_lock), so we can do racy operations such as looking for
  * remaining space left in packet and write, since mutual exclusion
  * protects us from concurrent writes.
  */
@@ -990,24 +977,21 @@ end:
  * @transport: transport structure
  *
  * Registers a transport which can be used as output to extract the data out of
- * LTTng.
+ * LTTng. Called with ust_lock held.
  */
 void ltt_transport_register(struct ltt_transport *transport)
 {
-       pthread_mutex_lock(&sessions_mutex);
        cds_list_add_tail(&transport->node, &ltt_transport_list);
-       pthread_mutex_unlock(&sessions_mutex);
 }
 
 /**
  * ltt_transport_unregister - LTT transport unregistration
  * @transport: transport structure
+ * Called with ust_lock held.
  */
 void ltt_transport_unregister(struct ltt_transport *transport)
 {
-       pthread_mutex_lock(&sessions_mutex);
        cds_list_del(&transport->node);
-       pthread_mutex_unlock(&sessions_mutex);
 }
 
 void ltt_events_exit(void)
index 76156f03d5bb7490ba67e8468c3b499d9ee99700..04392d8e6f479ddd452eb388933720a58f753d39 100644 (file)
@@ -17,7 +17,7 @@
 #include "ltt-tracer-core.h"
 
 /*
- * probe list is protected by lock_ust()/unlock_ust().
+ * probe list is protected by ust_lock()/ust_unlock().
  */
 static CDS_LIST_HEAD(probe_list);
 
@@ -41,7 +41,7 @@ int ltt_probe_register(struct lttng_probe_desc *desc)
        int ret = 0;
        int i;
 
-       lock_ust();
+       ust_lock();
        /*
         * TODO: This is O(N^2). Turn into a hash table when probe registration
         * overhead becomes an issue.
@@ -62,15 +62,15 @@ int ltt_probe_register(struct lttng_probe_desc *desc)
                assert(!ret);
        }
 end:
-       unlock_ust();
+       ust_unlock();
        return ret;
 }
 
 void ltt_probe_unregister(struct lttng_probe_desc *desc)
 {
-       lock_ust();
+       ust_lock();
        cds_list_del(&desc->head);
-       unlock_ust();
+       ust_unlock();
 }
 
 /*
index 1a329780f7875e6105df70b3cf88733640849633..fe2ecb9e455a4e9b39d4843d8a4b00745eaaa43e 100644 (file)
@@ -34,7 +34,7 @@ struct ltt_session;
 struct ltt_channel;
 struct ltt_event;
 
-void lock_ust(void);
-void unlock_ust(void);
+void ust_lock(void);
+void ust_unlock(void);
 
 #endif /* _LTT_TRACER_CORE_H */
index 8afac10d8b2760000a40be6b5b145445c801c75e..53dd9a27c3e5793da02c8a437e69ace195d3c0c8 100644 (file)
@@ -188,6 +188,7 @@ void objd_table_destroy(void)
                        ops->release(i);
        }
        free(objd_table.array);
+       objd_table.freelist_head = -1;
 }
 
 /*
index 1e4031be273fcbcc7a29d397ab0bb8db3bbcfa22..b6c5de51e456e5e3d8f501fd75265819eeebf199 100644 (file)
 static int initialized;
 
 /*
- * communication thread mutex. Held when handling a command, also held
- * by fork() to deal with removal of threads, and by exit path.
+ * The ust_lock/ust_unlock lock is used as a communication thread mutex.
+ * Held when handling a command, also held by fork() to deal with
+ * removal of threads, and by exit path.
  */
-static pthread_mutex_t lttng_ust_comm_mutex = PTHREAD_MUTEX_INITIALIZER;
 
 /* Should the ust comm thread quit ? */
 static int lttng_ust_comm_should_quit;
@@ -207,7 +207,7 @@ int handle_message(struct sock_info *sock_info,
        const struct objd_ops *ops;
        struct lttcomm_ust_reply lur;
 
-       pthread_mutex_lock(&lttng_ust_comm_mutex);
+       ust_lock();
 
        memset(&lur, 0, sizeof(lur));
 
@@ -255,7 +255,7 @@ end:
        }
        ret = send_reply(sock, &lur);
 
-       pthread_mutex_unlock(&lttng_ust_comm_mutex);
+       ust_unlock();
        return ret;
 }
 
@@ -295,10 +295,10 @@ void *ust_listener_thread(void *arg)
 
        /* Restart trying to connect to the session daemon */
 restart:
-       pthread_mutex_lock(&lttng_ust_comm_mutex);
+       ust_lock();
 
        if (lttng_ust_comm_should_quit) {
-               pthread_mutex_unlock(&lttng_ust_comm_mutex);
+               ust_unlock();
                goto quit;
        }
 
@@ -322,7 +322,7 @@ restart:
                 */
                ret = handle_register_done(sock_info);
                assert(!ret);
-               pthread_mutex_unlock(&lttng_ust_comm_mutex);
+               ust_unlock();
                sleep(5);
                goto restart;
        }
@@ -337,7 +337,7 @@ restart:
                ret = lttng_abi_create_root_handle();
                if (ret) {
                        ERR("Error creating root handle");
-                       pthread_mutex_unlock(&lttng_ust_comm_mutex);
+                       ust_unlock();
                        goto quit;
                }
                sock_info->root_handle = ret;
@@ -352,11 +352,11 @@ restart:
                 */
                ret = handle_register_done(sock_info);
                assert(!ret);
-               pthread_mutex_unlock(&lttng_ust_comm_mutex);
+               ust_unlock();
                sleep(5);
                goto restart;
        }
-       pthread_mutex_unlock(&lttng_ust_comm_mutex);
+       ust_unlock();
 
        for (;;) {
                ssize_t len;
@@ -501,6 +501,27 @@ void __attribute__((constructor)) lttng_ust_init(void)
        }
 }
 
+static
+void lttng_ust_cleanup(int exiting)
+{
+       cleanup_sock_info(&global_apps);
+       if (local_apps.allowed) {
+               cleanup_sock_info(&local_apps);
+       }
+       lttng_ust_abi_exit();
+       ltt_events_exit();
+       ltt_ring_buffer_client_discard_exit();
+       ltt_ring_buffer_client_overwrite_exit();
+       ltt_ring_buffer_metadata_client_exit();
+       exit_tracepoint();
+       if (!exiting) {
+               /* Reinitialize values for fork */
+               sem_count = 2;
+               lttng_ust_comm_should_quit = 0;
+               initialized = 0;
+       }
+}
+
 void __attribute__((destructor)) lttng_ust_exit(void)
 {
        int ret;
@@ -516,32 +537,21 @@ void __attribute__((destructor)) lttng_ust_exit(void)
         * mutexes to ensure it is not in a mutex critical section when
         * pthread_cancel is later called.
         */
-       pthread_mutex_lock(&lttng_ust_comm_mutex);
+       ust_lock();
        lttng_ust_comm_should_quit = 1;
-       pthread_mutex_unlock(&lttng_ust_comm_mutex);
+       ust_unlock();
 
        ret = pthread_cancel(global_apps.ust_listener);
        if (ret) {
                ERR("Error cancelling global ust listener thread");
        }
-
-       cleanup_sock_info(&global_apps);
-
        if (local_apps.allowed) {
                ret = pthread_cancel(local_apps.ust_listener);
                if (ret) {
                        ERR("Error cancelling local ust listener thread");
                }
-
-               cleanup_sock_info(&local_apps);
        }
-
-       lttng_ust_abi_exit();
-       ltt_events_exit();
-       ltt_ring_buffer_client_discard_exit();
-       ltt_ring_buffer_client_overwrite_exit();
-       ltt_ring_buffer_metadata_client_exit();
-       exit_tracepoint();
+       lttng_ust_cleanup(1);
 }
 
 /*
@@ -568,7 +578,7 @@ void ust_before_fork(ust_fork_info_t *fork_info)
        if (ret == -1) {
                PERROR("sigprocmask");
        }
-       pthread_mutex_lock(&lttng_ust_comm_mutex);
+       ust_lock();
        rcu_bp_before_fork();
 }
 
@@ -576,7 +586,8 @@ static void ust_after_fork_common(ust_fork_info_t *fork_info)
 {
        int ret;
 
-       pthread_mutex_unlock(&lttng_ust_comm_mutex);
+       DBG("process %d", getpid());
+       ust_unlock();
        /* Restore signals */
        ret = sigprocmask(SIG_SETMASK, &fork_info->orig_sigs, NULL);
        if (ret == -1) {
@@ -586,15 +597,28 @@ static void ust_after_fork_common(ust_fork_info_t *fork_info)
 
 void ust_after_fork_parent(ust_fork_info_t *fork_info)
 {
+       DBG("process %d", getpid());
        rcu_bp_after_fork_parent();
        /* Release mutexes and reenable signals */
        ust_after_fork_common(fork_info);
 }
 
+/*
+ * After fork, in the child, we need to cleanup all the leftover state,
+ * except the worker thread which already magically disappeared thanks
+ * to the weird Linux fork semantics. After tyding up, we call
+ * lttng_ust_init() again to start over as a new PID.
+ *
+ * This is meant for forks() that have tracing in the child between the
+ * fork and following exec call (if there is any).
+ */
 void ust_after_fork_child(ust_fork_info_t *fork_info)
 {
+       DBG("process %d", getpid());
        /* Release urcu mutexes */
        rcu_bp_after_fork_child();
+       lttng_ust_cleanup(0);
+       lttng_ust_init();
        /* Release mutexes and reenable signals */
        ust_after_fork_common(fork_info);
 }
index a006f700cc18af292d3061e622b6fb1a9ba46f3b..c5121995cdfadb1adce16195922fe7301c257092 100644 (file)
@@ -40,29 +40,12 @@ static void (*new_tracepoint_cb)(struct tracepoint *);
 static CDS_LIST_HEAD(libs);
 
 /*
- * Allow nested mutex for mutex listing and nested enable.
+ * The UST lock protects the library tracepoints, the hash table, and
+ * the library list.
+ * All calls to the tracepoint API must be protected by the UST lock,
+ * excepts calls to tracepoint_register_lib and
+ * tracepoint_unregister_lib, which take the UST lock themselves.
  */
-static __thread int nested_mutex;
-
-/*
- * Tracepoints mutex protects the library tracepoints, the hash table,
- * and the library list.
- */
-static pthread_mutex_t tracepoints_mutex;
-
-static
-void lock_tracepoints(void)
-{
-       if (!(nested_mutex++))
-               pthread_mutex_lock(&tracepoints_mutex);
-}
-
-static
-void unlock_tracepoints(void)
-{
-       if (!(--nested_mutex))
-               pthread_mutex_unlock(&tracepoints_mutex);
-}
 
 /*
  * Tracepoint hash table, containing the active tracepoints.
@@ -257,7 +240,7 @@ static struct tracepoint_entry *add_tracepoint(const char *name)
 
 /*
  * Remove the tracepoint from the tracepoint hash table. Must be called with
- * mutex_lock held.
+ * ust_lock held.
  */
 static inline void remove_tracepoint(struct tracepoint_entry *e)
 {
@@ -331,12 +314,10 @@ static void lib_update_tracepoints(void)
 {
        struct tracepoint_lib *lib;
 
-       lock_tracepoints();
        cds_list_for_each_entry(lib, &libs, list) {
                tracepoint_update_probe_range(lib->tracepoints_start,
                                lib->tracepoints_start + lib->tracepoints_count);
        }
-       unlock_tracepoints();
 }
 
 /*
@@ -373,14 +354,13 @@ tracepoint_add_probe(const char *name, void *probe, void *data)
  *
  * Returns 0 if ok, error value on error.
  * The probe address must at least be aligned on the architecture pointer size.
+ * Called with the UST lock held.
  */
 int __tracepoint_probe_register(const char *name, void *probe, void *data)
 {
        void *old;
 
-       lock_tracepoints();
        old = tracepoint_add_probe(name, probe, data);
-       unlock_tracepoints();
        if (IS_ERR(old))
                return PTR_ERR(old);
 
@@ -411,18 +391,13 @@ static void *tracepoint_remove_probe(const char *name, void *probe, void *data)
  * @probe: probe function pointer
  * @probe: probe data pointer
  *
- * We do not need to call a synchronize_sched to make sure the probes have
- * finished running before doing a module unload, because the module unload
- * itself uses stop_machine(), which insures that every preempt disabled section
- * have finished.
+ * Called with the UST lock held.
  */
 int __tracepoint_probe_unregister(const char *name, void *probe, void *data)
 {
        void *old;
 
-       lock_tracepoints();
        old = tracepoint_remove_probe(name, probe, data);
-       unlock_tracepoints();
        if (IS_ERR(old))
                return PTR_ERR(old);
 
@@ -447,20 +422,18 @@ static void tracepoint_add_old_probes(void *old)
  * @probe: probe handler
  *
  * caller must call tracepoint_probe_update_all()
+ * Called with the UST lock held.
  */
 int tracepoint_probe_register_noupdate(const char *name, void *probe,
                                       void *data)
 {
        void *old;
 
-       lock_tracepoints();
        old = tracepoint_add_probe(name, probe, data);
        if (IS_ERR(old)) {
-               unlock_tracepoints();
                return PTR_ERR(old);
        }
        tracepoint_add_old_probes(old);
-       unlock_tracepoints();
        return 0;
 }
 
@@ -470,40 +443,36 @@ int tracepoint_probe_register_noupdate(const char *name, void *probe,
  * @probe: probe function pointer
  *
  * caller must call tracepoint_probe_update_all()
+ * Called with the UST lock held.
  */
 int tracepoint_probe_unregister_noupdate(const char *name, void *probe,
                                         void *data)
 {
        void *old;
 
-       lock_tracepoints();
        old = tracepoint_remove_probe(name, probe, data);
        if (IS_ERR(old)) {
-               unlock_tracepoints();
                return PTR_ERR(old);
        }
        tracepoint_add_old_probes(old);
-       unlock_tracepoints();
        return 0;
 }
 
 /**
  * tracepoint_probe_update_all -  update tracepoints
+ * Called with the UST lock held.
  */
 void tracepoint_probe_update_all(void)
 {
        CDS_LIST_HEAD(release_probes);
        struct tp_probes *pos, *next;
 
-       lock_tracepoints();
        if (!need_update) {
-               unlock_tracepoints();
                return;
        }
        if (!cds_list_empty(&old_probes))
                cds_list_replace_init(&old_probes, &release_probes);
        need_update = 0;
-       unlock_tracepoints();
 
        tracepoint_update_probes();
        cds_list_for_each_entry_safe(pos, next, &release_probes, u.list) {
@@ -578,14 +547,16 @@ static void tracepoint_get_iter(struct tracepoint_iter *iter)
                tracepoint_iter_reset(iter);
 }
 
+/*
+ * Called with UST lock held.
+ */
 void tracepoint_iter_start(struct tracepoint_iter *iter)
 {
-       lock_tracepoints();
        tracepoint_get_iter(iter);
 }
 
 /*
- * Called with tracepoint mutex held.
+ * Called with UST lock held.
  */
 void tracepoint_iter_next(struct tracepoint_iter *iter)
 {
@@ -598,9 +569,11 @@ void tracepoint_iter_next(struct tracepoint_iter *iter)
        tracepoint_get_iter(iter);
 }
 
+/*
+ * Called with UST lock held.
+ */
 void tracepoint_iter_stop(struct tracepoint_iter *iter)
 {
-       unlock_tracepoints();
 }
 
 void tracepoint_iter_reset(struct tracepoint_iter *iter)
@@ -637,7 +610,7 @@ int tracepoint_register_lib(struct tracepoint * const *tracepoints_start,
        pl->tracepoints_start = tracepoints_start;
        pl->tracepoints_count = tracepoints_count;
 
-       lock_tracepoints();
+       ust_lock();
        /*
         * We sort the libs by struct lib pointer address.
         */
@@ -652,12 +625,11 @@ int tracepoint_register_lib(struct tracepoint * const *tracepoints_start,
        /* We should be added at the head of the list */
        cds_list_add(&pl->list, &libs);
 lib_added:
-       unlock_tracepoints();
-
        new_tracepoints(tracepoints_start, tracepoints_start + tracepoints_count);
 
        /* TODO: update just the loaded lib */
        lib_update_tracepoints();
+       ust_unlock();
 
        DBG("just registered a tracepoints section from %p and having %d tracepoints",
                tracepoints_start, tracepoints_count);
@@ -669,7 +641,7 @@ int tracepoint_unregister_lib(struct tracepoint * const *tracepoints_start)
 {
        struct tracepoint_lib *lib;
 
-       lock_tracepoints();
+       ust_lock();
        cds_list_for_each_entry(lib, &libs, list) {
                if (lib->tracepoints_start == tracepoints_start) {
                        struct tracepoint_lib *lib2free = lib;
@@ -678,7 +650,7 @@ int tracepoint_unregister_lib(struct tracepoint * const *tracepoints_start)
                        break;
                }
        }
-       unlock_tracepoints();
+       ust_unlock();
 
        return 0;
 }
@@ -692,4 +664,5 @@ void init_tracepoint(void)
 
 void exit_tracepoint(void)
 {
+       initialized = 0;
 }
This page took 0.034141 seconds and 4 git commands to generate.