From 33f43caa08bc332900bc8671a156912a09e88aa2 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Wed, 9 Dec 2020 09:30:23 -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. Use the stronger "cmm_smp_mb()" to provide store-release/load-acquire semantics because the explicit store-release and load-acquire are not implemented in the CMM model yet. Signed-off-by: Mathieu Desnoyers Change-Id: I3bbeecc00ff33c5d8ad1e3fd41c8a0dc866f1943 --- liblttng-ust/event-notifier-notification.c | 15 ++++++++++++--- liblttng-ust/lttng-ust-abi.c | 10 +++++++++- 2 files changed, 21 insertions(+), 4 deletions(-) diff --git a/liblttng-ust/event-notifier-notification.c b/liblttng-ust/event-notifier-notification.c index 5f7d9d58..ed2c8d28 100644 --- a/liblttng-ust/event-notifier-notification.c +++ b/liblttng-ust/event-notifier-notification.c @@ -284,17 +284,26 @@ static 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; + error_counter = CMM_LOAD_SHARED(event_notifier_group->error_counter); + /* + * load-acquire paired with store-release orders creation of the + * error counter and setting error_counter_len before the + * error_counter is used. + * Currently a full memory barrier is used, which could be + * turned into acquire-release barriers. + */ + cmm_smp_mb(); /* 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, - dimension_index, 1); + error_counter->counter, dimension_index, 1); if (ret) WARN_ON_ONCE(1); } diff --git a/liblttng-ust/lttng-ust-abi.c b/liblttng-ust/lttng-ust-abi.c index a024b616..aad64a6d 100644 --- a/liblttng-ust/lttng-ust-abi.c +++ b/liblttng-ust/lttng-ust-abi.c @@ -846,8 +846,16 @@ int lttng_ust_event_notifier_group_create_error_counter(int event_notifier_group goto create_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. + * Currently a full memory barrier is used, which could be + * turned into acquire-release barriers. + */ + cmm_smp_mb(); + CMM_STORE_SHARED(event_notifier_group->error_counter, counter); counter->objd = counter_objd; counter->event_notifier_group = event_notifier_group; /* owner */ -- 2.34.1