From: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Date: Fri, 3 Jun 2022 15:32:44 +0000 (-0400)
Subject: Custom upgrade: allow N lttng-ust instances per process
X-Git-Url: https://git.efficios.com/?a=commitdiff_plain;h=0b6b8118623769187e5c8363672460ce39397316;p=deliverable%2Flttng-tools.git

Custom upgrade: allow N lttng-ust instances per process

The objective of this path is to allow multiple versions of lttng-ust for
a given process to register themselves.

To achieve this, we relax the constraint of the ust_app_ht key and allow
duplicate for a given pid.

This hash table is mostly used by the tracker subsystem and only need to
provide a way to check for the existence of an ust_app for a given pid.

Most of the code relies on the `by socket` hash table to lookup ust_app or
it simply iterate over the hash table.

The registration code path also need some modification since it relies
on the pid to match the incoming notify socket to a ust_app pending
registration. For now, we can use the version from the registration data
to find the proper ust_app. This is valid since that, for now, only
lttng-ust instances of different version are allowed to co-exist in a
given process. This will not hold if we ever change this in the future.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Change-Id: I5d8c0211593fed599b121e57d29b62f1e70a4a2f
---

diff --git a/src/bin/lttng-sessiond/dispatch.c b/src/bin/lttng-sessiond/dispatch.c
index e764af7f2..f38617a4d 100644
--- a/src/bin/lttng-sessiond/dispatch.c
+++ b/src/bin/lttng-sessiond/dispatch.c
@@ -343,14 +343,36 @@ static void *thread_dispatch_ust_registration(void *data)
 				cds_list_for_each_entry_safe(wait_node, tmp_wait_node,
 						&wait_queue.head, head) {
 					health_code_update();
-					if (wait_node->app->pid == ust_cmd->reg_msg.pid) {
+					/*
+					 * Only using the pid number to match
+					 * notify socket is not enough when
+					 * dealing with multiple instances of
+					 * lttng-ust for a given process. A
+					 * similar problem will rise up when
+					 * dealing with multiple similar pid
+					 * across namespaces in the futur.
+					 */
+					if (wait_node->app->pid ==
+							ust_cmd->reg_msg.pid) {
+						if ((wait_node->app->v_major !=
+								    ust_cmd->reg_msg.major) &&
+								(wait_node->app->v_minor !=
+										ust_cmd->reg_msg.minor)) {
+							DBG("Skipping notify socket assignment (pid: %d) based on version mismatch. Notify socket registration version %d.%d, under iteration app version %d.%d.",
+									wait_node->app->pid,
+									ust_cmd->reg_msg.major,
+									ust_cmd->reg_msg.minor,
+									wait_node->app->v_major,
+									wait_node->app->v_minor);
+							continue;
+						}
 						wait_node->app->notify_sock = ust_cmd->sock;
 						cds_list_del(&wait_node->head);
 						wait_queue.count--;
 						app = wait_node->app;
 						free(wait_node);
 						wait_node = NULL;
-						DBG3("UST app notify socket %d is set", ust_cmd->sock);
+						DBG3("UST app notify socket %d is set for app sock %d", ust_cmd->sock, app->sock);
 						break;
 					}
 				}
diff --git a/src/bin/lttng-sessiond/ust-app.c b/src/bin/lttng-sessiond/ust-app.c
index e5d6857a3..254e12cec 100644
--- a/src/bin/lttng-sessiond/ust-app.c
+++ b/src/bin/lttng-sessiond/ust-app.c
@@ -4051,10 +4051,13 @@ void ust_app_add(struct ust_app *app)
 	rcu_read_lock();
 
 	/*
-	 * On a re-registration, we want to kick out the previous registration of
-	 * that pid
+	 * Accept duplicate pid to accommodate the possibility of multiple
+	 * lttng-ust per process. Both lttng-ust instance will register
+	 * themselves and be unique in term of socket.
+	 * All operations on pid should expects that multiple "app" be present
+	 * with the same pid.
 	 */
-	lttng_ht_add_replace_ulong(ust_app_ht, &app->pid_n);
+	lttng_ht_add_ulong(ust_app_ht, &app->pid_n);
 
 	/*
 	 * The socket _should_ be unique until _we_ call close. So, a add_unique
@@ -4328,17 +4331,9 @@ void ust_app_unregister(int sock)
 	iter.iter.node = &lta->notify_sock_n.node;
 	(void) lttng_ht_del(ust_app_ht_by_notify_sock, &iter);
 
-	/*
-	 * Ignore return value since the node might have been removed before by an
-	 * add replace during app registration because the PID can be reassigned by
-	 * the OS.
-	 */
 	iter.iter.node = &lta->pid_n.node;
 	ret = lttng_ht_del(ust_app_ht, &iter);
-	if (ret) {
-		DBG3("Unregister app by PID %d failed. This can happen on pid reuse",
-				lta->pid);
-	}
+	assert(!ret);
 
 	/* Free memory */
 	call_rcu(&lta->pid_n.head, delete_ust_app_rcu);