All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH lttng-ust 1/8] Remove duplicate provider name checks
       [not found] <1517600891-23632-1-git-send-email-francis.deslauriers@efficios.com>
@ 2018-02-02 19:48 ` Francis Deslauriers
  2018-02-02 19:48 ` [PATCH lttng-ust 2/8] Fix: missing event removal from the event hashtable Francis Deslauriers
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 29+ messages in thread
From: Francis Deslauriers @ 2018-02-02 19:48 UTC (permalink / raw)
  To: lttng-dev

From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

It's now possible to register a probe provider with a name that has
already been registered. This is useful when wanting to load a new
version of a shared library on a already running process.

Changes are necessary in the lttng-session daemon to support cases where
the newly register event has a different probe payload.

Taking a simple case where a probe provider is registered twice, the
tracepoint call site will have two probes registered to it and thus will
generate two events in the trace.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
---
 include/lttng/ust-tracepoint-event.h |  2 +-
 liblttng-ust/lttng-events.c          | 11 -----------
 liblttng-ust/lttng-probes.c          | 23 +----------------------
 3 files changed, 2 insertions(+), 34 deletions(-)

diff --git a/include/lttng/ust-tracepoint-event.h b/include/lttng/ust-tracepoint-event.h
index 15399c7..ec292d2 100644
--- a/include/lttng/ust-tracepoint-event.h
+++ b/include/lttng/ust-tracepoint-event.h
@@ -1007,7 +1007,7 @@ _TP_COMBINE_TOKENS(__lttng_events_init__, TRACEPOINT_PROVIDER)(void)
 	_TP_COMBINE_TOKENS(__tracepoint_provider_check_, TRACEPOINT_PROVIDER)();
 	ret = lttng_probe_register(&_TP_COMBINE_TOKENS(__probe_desc___, TRACEPOINT_PROVIDER));
 	if (ret) {
-		fprintf(stderr, "LTTng-UST: Error (%d) while registering tracepoint probe. Duplicate registration of tracepoint probes having the same name is not allowed.\n", ret);
+		fprintf(stderr, "LTTng-UST: Error (%d) while registering tracepoint probe.\n", ret);
 		abort();
 	}
 }
diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
index bfdae4f..f4a7ccc 100644
--- a/liblttng-ust/lttng-events.c
+++ b/liblttng-ust/lttng-events.c
@@ -537,7 +537,6 @@ int lttng_event_create(const struct lttng_event_desc *desc,
 	struct lttng_event *event;
 	struct lttng_session *session = chan->session;
 	struct cds_hlist_head *head;
-	struct cds_hlist_node *node;
 	int ret = 0;
 	size_t name_len = strlen(event_name);
 	uint32_t hash;
@@ -546,15 +545,6 @@ int lttng_event_create(const struct lttng_event_desc *desc,
 
 	hash = jhash(event_name, name_len, 0);
 	head = &chan->session->events_ht.table[hash & (LTTNG_UST_EVENT_HT_SIZE - 1)];
-	cds_hlist_for_each_entry(event, node, head, hlist) {
-		assert(event->desc);
-		if (!strncmp(event->desc->name, desc->name,
-					LTTNG_UST_SYM_NAME_LEN - 1)
-				&& chan == event->chan) {
-			ret = -EEXIST;
-			goto exist;
-		}
-	}
 
 	notify_socket = lttng_get_notify_socket(session->owner);
 	if (notify_socket < 0) {
@@ -623,7 +613,6 @@ sessiond_register_error:
 cache_error:
 create_enum_error:
 socket_error:
-exist:
 	return ret;
 }
 
diff --git a/liblttng-ust/lttng-probes.c b/liblttng-ust/lttng-probes.c
index 390265a..a09497f 100644
--- a/liblttng-ust/lttng-probes.c
+++ b/liblttng-ust/lttng-probes.c
@@ -148,20 +148,6 @@ struct cds_list_head *lttng_get_probe_list_head(void)
 }
 
 static
-const struct lttng_probe_desc *find_provider(const char *provider)
-{
-	struct lttng_probe_desc *iter;
-	struct cds_list_head *probe_list;
-
-	probe_list = lttng_get_probe_list_head();
-	cds_list_for_each_entry(iter, probe_list, head) {
-		if (!strcmp(iter->provider, provider))
-			return iter;
-	}
-	return NULL;
-}
-
-static
 int check_provider_version(struct lttng_probe_desc *desc)
 {
 	/*
@@ -206,13 +192,6 @@ int lttng_probe_register(struct lttng_probe_desc *desc)
 
 	ust_lock_nocheck();
 
-	/*
-	 * Check if the provider has already been registered.
-	 */
-	if (find_provider(desc->provider)) {
-		ret = -EEXIST;
-		goto end;
-	}
 	cds_list_add(&desc->lazy_init_head, &lazy_probe_init);
 	desc->lazy = 1;
 	DBG("adding probe %s containing %u events to lazy registration list",
@@ -224,7 +203,7 @@ int lttng_probe_register(struct lttng_probe_desc *desc)
 	 */
 	if (lttng_session_active())
 		fixup_lazy_probes();
-end:
+
 	ust_unlock();
 	return ret;
 }
-- 
2.7.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH lttng-ust 2/8] Fix: missing event removal from the event hashtable
       [not found] <1517600891-23632-1-git-send-email-francis.deslauriers@efficios.com>
  2018-02-02 19:48 ` [PATCH lttng-ust 1/8] Remove duplicate provider name checks Francis Deslauriers
@ 2018-02-02 19:48 ` Francis Deslauriers
  2018-02-02 19:48 ` [PATCH lttng-ust 3/8] Cleanup: Move version numbers in separate variables in configure script Francis Deslauriers
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 29+ messages in thread
From: Francis Deslauriers @ 2018-02-02 19:48 UTC (permalink / raw)
  To: lttng-dev

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
---
 liblttng-ust/lttng-events.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
index f4a7ccc..7419f78 100644
--- a/liblttng-ust/lttng-events.c
+++ b/liblttng-ust/lttng-events.c
@@ -883,7 +883,11 @@ void _lttng_event_destroy(struct lttng_event *event)
 {
 	struct lttng_enabler_ref *enabler_ref, *tmp_enabler_ref;
 
+	/* Remove from event list. */
 	cds_list_del(&event->node);
+	/* Remove from event hash table. */
+	cds_hlist_del(&event->hlist);
+
 	lttng_destroy_context(event->ctx);
 	lttng_free_event_filter_runtime(event);
 	/* Free event enabler refs */
-- 
2.7.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH lttng-ust 3/8] Cleanup: Move version numbers in separate variables in configure script
       [not found] <1517600891-23632-1-git-send-email-francis.deslauriers@efficios.com>
  2018-02-02 19:48 ` [PATCH lttng-ust 1/8] Remove duplicate provider name checks Francis Deslauriers
  2018-02-02 19:48 ` [PATCH lttng-ust 2/8] Fix: missing event removal from the event hashtable Francis Deslauriers
@ 2018-02-02 19:48 ` Francis Deslauriers
  2018-02-02 19:48 ` [PATCH lttng-ust 4/8] Add probe provider unregister function Francis Deslauriers
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 29+ messages in thread
From: Francis Deslauriers @ 2018-02-02 19:48 UTC (permalink / raw)
  To: lttng-dev

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
---
 configure.ac | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index fb2f278..b0b4157 100644
--- a/configure.ac
+++ b/configure.ac
@@ -20,7 +20,11 @@ AC_SUBST([PATCHLEVEL_VERSION], [V_PATCH])
 # Following the numbering scheme proposed by libtool for the library version
 # http://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html
 # This is the library version of liblttng-ust.
-AC_SUBST([LTTNG_UST_LIBRARY_VERSION], [0:0:0])
+m4_define([UST_LIB_V_MAJOR], [0])
+m4_define([UST_LIB_V_MINOR], [0])
+m4_define([UST_LIB_V_PATCH], [0])
+
+AC_SUBST([LTTNG_UST_LIBRARY_VERSION], [UST_LIB_V_MAJOR:UST_LIB_V_MINOR:UST_LIB_V_PATCH])
 # note: remember to update tracepoint.h dlopen() to match this version
 # number. TODO: eventually automate by exporting the major number.
 
-- 
2.7.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH lttng-ust 4/8] Add probe provider unregister function
       [not found] <1517600891-23632-1-git-send-email-francis.deslauriers@efficios.com>
                   ` (2 preceding siblings ...)
  2018-02-02 19:48 ` [PATCH lttng-ust 3/8] Cleanup: Move version numbers in separate variables in configure script Francis Deslauriers
@ 2018-02-02 19:48 ` Francis Deslauriers
  2018-02-02 19:48 ` [PATCH lttng-ust 5/8] Fix: missing enum removal from the enum hashtable Francis Deslauriers
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 29+ messages in thread
From: Francis Deslauriers @ 2018-02-02 19:48 UTC (permalink / raw)
  To: lttng-dev

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
---
 include/lttng/ust-events.h  |  1 +
 liblttng-ust/lttng-events.c | 89 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 90 insertions(+)

diff --git a/include/lttng/ust-events.h b/include/lttng/ust-events.h
index caf7e63..019b0eb 100644
--- a/include/lttng/ust-events.h
+++ b/include/lttng/ust-events.h
@@ -656,6 +656,7 @@ void synchronize_trace(void);
 
 int lttng_probe_register(struct lttng_probe_desc *desc);
 void lttng_probe_unregister(struct lttng_probe_desc *desc);
+void lttng_probe_provider_unregister_events(struct lttng_probe_desc *desc);
 int lttng_fix_pending_events(void);
 int lttng_probes_init(void);
 void lttng_probes_exit(void);
diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
index 7419f78..e8d4857 100644
--- a/liblttng-ust/lttng-events.c
+++ b/liblttng-ust/lttng-events.c
@@ -789,6 +789,95 @@ void lttng_create_event_if_missing(struct lttng_enabler *enabler)
 }
 
 /*
+ * Iterate over all the UST sessions to unregister and destroy all probes from
+ * the probe provider descriptor passed in arguments. Must me called with the
+ * ust_lock held.
+ */
+void lttng_probe_provider_unregister_events(struct lttng_probe_desc *provider_desc)
+{
+	int i;
+	struct cds_list_head *sessionsp;
+	struct lttng_session *session;
+	struct cds_hlist_head *head;
+	struct cds_hlist_node *node;
+	struct lttng_event *event;
+
+	/* Get handle on list of sessions. */
+	sessionsp = _lttng_get_sessions();
+
+	/*
+	 * Iterate over all events in the probe provider descriptions and sessions
+	 * to queue the unregistration of the events.
+	 */
+	for (i = 0; i < provider_desc->nr_events; i++) {
+		const char *event_name;
+		uint32_t hash;
+		size_t name_len;
+		const struct lttng_event_desc *event_desc;
+
+		event_desc = provider_desc->event_desc[i];
+		event_name = event_desc->name;
+		name_len = strlen(event_name);
+		hash = jhash(event_name, name_len, 0);
+
+		/* Iterate over all session to find the current event description. */
+		cds_list_for_each_entry(session, sessionsp, node) {
+
+			/*
+			 * Get the list of events in the hashtable bucket and iterate to
+			 * find the event matching this descriptor.
+			 */
+			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) {
+
+					/* Queue the unregistration of this event. */
+					_lttng_event_unregister(event);
+					break;
+				}
+			}
+		}
+	}
+
+	/* Wait for grace period. */
+	synchronize_trace();
+	/* Prune the unregistration queue. */
+	__tracepoint_probe_prune_release_queue();
+
+	/*
+	 * It is now safe to destroy the events and remove them from the event list
+	 * and hashtables.
+	 */
+	for (i = 0; i < provider_desc->nr_events; i++) {
+		const char *event_name;
+		uint32_t hash;
+		size_t name_len;
+		const struct lttng_event_desc *event_desc;
+
+		event_desc = provider_desc->event_desc[i];
+		event_name = event_desc->name;
+		name_len = strlen(event_name);
+		hash = jhash(event_name, name_len, 0);
+
+		/* Iterate over all sessions to find the current event description. */
+		cds_list_for_each_entry(session, sessionsp, node) {
+
+			/*
+			 * Get the list of events in the hashtable bucket and iterate to
+			 * find the event matching this descriptor.
+			 */
+			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) {
+					_lttng_event_destroy(event);
+					break;
+				}
+			}
+		}
+	}
+}
+
+/*
  * Create events associated with an enabler (if not already present),
  * and add backward reference from the event to the enabler.
  */
-- 
2.7.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH lttng-ust 5/8] Fix: missing enum removal from the enum hashtable
       [not found] <1517600891-23632-1-git-send-email-francis.deslauriers@efficios.com>
                   ` (3 preceding siblings ...)
  2018-02-02 19:48 ` [PATCH lttng-ust 4/8] Add probe provider unregister function Francis Deslauriers
@ 2018-02-02 19:48 ` Francis Deslauriers
  2018-02-02 19:48 ` [PATCH lttng-ust 6/8] Manually dlopen() liblttng-ust.so to prevent unloading Francis Deslauriers
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 29+ messages in thread
From: Francis Deslauriers @ 2018-02-02 19:48 UTC (permalink / raw)
  To: lttng-dev

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
---
 liblttng-ust/lttng-events.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
index e8d4857..2b679b5 100644
--- a/liblttng-ust/lttng-events.c
+++ b/liblttng-ust/lttng-events.c
@@ -990,6 +990,7 @@ static
 void _lttng_enum_destroy(struct lttng_enum *_enum)
 {
 	cds_list_del(&_enum->node);
+	cds_hlist_del(&_enum->hlist);
 	free(_enum);
 }
 
-- 
2.7.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH lttng-ust 6/8] Manually dlopen() liblttng-ust.so to prevent unloading
       [not found] <1517600891-23632-1-git-send-email-francis.deslauriers@efficios.com>
                   ` (4 preceding siblings ...)
  2018-02-02 19:48 ` [PATCH lttng-ust 5/8] Fix: missing enum removal from the enum hashtable Francis Deslauriers
@ 2018-02-02 19:48 ` Francis Deslauriers
  2018-02-02 19:48 ` [PATCH lttng-ust 7/8] Rename lttng_ust_enum_get to lttng_ust_enum_get_from_desc Francis Deslauriers
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 29+ messages in thread
From: Francis Deslauriers @ 2018-02-02 19:48 UTC (permalink / raw)
  To: lttng-dev

dlopen() increments the refcount of the library thus preventing the
refcount to reach zero in the case of dlclose;

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
---
 configure.ac                  |  1 +
 liblttng-ust/Makefile.am      |  2 ++
 liblttng-ust/lttng-ust-comm.c | 22 ++++++++++++++++++++++
 3 files changed, 25 insertions(+)

diff --git a/configure.ac b/configure.ac
index b0b4157..4fc6f9c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -25,6 +25,7 @@ m4_define([UST_LIB_V_MINOR], [0])
 m4_define([UST_LIB_V_PATCH], [0])
 
 AC_SUBST([LTTNG_UST_LIBRARY_VERSION], [UST_LIB_V_MAJOR:UST_LIB_V_MINOR:UST_LIB_V_PATCH])
+AC_SUBST([LTTNG_UST_LIBRARY_VERSION_MAJOR], [UST_LIB_V_MAJOR])
 # note: remember to update tracepoint.h dlopen() to match this version
 # number. TODO: eventually automate by exporting the major number.
 
diff --git a/liblttng-ust/Makefile.am b/liblttng-ust/Makefile.am
index 982be69..a7edfd5 100644
--- a/liblttng-ust/Makefile.am
+++ b/liblttng-ust/Makefile.am
@@ -60,6 +60,8 @@ liblttng_ust_runtime_la_SOURCES = \
 	string-utils.c \
 	string-utils.h
 
+liblttng_ust_runtime_la_CFLAGS = -DLTTNG_UST_LIBRARY_VERSION_MAJOR=\"$(LTTNG_UST_LIBRARY_VERSION_MAJOR)\"
+
 if HAVE_PERF_EVENT
 liblttng_ust_runtime_la_SOURCES += \
 	lttng-context-perf-counters.c \
diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c
index 511b9cf..ed912b8 100644
--- a/liblttng-ust/lttng-ust-comm.c
+++ b/liblttng-ust/lttng-ust-comm.c
@@ -27,6 +27,7 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/wait.h>
+#include <dlfcn.h>
 #include <fcntl.h>
 #include <unistd.h>
 #include <errno.h>
@@ -59,6 +60,9 @@
 #include "../libringbuffer/getcpu.h"
 #include "getenv.h"
 
+/* Concatenate lttng ust shared library name with its major version number. */
+#define LTTNG_UST_LIB_SO_NAME "liblttng-ust.so." LTTNG_UST_LIBRARY_VERSION_MAJOR
+
 /*
  * Has lttng ust comm constructor been called ?
  */
@@ -1648,6 +1652,7 @@ void __attribute__((constructor)) lttng_ust_init(void)
 	pthread_attr_t thread_attr;
 	int timeout_mode;
 	int ret;
+	void *handle;
 
 	if (uatomic_xchg(&initialized, 1) == 1)
 		return;
@@ -1662,6 +1667,23 @@ void __attribute__((constructor)) lttng_ust_init(void)
 	lttng_ust_loaded = 1;
 
 	/*
+	 * Manually load liblttng-ust.so to increment the dynamic loader's internal
+	 * refcount for this library so it never becomes zero, thus never gets
+	 * unloaded from the address space of the process. Since we are already
+	 * running in the constructor of the LTTNG_UST_LIB_SO_NAME library, calling
+	 * dlopen will simply increment the refcount and no additionnal work is
+	 * needed by the dynamic loader as the shared library is already loaded in
+	 * the address space. As a safe guard, we use the RTLD_NODELETE flag to
+	 * prevent unloading of the UST library if its refcount becomes zero
+	 * (which should never happen). Do the return value check but discard the
+	 * handle at the end of the function as it's not needed.
+	 */
+	handle = dlopen(LTTNG_UST_LIB_SO_NAME, RTLD_LAZY | RTLD_NODELETE);
+	if (!handle) {
+		ERR("dlopen of liblttng-ust shared library (%s).", LTTNG_UST_LIB_SO_NAME);
+	}
+
+	/*
 	 * We want precise control over the order in which we construct
 	 * our sub-libraries vs starting to receive commands from
 	 * sessiond (otherwise leading to errors when trying to create
-- 
2.7.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH lttng-ust 7/8] Rename lttng_ust_enum_get to lttng_ust_enum_get_from_desc
       [not found] <1517600891-23632-1-git-send-email-francis.deslauriers@efficios.com>
                   ` (5 preceding siblings ...)
  2018-02-02 19:48 ` [PATCH lttng-ust 6/8] Manually dlopen() liblttng-ust.so to prevent unloading Francis Deslauriers
@ 2018-02-02 19:48 ` Francis Deslauriers
  2018-02-02 19:48 ` [PATCH lttng-ust 8/8] Support unloading of probe providers Francis Deslauriers
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 29+ messages in thread
From: Francis Deslauriers @ 2018-02-02 19:48 UTC (permalink / raw)
  To: lttng-dev

Change the prototype to take a descriptor instead of a char *.
Now that provider names can have duplicates enum names are not
necessarily unique.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
---
 include/lttng/ust-events.h         |  5 +++--
 liblttng-ust-comm/lttng-ust-comm.c |  3 +--
 liblttng-ust/lttng-events.c        | 14 ++++++--------
 liblttng-ust/ust-core.c            | 11 +++++------
 4 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/include/lttng/ust-events.h b/include/lttng/ust-events.h
index 019b0eb..8673350 100644
--- a/include/lttng/ust-events.h
+++ b/include/lttng/ust-events.h
@@ -731,8 +731,9 @@ int lttng_session_active(void);
 typedef int (*t_statedump_func_ptr)(struct lttng_session *session);
 void lttng_handle_pending_statedump(void *owner);
 struct cds_list_head *_lttng_get_sessions(void);
-struct lttng_enum *lttng_ust_enum_get(struct lttng_session *session,
-		const char *enum_name);
+
+struct lttng_enum *lttng_ust_enum_get_from_desc(struct lttng_session *session,
+		const struct lttng_enum_desc *enum_desc);
 
 void lttng_ust_dl_update(void *ip);
 void lttng_ust_fixup_fd_tracker_tls(void);
diff --git a/liblttng-ust-comm/lttng-ust-comm.c b/liblttng-ust-comm/lttng-ust-comm.c
index 9fe6d28..d95e31d 100644
--- a/liblttng-ust-comm/lttng-ust-comm.c
+++ b/liblttng-ust-comm/lttng-ust-comm.c
@@ -874,8 +874,7 @@ int serialize_basic_type(struct lttng_session *session,
 		if (session) {
 			const struct lttng_enum *_enum;
 
-			_enum = lttng_ust_enum_get(session,
-					lbt->enumeration.desc->name);
+			_enum = lttng_ust_enum_get_from_desc(session, lbt->enumeration.desc);
 			if (!_enum)
 				return -EINVAL;
 			ubt->enumeration.id = _enum->id;
diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
index 2b679b5..07385d7 100644
--- a/liblttng-ust/lttng-events.c
+++ b/liblttng-ust/lttng-events.c
@@ -257,21 +257,19 @@ int lttng_enum_create(const struct lttng_enum_desc *desc,
 	const char *enum_name = desc->name;
 	struct lttng_enum *_enum;
 	struct cds_hlist_head *head;
-	struct cds_hlist_node *node;
 	int ret = 0;
 	size_t name_len = strlen(enum_name);
 	uint32_t hash;
 	int notify_socket;
 
+	/* Check if this enum is already registered for this session. */
 	hash = jhash(enum_name, name_len, 0);
 	head = &session->enums_ht.table[hash & (LTTNG_UST_ENUM_HT_SIZE - 1)];
-	cds_hlist_for_each_entry(_enum, node, head, hlist) {
-		assert(_enum->desc);
-		if (!strncmp(_enum->desc->name, desc->name,
-				LTTNG_UST_SYM_NAME_LEN - 1)) {
-			ret = -EEXIST;
-			goto exist;
-		}
+
+	_enum = lttng_ust_enum_get_from_desc(session, desc);
+	if (_enum) {
+		ret = -EEXIST;
+		goto exist;
 	}
 
 	notify_socket = lttng_get_notify_socket(session->owner);
diff --git a/liblttng-ust/ust-core.c b/liblttng-ust/ust-core.c
index 5355f5c..76f729a 100644
--- a/liblttng-ust/ust-core.c
+++ b/liblttng-ust/ust-core.c
@@ -63,21 +63,20 @@ void lttng_transport_unregister(struct lttng_transport *transport)
 /*
  * Needed by comm layer.
  */
-struct lttng_enum *lttng_ust_enum_get(struct lttng_session *session,
-		const char *enum_name)
+struct lttng_enum *lttng_ust_enum_get_from_desc(struct lttng_session *session,
+		const struct lttng_enum_desc *enum_desc)
 {
 	struct lttng_enum *_enum;
 	struct cds_hlist_head *head;
 	struct cds_hlist_node *node;
-	size_t name_len = strlen(enum_name);
+	size_t name_len = strlen(enum_desc->name);
 	uint32_t hash;
 
-	hash = jhash(enum_name, name_len, 0);
+	hash = jhash(enum_desc->name, name_len, 0);
 	head = &session->enums_ht.table[hash & (LTTNG_UST_ENUM_HT_SIZE - 1)];
 	cds_hlist_for_each_entry(_enum, node, head, hlist) {
 		assert(_enum->desc);
-		if (!strncmp(_enum->desc->name, enum_name,
-				LTTNG_UST_SYM_NAME_LEN - 1))
+		if (_enum->desc == enum_desc)
 			return _enum;
 	}
 	return NULL;
-- 
2.7.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH lttng-ust 8/8] Support unloading of probe providers
       [not found] <1517600891-23632-1-git-send-email-francis.deslauriers@efficios.com>
                   ` (6 preceding siblings ...)
  2018-02-02 19:48 ` [PATCH lttng-ust 7/8] Rename lttng_ust_enum_get to lttng_ust_enum_get_from_desc Francis Deslauriers
@ 2018-02-02 19:48 ` Francis Deslauriers
       [not found] ` <1517600891-23632-3-git-send-email-francis.deslauriers@efficios.com>
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 29+ messages in thread
From: Francis Deslauriers @ 2018-02-02 19:48 UTC (permalink / raw)
  To: lttng-dev

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@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;
 	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;
-- 
2.7.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH lttng-ust 2/8] Fix: missing event removal from the event hashtable
       [not found] ` <1517600891-23632-3-git-send-email-francis.deslauriers@efficios.com>
@ 2018-02-07 20:50   ` Mathieu Desnoyers
       [not found]   ` <785880654.17806.1518036643540.JavaMail.zimbra@efficios.com>
  1 sibling, 0 replies; 29+ messages in thread
From: Mathieu Desnoyers @ 2018-02-07 20:50 UTC (permalink / raw)
  To: Francis Deslauriers; +Cc: lttng-dev



----- On Feb 2, 2018, at 2:48 PM, Francis Deslauriers francis.deslauriers@efficios.com wrote:

Is this really a fix ? Or is it a preparation step in order to be able to
remove events before the end of the session lifetime ?

Thanks,

Mathieu


> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
> ---
> liblttng-ust/lttng-events.c | 4 ++++
> 1 file changed, 4 insertions(+)
> 
> diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
> index f4a7ccc..7419f78 100644
> --- a/liblttng-ust/lttng-events.c
> +++ b/liblttng-ust/lttng-events.c
> @@ -883,7 +883,11 @@ void _lttng_event_destroy(struct lttng_event *event)
> {
> 	struct lttng_enabler_ref *enabler_ref, *tmp_enabler_ref;
> 
> +	/* Remove from event list. */
> 	cds_list_del(&event->node);
> +	/* Remove from event hash table. */
> +	cds_hlist_del(&event->hlist);
> +
> 	lttng_destroy_context(event->ctx);
> 	lttng_free_event_filter_runtime(event);
> 	/* Free event enabler refs */
> --
> 2.7.4

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH lttng-ust 4/8] Add probe provider unregister function
       [not found] ` <1517600891-23632-5-git-send-email-francis.deslauriers@efficios.com>
@ 2018-02-07 20:54   ` Mathieu Desnoyers
       [not found]   ` <1464902041.17817.1518036865129.JavaMail.zimbra@efficios.com>
  1 sibling, 0 replies; 29+ messages in thread
From: Mathieu Desnoyers @ 2018-02-07 20:54 UTC (permalink / raw)
  To: Francis Deslauriers; +Cc: lttng-dev


----- On Feb 2, 2018, at 2:48 PM, Francis Deslauriers francis.deslauriers@efficios.com wrote:

> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
> ---
> include/lttng/ust-events.h  |  1 +
> liblttng-ust/lttng-events.c | 89 +++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 90 insertions(+)
> 
> diff --git a/include/lttng/ust-events.h b/include/lttng/ust-events.h
> index caf7e63..019b0eb 100644
> --- a/include/lttng/ust-events.h
> +++ b/include/lttng/ust-events.h
> @@ -656,6 +656,7 @@ void synchronize_trace(void);
> 
> int lttng_probe_register(struct lttng_probe_desc *desc);
> void lttng_probe_unregister(struct lttng_probe_desc *desc);
> +void lttng_probe_provider_unregister_events(struct lttng_probe_desc *desc);
> int lttng_fix_pending_events(void);
> int lttng_probes_init(void);
> void lttng_probes_exit(void);
> diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
> index 7419f78..e8d4857 100644
> --- a/liblttng-ust/lttng-events.c
> +++ b/liblttng-ust/lttng-events.c
> @@ -789,6 +789,95 @@ void lttng_create_event_if_missing(struct lttng_enabler
> *enabler)
> }
> 
> /*
> + * Iterate over all the UST sessions to unregister and destroy all probes from
> + * the probe provider descriptor passed in arguments. Must me called with the

passed in arguments -> received as argument

> + * ust_lock held.
> + */
> +void lttng_probe_provider_unregister_events(struct lttng_probe_desc
> *provider_desc)
> +{
> +	int i;

move int i; to the last variable (shortest line).

> +	struct cds_list_head *sessionsp;
> +	struct lttng_session *session;
> +	struct cds_hlist_head *head;
> +	struct cds_hlist_node *node;
> +	struct lttng_event *event;
> +
> +	/* Get handle on list of sessions. */
> +	sessionsp = _lttng_get_sessions();
> +
> +	/*
> +	 * Iterate over all events in the probe provider descriptions and sessions
> +	 * to queue the unregistration of the events.
> +	 */
> +	for (i = 0; i < provider_desc->nr_events; i++) {
> +		const char *event_name;
> +		uint32_t hash;
> +		size_t name_len;
> +		const struct lttng_event_desc *event_desc;

same here about var definition layout.

> +
> +		event_desc = provider_desc->event_desc[i];
> +		event_name = event_desc->name;
> +		name_len = strlen(event_name);
> +		hash = jhash(event_name, name_len, 0);
> +
> +		/* Iterate over all session to find the current event description. */
> +		cds_list_for_each_entry(session, sessionsp, node) {

remove whiteline.

> +
> +			/*
> +			 * Get the list of events in the hashtable bucket and iterate to
> +			 * find the event matching this descriptor.
> +			 */
> +			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) {
> +
> +					/* Queue the unregistration of this event. */
> +					_lttng_event_unregister(event);
> +					break;
> +				}
> +			}
> +		}
> +	}
> +
> +	/* Wait for grace period. */
> +	synchronize_trace();
> +	/* Prune the unregistration queue. */
> +	__tracepoint_probe_prune_release_queue();
> +
> +	/*
> +	 * It is now safe to destroy the events and remove them from the event list
> +	 * and hashtables.
> +	 */
> +	for (i = 0; i < provider_desc->nr_events; i++) {
> +		const char *event_name;
> +		uint32_t hash;
> +		size_t name_len;
> +		const struct lttng_event_desc *event_desc;

same.

> +
> +		event_desc = provider_desc->event_desc[i];
> +		event_name = event_desc->name;
> +		name_len = strlen(event_name);
> +		hash = jhash(event_name, name_len, 0);
> +



> +		/* Iterate over all sessions to find the current event description. */
> +		cds_list_for_each_entry(session, sessionsp, node) {

whiteline.

> +
> +			/*
> +			 * Get the list of events in the hashtable bucket and iterate to
> +			 * find the event matching this descriptor.
> +			 */

beware for the iteration below: you may need to use a cds_list_for_each_entry_safe()
if you can remove the list entry while you iterate.

> +			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) {
> +					_lttng_event_destroy(event);

you have a "break" here. Is there a possibility that you can find more than one match ?

Thanks,

Mathieu

> +					break;
> +				}
> +			}
> +		}
> +	}
> +}
> +
> +/*
>  * Create events associated with an enabler (if not already present),
>  * and add backward reference from the event to the enabler.
>  */
> --
> 2.7.4

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH lttng-ust 5/8] Fix: missing enum removal from the enum hashtable
       [not found] ` <1517600891-23632-6-git-send-email-francis.deslauriers@efficios.com>
@ 2018-02-07 20:55   ` Mathieu Desnoyers
       [not found]   ` <271937624.17827.1518036900541.JavaMail.zimbra@efficios.com>
  1 sibling, 0 replies; 29+ messages in thread
From: Mathieu Desnoyers @ 2018-02-07 20:55 UTC (permalink / raw)
  To: Francis Deslauriers; +Cc: lttng-dev

----- On Feb 2, 2018, at 2:48 PM, Francis Deslauriers francis.deslauriers@efficios.com wrote:

Is it a fix or a preparation step for a new feature ?

Thanks,

Mathieu

> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
> ---
> liblttng-ust/lttng-events.c | 1 +
> 1 file changed, 1 insertion(+)
> 
> diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
> index e8d4857..2b679b5 100644
> --- a/liblttng-ust/lttng-events.c
> +++ b/liblttng-ust/lttng-events.c
> @@ -990,6 +990,7 @@ static
> void _lttng_enum_destroy(struct lttng_enum *_enum)
> {
> 	cds_list_del(&_enum->node);
> +	cds_hlist_del(&_enum->hlist);
> 	free(_enum);
> }
> 
> --
> 2.7.4

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH lttng-ust 6/8] Manually dlopen() liblttng-ust.so to prevent unloading
       [not found] ` <1517600891-23632-7-git-send-email-francis.deslauriers@efficios.com>
@ 2018-02-07 20:59   ` Mathieu Desnoyers
       [not found]   ` <241397831.17844.1518037156142.JavaMail.zimbra@efficios.com>
  1 sibling, 0 replies; 29+ messages in thread
From: Mathieu Desnoyers @ 2018-02-07 20:59 UTC (permalink / raw)
  To: Francis Deslauriers; +Cc: lttng-dev

----- On Feb 2, 2018, at 2:48 PM, Francis Deslauriers francis.deslauriers@efficios.com wrote:

> dlopen() increments the refcount of the library thus preventing the
> refcount to reach zero in the case of dlclose;

The changelog and comment do not explain _why_ this is needed.

The scenario is:

- an application is _not_ linked against liblttng-ust.so
- the application dlopen() a tracepoint probe,
- the tracepoint probe .so is linked against liblttng-ust, and this is what ends up
  getting lttng-ust loaded,

Given that our goal is to allow dlclose() of tracepoint probes (new features), we don't
want the side effect of this dlclose() to also try to unload liblttng-ust. Because
liblttng-ust does _not_ support being dlclose'd, we need to "pin" it in memory by
grabbing an extra reference on the library, with a NODELETE RTLD flag.

Thanks,

Mathieu


> 
> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
> ---
> configure.ac                  |  1 +
> liblttng-ust/Makefile.am      |  2 ++
> liblttng-ust/lttng-ust-comm.c | 22 ++++++++++++++++++++++
> 3 files changed, 25 insertions(+)
> 
> diff --git a/configure.ac b/configure.ac
> index b0b4157..4fc6f9c 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -25,6 +25,7 @@ m4_define([UST_LIB_V_MINOR], [0])
> m4_define([UST_LIB_V_PATCH], [0])
> 
> AC_SUBST([LTTNG_UST_LIBRARY_VERSION],
> [UST_LIB_V_MAJOR:UST_LIB_V_MINOR:UST_LIB_V_PATCH])
> +AC_SUBST([LTTNG_UST_LIBRARY_VERSION_MAJOR], [UST_LIB_V_MAJOR])
> # note: remember to update tracepoint.h dlopen() to match this version
> # number. TODO: eventually automate by exporting the major number.
> 
> diff --git a/liblttng-ust/Makefile.am b/liblttng-ust/Makefile.am
> index 982be69..a7edfd5 100644
> --- a/liblttng-ust/Makefile.am
> +++ b/liblttng-ust/Makefile.am
> @@ -60,6 +60,8 @@ liblttng_ust_runtime_la_SOURCES = \
> 	string-utils.c \
> 	string-utils.h
> 
> +liblttng_ust_runtime_la_CFLAGS =
> -DLTTNG_UST_LIBRARY_VERSION_MAJOR=\"$(LTTNG_UST_LIBRARY_VERSION_MAJOR)\"
> +
> if HAVE_PERF_EVENT
> liblttng_ust_runtime_la_SOURCES += \
> 	lttng-context-perf-counters.c \
> diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c
> index 511b9cf..ed912b8 100644
> --- a/liblttng-ust/lttng-ust-comm.c
> +++ b/liblttng-ust/lttng-ust-comm.c
> @@ -27,6 +27,7 @@
> #include <sys/stat.h>
> #include <sys/types.h>
> #include <sys/wait.h>
> +#include <dlfcn.h>
> #include <fcntl.h>
> #include <unistd.h>
> #include <errno.h>
> @@ -59,6 +60,9 @@
> #include "../libringbuffer/getcpu.h"
> #include "getenv.h"
> 
> +/* Concatenate lttng ust shared library name with its major version number. */
> +#define LTTNG_UST_LIB_SO_NAME "liblttng-ust.so."
> LTTNG_UST_LIBRARY_VERSION_MAJOR
> +
> /*
>  * Has lttng ust comm constructor been called ?
>  */
> @@ -1648,6 +1652,7 @@ void __attribute__((constructor)) lttng_ust_init(void)
> 	pthread_attr_t thread_attr;
> 	int timeout_mode;
> 	int ret;
> +	void *handle;
> 
> 	if (uatomic_xchg(&initialized, 1) == 1)
> 		return;
> @@ -1662,6 +1667,23 @@ void __attribute__((constructor)) lttng_ust_init(void)
> 	lttng_ust_loaded = 1;
> 
> 	/*
> +	 * Manually load liblttng-ust.so to increment the dynamic loader's internal
> +	 * refcount for this library so it never becomes zero, thus never gets
> +	 * unloaded from the address space of the process. Since we are already
> +	 * running in the constructor of the LTTNG_UST_LIB_SO_NAME library, calling
> +	 * dlopen will simply increment the refcount and no additionnal work is
> +	 * needed by the dynamic loader as the shared library is already loaded in
> +	 * the address space. As a safe guard, we use the RTLD_NODELETE flag to
> +	 * prevent unloading of the UST library if its refcount becomes zero
> +	 * (which should never happen). Do the return value check but discard the
> +	 * handle at the end of the function as it's not needed.
> +	 */
> +	handle = dlopen(LTTNG_UST_LIB_SO_NAME, RTLD_LAZY | RTLD_NODELETE);
> +	if (!handle) {
> +		ERR("dlopen of liblttng-ust shared library (%s).", LTTNG_UST_LIB_SO_NAME);
> +	}
> +
> +	/*
> 	 * We want precise control over the order in which we construct
> 	 * our sub-libraries vs starting to receive commands from
> 	 * sessiond (otherwise leading to errors when trying to create
> --
> 2.7.4

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH lttng-ust 8/8] Support unloading of probe providers
       [not found] ` <1517600891-23632-9-git-send-email-francis.deslauriers@efficios.com>
