Fix: many-events registration/unregistration speed

Batch invocation of synchronize_rcu() when unregistering many events
from a session.

Also batch invocation of synchronize_rcu() when registering the same
events within many concurrent sessions (starting from the 2nd session).

Those slowdowns are noticeable with applications processes that have a
short life-time, e.g. shell scripts spawning multiple short-lived
processes take significantly longer to complete when LD_PRELOADing a UST
probe provider.

This slowdown only occurs when UST tracing sessions are created in the
session daemon.

tracepoint_probe_update_all() (currently unused) implements a similar
mechanism which has the downside of iterating on all events in all probe
libraries (not as efficient). Move synchronize_rcu() in
tracepoint_probe_update_all() outside of the iteration on all events to
free in this function, because it is only needed between the last
callsite update and the first memory reclaim, not between list removal
and reclaim.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
index 373995d08e61135bdd71b7991ba10749132d9116..64f70f92bd9701ff15936d9fb6ec84176d389187 100644 (file)
--- a/liblttng-ust/lttng-events.c
+++ b/liblttng-ust/lttng-events.c
@@ -184,7 +184,7 @@ void register_event(struct lttng_event *event)
 
        assert(event->registered == 0);
        desc = event->desc;
-       ret = __tracepoint_probe_register(desc->name,
+       ret = __tracepoint_probe_register_queue_release(desc->name,
                        desc->probe_callback,
                        event, desc->signature);
        WARN_ON_ONCE(ret);
@@ -200,7 +200,7 @@ void unregister_event(struct lttng_event *event)
 
        assert(event->registered == 1);
        desc = event->desc;
-       ret = __tracepoint_probe_unregister(desc->name,
+       ret = __tracepoint_probe_unregister_queue_release(desc->name,
                        desc->probe_callback,
                        event);
        WARN_ON_ONCE(ret);
@@ -230,6 +230,7 @@ void lttng_session_destroy(struct lttng_session *session)
                _lttng_event_unregister(event);
        }
        synchronize_trace();    /* Wait for in-flight events to complete */
+       __tracepoint_probe_prune_release_queue();
        cds_list_for_each_entry_safe(enabler, tmpenabler,
                        &session->enablers_head, node)
                lttng_enabler_destroy(enabler);
@@ -1125,6 +1126,7 @@ void lttng_session_sync_enablers(struct lttng_session *session)
                        lttng_filter_sync_state(runtime);
                }
        }
+       __tracepoint_probe_prune_release_queue();
 }
 
 /*

diff --git a/liblttng-ust/tracepoint-internal.h b/liblttng-ust/tracepoint-internal.h
index 70842382c9036bb97304bec2cc5ac67a1fe2e781..298f78e7313f7e7299b6ffc84f1cdd13a20148f5 100644 (file)
--- a/liblttng-ust/tracepoint-internal.h
+++ b/liblttng-ust/tracepoint-internal.h
@@ -38,6 +38,11 @@ extern int tracepoint_probe_register_noupdate(const char *name,
 extern int tracepoint_probe_unregister_noupdate(const char *name,
                void (*callback)(void), void *priv);
 extern void tracepoint_probe_update_all(void);
+extern int __tracepoint_probe_register_queue_release(const char *name,
+               void (*func)(void), void *data, const char *signature);
+extern int __tracepoint_probe_unregister_queue_release(const char *name,
+               void (*func)(void), void *data);
+extern void __tracepoint_probe_prune_release_queue(void);
 
 /*
  * call after disconnection of last probe implemented within a

diff --git a/liblttng-ust/tracepoint.c b/liblttng-ust/tracepoint.c
index db5886009de5f620f24880e884b1345618470a2d..14b8231f879fca9d3aa8e3bde5a6a661cbb6b4c2 100644 (file)
--- a/liblttng-ust/tracepoint.c
+++ b/liblttng-ust/tracepoint.c
@@ -93,6 +93,9 @@ static struct cds_hlist_head tracepoint_table[TRACEPOINT_TABLE_SIZE];
 static CDS_LIST_HEAD(old_probes);
 static int need_update;
 
+static CDS_LIST_HEAD(release_queue);
+static int release_queue_need_update;
+
 /*
  * Note about RCU :
  * It is used to to delay the free of multiple probes array until a quiescent
@@ -553,6 +556,16 @@ tracepoint_add_probe(const char *name, void (*probe)(void), void *data,
        return old;
 }
 
+static void tracepoint_release_queue_add_old_probes(void *old)
+{
+       release_queue_need_update = 1;
+       if (old) {
+               struct tp_probes *tp_probes = caa_container_of(old,
+                       struct tp_probes, probes[0]);
+               cds_list_add(&tp_probes->u.list, &release_queue);
+       }
+}
+
 /**
  * __tracepoint_probe_register -  Connect a probe to a tracepoint
  * @name: tracepoint name
@@ -584,6 +597,33 @@ end:
        return ret;
 }
 
+/*
+ * Caller needs to invoke __tracepoint_probe_release_queue() after
+ * calling __tracepoint_probe_register_queue_release() one or multiple
+ * times to ensure it does not leak memory.
+ */
+int __tracepoint_probe_register_queue_release(const char *name,
+               void (*probe)(void), void *data, const char *signature)
+{
+       void *old;
+       int ret = 0;
+
+       DBG("Registering probe to tracepoint %s. Queuing release.", name);
+
+       pthread_mutex_lock(&tracepoint_mutex);
+       old = tracepoint_add_probe(name, probe, data, signature);
+       if (IS_ERR(old)) {
+               ret = PTR_ERR(old);
+               goto end;
+       }
+
+       tracepoint_sync_callsites(name);
+       tracepoint_release_queue_add_old_probes(old);
+end:
+       pthread_mutex_unlock(&tracepoint_mutex);
+       return ret;
+}
+
 static void *tracepoint_remove_probe(const char *name, void (*probe)(void),
                void *data)
 {
@@ -628,6 +668,57 @@ end:
        return ret;
 }
 
+/*
+ * Caller needs to invoke __tracepoint_probe_release_queue() after
+ * calling __tracepoint_probe_unregister_queue_release() one or multiple
+ * times to ensure it does not leak memory.
+ */
+int __tracepoint_probe_unregister_queue_release(const char *name,
+               void (*probe)(void), void *data)
+{
+       void *old;
+       int ret = 0;
+
+       DBG("Un-registering probe from tracepoint %s. Queuing release.", name);
+
+       pthread_mutex_lock(&tracepoint_mutex);
+       old = tracepoint_remove_probe(name, probe, data);
+       if (IS_ERR(old)) {
+               ret = PTR_ERR(old);
+               goto end;
+       }
+       tracepoint_sync_callsites(name);
+       tracepoint_release_queue_add_old_probes(old);
+end:
+       pthread_mutex_unlock(&tracepoint_mutex);
+       return ret;
+}
+
+void __tracepoint_probe_prune_release_queue(void)
+{
+       CDS_LIST_HEAD(release_probes);
+       struct tp_probes *pos, *next;
+
+       DBG("Release queue of unregistered tracepoint probes.");
+
+       pthread_mutex_lock(&tracepoint_mutex);
+       if (!release_queue_need_update)
+               goto end;
+       if (!cds_list_empty(&release_queue))
+               cds_list_replace_init(&release_queue, &release_probes);
+       release_queue_need_update = 0;
+
+       /* Wait for grace period between all sync_callsites and free. */
+       synchronize_rcu();
+
+       cds_list_for_each_entry_safe(pos, next, &release_probes, u.list) {
+               cds_list_del(&pos->u.list);
+               free(pos);
+       }
+end:
+       pthread_mutex_unlock(&tracepoint_mutex);
+}
+
 static void tracepoint_add_old_probes(void *old)
 {
        need_update = 1;
@@ -708,9 +799,10 @@ void tracepoint_probe_update_all(void)
        need_update = 0;
 
        tracepoint_update_probes();
+       /* Wait for grace period between update_probes and free. */
+       synchronize_rcu();
        cds_list_for_each_entry_safe(pos, next, &release_probes, u.list) {
                cds_list_del(&pos->u.list);
-               synchronize_rcu();
                free(pos);
        }
 end: