diff mbox series

[lttng-ust,8/8] Support unloading of probe providers

Message ID 1517600891-23632-9-git-send-email-francis.deslauriers@efficios.com
State Superseded, archived
Headers show
Series Support provider duplicates and unloading | expand

Commit Message

Francis Deslauriers Feb. 2, 2018, 7:48 p.m. UTC
With this commit, it's now possible to dlclose() a library containing an
actively used probe provider.

The destructor of such library will now iterate over all the sessions
and over all probe definitions to unregister them from the respective
callsites in the process.

Signed-off-by: Francis Deslauriers <francis.deslauriers at efficios.com>
---
 liblttng-ust/lttng-events.c | 22 +++++++++++++++++++++-
 liblttng-ust/lttng-probes.c |  5 ++++-
 liblttng-ust/tracepoint.c   | 43 ++++++++++++++++++++++++++++++++++---------
 3 files changed, 59 insertions(+), 11 deletions(-)

Comments

Mathieu Desnoyers Feb. 7, 2018, 9:03 p.m. UTC | #1
----- On Feb 2, 2018, at 2:48 PM, Francis Deslauriers francis.deslauriers at efficios.com wrote:

> With this commit, it's now possible to dlclose() a library containing an
> actively used probe provider.
> 
> The destructor of such library will now iterate over all the sessions
> and over all probe definitions to unregister them from the respective
> callsites in the process.
> 
> Signed-off-by: Francis Deslauriers <francis.deslauriers at efficios.com>
> ---
> liblttng-ust/lttng-events.c | 22 +++++++++++++++++++++-
> liblttng-ust/lttng-probes.c |  5 ++++-
> liblttng-ust/tracepoint.c   | 43 ++++++++++++++++++++++++++++++++++---------
> 3 files changed, 59 insertions(+), 11 deletions(-)
> 
> diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
> index 07385d7..698210c 100644
> --- a/liblttng-ust/lttng-events.c
> +++ b/liblttng-ust/lttng-events.c
> @@ -793,7 +793,7 @@ void lttng_create_event_if_missing(struct lttng_enabler
> *enabler)
>  */
> void lttng_probe_provider_unregister_events(struct lttng_probe_desc
> *provider_desc)
> {
> -	int i;
> +	unsigned int i, j;

shorter lines below whenever possible.

> 	struct cds_list_head *sessionsp;
> 	struct lttng_session *session;
> 	struct cds_hlist_head *head;
> @@ -867,7 +867,27 @@ void lttng_probe_provider_unregister_events(struct
> lttng_probe_desc *provider_de
> 			head = &session->events_ht.table[hash & (LTTNG_UST_EVENT_HT_SIZE - 1)];
> 			cds_hlist_for_each_entry(event, node, head, hlist) {
> 				if (event_desc == event->desc) {
> +					/* Destroy enums of the current event. */
> +					for (j = 0; j < event->desc->nr_fields; j++) {
> +						const struct lttng_event_field *field;
> +						const struct lttng_enum_desc *enum_desc;
> +						struct lttng_enum *curr_enum;
> +
> +						field = &(event->desc->fields[j]);
> +						if (field->type.atype != atype_enum) {
> +							continue;
> +						}
> +
> +						enum_desc = field->type.u.basic.enumeration.desc;
> +						curr_enum = lttng_ust_enum_get_from_desc(session, enum_desc);
> +						if (curr_enum) {
> +							_lttng_enum_destroy(curr_enum);
> +						}
> +					}
> +
> +					/* Destroy event. */
> 					_lttng_event_destroy(event);
> +
> 					break;
> 				}
> 			}
> diff --git a/liblttng-ust/lttng-probes.c b/liblttng-ust/lttng-probes.c
> index a09497f..862b19e 100644
> --- a/liblttng-ust/lttng-probes.c
> +++ b/liblttng-ust/lttng-probes.c
> @@ -226,7 +226,10 @@ void lttng_probe_unregister(struct lttng_probe_desc *desc)
> 		cds_list_del(&desc->head);
> 	else
> 		cds_list_del(&desc->lazy_init_head);
> -	DBG("just unregistered probe %s", desc->provider);
> +
> +	lttng_probe_provider_unregister_events(desc);
> +	DBG("just unregistered probes of provider %s", desc->provider);
> +
> 	ust_unlock();
> }
> 
> diff --git a/liblttng-ust/tracepoint.c b/liblttng-ust/tracepoint.c
> index 14b8231..8c630a6 100644
> --- a/liblttng-ust/tracepoint.c
> +++ b/liblttng-ust/tracepoint.c
> @@ -107,8 +107,8 @@ struct tracepoint_entry {
> 	struct lttng_ust_tracepoint_probe *probes;
> 	int refcount;	/* Number of times armed. 0 if disarmed. */
> 	int callsite_refcount;	/* how many libs use this tracepoint */
> -	const char *signature;
> -	char name[0];
> +	char *signature;
> +	char *name;
> };
> 
> struct tp_probes {
> @@ -132,6 +132,7 @@ struct callsite_entry {
> 	struct cds_hlist_node hlist;	/* hash table node */
> 	struct cds_list_head node;	/* lib list of callsites node */
> 	struct lttng_ust_tracepoint *tp;
> +	bool tp_entry_callsite_ref; /* Has a tp_entry took a ref on this callsite*/

missing space after callsite.

Thanks,

Mathieu

> };
> 
> /* coverity[+alloc] */
> @@ -284,6 +285,8 @@ static struct tracepoint_entry *add_tracepoint(const char
> *name,
> 	struct cds_hlist_node *node;
> 	struct tracepoint_entry *e;
> 	size_t name_len = strlen(name);
> +	size_t sig_len = strlen(signature);
> +	size_t sig_off, name_off;
> 	uint32_t hash;
> 
> 	if (name_len > LTTNG_UST_SYM_NAME_LEN - 1) {
> @@ -298,19 +301,29 @@ static struct tracepoint_entry *add_tracepoint(const char
> *name,
> 			return ERR_PTR(-EEXIST);	/* Already there */
> 		}
> 	}
> +
> 	/*
> -	 * Using zmalloc here to allocate a variable length element. Could
> -	 * cause some memory fragmentation if overused.
> +	 * Using zmalloc here to allocate a variable length elements: name and
> +	 * signature. Could cause some memory fragmentation if overused.
> 	 */
> -	e = zmalloc(sizeof(struct tracepoint_entry) + name_len + 1);
> +	name_off = sizeof(struct tracepoint_entry);
> +	sig_off = name_off + name_len + 1;
> +
> +	e = zmalloc(sizeof(struct tracepoint_entry) + name_len + 1 + sig_len + 1);
> 	if (!e)
> 		return ERR_PTR(-ENOMEM);
> -	memcpy(&e->name[0], name, name_len + 1);
> +	e->name = (char *) e + name_off;
> +	memcpy(e->name, name, name_len + 1);
> 	e->name[name_len] = '\0';
> +
> +	e->signature = (char *) e + sig_off;
> +	memcpy(e->signature, signature, sig_len + 1);
> +	e->signature[sig_len] = '\0';
> +
> 	e->probes = NULL;
> 	e->refcount = 0;
> 	e->callsite_refcount = 0;
> -	e->signature = signature;
> +
> 	cds_hlist_add_head(&e->hlist, head);
> 	return e;
> }
> @@ -405,6 +418,7 @@ static void add_callsite(struct tracepoint_lib * lib, struct
> lttng_ust_tracepoin
> 	if (!tp_entry)
> 		return;
> 	tp_entry->callsite_refcount++;
> +	e->tp_entry_callsite_ref = true;
> }
> 
> /*
> @@ -417,7 +431,8 @@ static void remove_callsite(struct callsite_entry *e)
> 
> 	tp_entry = get_tracepoint(e->tp->name);
> 	if (tp_entry) {
> -		tp_entry->callsite_refcount--;
> +		if (e->tp_entry_callsite_ref)
> +			tp_entry->callsite_refcount--;
> 		if (tp_entry->callsite_refcount == 0)
> 			disable_tracepoint(e->tp);
> 	}
> @@ -453,10 +468,15 @@ static void tracepoint_sync_callsites(const char *name)
> 		if (strncmp(name, tp->name, LTTNG_UST_SYM_NAME_LEN - 1))
> 			continue;
> 		if (tp_entry) {
> +			if (!e->tp_entry_callsite_ref) {
> +				tp_entry->callsite_refcount++;
> +				e->tp_entry_callsite_ref = true;
> +			}
> 			set_tracepoint(&tp_entry, tp,
> 					!!tp_entry->refcount);
> 		} else {
> 			disable_tracepoint(tp);
> +			e->tp_entry_callsite_ref = false;
> 		}
> 	}
> }
> @@ -545,7 +565,12 @@ tracepoint_add_probe(const char *name, void (*probe)(void),
> void *data,
> 	struct lttng_ust_tracepoint_probe *old;
> 
> 	entry = get_tracepoint(name);
> -	if (!entry) {
> +	if (entry) {
> +		if (strcmp(entry->signature, signature) != 0) {
> +			ERR("Tracepoint and probe signature do not match.");
> +			return ERR_PTR(-EINVAL);
> +		}
> +	} else {
> 		entry = add_tracepoint(name, signature);
> 		if (IS_ERR(entry))
> 			return (struct lttng_ust_tracepoint_probe *)entry;
> --
> 2.7.4
diff mbox series

Patch

diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
index 07385d7..698210c 100644
--- a/liblttng-ust/lttng-events.c
+++ b/liblttng-ust/lttng-events.c
@@ -793,7 +793,7 @@  void lttng_create_event_if_missing(struct lttng_enabler *enabler)
  */
 void lttng_probe_provider_unregister_events(struct lttng_probe_desc *provider_desc)
 {
-	int i;
+	unsigned int i, j;
 	struct cds_list_head *sessionsp;
 	struct lttng_session *session;
 	struct cds_hlist_head *head;
@@ -867,7 +867,27 @@  void lttng_probe_provider_unregister_events(struct lttng_probe_desc *provider_de
 			head = &session->events_ht.table[hash & (LTTNG_UST_EVENT_HT_SIZE - 1)];
 			cds_hlist_for_each_entry(event, node, head, hlist) {
 				if (event_desc == event->desc) {
+					/* Destroy enums of the current event. */
+					for (j = 0; j < event->desc->nr_fields; j++) {
+						const struct lttng_event_field *field;
+						const struct lttng_enum_desc *enum_desc;
+						struct lttng_enum *curr_enum;
+
+						field = &(event->desc->fields[j]);
+						if (field->type.atype != atype_enum) {
+							continue;
+						}
+
+						enum_desc = field->type.u.basic.enumeration.desc;
+						curr_enum = lttng_ust_enum_get_from_desc(session, enum_desc);
+						if (curr_enum) {
+							_lttng_enum_destroy(curr_enum);
+						}
+					}
+
+					/* Destroy event. */
 					_lttng_event_destroy(event);
+
 					break;
 				}
 			}
diff --git a/liblttng-ust/lttng-probes.c b/liblttng-ust/lttng-probes.c
index a09497f..862b19e 100644
--- a/liblttng-ust/lttng-probes.c
+++ b/liblttng-ust/lttng-probes.c
@@ -226,7 +226,10 @@  void lttng_probe_unregister(struct lttng_probe_desc *desc)
 		cds_list_del(&desc->head);
 	else
 		cds_list_del(&desc->lazy_init_head);
-	DBG("just unregistered probe %s", desc->provider);
+
+	lttng_probe_provider_unregister_events(desc);
+	DBG("just unregistered probes of provider %s", desc->provider);
+
 	ust_unlock();
 }
 
diff --git a/liblttng-ust/tracepoint.c b/liblttng-ust/tracepoint.c
index 14b8231..8c630a6 100644
--- a/liblttng-ust/tracepoint.c
+++ b/liblttng-ust/tracepoint.c
@@ -107,8 +107,8 @@  struct tracepoint_entry {
 	struct lttng_ust_tracepoint_probe *probes;
 	int refcount;	/* Number of times armed. 0 if disarmed. */
 	int callsite_refcount;	/* how many libs use this tracepoint */
-	const char *signature;
-	char name[0];
+	char *signature;
+	char *name;
 };
 
 struct tp_probes {
@@ -132,6 +132,7 @@  struct callsite_entry {
 	struct cds_hlist_node hlist;	/* hash table node */
 	struct cds_list_head node;	/* lib list of callsites node */
 	struct lttng_ust_tracepoint *tp;
+	bool tp_entry_callsite_ref; /* Has a tp_entry took a ref on this callsite*/
 };
 
 /* coverity[+alloc] */
@@ -284,6 +285,8 @@  static struct tracepoint_entry *add_tracepoint(const char *name,
 	struct cds_hlist_node *node;
 	struct tracepoint_entry *e;
 	size_t name_len = strlen(name);
+	size_t sig_len = strlen(signature);
+	size_t sig_off, name_off;
 	uint32_t hash;
 
 	if (name_len > LTTNG_UST_SYM_NAME_LEN - 1) {
@@ -298,19 +301,29 @@  static struct tracepoint_entry *add_tracepoint(const char *name,
 			return ERR_PTR(-EEXIST);	/* Already there */
 		}
 	}
+
 	/*
-	 * Using zmalloc here to allocate a variable length element. Could
-	 * cause some memory fragmentation if overused.
+	 * Using zmalloc here to allocate a variable length elements: name and
+	 * signature. Could cause some memory fragmentation if overused.
 	 */
-	e = zmalloc(sizeof(struct tracepoint_entry) + name_len + 1);
+	name_off = sizeof(struct tracepoint_entry);
+	sig_off = name_off + name_len + 1;
+
+	e = zmalloc(sizeof(struct tracepoint_entry) + name_len + 1 + sig_len + 1);
 	if (!e)
 		return ERR_PTR(-ENOMEM);
-	memcpy(&e->name[0], name, name_len + 1);
+	e->name = (char *) e + name_off;
+	memcpy(e->name, name, name_len + 1);
 	e->name[name_len] = '\0';
+
+	e->signature = (char *) e + sig_off;
+	memcpy(e->signature, signature, sig_len + 1);
+	e->signature[sig_len] = '\0';
+
 	e->probes = NULL;
 	e->refcount = 0;
 	e->callsite_refcount = 0;
-	e->signature = signature;
+
 	cds_hlist_add_head(&e->hlist, head);
 	return e;
 }
@@ -405,6 +418,7 @@  static void add_callsite(struct tracepoint_lib * lib, struct lttng_ust_tracepoin
 	if (!tp_entry)
 		return;
 	tp_entry->callsite_refcount++;
+	e->tp_entry_callsite_ref = true;
 }
 
 /*
@@ -417,7 +431,8 @@  static void remove_callsite(struct callsite_entry *e)
 
 	tp_entry = get_tracepoint(e->tp->name);
 	if (tp_entry) {
-		tp_entry->callsite_refcount--;
+		if (e->tp_entry_callsite_ref)
+			tp_entry->callsite_refcount--;
 		if (tp_entry->callsite_refcount == 0)
 			disable_tracepoint(e->tp);
 	}
@@ -453,10 +468,15 @@  static void tracepoint_sync_callsites(const char *name)
 		if (strncmp(name, tp->name, LTTNG_UST_SYM_NAME_LEN - 1))
 			continue;
 		if (tp_entry) {
+			if (!e->tp_entry_callsite_ref) {
+				tp_entry->callsite_refcount++;
+				e->tp_entry_callsite_ref = true;
+			}
 			set_tracepoint(&tp_entry, tp,
 					!!tp_entry->refcount);
 		} else {
 			disable_tracepoint(tp);
+			e->tp_entry_callsite_ref = false;
 		}
 	}
 }
@@ -545,7 +565,12 @@  tracepoint_add_probe(const char *name, void (*probe)(void), void *data,
 	struct lttng_ust_tracepoint_probe *old;
 
 	entry = get_tracepoint(name);
-	if (!entry) {
+	if (entry) {
+		if (strcmp(entry->signature, signature) != 0) {
+			ERR("Tracepoint and probe signature do not match.");
+			return ERR_PTR(-EINVAL);
+		}
+	} else {
 		entry = add_tracepoint(name, signature);
 		if (IS_ERR(entry))
 			return (struct lttng_ust_tracepoint_probe *)entry;