@ 2018-02-07 21:03   ` Mathieu Desnoyers
  0 siblings, 0 replies; 29+ messages in thread
From: Mathieu Desnoyers @ 2018-02-07 21:03 UTC (permalink / raw)
  To: Francis Deslauriers; +Cc: lttng-dev

----- On Feb 2, 2018, at 2:48 PM, Francis Deslauriers francis.deslauriers@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@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

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH lttng-ust 4/8] Add probe provider unregister function
       [not found]   ` <1464902041.17817.1518036865129.JavaMail.zimbra@efficios.com>
@ 2018-02-08 18:54     ` Francis Deslauriers
  0 siblings, 0 replies; 29+ messages in thread
From: Francis Deslauriers @ 2018-02-08 18:54 UTC (permalink / raw)
  To: Mathieu Desnoyers; +Cc: lttng-dev

2018-02-07 15:54 GMT-05:00 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>:
>
> ----- On Feb 2, 2018, at 2:48 PM, Francis Deslauriers francis.deslauriers@efficios.com wrote:
>
>> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
>> ---
>> include/lttng/ust-events.h  |  1 +
>> liblttng-ust/lttng-events.c | 89 +++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 90 insertions(+)
>>
>> diff --git a/include/lttng/ust-events.h b/include/lttng/ust-events.h
>> index caf7e63..019b0eb 100644
>> --- a/include/lttng/ust-events.h
>> +++ b/include/lttng/ust-events.h
>> @@ -656,6 +656,7 @@ void synchronize_trace(void);
>>
>> int lttng_probe_register(struct lttng_probe_desc *desc);
>> void lttng_probe_unregister(struct lttng_probe_desc *desc);
>> +void lttng_probe_provider_unregister_events(struct lttng_probe_desc *desc);
>> int lttng_fix_pending_events(void);
>> int lttng_probes_init(void);
>> void lttng_probes_exit(void);
>> diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
>> index 7419f78..e8d4857 100644
>> --- a/liblttng-ust/lttng-events.c
>> +++ b/liblttng-ust/lttng-events.c
>> @@ -789,6 +789,95 @@ void lttng_create_event_if_missing(struct lttng_enabler
>> *enabler)
>> }
>>
>> /*
>> + * Iterate over all the UST sessions to unregister and destroy all probes from
>> + * the probe provider descriptor passed in arguments. Must me called with the
>
> passed in arguments -> received as argument
>
>> + * ust_lock held.
>> + */
>> +void lttng_probe_provider_unregister_events(struct lttng_probe_desc
>> *provider_desc)
>> +{
>> +     int i;
>
> move int i; to the last variable (shortest line).
>
>> +     struct cds_list_head *sessionsp;
>> +     struct lttng_session *session;
>> +     struct cds_hlist_head *head;
>> +     struct cds_hlist_node *node;
>> +     struct lttng_event *event;
>> +
>> +     /* Get handle on list of sessions. */
>> +     sessionsp = _lttng_get_sessions();
>> +
>> +     /*
>> +      * Iterate over all events in the probe provider descriptions and sessions
>> +      * to queue the unregistration of the events.
>> +      */
>> +     for (i = 0; i < provider_desc->nr_events; i++) {
>> +             const char *event_name;
>> +             uint32_t hash;
>> +             size_t name_len;
>> +             const struct lttng_event_desc *event_desc;
>
> same here about var definition layout.
>
>> +
>> +             event_desc = provider_desc->event_desc[i];
>> +             event_name = event_desc->name;
>> +             name_len = strlen(event_name);
>> +             hash = jhash(event_name, name_len, 0);
>> +
>> +             /* Iterate over all session to find the current event description. */
>> +             cds_list_for_each_entry(session, sessionsp, node) {
>
> remove whiteline.
>
>> +
>> +                     /*
>> +                      * Get the list of events in the hashtable bucket and iterate to
>> +                      * find the event matching this descriptor.
>> +                      */
>> +                     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) {
>> +
>> +                                     /* Queue the unregistration of this event. */
>> +                                     _lttng_event_unregister(event);
>> +                                     break;
>> +                             }
>> +                     }
>> +             }
>> +     }
>> +
>> +     /* Wait for grace period. */
>> +     synchronize_trace();
>> +     /* Prune the unregistration queue. */
>> +     __tracepoint_probe_prune_release_queue();
>> +
>> +     /*
>> +      * It is now safe to destroy the events and remove them from the event list
>> +      * and hashtables.
>> +      */
>> +     for (i = 0; i < provider_desc->nr_events; i++) {
>> +             const char *event_name;
>> +             uint32_t hash;
>> +             size_t name_len;
>> +             const struct lttng_event_desc *event_desc;
>
> same.
>
>> +
>> +             event_desc = provider_desc->event_desc[i];
>> +             event_name = event_desc->name;
>> +             name_len = strlen(event_name);
>> +             hash = jhash(event_name, name_len, 0);
>> +
>
>
>
>> +             /* Iterate over all sessions to find the current event description. */
>> +             cds_list_for_each_entry(session, sessionsp, node) {
>
> whiteline.
>
>> +
>> +                     /*
>> +                      * Get the list of events in the hashtable bucket and iterate to
>> +                      * find the event matching this descriptor.
>> +                      */
>
> beware for the iteration below: you may need to use a cds_list_for_each_entry_safe()
> if you can remove the list entry while you iterate.
Good point. Fixed.
>
>> +                     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) {
>> +                                     _lttng_event_destroy(event);
>
> you have a "break" here. Is there a possibility that you can find more than one match ?

When this function is called, we are in the process of unloading a
probe provider, we iterate over
description of every events in that provider description. The if
statement above compares
the pointers of event descriptor so there is only one match possible
in a given process.
The break is there to avoid continuing the iteration over the other
events on that hlist needlessly.

All the coding style comments above were addressed.

Thank you,
Francis
>
> Thanks,
>
> Mathieu
>
>> +                                     break;
>> +                             }
>> +                     }
>> +             }
>> +     }
>> +}
>> +
>> +/*
>>  * Create events associated with an enabler (if not already present),
>>  * and add backward reference from the event to the enabler.
>>  */
>> --
>> 2.7.4
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com



-- 
Francis Deslauriers
Software developer
EfficiOS inc.
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH lttng-ust 6/8] Manually dlopen() liblttng-ust.so to prevent unloading
       [not found]   ` <241397831.17844.1518037156142.JavaMail.zimbra@efficios.com>
@ 2018-02-08 20:24     ` Francis Deslauriers
       [not found]     ` <CADcCL0hfhancCuQvq6XaOmSbOdGh=E3hZavrjLW+h8CgRmKfUA@mail.gmail.com>
  1 sibling, 0 replies; 29+ messages in thread
From: Francis Deslauriers @ 2018-02-08 20:24 UTC (permalink / raw)
  To: Mathieu Desnoyers; +Cc: lttng-dev

2018-02-07 15:59 GMT-05:00 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>:
> ----- On Feb 2, 2018, at 2:48 PM, Francis Deslauriers francis.deslauriers@efficios.com wrote:
>
>> dlopen() increments the refcount of the library thus preventing the
>> refcount to reach zero in the case of dlclose;
>
> The changelog and comment do not explain _why_ this is needed.
>
> The scenario is:
>
> - an application is _not_ linked against liblttng-ust.so
> - the application dlopen() a tracepoint probe,
> - the tracepoint probe .so is linked against liblttng-ust, and this is what ends up
>   getting lttng-ust loaded,
>
> Given that our goal is to allow dlclose() of tracepoint probes (new features), we don't
> want the side effect of this dlclose() to also try to unload liblttng-ust. Because
> liblttng-ust does _not_ support being dlclose'd, we need to "pin" it in memory by
> grabbing an extra reference on the library, with a NODELETE RTLD flag.
>
I agree, here is a revised version of the commit message and comment:

Manually dlopen() liblttng-ust.so to prevent unloading

The support of dlclose() allows for the following problematic scenario:
- Application is not linked against the liblttng-ust.so
- Application dlopen() a probe provider library that is linked against
liblttng-ust.so
- Application dlclose() the probe provider

In this scenario, the probe provider has a dependency on liblttng-ust.so, so
when it's loaded by the application, liblttng-ust.so is loaded too. The probe
provider library now has the only reference to the liblttng-ust.so library.
When the application calls dlclose() on it, all its references are dropped,
thus triggering the unloading of both the probe provider library and
liblttng-ust.so.

This scenario is problematic because the probe provider needs the
liblttng-ust.so library to complete its own unloading. As we can't control the
order in which libraries are unloaded, we must ensure that liblttng-ust.so is
not unloaded before the probe provider. To prevent that unloading, we pin it in
memory by grabbing an extra reference on the library, with a RTLD_NODELETE
flag. This will prevent the dynamic loader from ever removing the
liblttng-ust.so library from the process' address space.


Thank you,
Francis

> Thanks,
>
> Mathieu
>
>
>>
>> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
>> ---
>> configure.ac                  |  1 +
>> liblttng-ust/Makefile.am      |  2 ++
>> liblttng-ust/lttng-ust-comm.c | 22 ++++++++++++++++++++++
>> 3 files changed, 25 insertions(+)
>>
>> diff --git a/configure.ac b/configure.ac
>> index b0b4157..4fc6f9c 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -25,6 +25,7 @@ m4_define([UST_LIB_V_MINOR], [0])
>> m4_define([UST_LIB_V_PATCH], [0])
>>
>> AC_SUBST([LTTNG_UST_LIBRARY_VERSION],
>> [UST_LIB_V_MAJOR:UST_LIB_V_MINOR:UST_LIB_V_PATCH])
>> +AC_SUBST([LTTNG_UST_LIBRARY_VERSION_MAJOR], [UST_LIB_V_MAJOR])
>> # note: remember to update tracepoint.h dlopen() to match this version
>> # number. TODO: eventually automate by exporting the major number.
>>
>> diff --git a/liblttng-ust/Makefile.am b/liblttng-ust/Makefile.am
>> index 982be69..a7edfd5 100644
>> --- a/liblttng-ust/Makefile.am
>> +++ b/liblttng-ust/Makefile.am
>> @@ -60,6 +60,8 @@ liblttng_ust_runtime_la_SOURCES = \
>>       string-utils.c \
>>       string-utils.h
>>
>> +liblttng_ust_runtime_la_CFLAGS =
>> -DLTTNG_UST_LIBRARY_VERSION_MAJOR=\"$(LTTNG_UST_LIBRARY_VERSION_MAJOR)\"
>> +
>> if HAVE_PERF_EVENT
>> liblttng_ust_runtime_la_SOURCES += \
>>       lttng-context-perf-counters.c \
>> diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c
>> index 511b9cf..ed912b8 100644
>> --- a/liblttng-ust/lttng-ust-comm.c
>> +++ b/liblttng-ust/lttng-ust-comm.c
>> @@ -27,6 +27,7 @@
>> #include <sys/stat.h>
>> #include <sys/types.h>
>> #include <sys/wait.h>
>> +#include <dlfcn.h>
>> #include <fcntl.h>
>> #include <unistd.h>
>> #include <errno.h>
>> @@ -59,6 +60,9 @@
>> #include "../libringbuffer/getcpu.h"
>> #include "getenv.h"
>>
>> +/* Concatenate lttng ust shared library name with its major version number. */
>> +#define LTTNG_UST_LIB_SO_NAME "liblttng-ust.so."
>> LTTNG_UST_LIBRARY_VERSION_MAJOR
>> +
>> /*
>>  * Has lttng ust comm constructor been called ?
>>  */
>> @@ -1648,6 +1652,7 @@ void __attribute__((constructor)) lttng_ust_init(void)
>>       pthread_attr_t thread_attr;
>>       int timeout_mode;
>>       int ret;
>> +     void *handle;
>>
>>       if (uatomic_xchg(&initialized, 1) == 1)
>>               return;
>> @@ -1662,6 +1667,23 @@ void __attribute__((constructor)) lttng_ust_init(void)
>>       lttng_ust_loaded = 1;
>>
>>       /*
>> +      * Manually load liblttng-ust.so to increment the dynamic loader's internal
>> +      * refcount for this library so it never becomes zero, thus never gets
>> +      * unloaded from the address space of the process. Since we are already
>> +      * running in the constructor of the LTTNG_UST_LIB_SO_NAME library, calling
>> +      * dlopen will simply increment the refcount and no additionnal work is
>> +      * needed by the dynamic loader as the shared library is already loaded in
>> +      * the address space. As a safe guard, we use the RTLD_NODELETE flag to
>> +      * prevent unloading of the UST library if its refcount becomes zero
>> +      * (which should never happen). Do the return value check but discard the
>> +      * handle at the end of the function as it's not needed.
>> +      */
>> +     handle = dlopen(LTTNG_UST_LIB_SO_NAME, RTLD_LAZY | RTLD_NODELETE);
>> +     if (!handle) {
>> +             ERR("dlopen of liblttng-ust shared library (%s).", LTTNG_UST_LIB_SO_NAME);
>> +     }
>> +
>> +     /*
>>        * We want precise control over the order in which we construct
>>        * our sub-libraries vs starting to receive commands from
>>        * sessiond (otherwise leading to errors when trying to create
>> --
>> 2.7.4
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com



-- 
Francis Deslauriers
Software developer
EfficiOS inc.
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH lttng-ust 2/8] Fix: missing event removal from the event hashtable
       [not found]   ` <785880654.17806.1518036643540.JavaMail.zimbra@efficios.com>
@ 2018-02-08 20:42     ` Francis Deslauriers
       [not found]     ` <CADcCL0iVq=qA5Teck2dL3wkXjn0ZVaUUt8q7Fv64ihhAEUUNiA@mail.gmail.com>
  1 sibling, 0 replies; 29+ messages in thread
From: Francis Deslauriers @ 2018-02-08 20:42 UTC (permalink / raw)
  To: Mathieu Desnoyers; +Cc: lttng-dev

2018-02-07 15:50 GMT-05:00 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>:
>
>
> ----- On Feb 2, 2018, at 2:48 PM, Francis Deslauriers francis.deslauriers@efficios.com wrote:
>
> Is this really a fix ? Or is it a preparation step in order to be able to
> remove events before the end of the session lifetime ?

It's not a fix in the sense that it was not causing issues because it
was only called at session destruction.
But it's a fix in the sense that the _lttng_event_destroy function was
not doing all the work necessary to destroy an event passed as
argument. Taken by itself, a call to this function would leave one of
the hlists of the hash table in a incorrect state by freeing an
element without cds_hlist_del() it from the hlist before.

That was my thought process. What do you think?

Thank you,
Francis

> Thanks,
>
> Mathieu
>
>
>> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
>> ---
>> liblttng-ust/lttng-events.c | 4 ++++
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
>> index f4a7ccc..7419f78 100644
>> --- a/liblttng-ust/lttng-events.c
>> +++ b/liblttng-ust/lttng-events.c
>> @@ -883,7 +883,11 @@ void _lttng_event_destroy(struct lttng_event *event)
>> {
>>       struct lttng_enabler_ref *enabler_ref, *tmp_enabler_ref;
>>
>> +     /* Remove from event list. */
>>       cds_list_del(&event->node);
>> +     /* Remove from event hash table. */
>> +     cds_hlist_del(&event->hlist);
>> +
>>       lttng_destroy_context(event->ctx);
>>       lttng_free_event_filter_runtime(event);
>>       /* Free event enabler refs */
>> --
>> 2.7.4
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com



-- 
Francis Deslauriers
Software developer
EfficiOS inc.
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH lttng-ust 5/8] Fix: missing enum removal from the enum hashtable
       [not found]   ` <271937624.17827.1518036900541.JavaMail.zimbra@efficios.com>
@ 2018-02-08 20:45     ` Francis Deslauriers
       [not found]     ` <CADcCL0h7joW64uTw3A6788smGTWrexGYdntF2TWQ_D32JSNZrA@mail.gmail.com>
  1 sibling, 0 replies; 29+ messages in thread
From: Francis Deslauriers @ 2018-02-08 20:45 UTC (permalink / raw)
  To: Mathieu Desnoyers; +Cc: lttng-dev

2018-02-07 15:55 GMT-05:00 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>:
> ----- On Feb 2, 2018, at 2:48 PM, Francis Deslauriers francis.deslauriers@efficios.com wrote:
>
> Is it a fix or a preparation step for a new feature ?
Same thought process that with: cds_hlist_del(&event->hlist);

Thanks,
Francis
>
> Thanks,
>
> Mathieu
>
>> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
>> ---
>> liblttng-ust/lttng-events.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
>> index e8d4857..2b679b5 100644
>> --- a/liblttng-ust/lttng-events.c
>> +++ b/liblttng-ust/lttng-events.c
>> @@ -990,6 +990,7 @@ static
>> void _lttng_enum_destroy(struct lttng_enum *_enum)
>> {
>>       cds_list_del(&_enum->node);
>> +     cds_hlist_del(&_enum->hlist);
>>       free(_enum);
>> }
>>
>> --
>> 2.7.4
>
> --
> Mathieu Desnoyers
> EfficiOS Inc.
> http://www.efficios.com



-- 
Francis Deslauriers
Software developer
EfficiOS inc.
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH lttng-ust 2/8] Fix: missing event removal from the event hashtable
       [not found]     ` <CADcCL0iVq=qA5Teck2dL3wkXjn0ZVaUUt8q7Fv64ihhAEUUNiA@mail.gmail.com>
@ 2018-02-08 20:52       ` Mathieu Desnoyers
  0 siblings, 0 replies; 29+ messages in thread
From: Mathieu Desnoyers @ 2018-02-08 20:52 UTC (permalink / raw)
  To: Francis Deslauriers; +Cc: lttng-dev

----- On Feb 8, 2018, at 3:42 PM, Francis Deslauriers francis.deslauriers@efficios.com wrote:

> 2018-02-07 15:50 GMT-05:00 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>:
>>
>>
>> ----- On Feb 2, 2018, at 2:48 PM, Francis Deslauriers
>> francis.deslauriers@efficios.com wrote:
>>
>> Is this really a fix ? Or is it a preparation step in order to be able to
>> remove events before the end of the session lifetime ?
> 
> It's not a fix in the sense that it was not causing issues because it
> was only called at session destruction.
> But it's a fix in the sense that the _lttng_event_destroy function was
> not doing all the work necessary to destroy an event passed as
> argument. Taken by itself, a call to this function would leave one of
> the hlists of the hash table in a incorrect state by freeing an
> element without cds_hlist_del() it from the hlist before.
> 
> That was my thought process. What do you think?

A fix is something I need to backport to make things work. I don't see
this as a fix based on that definition.

Thanks,

Mathieu

> 
> Thank you,
> Francis
> 
>> Thanks,
>>
>> Mathieu
>>
>>
>>> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
>>> ---
>>> liblttng-ust/lttng-events.c | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
>>> index f4a7ccc..7419f78 100644
>>> --- a/liblttng-ust/lttng-events.c
>>> +++ b/liblttng-ust/lttng-events.c
>>> @@ -883,7 +883,11 @@ void _lttng_event_destroy(struct lttng_event *event)
>>> {
>>>       struct lttng_enabler_ref *enabler_ref, *tmp_enabler_ref;
>>>
>>> +     /* Remove from event list. */
>>>       cds_list_del(&event->node);
>>> +     /* Remove from event hash table. */
>>> +     cds_hlist_del(&event->hlist);
>>> +
>>>       lttng_destroy_context(event->ctx);
>>>       lttng_free_event_filter_runtime(event);
>>>       /* Free event enabler refs */
>>> --
>>> 2.7.4
>>
>> --
>> Mathieu Desnoyers
>> EfficiOS Inc.
>> http://www.efficios.com
> 
> 
> 
> --
> Francis Deslauriers
> Software developer
> EfficiOS inc.

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH lttng-ust 5/8] Fix: missing enum removal from the enum hashtable
       [not found]     ` <CADcCL0h7joW64uTw3A6788smGTWrexGYdntF2TWQ_D32JSNZrA@mail.gmail.com>
@ 2018-02-08 20:52       ` Mathieu Desnoyers
  0 siblings, 0 replies; 29+ messages in thread
From: Mathieu Desnoyers @ 2018-02-08 20:52 UTC (permalink / raw)
  To: Francis Deslauriers; +Cc: lttng-dev

----- On Feb 8, 2018, at 3:45 PM, Francis Deslauriers francis.deslauriers@efficios.com wrote:

> 2018-02-07 15:55 GMT-05:00 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>:
>> ----- On Feb 2, 2018, at 2:48 PM, Francis Deslauriers
>> francis.deslauriers@efficios.com wrote:
>>
>> Is it a fix or a preparation step for a new feature ?
> Same thought process that with: cds_hlist_del(&event->hlist);

Not a fix then.

Thanks,

Mathieu

> 
> Thanks,
> Francis
>>
>> Thanks,
>>
>> Mathieu
>>
>>> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
>>> ---
>>> liblttng-ust/lttng-events.c | 1 +
>>> 1 file changed, 1 insertion(+)
>>>
>>> diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
>>> index e8d4857..2b679b5 100644
>>> --- a/liblttng-ust/lttng-events.c
>>> +++ b/liblttng-ust/lttng-events.c
>>> @@ -990,6 +990,7 @@ static
>>> void _lttng_enum_destroy(struct lttng_enum *_enum)
>>> {
>>>       cds_list_del(&_enum->node);
>>> +     cds_hlist_del(&_enum->hlist);
>>>       free(_enum);
>>> }
>>>
>>> --
>>> 2.7.4
>>
>> --
>> Mathieu Desnoyers
>> EfficiOS Inc.
>> http://www.efficios.com
> 
> 
> 
> --
> Francis Deslauriers
> Software developer
> EfficiOS inc.

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH lttng-ust 6/8] Manually dlopen() liblttng-ust.so to prevent unloading
       [not found]     ` <CADcCL0hfhancCuQvq6XaOmSbOdGh=E3hZavrjLW+h8CgRmKfUA@mail.gmail.com>
@ 2018-02-08 20:56       ` Mathieu Desnoyers
  0 siblings, 0 replies; 29+ messages in thread
From: Mathieu Desnoyers @ 2018-02-08 20:56 UTC (permalink / raw)
  To: Francis Deslauriers; +Cc: lttng-dev

----- On Feb 8, 2018, at 3:24 PM, Francis Deslauriers francis.deslauriers@efficios.com wrote:

> 2018-02-07 15:59 GMT-05:00 Mathieu Desnoyers <mathieu.desnoyers@efficios.com>:
>> ----- On Feb 2, 2018, at 2:48 PM, Francis Deslauriers
>> francis.deslauriers@efficios.com wrote:
>>
>>> dlopen() increments the refcount of the library thus preventing the
>>> refcount to reach zero in the case of dlclose;
>>
>> The changelog and comment do not explain _why_ this is needed.
>>
>> The scenario is:
>>
>> - an application is _not_ linked against liblttng-ust.so
>> - the application dlopen() a tracepoint probe,
>> - the tracepoint probe .so is linked against liblttng-ust, and this is what ends
>> up
>>   getting lttng-ust loaded,
>>
>> Given that our goal is to allow dlclose() of tracepoint probes (new features),
>> we don't
>> want the side effect of this dlclose() to also try to unload liblttng-ust.
>> Because
>> liblttng-ust does _not_ support being dlclose'd, we need to "pin" it in memory
>> by
>> grabbing an extra reference on the library, with a NODELETE RTLD flag.
>>
> I agree, here is a revised version of the commit message and comment:
> 
> Manually dlopen() liblttng-ust.so to prevent unloading
> 
> The support of dlclose() allows for the following problematic scenario:
> - Application is not linked against the liblttng-ust.so
> - Application dlopen() a probe provider library that is linked against
> liblttng-ust.so
> - Application dlclose() the probe provider
> 
> In this scenario, the probe provider has a dependency on liblttng-ust.so, so
> when it's loaded by the application, liblttng-ust.so is loaded too. The probe
> provider library now has the only reference to the liblttng-ust.so library.
> When the application calls dlclose() on it, all its references are dropped,
> thus triggering the unloading of both the probe provider library and
> liblttng-ust.so.
> 
> This scenario is problematic because the probe provider needs the
> liblttng-ust.so library to complete its own unloading. As we can't control the
> order in which libraries are unloaded, we must ensure that liblttng-ust.so is
> not unloaded before the probe provider. To prevent that unloading, we pin it in
> memory by grabbing an extra reference on the library, with a RTLD_NODELETE
> flag. This will prevent the dynamic loader from ever removing the
> liblttng-ust.so library from the process' address space.

I don't think this was the reason *why* we could not dlclose() liblttng-ust.
But I do not recall how that fails exactly. Thoughts ?

Thanks,

Mathieu


> 
> 
> Thank you,
> Francis
> 
>> Thanks,
>>
>> Mathieu
>>
>>
>>>
>>> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
>>> ---
>>> configure.ac                  |  1 +
>>> liblttng-ust/Makefile.am      |  2 ++
>>> liblttng-ust/lttng-ust-comm.c | 22 ++++++++++++++++++++++
>>> 3 files changed, 25 insertions(+)
>>>
>>> diff --git a/configure.ac b/configure.ac
>>> index b0b4157..4fc6f9c 100644
>>> --- a/configure.ac
>>> +++ b/configure.ac
>>> @@ -25,6 +25,7 @@ m4_define([UST_LIB_V_MINOR], [0])
>>> m4_define([UST_LIB_V_PATCH], [0])
>>>
>>> AC_SUBST([LTTNG_UST_LIBRARY_VERSION],
>>> [UST_LIB_V_MAJOR:UST_LIB_V_MINOR:UST_LIB_V_PATCH])
>>> +AC_SUBST([LTTNG_UST_LIBRARY_VERSION_MAJOR], [UST_LIB_V_MAJOR])
>>> # note: remember to update tracepoint.h dlopen() to match this version
>>> # number. TODO: eventually automate by exporting the major number.
>>>
>>> diff --git a/liblttng-ust/Makefile.am b/liblttng-ust/Makefile.am
>>> index 982be69..a7edfd5 100644
>>> --- a/liblttng-ust/Makefile.am
>>> +++ b/liblttng-ust/Makefile.am
>>> @@ -60,6 +60,8 @@ liblttng_ust_runtime_la_SOURCES = \
>>>       string-utils.c \
>>>       string-utils.h
>>>
>>> +liblttng_ust_runtime_la_CFLAGS =
>>> -DLTTNG_UST_LIBRARY_VERSION_MAJOR=\"$(LTTNG_UST_LIBRARY_VERSION_MAJOR)\"
>>> +
>>> if HAVE_PERF_EVENT
>>> liblttng_ust_runtime_la_SOURCES += \
>>>       lttng-context-perf-counters.c \
>>> diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c
>>> index 511b9cf..ed912b8 100644
>>> --- a/liblttng-ust/lttng-ust-comm.c
>>> +++ b/liblttng-ust/lttng-ust-comm.c
>>> @@ -27,6 +27,7 @@
>>> #include <sys/stat.h>
>>> #include <sys/types.h>
>>> #include <sys/wait.h>
>>> +#include <dlfcn.h>
>>> #include <fcntl.h>
>>> #include <unistd.h>
>>> #include <errno.h>
>>> @@ -59,6 +60,9 @@
>>> #include "../libringbuffer/getcpu.h"
>>> #include "getenv.h"
>>>
>>> +/* Concatenate lttng ust shared library name with its major version number. */
>>> +#define LTTNG_UST_LIB_SO_NAME "liblttng-ust.so."
>>> LTTNG_UST_LIBRARY_VERSION_MAJOR
>>> +
>>> /*
>>>  * Has lttng ust comm constructor been called ?
>>>  */
>>> @@ -1648,6 +1652,7 @@ void __attribute__((constructor)) lttng_ust_init(void)
>>>       pthread_attr_t thread_attr;
>>>       int timeout_mode;
>>>       int ret;
>>> +     void *handle;
>>>
>>>       if (uatomic_xchg(&initialized, 1) == 1)
>>>               return;
>>> @@ -1662,6 +1667,23 @@ void __attribute__((constructor)) lttng_ust_init(void)
>>>       lttng_ust_loaded = 1;
>>>
>>>       /*
>>> +      * Manually load liblttng-ust.so to increment the dynamic loader's
>>> internal
>>> +      * refcount for this library so it never becomes zero, thus never gets
>>> +      * unloaded from the address space of the process. Since we are already
>>> +      * running in the constructor of the LTTNG_UST_LIB_SO_NAME library,
>>> calling
>>> +      * dlopen will simply increment the refcount and no additionnal work is
>>> +      * needed by the dynamic loader as the shared library is already loaded in
>>> +      * the address space. As a safe guard, we use the RTLD_NODELETE flag to
>>> +      * prevent unloading of the UST library if its refcount becomes zero
>>> +      * (which should never happen). Do the return value check but discard the
>>> +      * handle at the end of the function as it's not needed.
>>> +      */
>>> +     handle = dlopen(LTTNG_UST_LIB_SO_NAME, RTLD_LAZY | RTLD_NODELETE);
>>> +     if (!handle) {
>>> +             ERR("dlopen of liblttng-ust shared library (%s).",
>>> LTTNG_UST_LIB_SO_NAME);
>>> +     }
>>> +
>>> +     /*
>>>        * We want precise control over the order in which we construct
>>>        * our sub-libraries vs starting to receive commands from
>>>        * sessiond (otherwise leading to errors when trying to create
>>> --
>>> 2.7.4
>>
>> --
>> Mathieu Desnoyers
>> EfficiOS Inc.
>> http://www.efficios.com
> 
> 
> 
> --
> Francis Deslauriers
> Software developer
> EfficiOS inc.

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH lttng-ust v2 0/6] Support provider duplicates and unloading
       [not found] <1517600891-23632-1-git-send-email-francis.deslauriers@efficios.com>
                   ` (12 preceding siblings ...)
       [not found] ` <1517600891-23632-9-git-send-email-francis.deslauriers@efficios.com>
@ 2018-02-09 20:15 ` Francis Deslauriers
       [not found] ` <1518207325-9641-1-git-send-email-francis.deslauriers@efficios.com>
  14 siblings, 0 replies; 29+ messages in thread
From: Francis Deslauriers @ 2018-02-09 20:15 UTC (permalink / raw)
  To: lttng-dev

This patch set adds the support for duplicated probe providers and
support for unloading probe providers using dlclose().

It allows to advance scenarios where probe providers can be upgraded
during tracing.

For example, during tracing the user could dlopen() a new version of a probe
provider and dlclose() the previous one. All this without stop tracing.

A patch set for lttng-tools project is necessary to take advantage of
those changes.

The lttng-tools changes can be found on this branch of my Github fork:
https://github.com/frdeso/lttng-tools/commits/multi-lib-support

Francis Deslauriers (5):
  Cleanup: Move version numbers in separate variables in configure
    script
  Add probe provider unregister function
  Manually dlopen() liblttng-ust.so to prevent unloading
  Rename lttng_ust_enum_get to lttng_ust_enum_get_from_desc
  Support unloading of probe providers

Mathieu Desnoyers (1):
  Remove duplicate provider name checks

 configure.ac                         |   7 +-
 include/lttng/ust-events.h           |   6 +-
 include/lttng/ust-tracepoint-event.h |   2 +-
 liblttng-ust-comm/lttng-ust-comm.c   |   3 +-
 liblttng-ust/Makefile.am             |   2 +
 liblttng-ust/lttng-events.c          | 135 ++++++++++++++++++++++++++++++-----
 liblttng-ust/lttng-probes.c          |  28 ++------
 liblttng-ust/lttng-ust-comm.c        |  25 +++++++
 liblttng-ust/tracepoint.c            |  43 ++++++++---
 liblttng-ust/ust-core.c              |  11 ++-
 10 files changed, 199 insertions(+), 63 deletions(-)

-- 
2.7.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [PATCH lttng-ust v2 1/6] Remove duplicate provider name checks
       [not found] ` <1518207325-9641-1-git-send-email-francis.deslauriers@efficios.com>
@ 2018-02-09 20:15   ` Francis Deslauriers
  2018-02-09 20:15   ` [PATCH lttng-ust v2 2/6] Cleanup: Move version numbers in separate variables in configure script Francis Deslauriers
                     ` (6 subsequent siblings)
  7 siblings, 0 replies; 29+ messages in thread
From: Francis Deslauriers @ 2018-02-09 20:15 UTC (permalink / raw)
  To: lttng-dev

From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

It's now possible to register a probe provider with a name that has
already been registered. This is useful when wanting to load a new
version of a shared library on a already running process.

Changes are necessary in the lttng-session daemon to support cases where
the newly register event has a different probe payload.

Taking a simple case where a probe provider is registered twice, the
tracepoint call site will have two probes registered to it and thus will
generate two events in the trace.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
---
 include/lttng/ust-tracepoint-event.h |  2 +-
 liblttng-ust/lttng-events.c          | 11 -----------
 liblttng-ust/lttng-probes.c          | 23 +----------------------
 3 files changed, 2 insertions(+), 34 deletions(-)

diff --git a/include/lttng/ust-tracepoint-event.h b/include/lttng/ust-tracepoint-event.h
index 15399c7..ec292d2 100644
--- a/include/lttng/ust-tracepoint-event.h
+++ b/include/lttng/ust-tracepoint-event.h
@@ -1007,7 +1007,7 @@ _TP_COMBINE_TOKENS(__lttng_events_init__, TRACEPOINT_PROVIDER)(void)
 	_TP_COMBINE_TOKENS(__tracepoint_provider_check_, TRACEPOINT_PROVIDER)();
 	ret = lttng_probe_register(&_TP_COMBINE_TOKENS(__probe_desc___, TRACEPOINT_PROVIDER));
 	if (ret) {
-		fprintf(stderr, "LTTng-UST: Error (%d) while registering tracepoint probe. Duplicate registration of tracepoint probes having the same name is not allowed.\n", ret);
+		fprintf(stderr, "LTTng-UST: Error (%d) while registering tracepoint probe.\n", ret);
 		abort();
 	}
 }
diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
index bfdae4f..f4a7ccc 100644
--- a/liblttng-ust/lttng-events.c
+++ b/liblttng-ust/lttng-events.c
@@ -537,7 +537,6 @@ int lttng_event_create(const struct lttng_event_desc *desc,
 	struct lttng_event *event;
 	struct lttng_session *session = chan->session;
 	struct cds_hlist_head *head;
-	struct cds_hlist_node *node;
 	int ret = 0;
 	size_t name_len = strlen(event_name);
 	uint32_t hash;
@@ -546,15 +545,6 @@ int lttng_event_create(const struct lttng_event_desc *desc,
 
 	hash = jhash(event_name, name_len, 0);
 	head = &chan->session->events_ht.table[hash & (LTTNG_UST_EVENT_HT_SIZE - 1)];
-	cds_hlist_for_each_entry(event, node, head, hlist) {
-		assert(event->desc);
-		if (!strncmp(event->desc->name, desc->name,
-					LTTNG_UST_SYM_NAME_LEN - 1)
-				&& chan == event->chan) {
-			ret = -EEXIST;
-			goto exist;
-		}
-	}
 
 	notify_socket = lttng_get_notify_socket(session->owner);
 	if (notify_socket < 0) {
@@ -623,7 +613,6 @@ sessiond_register_error:
 cache_error:
 create_enum_error:
 socket_error:
-exist:
 	return ret;
 }
 
diff --git a/liblttng-ust/lttng-probes.c b/liblttng-ust/lttng-probes.c
index 390265a..a09497f 100644
--- a/liblttng-ust/lttng-probes.c
+++ b/liblttng-ust/lttng-probes.c
@@ -148,20 +148,6 @@ struct cds_list_head *lttng_get_probe_list_head(void)
 }
 
 static
-const struct lttng_probe_desc *find_provider(const char *provider)
-{
-	struct lttng_probe_desc *iter;
-	struct cds_list_head *probe_list;
-
-	probe_list = lttng_get_probe_list_head();
-	cds_list_for_each_entry(iter, probe_list, head) {
-		if (!strcmp(iter->provider, provider))
-			return iter;
-	}
-	return NULL;
-}
-
-static
 int check_provider_version(struct lttng_probe_desc *desc)
 {
 	/*
@@ -206,13 +192,6 @@ int lttng_probe_register(struct lttng_probe_desc *desc)
 
 	ust_lock_nocheck();
 
-	/*
-	 * Check if the provider has already been registered.
-	 */
-	if (find_provider(desc->provider)) {
-		ret = -EEXIST;
-		goto end;
-	}
 	cds_list_add(&desc->lazy_init_head, &lazy_probe_init);
 	desc->lazy = 1;
 	DBG("adding probe %s containing %u events to lazy registration list",
@@ -224,7 +203,7 @@ int lttng_probe_register(struct lttng_probe_desc *desc)
 	 */
 	if (lttng_session_active())
 		fixup_lazy_probes();
-end:
+
 	ust_unlock();
 	return ret;
 }
-- 
2.7.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH lttng-ust v2 2/6] Cleanup: Move version numbers in separate variables in configure script
       [not found] ` <1518207325-9641-1-git-send-email-francis.deslauriers@efficios.com>
  2018-02-09 20:15   ` [PATCH lttng-ust v2 1/6] Remove duplicate provider name checks Francis Deslauriers
@ 2018-02-09 20:15   ` Francis Deslauriers
  2018-02-09 20:15   ` [PATCH lttng-ust v2 3/6] Add probe provider unregister function Francis Deslauriers
                     ` (5 subsequent siblings)
  7 siblings, 0 replies; 29+ messages in thread
From: Francis Deslauriers @ 2018-02-09 20:15 UTC (permalink / raw)
  To: lttng-dev

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
---
 configure.ac | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index fb2f278..b0b4157 100644
--- a/configure.ac
+++ b/configure.ac
@@ -20,7 +20,11 @@ AC_SUBST([PATCHLEVEL_VERSION], [V_PATCH])
 # Following the numbering scheme proposed by libtool for the library version
 # http://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html
 # This is the library version of liblttng-ust.
-AC_SUBST([LTTNG_UST_LIBRARY_VERSION], [0:0:0])
+m4_define([UST_LIB_V_MAJOR], [0])
+m4_define([UST_LIB_V_MINOR], [0])
+m4_define([UST_LIB_V_PATCH], [0])
+
+AC_SUBST([LTTNG_UST_LIBRARY_VERSION], [UST_LIB_V_MAJOR:UST_LIB_V_MINOR:UST_LIB_V_PATCH])
 # note: remember to update tracepoint.h dlopen() to match this version
 # number. TODO: eventually automate by exporting the major number.
 
-- 
2.7.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH lttng-ust v2 3/6] Add probe provider unregister function
       [not found] ` <1518207325-9641-1-git-send-email-francis.deslauriers@efficios.com>
  2018-02-09 20:15   ` [PATCH lttng-ust v2 1/6] Remove duplicate provider name checks Francis Deslauriers
  2018-02-09 20:15   ` [PATCH lttng-ust v2 2/6] Cleanup: Move version numbers in separate variables in configure script Francis Deslauriers
@ 2018-02-09 20:15   ` Francis Deslauriers
  2018-02-09 20:15   ` [PATCH lttng-ust v2 4/6] Manually dlopen() liblttng-ust.so to prevent unloading Francis Deslauriers
                     ` (4 subsequent siblings)
  7 siblings, 0 replies; 29+ messages in thread
From: Francis Deslauriers @ 2018-02-09 20:15 UTC (permalink / raw)
  To: lttng-dev

Also, ensure that enumerations and events are removed from their
respective hashtables when _lttng_{event, enum}_destroy functions are
called.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
---

v2:
use of cds_hlist_for_each_entry_safe to safely delete event from hashtable
during iteration
Various coding style fixes

---
 include/lttng/ust-events.h  |   1 +
 liblttng-ust/lttng-events.c | 110 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 111 insertions(+)

diff --git a/include/lttng/ust-events.h b/include/lttng/ust-events.h
index caf7e63..019b0eb 100644
--- a/include/lttng/ust-events.h
+++ b/include/lttng/ust-events.h
@@ -656,6 +656,7 @@ void synchronize_trace(void);
 
 int lttng_probe_register(struct lttng_probe_desc *desc);
 void lttng_probe_unregister(struct lttng_probe_desc *desc);
+void lttng_probe_provider_unregister_events(struct lttng_probe_desc *desc);
 int lttng_fix_pending_events(void);
 int lttng_probes_init(void);
 void lttng_probes_exit(void);
diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
index f4a7ccc..57eb6f7 100644
--- a/liblttng-ust/lttng-events.c
+++ b/liblttng-ust/lttng-events.c
@@ -789,6 +789,111 @@ void lttng_create_event_if_missing(struct lttng_enabler *enabler)
 }
 
 /*
+ * Iterate over all the UST sessions to unregister and destroy all probes from
+ * the probe provider descriptor received as argument. Must me called with the
+ * ust_lock held.
+ */
+void lttng_probe_provider_unregister_events(struct lttng_probe_desc *provider_desc)
+{
+	struct cds_hlist_node *node, *tmp_node;
+	struct cds_list_head *sessionsp;
+	struct lttng_session *session;
+	struct cds_hlist_head *head;
+	struct lttng_event *event;
+	int i;
+
+	/* Get handle on list of sessions. */
+	sessionsp = _lttng_get_sessions();
+
+	/*
+	 * Iterate over all events in the probe provider descriptions and sessions
+	 * to queue the unregistration of the events.
+	 */
+	for (i = 0; i < provider_desc->nr_events; i++) {
+		const struct lttng_event_desc *event_desc;
+		const char *event_name;
+		size_t name_len;
+		uint32_t hash;
+
+		event_desc = provider_desc->event_desc[i];
+		event_name = event_desc->name;
+		name_len = strlen(event_name);
+		hash = jhash(event_name, name_len, 0);
+
+		/* Iterate over all session to find the current event description. */
+		cds_list_for_each_entry(session, sessionsp, node) {
+			/*
+			 * Get the list of events in the hashtable bucket and iterate to
+			 * find the event matching this descriptor.
+			 */
+			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) {
+					/* Queue the unregistration of this event. */
+					_lttng_event_unregister(event);
+					break;
+				}
+			}
+		}
+	}
+
+	/* Wait for grace period. */
+	synchronize_trace();
+	/* Prune the unregistration queue. */
+	__tracepoint_probe_prune_release_queue();
+
+	/*
+	 * It is now safe to destroy the events and remove them from the event list
+	 * and hashtables.
+	 */
+	for (i = 0; i < provider_desc->nr_events; i++) {
+		const struct lttng_event_desc *event_desc;
+		const char *event_name;
+		size_t name_len;
+		uint32_t hash;
+
+		event_desc = provider_desc->event_desc[i];
+		event_name = event_desc->name;
+		name_len = strlen(event_name);
+		hash = jhash(event_name, name_len, 0);
+
+		/* Iterate over all sessions to find the current event description. */
+		cds_list_for_each_entry(session, sessionsp, node) {
+			/*
+			 * Get the list of events in the hashtable bucket and iterate to
+			 * find the event matching this descriptor.
+			 */
+			head = &session->events_ht.table[hash & (LTTNG_UST_EVENT_HT_SIZE - 1)];
+			cds_hlist_for_each_entry_safe(event, node, tmp_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_enum_desc *enum_desc;
+						const struct lttng_event_field *field;
+						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;
+				}
+			}
+		}
+	}
+}
+
+/*
  * Create events associated with an enabler (if not already present),
  * and add backward reference from the event to the enabler.
  */
@@ -883,7 +988,11 @@ void _lttng_event_destroy(struct lttng_event *event)
 {
 	struct lttng_enabler_ref *enabler_ref, *tmp_enabler_ref;
 
+	/* Remove from event list. */
 	cds_list_del(&event->node);
+	/* Remove from event hash table. */
+	cds_hlist_del(&event->hlist);
+
 	lttng_destroy_context(event->ctx);
 	lttng_free_event_filter_runtime(event);
 	/* Free event enabler refs */
@@ -897,6 +1006,7 @@ static
 void _lttng_enum_destroy(struct lttng_enum *_enum)
 {
 	cds_list_del(&_enum->node);
+	cds_hlist_del(&_enum->hlist);
 	free(_enum);
 }
 
-- 
2.7.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH lttng-ust v2 4/6] Manually dlopen() liblttng-ust.so to prevent unloading
       [not found] ` <1518207325-9641-1-git-send-email-francis.deslauriers@efficios.com>
                     ` (2 preceding siblings ...)
  2018-02-09 20:15   ` [PATCH lttng-ust v2 3/6] Add probe provider unregister function Francis Deslauriers
@ 2018-02-09 20:15   ` Francis Deslauriers
  2018-02-09 20:15   ` [PATCH lttng-ust v2 5/6] Rename lttng_ust_enum_get to lttng_ust_enum_get_from_desc Francis Deslauriers
                     ` (3 subsequent siblings)
  7 siblings, 0 replies; 29+ messages in thread
