From: Mathieu Desnoyers Date: Mon, 8 Aug 2016 21:08:00 +0000 (-0400) Subject: Fix: many-events registration/unregistration speed X-Git-Tag: v2.9.0-rc1~26 X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=baa1e0bcf6630bd3f9282b82586d1a730f3d7ae2;p=lttng-ust.git 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 --- diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c index 373995d0..64f70f92 100644 --- 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 70842382..298f78e7 100644 --- 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 db588600..14b8231f 100644 --- 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: