All of lore.kernel.org
 help / color / mirror / Atom feed
* [lttng-dev] [PATCH 1/2] lttng_ust_init_thread: initialise cached context values
@ 2022-07-18 21:59 Norbert Lange via lttng-dev
  2022-07-18 21:59 ` [lttng-dev] [PATCH 2/2] lttng_ust_init_thread: call urcu_register_thread Norbert Lange via lttng-dev
  2022-07-19 19:39 ` [lttng-dev] [PATCH 1/2] lttng_ust_init_thread: initialise cached context values Mathieu Desnoyers via lttng-dev
  0 siblings, 2 replies; 6+ messages in thread
From: Norbert Lange via lttng-dev @ 2022-07-18 21:59 UTC (permalink / raw)
  To: lttng-dev

Modify all relevant *_alloc_tls functions so that they take an
argument for 'init'. Setting this to 'true' will read
the actual context value and store it into a thread local
cache.

The function 'lttng_ust_init_thread' will use this to
precalculate context values. Tracepoints can then avoid
systemcalls.

Signed-off-by: Norbert Lange <nolange79@gmail.com>
---
*   see https://lore.kernel.org/all/CADYdroN3=pAe66crtsVT9aHE4T+bT61-tvHFKCuwZYkHhuyFjw@mail.gmail.com/
*   Will update docs after review, and do some more tests
    during the week
---
 src/lib/lttng-ust/lttng-context-cgroup-ns.c |  4 +++-
 src/lib/lttng-ust/lttng-context-ipc-ns.c    |  4 +++-
 src/lib/lttng-ust/lttng-context-net-ns.c    |  4 +++-
 src/lib/lttng-ust/lttng-context-procname.c  |  4 +++-
 src/lib/lttng-ust/lttng-context-provider.c  |  4 ++--
 src/lib/lttng-ust/lttng-context-time-ns.c   |  4 +++-
 src/lib/lttng-ust/lttng-context-uts-ns.c    |  4 +++-
 src/lib/lttng-ust/lttng-context-vtid.c      |  4 +++-
 src/lib/lttng-ust/lttng-probes.c            |  4 ++--
 src/lib/lttng-ust/lttng-tracer-core.h       | 16 +++++++-------
 src/lib/lttng-ust/lttng-ust-comm.c          | 24 ++++++++++-----------
 src/lib/lttng-ust/lttng-ust-statedump.c     |  2 +-
 12 files changed, 46 insertions(+), 32 deletions(-)

diff --git a/src/lib/lttng-ust/lttng-context-cgroup-ns.c b/src/lib/lttng-ust/lttng-context-cgroup-ns.c
index 34523ea1..3eb5ab5b 100644
--- a/src/lib/lttng-ust/lttng-context-cgroup-ns.c
+++ b/src/lib/lttng-ust/lttng-context-cgroup-ns.c
@@ -155,7 +155,9 @@ error_find_context:
 /*
  * Force a read (imply TLS allocation for dlopen) of TLS variables.
  */
-void lttng_cgroup_ns_alloc_tls(void)
+void lttng_cgroup_ns_alloc_tls(bool init)
 {
 	asm volatile ("" : : "m" (URCU_TLS(cached_cgroup_ns)));
+	if (init)
+		(void)get_cgroup_ns();
 }
diff --git a/src/lib/lttng-ust/lttng-context-ipc-ns.c b/src/lib/lttng-ust/lttng-context-ipc-ns.c
index 63401e8d..b68b939f 100644
--- a/src/lib/lttng-ust/lttng-context-ipc-ns.c
+++ b/src/lib/lttng-ust/lttng-context-ipc-ns.c
@@ -153,7 +153,9 @@ error_find_context:
 /*
  * Force a read (imply TLS allocation for dlopen) of TLS variables.
  */
-void lttng_ipc_ns_alloc_tls(void)
+void lttng_ipc_ns_alloc_tls(bool init)
 {
 	asm volatile ("" : : "m" (URCU_TLS(cached_ipc_ns)));
+	if (init)
+		(void)get_ipc_ns();
 }
diff --git a/src/lib/lttng-ust/lttng-context-net-ns.c b/src/lib/lttng-ust/lttng-context-net-ns.c
index 960591c2..93e574ad 100644
--- a/src/lib/lttng-ust/lttng-context-net-ns.c
+++ b/src/lib/lttng-ust/lttng-context-net-ns.c
@@ -153,7 +153,9 @@ error_find_context:
 /*
  * Force a read (imply TLS allocation for dlopen) of TLS variables.
  */
-void lttng_net_ns_alloc_tls(void)
+void lttng_net_ns_alloc_tls(bool init)
 {
 	asm volatile ("" : : "m" (URCU_TLS(cached_net_ns)));
+	if (init)
+		(void)get_net_ns();
 }
diff --git a/src/lib/lttng-ust/lttng-context-procname.c b/src/lib/lttng-ust/lttng-context-procname.c
index b5bf77be..95f6fe42 100644
--- a/src/lib/lttng-ust/lttng-context-procname.c
+++ b/src/lib/lttng-ust/lttng-context-procname.c
@@ -122,7 +122,9 @@ error_find_context:
 /*
  * Force a read (imply TLS allocation for dlopen) of TLS variables.
  */