From: Francis Deslauriers @ 2018-02-09 20:15 UTC (permalink / raw)
  To: lttng-dev

The support of probe provider dlclose() allows for the following problematic
scenario:
- Application is not linked against the liblttng-ust.so
- Application dlopen() a probe provider library that is linked against
	liblttng-ust.so
- Application dlclose() the probe provider

In this scenario, the probe provider has a dependency on liblttng-ust.so, so
when it's loaded by the application, liblttng-ust.so is loaded too. The probe
provider library now has the only reference to the liblttng-ust.so library.
When the application calls dlclose() on it, all its references are dropped,
thus triggering the unloading of both the probe provider library and
liblttng-ust.so.

This scenario is problematic because lttng ust_listener_threads are in DETACHED
state. We cannot join them and therefore we cannot unload the library
containing the code they run. Only the operating system can free those
resources.

The reason why those threads are in DETACHED state is to quickly teardown
applications on process exit.

A possible solution to investigate: if we can determine whether liblttng-ust.so
is being dlopen (directly or undirectly) or it's linked against the
application, we could set the detached state accordingly.

To prevent that unloading, we pin it in memory by grabbing an extra reference
on the library, with a RTLD_NODELETE flag. This will prevent the dynamic loader
from ever removing the liblttng-ust.so library from the process' address space.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
---

v2:
Rephrase commit message and comment
---

 configure.ac                  |  1 +
 liblttng-ust/Makefile.am      |  2 ++
 liblttng-ust/lttng-ust-comm.c | 25 +++++++++++++++++++++++++
 3 files changed, 28 insertions(+)

diff --git a/configure.ac b/configure.ac
index b0b4157..4fc6f9c 100644
--- a/configure.ac
+++ b/configure.ac
@@ -25,6 +25,7 @@ m4_define([UST_LIB_V_MINOR], [0])
 m4_define([UST_LIB_V_PATCH], [0])
 
 AC_SUBST([LTTNG_UST_LIBRARY_VERSION], [UST_LIB_V_MAJOR:UST_LIB_V_MINOR:UST_LIB_V_PATCH])
+AC_SUBST([LTTNG_UST_LIBRARY_VERSION_MAJOR], [UST_LIB_V_MAJOR])
 # note: remember to update tracepoint.h dlopen() to match this version
 # number. TODO: eventually automate by exporting the major number.
 
diff --git a/liblttng-ust/Makefile.am b/liblttng-ust/Makefile.am
index 982be69..a7edfd5 100644
--- a/liblttng-ust/Makefile.am
+++ b/liblttng-ust/Makefile.am
@@ -60,6 +60,8 @@ liblttng_ust_runtime_la_SOURCES = \
 	string-utils.c \
 	string-utils.h
 
+liblttng_ust_runtime_la_CFLAGS = -DLTTNG_UST_LIBRARY_VERSION_MAJOR=\"$(LTTNG_UST_LIBRARY_VERSION_MAJOR)\"
+
 if HAVE_PERF_EVENT
 liblttng_ust_runtime_la_SOURCES += \
 	lttng-context-perf-counters.c \
diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c
index 511b9cf..57e5ce6 100644
--- a/liblttng-ust/lttng-ust-comm.c
+++ b/liblttng-ust/lttng-ust-comm.c
@@ -27,6 +27,7 @@
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <sys/wait.h>
+#include <dlfcn.h>
 #include <fcntl.h>
 #include <unistd.h>
 #include <errno.h>
@@ -59,6 +60,9 @@
 #include "../libringbuffer/getcpu.h"
 #include "getenv.h"
 
+/* Concatenate lttng ust shared library name with its major version number. */
+#define LTTNG_UST_LIB_SO_NAME "liblttng-ust.so." LTTNG_UST_LIBRARY_VERSION_MAJOR
+
 /*
  * Has lttng ust comm constructor been called ?
  */
@@ -1648,6 +1652,7 @@ void __attribute__((constructor)) lttng_ust_init(void)
 	pthread_attr_t thread_attr;
 	int timeout_mode;
 	int ret;
+	void *handle;
 
 	if (uatomic_xchg(&initialized, 1) == 1)
 		return;
@@ -1662,6 +1667,26 @@ void __attribute__((constructor)) lttng_ust_init(void)
 	lttng_ust_loaded = 1;
 
 	/*
+	 * We need to ensure that the liblttng-ust library is not unloaded to avoid
+	 * the unloading of code used by the ust_listener_threads as we can not
+	 * reliably know when they exited. To do that, manually load
+	 * liblttng-ust.so to increment the dynamic loader's internal refcount for
+	 * this library so it never becomes zero, thus never gets unloaded from the
+	 * address space of the process. Since we are already running in the
+	 * constructor of the LTTNG_UST_LIB_SO_NAME library, calling dlopen will
+	 * simply increment the refcount and no additionnal work is needed by the
+	 * dynamic loader as the shared library is already loaded in the address
+	 * space. As a safe guard, we use the RTLD_NODELETE flag to prevent
+	 * unloading of the UST library if its refcount becomes zero (which should
+	 * never happen). Do the return value check but discard the handle at the
+	 * end of the function as it's not needed.
+	 */
+	handle = dlopen(LTTNG_UST_LIB_SO_NAME, RTLD_LAZY | RTLD_NODELETE);
+	if (!handle) {
+		ERR("dlopen of liblttng-ust shared library (%s).", LTTNG_UST_LIB_SO_NAME);
+	}
+
+	/*
 	 * We want precise control over the order in which we construct
 	 * our sub-libraries vs starting to receive commands from
 	 * sessiond (otherwise leading to errors when trying to create
-- 
2.7.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH lttng-ust v2 5/6] Rename lttng_ust_enum_get to lttng_ust_enum_get_from_desc
       [not found] ` <1518207325-9641-1-git-send-email-francis.deslauriers@efficios.com>
                     ` (3 preceding siblings ...)
  2018-02-09 20:15   ` [PATCH lttng-ust v2 4/6] Manually dlopen() liblttng-ust.so to prevent unloading Francis Deslauriers
@ 2018-02-09 20:15   ` Francis Deslauriers
  2018-02-09 20:15   ` [PATCH lttng-ust v2 6/6] Support unloading of probe providers Francis Deslauriers
                     ` (2 subsequent siblings)
  7 siblings, 0 replies; 29+ messages in thread
From: Francis Deslauriers @ 2018-02-09 20:15 UTC (permalink / raw)
  To: lttng-dev

Change the prototype to take a descriptor instead of a char *.
Now that provider names can have duplicates enum names are not
necessarily unique.

Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
---
 include/lttng/ust-events.h         |  5 +++--
 liblttng-ust-comm/lttng-ust-comm.c |  3 +--
 liblttng-ust/lttng-events.c        | 14 ++++++--------
 liblttng-ust/ust-core.c            | 11 +++++------
 4 files changed, 15 insertions(+), 18 deletions(-)

diff --git a/include/lttng/ust-events.h b/include/lttng/ust-events.h
index 019b0eb..8673350 100644
--- a/include/lttng/ust-events.h
+++ b/include/lttng/ust-events.h
@@ -731,8 +731,9 @@ int lttng_session_active(void);
 typedef int (*t_statedump_func_ptr)(struct lttng_session *session);
 void lttng_handle_pending_statedump(void *owner);
 struct cds_list_head *_lttng_get_sessions(void);
-struct lttng_enum *lttng_ust_enum_get(struct lttng_session *session,
-		const char *enum_name);
+
+struct lttng_enum *lttng_ust_enum_get_from_desc(struct lttng_session *session,
+		const struct lttng_enum_desc *enum_desc);
 
 void lttng_ust_dl_update(void *ip);
 void lttng_ust_fixup_fd_tracker_tls(void);
diff --git a/liblttng-ust-comm/lttng-ust-comm.c b/liblttng-ust-comm/lttng-ust-comm.c
index 9fe6d28..d95e31d 100644
--- a/liblttng-ust-comm/lttng-ust-comm.c
+++ b/liblttng-ust-comm/lttng-ust-comm.c
@@ -874,8 +874,7 @@ int serialize_basic_type(struct lttng_session *session,
 		if (session) {
 			const struct lttng_enum *_enum;
 
-			_enum = lttng_ust_enum_get(session,
-					lbt->enumeration.desc->name);
+			_enum = lttng_ust_enum_get_from_desc(session, lbt->enumeration.desc);
 			if (!_enum)
 				return -EINVAL;
 			ubt->enumeration.id = _enum->id;
diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
index 57eb6f7..e130ec6 100644
--- a/liblttng-ust/lttng-events.c
+++ b/liblttng-ust/lttng-events.c
@@ -257,21 +257,19 @@ int lttng_enum_create(const struct lttng_enum_desc *desc,
 	const char *enum_name = desc->name;
 	struct lttng_enum *_enum;
 	struct cds_hlist_head *head;
-	struct cds_hlist_node *node;
 	int ret = 0;
 	size_t name_len = strlen(enum_name);
 	uint32_t hash;
 	int notify_socket;
 
+	/* Check if this enum is already registered for this session. */
 	hash = jhash(enum_name, name_len, 0);
 	head = &session->enums_ht.table[hash & (LTTNG_UST_ENUM_HT_SIZE - 1)];
