From ab04d7b1e3708b0da0771d054e97e76d3e0f7182 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Fri, 27 Nov 2020 16:03:55 -0500 Subject: [PATCH] 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 --- src/lttng-abi.c | 7 ++++++- src/lttng-event-notifier-notification.c | 12 +++++++++--- src/lttng-events.c | 13 ++++++++----- 3 files changed, 23 insertions(+), 9 deletions(-) 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); -- 2.34.1