From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Date: Fri, 27 Nov 2020 21:03:55 +0000 (-0500)
Subject: Fix: notifier: use store-release/load-acquire for error counter
X-Git-Tag: v2.13.0-rc1~75
X-Git-Url: https://git.lttng.org./?a=commitdiff_plain;h=ab04d7b1e3708b0da0771d054e97e76d3e0f7182;p=lttng-modules.git

Fix: notifier: use store-release/load-acquire for error counter

The "record_error" operation is executed concurrently with setting the
error counter in the notifier group without locking, so we need to
explicitly provide existance guarantees.

The only visible transition is from NULL -> !NULL, because the only
situation reverting the error counter back to NULL is on destruction of
the notification group, after an RCU synchronisation guarantees that no
record_error can observe this pointer anymore.

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

diff --git a/src/lttng-abi.c b/src/lttng-abi.c
index a9beda75..83853436 100644
--- a/src/lttng-abi.c
+++ b/src/lttng-abi.c
@@ -2144,8 +2144,13 @@ long lttng_abi_event_notifier_group_create_error_counter(
 		goto counter_error;
 	}
 
-	event_notifier_group->error_counter = counter;
 	event_notifier_group->error_counter_len = counter_len;
+	/*
+	 * store-release to publish error counter matches load-acquire
+	 * in record_error. Ensures the counter is created and the
+	 * error_counter_len is set before they are used.
+	 */
+	smp_store_release(&event_notifier_group->error_counter, counter);
 
 	counter->file = counter_file;
 	counter->owner = event_notifier_group->file;
diff --git a/src/lttng-event-notifier-notification.c b/src/lttng-event-notifier-notification.c
index f681d9b7..77e72842 100644
--- a/src/lttng-event-notifier-notification.c
+++ b/src/lttng-event-notifier-notification.c
@@ -352,17 +352,23 @@ void record_error(struct lttng_event_notifier *event_notifier)
 {
 
 	struct lttng_event_notifier_group *event_notifier_group = event_notifier->group;
+	struct lttng_counter *error_counter;
 	size_t dimension_index[1];
 	int ret;
 
+	/*
+	 * smp_load_acquire paired with smp_store_release orders
+	 * creation of the error counter and setting error_counter_len
+	 * before the error_counter is used.
+	 */
+	error_counter = smp_load_acquire(&event_notifier_group->error_counter);
 	/* This group may not have an error counter attached to it. */
-	if (!event_notifier_group->error_counter)
+	if (!error_counter)
 		return;
 
 	dimension_index[0] = event_notifier->error_counter_index;
 
-	ret = event_notifier_group->error_counter->ops->counter_add(
-			event_notifier_group->error_counter->counter,
+	ret = error_counter->ops->counter_add(error_counter->counter,
 			dimension_index, 1);
 	if (ret)
 		WARN_ON_ONCE(1);
diff --git a/src/lttng-events.c b/src/lttng-events.c
index 3f9d0f9a..bcb817e5 100644
--- a/src/lttng-events.c
+++ b/src/lttng-events.c
@@ -412,6 +412,7 @@ void lttng_event_notifier_group_destroy(
 
 	if (event_notifier_group->error_counter) {
 		struct lttng_counter *error_counter = event_notifier_group->error_counter;
+
 		error_counter->ops->counter_destroy(error_counter->counter);
 		module_put(error_counter->transport->owner);
 		lttng_kvfree(error_counter);
@@ -1071,6 +1072,7 @@ struct lttng_event_notifier *_lttng_event_notifier_create(
 		void *filter, enum lttng_kernel_instrumentation itype)
 {
 	struct lttng_event_notifier *event_notifier;
+	struct lttng_counter *error_counter;
 	const char *event_name;
 	struct hlist_head *head;
 	int ret;
@@ -1234,9 +1236,12 @@ struct lttng_event_notifier *_lttng_event_notifier_create(
 
 	/*
 	 * Clear the error counter bucket. The sessiond keeps track of which
-	 * bucket is currently in use. We trust it.
+	 * bucket is currently in use. We trust it. The session lock
+	 * synchronizes against concurrent creation of the error
+	 * counter.
 	 */
-	if (event_notifier_group->error_counter) {
+	error_counter = event_notifier_group->error_counter;
+	if (error_counter) {
 		size_t dimension_index[1];
 
 		/*
@@ -1250,9 +1255,7 @@ struct lttng_event_notifier *_lttng_event_notifier_create(
 		}
 
 		dimension_index[0] = event_notifier->error_counter_index;
-		ret = event_notifier_group->error_counter->ops->counter_clear(
-				event_notifier_group->error_counter->counter,
-				dimension_index);
+		ret = error_counter->ops->counter_clear(error_counter->counter, dimension_index);
 		if (ret) {
 			printk(KERN_INFO "LTTng: event_notifier: Unable to clear error counter bucket %llu\n",
 				event_notifier->error_counter_index);