-void lttng_procname_alloc_tls(void)
+void lttng_procname_alloc_tls(bool init)
 {
 	asm volatile ("" : : "m" (URCU_TLS(cached_procname)[0]));
+	if (init)
+		(void)wrapper_getprocname();
 }
diff --git a/src/lib/lttng-ust/lttng-context-provider.c b/src/lib/lttng-ust/lttng-context-provider.c
index 4e7e429f..ba24aad7 100644
--- a/src/lib/lttng-ust/lttng-context-provider.c
+++ b/src/lib/lttng-ust/lttng-context-provider.c
@@ -67,7 +67,7 @@ struct lttng_ust_registered_context_provider *lttng_ust_context_provider_registe
 	size_t name_len = strlen(provider->name);
 	uint32_t hash;
 
-	lttng_ust_alloc_tls();
+	lttng_ust_alloc_tls(false);
 
 	/* Provider name starts with "$app.". */
 	if (strncmp("$app.", provider->name, strlen("$app.")) != 0)
@@ -101,7 +101,7 @@ end:
 
 void lttng_ust_context_provider_unregister(struct lttng_ust_registered_context_provider *reg_provider)
 {
-	lttng_ust_alloc_tls();
+	lttng_ust_alloc_tls(false);
 
 	if (ust_lock())
 		goto end;
diff --git a/src/lib/lttng-ust/lttng-context-time-ns.c b/src/lib/lttng-ust/lttng-context-time-ns.c
index ec32a1de..a957409b 100644
--- a/src/lib/lttng-ust/lttng-context-time-ns.c
+++ b/src/lib/lttng-ust/lttng-context-time-ns.c
@@ -152,7 +152,9 @@ error_find_context:
 /*
  * Force a read (imply TLS allocation for dlopen) of TLS variables.
  */
-void lttng_time_ns_alloc_tls(void)
+void lttng_time_ns_alloc_tls(bool init)
 {
 	asm volatile ("" : : "m" (URCU_TLS(cached_time_ns)));
+	if (init)
+		(void)get_time_ns();
 }
diff --git a/src/lib/lttng-ust/lttng-context-uts-ns.c b/src/lib/lttng-ust/lttng-context-uts-ns.c
index 06a978fd..0c802188 100644
--- a/src/lib/lttng-ust/lttng-context-uts-ns.c
+++ b/src/lib/lttng-ust/lttng-context-uts-ns.c
@@ -154,7 +154,9 @@ error_find_context:
 /*
  * Force a read (imply TLS allocation for dlopen) of TLS variables.
  */
-void lttng_uts_ns_alloc_tls(void)
+void lttng_uts_ns_alloc_tls(bool init)
 {
 	asm volatile ("" : : "m" (URCU_TLS(cached_uts_ns)));
+	if (init)
+		(void)get_uts_ns();
 }
diff --git a/src/lib/lttng-ust/lttng-context-vtid.c b/src/lib/lttng-ust/lttng-context-vtid.c
index 880e34b9..d3c9863e 100644
--- a/src/lib/lttng-ust/lttng-context-vtid.c
+++ b/src/lib/lttng-ust/lttng-context-vtid.c
@@ -112,7 +112,9 @@ error_find_context:
 /*
  * Force a read (imply TLS allocation for dlopen) of TLS variables.
  */
-void lttng_vtid_alloc_tls(void)
+void lttng_vtid_alloc_tls(bool init)
 {
 	asm volatile ("" : : "m" (URCU_TLS(cached_vtid)));
+	if (init)
+		(void)wrapper_getvtid();
 }
diff --git a/src/lib/lttng-ust/lttng-probes.c b/src/lib/lttng-ust/lttng-probes.c
index e8e08ff1..314cb7a0 100644
--- a/src/lib/lttng-ust/lttng-probes.c
+++ b/src/lib/lttng-ust/lttng-probes.c
@@ -231,7 +231,7 @@ struct lttng_ust_registered_probe *lttng_ust_probe_register(const struct lttng_u
 {
 	struct lttng_ust_registered_probe *reg_probe = NULL;
 
-	lttng_ust_alloc_tls();
+	lttng_ust_alloc_tls(false);
 
 	/*
 	 * If version mismatch, don't register, but don't trigger assert
@@ -269,7 +269,7 @@ end:
 
 void lttng_ust_probe_unregister(struct lttng_ust_registered_probe *reg_probe)
 {
-	lttng_ust_alloc_tls();
+	lttng_ust_alloc_tls(false);
 
 	if (!reg_probe)
 		return;
diff --git a/src/lib/lttng-ust/lttng-tracer-core.h b/src/lib/lttng-ust/lttng-tracer-core.h
index eadc43ed..dbb3bd9f 100644
--- a/src/lib/lttng-ust/lttng-tracer-core.h
+++ b/src/lib/lttng-ust/lttng-tracer-core.h
@@ -34,28 +34,28 @@ void ust_lock_nocheck(void)
 void ust_unlock(void)
 	__attribute__((visibility("hidden")));
 
-void lttng_ust_alloc_tls(void)
+void lttng_ust_alloc_tls(bool init)
 	__attribute__((visibility("hidden")));
 
-void lttng_vtid_alloc_tls(void)
+void lttng_vtid_alloc_tls(bool init)
 	__attribute__((visibility("hidden")));
 
-void lttng_procname_alloc_tls(void)
+void lttng_procname_alloc_tls(bool init)
 	__attribute__((visibility("hidden")));
 
-void lttng_cgroup_ns_alloc_tls(void)
+void lttng_cgroup_ns_alloc_tls(bool init)
 	__attribute__((visibility("hidden")));
 
-void lttng_ipc_ns_alloc_tls(void)
+void lttng_ipc_ns_alloc_tls(bool init)
 	__attribute__((visibility("hidden")));
 
-void lttng_net_ns_alloc_tls(void)
+void lttng_net_ns_alloc_tls(bool init)
 	__attribute__((visibility("hidden")));
 
-void lttng_time_ns_alloc_tls(void)
+void lttng_time_ns_alloc_tls(bool init)
 	__attribute__((visibility("hidden")));
 
-void lttng_uts_ns_alloc_tls(void)
+void lttng_uts_ns_alloc_tls(bool init)
 	__attribute__((visibility("hidden")));
 
 const char *lttng_ust_obj_get_name(int id)
diff --git a/src/lib/lttng-ust/lttng-ust-comm.c b/src/lib/lttng-ust/lttng-ust-comm.c
index 7f34efe7..ba0bd985 100644
--- a/src/lib/lttng-ust/lttng-ust-comm.c
+++ b/src/lib/lttng-ust/lttng-ust-comm.c
@@ -407,21 +407,21 @@ void lttng_lttng_ust_urcu_alloc_tls(void)
 	(void) lttng_ust_urcu_read_ongoing();
 }
 
-void lttng_ust_alloc_tls(void)
+void lttng_ust_alloc_tls(bool init)
 {
 	lttng_lttng_ust_urcu_alloc_tls();
 	lttng_ringbuffer_alloc_tls();
-	lttng_vtid_alloc_tls();
+	lttng_vtid_alloc_tls(init);
 	lttng_nest_count_alloc_tls();
-	lttng_procname_alloc_tls();
+	lttng_procname_alloc_tls(init);
 	lttng_ust_mutex_nest_alloc_tls();
 	lttng_ust_perf_counter_alloc_tls();
 	lttng_ust_common_alloc_tls();
-	lttng_cgroup_ns_alloc_tls();
-	lttng_ipc_ns_alloc_tls();
-	lttng_net_ns_alloc_tls();
-	lttng_time_ns_alloc_tls();
-	lttng_uts_ns_alloc_tls();
+	lttng_cgroup_ns_alloc_tls(init);
+	lttng_ipc_ns_alloc_tls(init);
+	lttng_net_ns_alloc_tls(init);
+	lttng_time_ns_alloc_tls(init);
+	lttng_uts_ns_alloc_tls(init);
 	lttng_ust_ring_buffer_client_discard_alloc_tls();
 	lttng_ust_ring_buffer_client_discard_rt_alloc_tls();
 	lttng_ust_ring_buffer_client_overwrite_alloc_tls();
@@ -446,7 +446,7 @@ void lttng_ust_init_thread(void)
 	 * ensure those are initialized before a signal handler nesting over
 	 * this thread attempts to use them.
 	 */
-	lttng_ust_alloc_tls();
+	lttng_ust_alloc_tls(true);
 }
 
 int lttng_get_notify_socket(void *owner)
@@ -1815,7 +1815,7 @@ void *ust_listener_thread(void *arg)
 	int sock, ret, prev_connect_failed = 0, has_waited = 0, fd;
 	long timeout;
 
-	lttng_ust_alloc_tls();
+	lttng_ust_alloc_tls(false);
 	/*
 	 * If available, add '-ust' to the end of this thread's
 	 * process name
@@ -2183,7 +2183,7 @@ void lttng_ust_ctor(void)
 	 * to be the dynamic linker mutex) and ust_lock, taken within
 	 * the ust lock.
 	 */
-	lttng_ust_alloc_tls();
+	lttng_ust_alloc_tls(false);
 
 	lttng_ust_loaded = 1;
 
@@ -2497,7 +2497,7 @@ void lttng_ust_before_fork(sigset_t *save_sigset)
 	int ret;
 
 	/* Allocate lttng-ust TLS. */
-	lttng_ust_alloc_tls();
+	lttng_ust_alloc_tls(false);
 
 	if (URCU_TLS(lttng_ust_nest_count))
 		return;
diff --git a/src/lib/lttng-ust/lttng-ust-statedump.c b/src/lib/lttng-ust/lttng-ust-statedump.c
index 309a98fa..ee180a0d 100644
--- a/src/lib/lttng-ust/lttng-ust-statedump.c
+++ b/src/lib/lttng-ust/lttng-ust-statedump.c
@@ -557,7 +557,7 @@ void lttng_ust_dl_update(void *ip)
 	 * Force the allocation of lttng-ust TLS variables when called from
 	 * dlopen/dlclose instrumentation.
 	 */
-	lttng_ust_alloc_tls();
+	lttng_ust_alloc_tls(false);
 
 	data.exec_found = 0;
 	data.first = true;
-- 
2.35.1

_______________________________________________
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] 6+ messages in thread
* [lttng-dev] [PATCH 2/2] lttng_ust_init_thread: call urcu_register_thread
  2022-07-18 21:59 [lttng-dev] [PATCH 1/2] lttng_ust_init_thread: initialise cached context values Norbert Lange via lttng-dev
@ 2022-07-18 21:59 ` Norbert Lange via lttng-dev
  2022-07-19 19:40   ` Mathieu Desnoyers via lttng-dev
  2022-07-19 19:39 ` [lttng-dev] [PATCH 1/2] lttng_ust_init_thread: initialise cached context values Mathieu Desnoyers via lttng-dev
  1 sibling, 1 reply; 6+ messages in thread
From: Norbert Lange via lttng-dev @ 2022-07-18 21:59 UTC (permalink / raw)
  To: lttng-dev

Eagerly register the thread, and avoid taking mutex during the
first tracepoint.

Signed-off-by: Norbert Lange <nolange79@gmail.com>
---
 src/lib/lttng-ust/lttng-ust-comm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/lib/lttng-ust/lttng-ust-comm.c b/src/lib/lttng-ust/lttng-ust-comm.c
index ba0bd985..3ff6b086 100644
--- a/src/lib/lttng-ust/lttng-ust-comm.c
+++ b/src/lib/lttng-ust/lttng-ust-comm.c
@@ -447,6 +447,8 @@ void lttng_ust_init_thread(void)
 	 * this thread attempts to use them.
 	 */
 	lttng_ust_alloc_tls(true);
+
+	lttng_ust_urcu_register_thread();
 }
 
 int lttng_get_notify_socket(void *owner)
-- 
2.35.1

_______________________________________________
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] 6+ messages in thread
* Re: [lttng-dev] [PATCH 1/2] lttng_ust_init_thread: initialise cached context values
  2022-07-18 21:59 [lttng-dev] [PATCH 1/2] lttng_ust_init_thread: initialise cached context values Norbert Lange via lttng-dev
  2022-07-18 21:59 ` [lttng-dev] [PATCH 2/2] lttng_ust_init_thread: call urcu_register_thread Norbert Lange via lttng-dev
@ 2022-07-19 19:39 ` Mathieu Desnoyers via lttng-dev
  2022-07-20 20:00   ` Norbert Lange via lttng-dev
  1 sibling, 1 reply; 6+ messages in thread
From: Mathieu Desnoyers via lttng-dev @ 2022-07-19 19:39 UTC (permalink / raw)
  To: Norbert Lange; +Cc: lttng-dev

----- On Jul 18, 2022, at 5:59 PM, Norbert Lange via lttng-dev lttng-dev@lists.lttng.org wrote:

> Modify all relevant *_alloc_tls functions so that they take an
> argument for 'init'. Setting this to 'true' will read
> the actual context value and store it into a thread local
> cache.
> 
> The function 'lttng_ust_init_thread' will use this to
> precalculate context values. Tracepoints can then avoid
> systemcalls.

Rather than integrating two unrelated things within "alloc_tls"
functions, I would prefer that we split things like this, e.g.:

keep lttng_cgroup_ns_alloc_tls() as is.

Introduce lttng_ust_cgroup_init_thread() which would
call (void)get_cgroup_ns().

Likewise for all other contexts.

Thoughts ?

Thanks,

Mathieu


> 
> Signed-off-by: Norbert Lange <nolange79@gmail.com>
> ---
> *   see
> https://lore.kernel.org/all/CADYdroN3=pAe66crtsVT9aHE4T+bT61-tvHFKCuwZYkHhuyFjw@mail.gmail.com/
> *   Will update docs after review, and do some more tests
>    during the week
> ---
> src/lib/lttng-ust/lttng-context-cgroup-ns.c |  4 +++-
> src/lib/lttng-ust/lttng-context-ipc-ns.c    |  4 +++-
> src/lib/lttng-ust/lttng-context-net-ns.c    |  4 +++-
> src/lib/lttng-ust/lttng-context-procname.c  |  4 +++-
> src/lib/lttng-ust/lttng-context-provider.c  |  4 ++--
> src/lib/lttng-ust/lttng-context-time-ns.c   |  4 +++-
> src/lib/lttng-ust/lttng-context-uts-ns.c    |  4 +++-
> src/lib/lttng-ust/lttng-context-vtid.c      |  4 +++-
> src/lib/lttng-ust/lttng-probes.c            |  4 ++--
> src/lib/lttng-ust/lttng-tracer-core.h       | 16 +++++++-------
> src/lib/lttng-ust/lttng-ust-comm.c          | 24 ++++++++++-----------
> src/lib/lttng-ust/lttng-ust-statedump.c     |  2 +-
> 12 files changed, 46 insertions(+), 32 deletions(-)
> 
> diff --git a/src/lib/lttng-ust/lttng-context-cgroup-ns.c
> b/src/lib/lttng-ust/lttng-context-cgroup-ns.c
> index 34523ea1..3eb5ab5b 100644
> --- a/src/lib/lttng-ust/lttng-context-cgroup-ns.c
> +++ b/src/lib/lttng-ust/lttng-context-cgroup-ns.c
> @@ -155,7 +155,9 @@ error_find_context:
> /*
>  * Force a read (imply TLS allocation for dlopen) of TLS variables.
>  */
> -void lttng_cgroup_ns_alloc_tls(void)
> +void lttng_cgroup_ns_alloc_tls(bool init)
> {
> 	asm volatile ("" : : "m" (URCU_TLS(cached_cgroup_ns)));
> +	if (init)
> +		(void)get_cgroup_ns();
> }
> diff --git a/src/lib/lttng-ust/lttng-context-ipc-ns.c
> b/src/lib/lttng-ust/lttng-context-ipc-ns.c
> index 63401e8d..b68b939f 100644
> --- a/src/lib/lttng-ust/lttng-context-ipc-ns.c
> +++ b/src/lib/lttng-ust/lttng-context-ipc-ns.c
> @@ -153,7 +153,9 @@ error_find_context:
> /*
>  * Force a read (imply TLS allocation for dlopen) of TLS variables.
>  */
> -void lttng_ipc_ns_alloc_tls(void)
> +void lttng_ipc_ns_alloc_tls(bool init)
> {
> 	asm volatile ("" : : "m" (URCU_TLS(cached_ipc_ns)));
> +	if (init)
> +		(void)get_ipc_ns();
> }
> diff --git a/src/lib/lttng-ust/lttng-context-net-ns.c
> b/src/lib/lttng-ust/lttng-context-net-ns.c
> index 960591c2..93e574ad 100644
> --- a/src/lib/lttng-ust/lttng-context-net-ns.c
> +++ b/src/lib/lttng-ust/lttng-context-net-ns.c
> @@ -153,7 +153,9 @@ error_find_context:
> /*
>  * Force a read (imply TLS allocation for dlopen) of TLS variables.
>  */
> -void lttng_net_ns_alloc_tls(void)
> +void lttng_net_ns_alloc_tls(bool init)
> {
> 	asm volatile ("" : : "m" (URCU_TLS(cached_net_ns)));
> +	if (init)
> +		(void)get_net_ns();
> }
> diff --git a/src/lib/lttng-ust/lttng-context-procname.c
> b/src/lib/lttng-ust/lttng-context-procname.c
> index b5bf77be..95f6fe42 100644
> --- a/src/lib/lttng-ust/lttng-context-procname.c
> +++ b/src/lib/lttng-ust/lttng-context-procname.c
> @@ -122,7 +122,9 @@ error_find_context:
> /*
>  * Force a read (imply TLS allocation for dlopen) of TLS variables.
>  */
> -void lttng_procname_alloc_tls(void)
> +void lttng_procname_alloc_tls(bool init)
> {
> 	asm volatile ("" : : "m" (URCU_TLS(cached_procname)[0]));
> +	if (init)
> +		(void)wrapper_getprocname();
> }
> diff --git a/src/lib/lttng-ust/lttng-context-provider.c
> b/src/lib/lttng-ust/lttng-context-provider.c
> index 4e7e429f..ba24aad7 100644
> --- a/src/lib/lttng-ust/lttng-context-provider.c
> +++ b/src/lib/lttng-ust/lttng-context-provider.c
> @@ -67,7 +67,7 @@ struct lttng_ust_registered_context_provider
> *lttng_ust_context_provider_registe
> 	size_t name_len = strlen(provider->name);
> 	uint32_t hash;
> 
> -	lttng_ust_alloc_tls();
> +	lttng_ust_alloc_tls(false);
> 
> 	/* Provider name starts with "$app.". */
> 	if (strncmp("$app.", provider->name, strlen("$app.")) != 0)
> @@ -101,7 +101,7 @@ end:
> 
> void lttng_ust_context_provider_unregister(struct
> lttng_ust_registered_context_provider *reg_provider)
> {
> -	lttng_ust_alloc_tls();
> +	lttng_ust_alloc_tls(false);
> 
> 	if (ust_lock())
> 		goto end;
> diff --git a/src/lib/lttng-ust/lttng-context-time-ns.c
> b/src/lib/lttng-ust/lttng-context-time-ns.c
> index ec32a1de..a957409b 100644
> --- a/src/lib/lttng-ust/lttng-context-time-ns.c
> +++ b/src/lib/lttng-ust/lttng-context-time-ns.c
> @@ -152,7 +152,9 @@ error_find_context:
> /*
>  * Force a read (imply TLS allocation for dlopen) of TLS variables.
>  */
> -void lttng_time_ns_alloc_tls(void)
> +void lttng_time_ns_alloc_tls(bool init)
> {
> 	asm volatile ("" : : "m" (URCU_TLS(cached_time_ns)));
> +	if (init)
> +		(void)get_time_ns();
> }
> diff --git a/src/lib/lttng-ust/lttng-context-uts-ns.c
> b/src/lib/lttng-ust/lttng-context-uts-ns.c
> index 06a978fd..0c802188 100644
> --- a/src/lib/lttng-ust/lttng-context-uts-ns.c
> +++ b/src/lib/lttng-ust/lttng-context-uts-ns.c
> @@ -154,7 +154,9 @@ error_find_context:
> /*
>  * Force a read (imply TLS allocation for dlopen) of TLS variables.
>  */
> -void lttng_uts_ns_alloc_tls(void)
> +void lttng_uts_ns_alloc_tls(bool init)
> {
> 	asm volatile ("" : : "m" (URCU_TLS(cached_uts_ns)));
> +	if (init)
> +		(void)get_uts_ns();
> }
> diff --git a/src/lib/lttng-ust/lttng-context-vtid.c
> b/src/lib/lttng-ust/lttng-context-vtid.c
> index 880e34b9..d3c9863e 100644
> --- a/src/lib/lttng-ust/lttng-context-vtid.c
> +++ b/src/lib/lttng-ust/lttng-context-vtid.c
> @@ -112,7 +112,9 @@ error_find_context:
> /*
>  * Force a read (imply TLS allocation for dlopen) of TLS variables.
>  */
> -void lttng_vtid_alloc_tls(void)
> +void lttng_vtid_alloc_tls(bool init)
> {
> 	asm volatile ("" : : "m" (URCU_TLS(cached_vtid)));
> +	if (init)
> +		(void)wrapper_getvtid();
> }
> diff --git a/src/lib/lttng-ust/lttng-probes.c b/src/lib/lttng-ust/lttng-probes.c
> index e8e08ff1..314cb7a0 100644
> --- a/src/lib/lttng-ust/lttng-probes.c
> +++ b/src/lib/lttng-ust/lttng-probes.c
> @@ -231,7 +231,7 @@ struct lttng_ust_registered_probe
> *lttng_ust_probe_register(const struct lttng_u
> {
> 	struct lttng_ust_registered_probe *reg_probe = NULL;
> 
> -	lttng_ust_alloc_tls();
> +	lttng_ust_alloc_tls(false);
> 
> 	/*
> 	 * If version mismatch, don't register, but don't trigger assert
> @@ -269,7 +269,7 @@ end:
> 
> void lttng_ust_probe_unregister(struct lttng_ust_registered_probe *reg_probe)
> {
> -	lttng_ust_alloc_tls();
> +	lttng_ust_alloc_tls(false);
> 
> 	if (!reg_probe)
> 		return;
> diff --git a/src/lib/lttng-ust/lttng-tracer-core.h
> b/src/lib/lttng-ust/lttng-tracer-core.h
> index eadc43ed..dbb3bd9f 100644
> --- a/src/lib/lttng-ust/lttng-tracer-core.h
> +++ b/src/lib/lttng-ust/lttng-tracer-core.h
> @@ -34,28 +34,28 @@ void ust_lock_nocheck(void)
> void ust_unlock(void)
> 	__attribute__((visibility("hidden")));
> 
> -void lttng_ust_alloc_tls(void)
> +void lttng_ust_alloc_tls(bool init)
> 	__attribute__((visibility("hidden")));
> 
> -void lttng_vtid_alloc_tls(void)
> +void lttng_vtid_alloc_tls(bool init)
> 	__attribute__((visibility("hidden")));
> 
> -void lttng_procname_alloc_tls(void)
> +void lttng_procname_alloc_tls(bool init)
> 	__attribute__((visibility("hidden")));
> 
> -void lttng_cgroup_ns_alloc_tls(void)
> +void lttng_cgroup_ns_alloc_tls(bool init)
> 	__attribute__((visibility("hidden")));
> 
> -void lttng_ipc_ns_alloc_tls(void)
> +void lttng_ipc_ns_alloc_tls(bool init)
> 	__attribute__((visibility("hidden")));
> 
> -void lttng_net_ns_alloc_tls(void)
> +void lttng_net_ns_alloc_tls(bool init)
> 	__attribute__((visibility("hidden")));
> 
> -void lttng_time_ns_alloc_tls(void)
> +void lttng_time_ns_alloc_tls(bool init)
> 	__attribute__((visibility("hidden")));
> 
> -void lttng_uts_ns_alloc_tls(void)
> +void lttng_uts_ns_alloc_tls(bool init)
> 	__attribute__((visibility("hidden")));
> 
> const char *lttng_ust_obj_get_name(int id)
> diff --git a/src/lib/lttng-ust/lttng-ust-comm.c
> b/src/lib/lttng-ust/lttng-ust-comm.c
> index 7f34efe7..ba0bd985 100644
> --- a/src/lib/lttng-ust/lttng-ust-comm.c
> +++ b/src/lib/lttng-ust/lttng-ust-comm.c
> @@ -407,21 +407,21 @@ void lttng_lttng_ust_urcu_alloc_tls(void)
> 	(void) lttng_ust_urcu_read_ongoing();
> }
> 
> -void lttng_ust_alloc_tls(void)
> +void lttng_ust_alloc_tls(bool init)
> {
> 	lttng_lttng_ust_urcu_alloc_tls();
> 	lttng_ringbuffer_alloc_tls();
> -	lttng_vtid_alloc_tls();
> +	lttng_vtid_alloc_tls(init);
> 	lttng_nest_count_alloc_tls();
> -	lttng_procname_alloc_tls();
> +	lttng_procname_alloc_tls(init);
> 	lttng_ust_mutex_nest_alloc_tls();
> 	lttng_ust_perf_counter_alloc_tls();
> 	lttng_ust_common_alloc_tls();
> -	lttng_cgroup_ns_alloc_tls();
> -	lttng_ipc_ns_alloc_tls();
> -	lttng_net_ns_alloc_tls();
> -	lttng_time_ns_alloc_tls();
> -	lttng_uts_ns_alloc_tls();
> +	lttng_cgroup_ns_alloc_tls(init);
> +	lttng_ipc_ns_alloc_tls(init);
> +	lttng_net_ns_alloc_tls(init);
> +	lttng_time_ns_alloc_tls(init);
> +	lttng_uts_ns_alloc_tls(init);
> 	lttng_ust_ring_buffer_client_discard_alloc_tls();
> 	lttng_ust_ring_buffer_client_discard_rt_alloc_tls();
> 	lttng_ust_ring_buffer_client_overwrite_alloc_tls();
> @@ -446,7 +446,7 @@ void lttng_ust_init_thread(void)
> 	 * ensure those are initialized before a signal handler nesting over
> 	 * this thread attempts to use them.
> 	 */
> -	lttng_ust_alloc_tls();
> +	lttng_ust_alloc_tls(true);
> }
> 
> int lttng_get_notify_socket(void *owner)
> @@ -1815,7 +1815,7 @@ void *ust_listener_thread(void *arg)
> 	int sock, ret, prev_connect_failed = 0, has_waited = 0, fd;
> 	long timeout;
> 
> -	lttng_ust_alloc_tls();
> +	lttng_ust_alloc_tls(false);
> 	/*
> 	 * If available, add '-ust' to the end of this thread's
> 	 * process name
> @@ -2183,7 +2183,7 @@ void lttng_ust_ctor(void)
> 	 * to be the dynamic linker mutex) and ust_lock, taken within
> 	 * the ust lock.
> 	 */
> -	lttng_ust_alloc_tls();
> +	lttng_ust_alloc_tls(false);
> 
> 	lttng_ust_loaded = 1;
> 
> @@ -2497,7 +2497,7 @@ void lttng_ust_before_fork(sigset_t *save_sigset)
> 	int ret;
> 
> 	/* Allocate lttng-ust TLS. */
> -	lttng_ust_alloc_tls();
> +	lttng_ust_alloc_tls(false);
> 
> 	if (URCU_TLS(lttng_ust_nest_count))
> 		return;
> diff --git a/src/lib/lttng-ust/lttng-ust-statedump.c
> b/src/lib/lttng-ust/lttng-ust-statedump.c
> index 309a98fa..ee180a0d 100644
> --- a/src/lib/lttng-ust/lttng-ust-statedump.c
> +++ b/src/lib/lttng-ust/lttng-ust-statedump.c
> @@ -557,7 +557,7 @@ void lttng_ust_dl_update(void *ip)
> 	 * Force the allocation of lttng-ust TLS variables when called from
> 	 * dlopen/dlclose instrumentation.
> 	 */
> -	lttng_ust_alloc_tls();
> +	lttng_ust_alloc_tls(false);
> 
> 	data.exec_found = 0;
> 	data.first = true;
> --
> 2.35.1
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

-- 
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] 6+ messages in thread
* Re: [lttng-dev] [PATCH 2/2] lttng_ust_init_thread: call urcu_register_thread
  2022-07-18 21:59 ` [lttng-dev] [PATCH 2/2] lttng_ust_init_thread: call urcu_register_thread Norbert Lange via lttng-dev
@ 2022-07-19 19:40   ` Mathieu Desnoyers via lttng-dev
  0 siblings, 0 replies; 6+ messages in thread
From: Mathieu Desnoyers via lttng-dev @ 2022-07-19 19:40 UTC (permalink / raw)
  To: Norbert Lange; +Cc: lttng-dev

----- On Jul 18, 2022, at 5:59 PM, Norbert Lange via lttng-dev lttng-dev@lists.lttng.org wrote:

> Eagerly register the thread, and avoid taking mutex during the
> first tracepoint.

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

Please resubmit with the next round with my acked-by tag.

Mathieu

> 
> Signed-off-by: Norbert Lange <nolange79@gmail.com>
> ---
> src/lib/lttng-ust/lttng-ust-comm.c | 2 ++
> 1 file changed, 2 insertions(+)
> 
> diff --git a/src/lib/lttng-ust/lttng-ust-comm.c
> b/src/lib/lttng-ust/lttng-ust-comm.c
> index ba0bd985..3ff6b086 100644
> --- a/src/lib/lttng-ust/lttng-ust-comm.c
> +++ b/src/lib/lttng-ust/lttng-ust-comm.c
> @@ -447,6 +447,8 @@ void lttng_ust_init_thread(void)
> 	 * this thread attempts to use them.
> 	 */
> 	lttng_ust_alloc_tls(true);
> +
> +	lttng_ust_urcu_register_thread();
> }
> 
> int lttng_get_notify_socket(void *owner)
> --
> 2.35.1
> 
> _______________________________________________
> lttng-dev mailing list
> lttng-dev@lists.lttng.org
> https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev

-- 
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] 6+ messages in thread
* Re: [lttng-dev] [PATCH 1/2] lttng_ust_init_thread: initialise cached context values
  2022-07-19 19:39 ` [lttng-dev] [PATCH 1/2] lttng_ust_init_thread: initialise cached context values Mathieu Desnoyers via lttng-dev
@ 2022-07-20 20:00   ` Norbert Lange via lttng-dev
  2022-07-20 20:48     ` Mathieu Desnoyers via lttng-dev
  0 siblings, 1 reply; 6+ messages in thread
From: Norbert Lange via lttng-dev @ 2022-07-20 20:00 UTC (permalink / raw)
  To: Mathieu Desnoyers; +Cc: lttng-dev

Am Di., 19. Juli 2022 um 21:39 Uhr schrieb Mathieu Desnoyers
<mathieu.desnoyers@efficios.com>:
>
> ----- On Jul 18, 2022, at 5:59 PM, Norbert Lange via lttng-dev lttng-dev@lists.lttng.org wrote:
>
> > Modify all relevant *_alloc_tls functions so that they take an
> > argument for 'init'. Setting this to 'true' will read
> > the actual context value and store it into a thread local
> > cache.
> >
> > The function 'lttng_ust_init_thread' will use this to
> > precalculate context values. Tracepoints can then avoid
> > systemcalls.
>
> Rather than integrating two unrelated things within "alloc_tls"
> functions, I would prefer that we split things like this, e.g.:
>
> keep lttng_cgroup_ns_alloc_tls() as is.
>
> Introduce lttng_ust_cgroup_init_thread() which would
> call (void)get_cgroup_ns().
>
> Likewise for all other contexts.
>
> Thoughts ?

is this just a matter of the function name? The "things" are
related as they both prepare the same thread local cache.

ie.
rename lttng_cgroup_ns_alloc_tls(bool init) to
lttng_ust_cgroup_init_thread(bool initcache)?

Don't care much either way,I  just usually try to limit symbols.

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

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [lttng-dev] [PATCH 1/2] lttng_ust_init_thread: initialise cached context values
  2022-07-20 20:00   ` Norbert Lange via lttng-dev
@ 2022-07-20 20:48     ` Mathieu Desnoyers via lttng-dev
  0 siblings, 0 replies; 6+ messages in thread
From: Mathieu Desnoyers via lttng-dev @ 2022-07-20 20:48 UTC (permalink / raw)
  To: Norbert Lange; +Cc: lttng-dev

----- On Jul 20, 2022, at 4:00 PM, Norbert Lange nolange79@gmail.com wrote:

> Am Di., 19. Juli 2022 um 21:39 Uhr schrieb Mathieu Desnoyers
> <mathieu.desnoyers@efficios.com>:
>>
>> ----- On Jul 18, 2022, at 5:59 PM, Norbert Lange via lttng-dev
>> lttng-dev@lists.lttng.org wrote:
>>
>> > Modify all relevant *_alloc_tls functions so that they take an
>> > argument for 'init'. Setting this to 'true' will read
>> > the actual context value and store it into a thread local
>> > cache.
>> >
>> > The function 'lttng_ust_init_thread' will use this to
>> > precalculate context values. Tracepoints can then avoid
>> > systemcalls.
>>
>> Rather than integrating two unrelated things within "alloc_tls"
>> functions, I would prefer that we split things like this, e.g.:
>>
>> keep lttng_cgroup_ns_alloc_tls() as is.
>>
>> Introduce lttng_ust_cgroup_init_thread() which would
>> call (void)get_cgroup_ns().
>>
>> Likewise for all other contexts.
>>
>> Thoughts ?
> 
> is this just a matter of the function name? The "things" are
> related as they both prepare the same thread local cache.
> 
> ie.
> rename lttng_cgroup_ns_alloc_tls(bool init) to
> lttng_ust_cgroup_init_thread(bool initcache)?
> 
> Don't care much either way,I  just usually try to limit symbols.

So AFAIU when only alloc_tls is needed, you would just change that
for lttng_ust_cgroup_init_thread(false), correct ?

I would then prefer that we introduce flags rather than rely on a
boolean to make it immediately clear what is done from the callsite,
e.g.:

enum lttng_ust_init_thread_flags {
  LTTNG_UST_INIT_THREAD_TLS = (1 << 0),
  LTTNG_UST_INIT_THREAD_CONTEXT_CACHE = (1 << 1),
}

lttng_ust_cgroup_init_thread(int init_thread_flags)

And then call either:

lttng_ust_cgroup_init_thread(LTTNG_UST_INIT_THREAD_TLS)
to initialize the tls.

lttng_ust_cgroup_init_thread(LTTNG_UST_INIT_THREAD_CONTEXT_CACHE)
to initialize the thread context cache.

or
lttng_ust_cgroup_init_thread(LTTNG_UST_INIT_THREAD_TLS | LTTNG_UST_INIT_THREAD_CONTEXT_CACHE)
to initialize both.

Thoughts ?

Thanks,

Mathieu

> 
> Regards,
> Norbert

-- 
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] 6+ messages in thread
end of thread, other threads:[~2022-07-20 20:48 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-18 21:59 [lttng-dev] [PATCH 1/2] lttng_ust_init_thread: initialise cached context values Norbert Lange via lttng-dev
2022-07-18 21:59 ` [lttng-dev] [PATCH 2/2] lttng_ust_init_thread: call urcu_register_thread Norbert Lange via lttng-dev
2022-07-19 19:40   ` Mathieu Desnoyers via lttng-dev
2022-07-19 19:39 ` [lttng-dev] [PATCH 1/2] lttng_ust_init_thread: initialise cached context values Mathieu Desnoyers via lttng-dev
2022-07-20 20:00   ` Norbert Lange via lttng-dev
2022-07-20 20:48     ` Mathieu Desnoyers via lttng-dev

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.