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 a9beda756d7bbee4b9127eaec6c9ebb65353045f..838534368d0c018b9ac8cce9940a2a8ad77296fe 100644 (file)
--- 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 f681d9b79d24486ca03fa911967f6cc5fe7e3200..77e72842d410782e89aaa21bdb9f1e835e0ad936 100644 (file)
--- 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 3f9d0f9a91fdf7b50b7a647d1aea55413d8412b0..bcb817e55cf721490c4e6f4656b8f7626e9a9ebb 100644 (file)
--- 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);