(trafficserver) branch master updated: Change volatile to atomic in EThread.h. (#10929)

[wkaras]

This is an automated email from the ASF dual-hosted git repository.

wkaras pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new 1c9c902915 Change volatile to atomic in EThread.h. (#10929)
1c9c902915 is described below

commit 1c9c9029155d3d173289084bb91dfe6d73bcb273
Author: Walt Karas <wka...@yahooinc.com>
AuthorDate: Wed Dec 13 09:35:45 2023 -0500

    Change volatile to atomic in EThread.h. (#10929)
---
 include/iocore/eventsystem/EThread.h  |  6 ++++--
 src/iocore/eventsystem/UnixEThread.cc | 21 ++++++++++++---------
 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/include/iocore/eventsystem/EThread.h 
b/include/iocore/eventsystem/EThread.h
index 8f7e069773..0bb0d3f17c 100644
--- a/include/iocore/eventsystem/EThread.h
+++ b/include/iocore/eventsystem/EThread.h
@@ -24,6 +24,8 @@
 
 #pragma once
 
+#include <atomic>
+
 #include "tscore/ink_platform.h"
 #include "tscore/ink_rand.h"
 #include "tscore/Version.h"
@@ -479,7 +481,7 @@ public:
     /// The slices.
     std::array<Slice, N_SLICES> _slice;
 
-    Slice *volatile current_slice = nullptr; ///< The current slice.
+    std::atomic<Slice *> current_slice{nullptr}; ///< The current slice.
 
     /** The number of time scales used in the event statistics.
         Currently these are 10s, 100s, 1000s.
@@ -599,7 +601,7 @@ inline auto
 EThread::Metrics::record_loop_time(ink_hrtime delta) -> self_type &
 {
   static auto constexpr DIVISOR = 
std::chrono::duration_cast<ts_nanoseconds>(LOOP_HISTOGRAM_BUCKET_SIZE).count();
-  current_slice->record_loop_duration(delta);
+  current_slice.load(std::memory_order_acquire)->record_loop_duration(delta);
   _loop_timing(delta / DIVISOR);
   return *this;
 }
diff --git a/src/iocore/eventsystem/UnixEThread.cc 
b/src/iocore/eventsystem/UnixEThread.cc
index 492b442cfe..97954b50b8 100644
--- a/src/iocore/eventsystem/UnixEThread.cc
+++ b/src/iocore/eventsystem/UnixEThread.cc
@@ -221,22 +221,24 @@ EThread::execute_regular()
   // A statically initialized instance we can use as a prototype for 
initializing other instances.
   static const Metrics::Slice SLICE_INIT;
 
+  Metrics::Slice *current_slice{nullptr};
+
   // give priority to immediate events
   while (!TSSystemState::is_event_system_shut_down()) {
     loop_start_time = ink_get_hrtime();
     nq_count        = 0; // count # of elements put on negative queue.
     ev_count        = 0; // # of events handled.
 
-    metrics.current_slice = metrics._slice.data() + (loop_start_time / 
HRTIME_SECOND) % Metrics::N_SLICES;
-    if (metrics.current_slice != prev_slice) {
+    current_slice = metrics._slice.data() + (loop_start_time / HRTIME_SECOND) 
% Metrics::N_SLICES;
+    metrics.current_slice.store(current_slice, std::memory_order_release);
+    if (current_slice != prev_slice) {
       // I have observed multi-second event loops in production, making this 
necessary. [amc]
       do {
-        // Need @c const_cast to cast away @c volatile
         memcpy(prev_slice = this->metrics.next_slice(prev_slice), &SLICE_INIT, 
sizeof(SLICE_INIT));
-      } while (metrics.current_slice != prev_slice);
-      metrics.current_slice->record_loop_start(loop_start_time);
+      } while (current_slice != prev_slice);
+      current_slice->record_loop_start(loop_start_time);
     }
-    ++(metrics.current_slice->_count); // loop started, bump count.
+    ++(current_slice->_count); // loop started, bump count.
 
     process_queue(&NegativeQueue, &ev_count, &nq_count);
 
@@ -277,7 +279,7 @@ EThread::execute_regular()
         // Therefore, we have to set the limitation of sleep time in order to 
handle the next retry in time.
         sleep_time = std::min(sleep_time, DELAY_FOR_RETRY);
       }
-      ++(metrics.current_slice->_wait);
+      ++(current_slice->_wait);
     } else {
       sleep_time = 0;
     }
@@ -293,7 +295,7 @@ EThread::execute_regular()
 
     metrics.decay();
     metrics.record_loop_time(delta);
-    metrics.current_slice->record_event_count(ev_count);
+    current_slice->record_event_count(ev_count);
   }
 }
 
@@ -367,7 +369,8 @@ EThread::Metrics::summarize(Metrics &global)
 
   // To avoid race conditions, we back up one from the current metric block. 
It's close enough
   // and won't be updated during the time this method runs so it should be 
thread safe.
-  Slice *slice = this->prev_slice(current_slice);
+  // The acquire fence ensures we see all the preceeding updates to the 
previous slice.
+  Slice *slice = 
this->prev_slice(current_slice.load(std::memory_order_acquire));
 
   for (unsigned t = 0; t < N_TIMESCALES; ++t) {
     int count = SLICE_SAMPLE_COUNT[t];