-	cds_hlist_for_each_entry(_enum, node, head, hlist) {
-		assert(_enum->desc);
-		if (!strncmp(_enum->desc->name, desc->name,
-				LTTNG_UST_SYM_NAME_LEN - 1)) {
-			ret = -EEXIST;
-			goto exist;
-		}
+
+	_enum = lttng_ust_enum_get_from_desc(session, desc);
+	if (_enum) {
+		ret = -EEXIST;
+		goto exist;
 	}
 
 	notify_socket = lttng_get_notify_socket(session->owner);
diff --git a/liblttng-ust/ust-core.c b/liblttng-ust/ust-core.c
index 5355f5c..76f729a 100644
--- a/liblttng-ust/ust-core.c
+++ b/liblttng-ust/ust-core.c
@@ -63,21 +63,20 @@ void lttng_transport_unregister(struct lttng_transport *transport)
 /*
  * Needed by comm layer.
  */
-struct lttng_enum *lttng_ust_enum_get(struct lttng_session *session,
-		const char *enum_name)
+struct lttng_enum *lttng_ust_enum_get_from_desc(struct lttng_session *session,
+		const struct lttng_enum_desc *enum_desc)
 {
 	struct lttng_enum *_enum;
 	struct cds_hlist_head *head;
 	struct cds_hlist_node *node;
-	size_t name_len = strlen(enum_name);
+	size_t name_len = strlen(enum_desc->name);
 	uint32_t hash;
 
-	hash = jhash(enum_name, name_len, 0);
+	hash = jhash(enum_desc->name, name_len, 0);
 	head = &session->enums_ht.table[hash & (LTTNG_UST_ENUM_HT_SIZE - 1)];
 	cds_hlist_for_each_entry(_enum, node, head, hlist) {
 		assert(_enum->desc);
-		if (!strncmp(_enum->desc->name, enum_name,
-				LTTNG_UST_SYM_NAME_LEN - 1))
+		if (_enum->desc == enum_desc)
 			return _enum;
 	}
 	return NULL;
-- 
2.7.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [PATCH lttng-ust v2 6/6] Support unloading of probe providers
       [not found] ` <1518207325-9641-1-git-send-email-francis.deslauriers@efficios.com>
                     ` (4 preceding siblings ...)
  2018-02-09 20:15   ` [PATCH lttng-ust v2 5/6] Rename lttng_ust_enum_get to lttng_ust_enum_get_from_desc Francis Deslauriers
@ 2018-02-09 20:15   ` Francis Deslauriers
       [not found]   ` <1518207325-9641-5-git-send-email-francis.deslauriers@efficios.com>
  2018-02-09 20:43   ` [PATCH lttng-ust v2 0/6] Support provider duplicates and unloading Mathieu Desnoyers
  7 siblings, 0 replies; 29+ messages in thread
From: Francis Deslauriers @ 2018-02-09 20:15 UTC (permalink / raw)
  To: lttng-dev

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@efficios.com>
---

v2:
Coding style fixes.
---
 liblttng-ust/lttng-events.c |  2 +-
 liblttng-ust/lttng-probes.c |  5 ++++-
 liblttng-ust/tracepoint.c   | 43 ++++++++++++++++++++++++++++++++++---------
 3 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/liblttng-ust/lttng-events.c b/liblttng-ust/lttng-events.c
index e130ec6..255c4b9 100644
--- a/liblttng-ust/lttng-events.c
+++ b/liblttng-ust/lttng-events.c
@@ -798,7 +798,7 @@ void lttng_probe_provider_unregister_events(struct lttng_probe_desc *provider_de
 	struct lttng_session *session;
 	struct cds_hlist_head *head;
 	struct lttng_event *event;
-	int i;
+	unsigned int i, j;
 
 	/* Get handle on list of sessions. */
 	sessionsp = _lttng_get_sessions();
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..2c25d46 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;
-- 
2.7.4

_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [PATCH lttng-ust v2 4/6] Manually dlopen() liblttng-ust.so to prevent unloading
       [not found]   ` <1518207325-9641-5-git-send-email-francis.deslauriers@efficios.com>
@ 2018-02-09 20:43     ` Mathieu Desnoyers
  0 siblings, 0 replies; 29+ messages in thread
From: Mathieu Desnoyers @ 2018-02-09 20:43 UTC (permalink / raw)
  To: Francis Deslauriers; +Cc: lttng-dev

Cleaned up this changelog, which has tabs (should be spaces), and has
lines above ~74 columns (which is odd in a git log).

Thanks,

Mathieu

----- On Feb 9, 2018, at 3:15 PM, Francis Deslauriers francis.deslauriers@efficios.com wrote:

> The support of probe provider dlclose() allows for the following problematic
> scenario:
> - Application is not linked against the liblttng-ust.so
> - Application dlopen() a probe provider library that is linked against
>	liblttng-ust.so
> - Application dlclose() the probe provider
> 
> In this scenario, the probe provider has a dependency on liblttng-ust.so, so
> when it's loaded by the application, liblttng-ust.so is loaded too. The probe
> provider library now has the only reference to the liblttng-ust.so library.
> When the application calls dlclose() on it, all its references are dropped,
> thus triggering the unloading of both the probe provider library and
> liblttng-ust.so.
> 
> This scenario is problematic because lttng ust_listener_threads are in DETACHED
> state. We cannot join them and therefore we cannot unload the library
> containing the code they run. Only the operating system can free those
> resources.
> 
> The reason why those threads are in DETACHED state is to quickly teardown
> applications on process exit.
> 
> A possible solution to investigate: if we can determine whether liblttng-ust.so
> is being dlopen (directly or undirectly) or it's linked against the
> application, we could set the detached state accordingly.
> 
> To prevent that unloading, we pin it in memory by grabbing an extra reference
> on the library, with a RTLD_NODELETE flag. This will prevent the dynamic loader
> from ever removing the liblttng-ust.so library from the process' address space.
> 
> Signed-off-by: Francis Deslauriers <francis.deslauriers@efficios.com>
> ---
> 
> v2:
> Rephrase commit message and comment
> ---
> 
> configure.ac                  |  1 +
> liblttng-ust/Makefile.am      |  2 ++
> liblttng-ust/lttng-ust-comm.c | 25 +++++++++++++++++++++++++
> 3 files changed, 28 insertions(+)
> 
> diff --git a/configure.ac b/configure.ac
> index b0b4157..4fc6f9c 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -25,6 +25,7 @@ m4_define([UST_LIB_V_MINOR], [0])
> m4_define([UST_LIB_V_PATCH], [0])
> 
> AC_SUBST([LTTNG_UST_LIBRARY_VERSION],
> [UST_LIB_V_MAJOR:UST_LIB_V_MINOR:UST_LIB_V_PATCH])
> +AC_SUBST([LTTNG_UST_LIBRARY_VERSION_MAJOR], [UST_LIB_V_MAJOR])
> # note: remember to update tracepoint.h dlopen() to match this version
> # number. TODO: eventually automate by exporting the major number.
> 
> diff --git a/liblttng-ust/Makefile.am b/liblttng-ust/Makefile.am
> index 982be69..a7edfd5 100644
> --- a/liblttng-ust/Makefile.am
> +++ b/liblttng-ust/Makefile.am
> @@ -60,6 +60,8 @@ liblttng_ust_runtime_la_SOURCES = \
> 	string-utils.c \
> 	string-utils.h
> 
> +liblttng_ust_runtime_la_CFLAGS =
> -DLTTNG_UST_LIBRARY_VERSION_MAJOR=\"$(LTTNG_UST_LIBRARY_VERSION_MAJOR)\"
> +
> if HAVE_PERF_EVENT
> liblttng_ust_runtime_la_SOURCES += \
> 	lttng-context-perf-counters.c \
> diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c
> index 511b9cf..57e5ce6 100644
> --- a/liblttng-ust/lttng-ust-comm.c
> +++ b/liblttng-ust/lttng-ust-comm.c
> @@ -27,6 +27,7 @@
> #include <sys/stat.h>
> #include <sys/types.h>
> #include <sys/wait.h>
> +#include <dlfcn.h>
> #include <fcntl.h>
> #include <unistd.h>
> #include <errno.h>
> @@ -59,6 +60,9 @@
> #include "../libringbuffer/getcpu.h"
> #include "getenv.h"
> 
> +/* Concatenate lttng ust shared library name with its major version number. */
> +#define LTTNG_UST_LIB_SO_NAME "liblttng-ust.so."
> LTTNG_UST_LIBRARY_VERSION_MAJOR
> +
> /*
>  * Has lttng ust comm constructor been called ?
>  */
> @@ -1648,6 +1652,7 @@ void __attribute__((constructor)) lttng_ust_init(void)
> 	pthread_attr_t thread_attr;
> 	int timeout_mode;
> 	int ret;
> +	void *handle;
> 
> 	if (uatomic_xchg(&initialized, 1) == 1)
> 		return;
> @@ -1662,6 +1667,26 @@ void __attribute__((constructor)) lttng_ust_init(void)
> 	lttng_ust_loaded = 1;
> 
> 	/*
> +	 * We need to ensure that the liblttng-ust library is not unloaded to avoid
> +	 * the unloading of code used by the ust_listener_threads as we can not
> +	 * reliably know when they exited. To do that, manually load
> +	 * liblttng-ust.so to increment the dynamic loader's internal refcount for
> +	 * this library so it never becomes zero, thus never gets unloaded from the
> +	 * address space of the process. Since we are already running in the
> +	 * constructor of the LTTNG_UST_LIB_SO_NAME library, calling dlopen will
> +	 * simply increment the refcount and no additionnal work is needed by the
> +	 * dynamic loader as the shared library is already loaded in the address
> +	 * space. As a safe guard, we use the RTLD_NODELETE flag to prevent
> +	 * unloading of the UST library if its refcount becomes zero (which should
> +	 * never happen). Do the return value check but discard the handle at the
> +	 * end of the function as it's not needed.
> +	 */
> +	handle = dlopen(LTTNG_UST_LIB_SO_NAME, RTLD_LAZY | RTLD_NODELETE);
> +	if (!handle) {
> +		ERR("dlopen of liblttng-ust shared library (%s).", LTTNG_UST_LIB_SO_NAME);
> +	}
> +
> +	/*
> 	 * We want precise control over the order in which we construct
> 	 * our sub-libraries vs starting to receive commands from
> 	 * sessiond (otherwise leading to errors when trying to create
> --
> 2.7.4

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [PATCH lttng-ust v2 0/6] Support provider duplicates and unloading
       [not found] ` <1518207325-9641-1-git-send-email-francis.deslauriers@efficios.com>
                     ` (6 preceding siblings ...)
       [not found]   ` <1518207325-9641-5-git-send-email-francis.deslauriers@efficios.com>
@ 2018-02-09 20:43   ` Mathieu Desnoyers
  7 siblings, 0 replies; 29+ messages in thread
From: Mathieu Desnoyers @ 2018-02-09 20:43 UTC (permalink / raw)
  To: Francis Deslauriers; +Cc: lttng-dev

All merged into lttng-ust master, thanks!

Mathieu

----- On Feb 9, 2018, at 3:15 PM, Francis Deslauriers francis.deslauriers@efficios.com wrote:

> This patch set adds the support for duplicated probe providers and
> support for unloading probe providers using dlclose().
> 
> It allows to advance scenarios where probe providers can be upgraded
> during tracing.
> 
> For example, during tracing the user could dlopen() a new version of a probe
> provider and dlclose() the previous one. All this without stop tracing.
> 
> A patch set for lttng-tools project is necessary to take advantage of
> those changes.
> 
> The lttng-tools changes can be found on this branch of my Github fork:
> https://github.com/frdeso/lttng-tools/commits/multi-lib-support
> 
> Francis Deslauriers (5):
>  Cleanup: Move version numbers in separate variables in configure
>    script
>  Add probe provider unregister function
>  Manually dlopen() liblttng-ust.so to prevent unloading
>  Rename lttng_ust_enum_get to lttng_ust_enum_get_from_desc
>  Support unloading of probe providers
> 
> Mathieu Desnoyers (1):
>  Remove duplicate provider name checks
> 
> configure.ac                         |   7 +-
> include/lttng/ust-events.h           |   6 +-
> include/lttng/ust-tracepoint-event.h |   2 +-
> liblttng-ust-comm/lttng-ust-comm.c   |   3 +-
> liblttng-ust/Makefile.am             |   2 +
> liblttng-ust/lttng-events.c          | 135 ++++++++++++++++++++++++++++++-----
> liblttng-ust/lttng-probes.c          |  28 ++------
> liblttng-ust/lttng-ust-comm.c        |  25 +++++++
> liblttng-ust/tracepoint.c            |  43 ++++++++---
> liblttng-ust/ust-core.c              |  11 ++-
> 10 files changed, 199 insertions(+), 63 deletions(-)
> 
> --
> 2.7.4

-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2018-02-09 20:42 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1517600891-23632-1-git-send-email-francis.deslauriers@efficios.com>
2018-02-02 19:48 ` [PATCH lttng-ust 1/8] Remove duplicate provider name checks Francis Deslauriers
2018-02-02 19:48 ` [PATCH lttng-ust 2/8] Fix: missing event removal from the event hashtable Francis Deslauriers
2018-02-02 19:48 ` [PATCH lttng-ust 3/8] Cleanup: Move version numbers in separate variables in configure script Francis Deslauriers
2018-02-02 19:48 ` [PATCH lttng-ust 4/8] Add probe provider unregister function Francis Deslauriers
2018-02-02 19:48 ` [PATCH lttng-ust 5/8] Fix: missing enum removal from the enum hashtable Francis Deslauriers
2018-02-02 19:48 ` [PATCH lttng-ust 6/8] Manually dlopen() liblttng-ust.so to prevent unloading Francis Deslauriers
2018-02-02 19:48 ` [PATCH lttng-ust 7/8] Rename lttng_ust_enum_get to lttng_ust_enum_get_from_desc Francis Deslauriers
2018-02-02 19:48 ` [PATCH lttng-ust 8/8] Support unloading of probe providers Francis Deslauriers
     [not found] ` <1517600891-23632-3-git-send-email-francis.deslauriers@efficios.com>
2018-02-07 20:50   ` [PATCH lttng-ust 2/8] Fix: missing event removal from the event hashtable Mathieu Desnoyers
     [not found]   ` <785880654.17806.1518036643540.JavaMail.zimbra@efficios.com>
2018-02-08 20:42     ` Francis Deslauriers
     [not found]     ` <CADcCL0iVq=qA5Teck2dL3wkXjn0ZVaUUt8q7Fv64ihhAEUUNiA@mail.gmail.com>
2018-02-08 20:52       ` Mathieu Desnoyers
     [not found] ` <1517600891-23632-5-git-send-email-francis.deslauriers@efficios.com>
2018-02-07 20:54   ` [PATCH lttng-ust 4/8] Add probe provider unregister function Mathieu Desnoyers
     [not found]   ` <1464902041.17817.1518036865129.JavaMail.zimbra@efficios.com>
2018-02-08 18:54     ` Francis Deslauriers
     [not found] ` <1517600891-23632-6-git-send-email-francis.deslauriers@efficios.com>
2018-02-07 20:55   ` [PATCH lttng-ust 5/8] Fix: missing enum removal from the enum hashtable Mathieu Desnoyers
     [not found]   ` <271937624.17827.1518036900541.JavaMail.zimbra@efficios.com>
2018-02-08 20:45     ` Francis Deslauriers
     [not found]     ` <CADcCL0h7joW64uTw3A6788smGTWrexGYdntF2TWQ_D32JSNZrA@mail.gmail.com>
2018-02-08 20:52       ` Mathieu Desnoyers
     [not found] ` <1517600891-23632-7-git-send-email-francis.deslauriers@efficios.com>
2018-02-07 20:59   ` [PATCH lttng-ust 6/8] Manually dlopen() liblttng-ust.so to prevent unloading Mathieu Desnoyers
     [not found]   ` <241397831.17844.1518037156142.JavaMail.zimbra@efficios.com>
2018-02-08 20:24     ` Francis Deslauriers
     [not found]     ` <CADcCL0hfhancCuQvq6XaOmSbOdGh=E3hZavrjLW+h8CgRmKfUA@mail.gmail.com>
2018-02-08 20:56       ` Mathieu Desnoyers
     [not found] ` <1517600891-23632-9-git-send-email-francis.deslauriers@efficios.com>
2018-02-07 21:03   ` [PATCH lttng-ust 8/8] Support unloading of probe providers Mathieu Desnoyers
2018-02-09 20:15 ` [PATCH lttng-ust v2 0/6] Support provider duplicates and unloading Francis Deslauriers
     [not found] ` <1518207325-9641-1-git-send-email-francis.deslauriers@efficios.com>
2018-02-09 20:15   ` [PATCH lttng-ust v2 1/6] Remove duplicate provider name checks Francis Deslauriers
2018-02-09 20:15   ` [PATCH lttng-ust v2 2/6] Cleanup: Move version numbers in separate variables in configure script Francis Deslauriers
2018-02-09 20:15   ` [PATCH lttng-ust v2 3/6] Add probe provider unregister function Francis Deslauriers
2018-02-09 20:15   ` [PATCH lttng-ust v2 4/6] Manually dlopen() liblttng-ust.so to prevent unloading Francis Deslauriers
2018-02-09 20:15   ` [PATCH lttng-ust v2 5/6] Rename lttng_ust_enum_get to lttng_ust_enum_get_from_desc Francis Deslauriers
2018-02-09 20:15   ` [PATCH lttng-ust v2 6/6] Support unloading of probe providers Francis Deslauriers
     [not found]   ` <1518207325-9641-5-git-send-email-francis.deslauriers@efficios.com>
2018-02-09 20:43     ` [PATCH lttng-ust v2 4/6] Manually dlopen() liblttng-ust.so to prevent unloading Mathieu Desnoyers
2018-02-09 20:43   ` [PATCH lttng-ust v2 0/6] Support provider duplicates and unloading Mathieu Desnoyers

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.