Use std::atomic in the signalling interfaces
Test: vsoc_stress_test
Bug: 70635412
Change-Id: I5f4487bafa6e8058d52778ace1a749607c7722f6
diff --git a/common/vsoc/lib/lock_common.cpp b/common/vsoc/lib/lock_common.cpp
index ed7962e..2744ba3 100644
--- a/common/vsoc/lib/lock_common.cpp
+++ b/common/vsoc/lib/lock_common.cpp
@@ -116,20 +116,18 @@
}
void layout::GuestAndHostLock::Lock(RegionView* region) {
- uint32_t* uaddr = reinterpret_cast<uint32_t*>(&lock_uint32_);
uint32_t expected;
uint32_t tid = gettid();
while (1) {
if (TryLock(tid, &expected)) {
return;
}
- region->WaitForSignal(uaddr, expected);
+ region->WaitForSignal(&lock_uint32_, expected);
}
}
void layout::GuestAndHostLock::Unlock(RegionView* region) {
- uint32_t* uaddr = reinterpret_cast<uint32_t*>(&lock_uint32_);
- region->SendSignal(UnlockCommon(gettid()), uaddr);
+ region->SendSignal(UnlockCommon(gettid()), &lock_uint32_);
}
} // namespace vsoc
diff --git a/common/vsoc/lib/mock_region_view.h b/common/vsoc/lib/mock_region_view.h
index 29ccad9..b6bcc45 100644
--- a/common/vsoc/lib/mock_region_view.h
+++ b/common/vsoc/lib/mock_region_view.h
@@ -54,12 +54,13 @@
};
virtual void SendSignal(vsoc::layout::Sides /* sides_to_signal */,
- uint32_t* uaddr) {
+ std::atomic<uint32_t>* uaddr) {
syscall(SYS_futex, reinterpret_cast<int32_t*>(uaddr), FUTEX_WAKE, -1,
nullptr, nullptr, 0);
}
- virtual void WaitForSignal(uint32_t* uaddr, uint32_t expected_value) {
+ virtual void WaitForSignal(std::atomic<uint32_t>* uaddr,
+ uint32_t expected_value) {
{
std::lock_guard<std::mutex> guard(mutex_);
if (tid_to_addr_.find(std::this_thread::get_id()) != tid_to_addr_.end()) {
@@ -107,7 +108,7 @@
void* region_base_{};
std::mutex mutex_;
std::condition_variable map_changed_;
- std::unordered_map<std::thread::id, uint32_t*> tid_to_addr_;
+ std::unordered_map<std::thread::id, std::atomic<uint32_t>*> tid_to_addr_;
};
template <typename Layout>
diff --git a/common/vsoc/lib/region_signaling_interface.h b/common/vsoc/lib/region_signaling_interface.h
index b997387..d5bfbea 100644
--- a/common/vsoc/lib/region_signaling_interface.h
+++ b/common/vsoc/lib/region_signaling_interface.h
@@ -36,7 +36,7 @@
//
// signal_addr: the memory location to signal. Must be within the region.
virtual void SendSignal(layout::Sides sides_to_signal,
- uint32_t* signal_addr) = 0;
+ std::atomic<uint32_t>* signal_addr) = 0;
// This implements the following:
// if (*signal_addr == last_observed_value)
@@ -49,7 +49,7 @@
// signal_addr: the memory that will be signaled. Must be within the region.
//
// last_observed_value: the value that motivated the calling code to wait.
- virtual void WaitForSignal(uint32_t* signal_addr,
+ virtual void WaitForSignal(std::atomic<uint32_t>* signal_addr,
uint32_t last_observed_value) = 0;
};
diff --git a/common/vsoc/lib/region_view.cpp b/common/vsoc/lib/region_view.cpp
index 5c53b12..4f222f1 100644
--- a/common/vsoc/lib/region_view.cpp
+++ b/common/vsoc/lib/region_view.cpp
@@ -22,7 +22,7 @@
if (stopping_) {
return;
}
- region_->ProcessSignalsFromPeer([](uint32_t* uaddr) {
+ region_->ProcessSignalsFromPeer([](std::atomic<uint32_t>* uaddr) {
syscall(SYS_futex, uaddr, FUTEX_WAKE, -1, nullptr, nullptr, 0);
});
}
@@ -83,7 +83,7 @@
}
void vsoc::RegionView::ProcessSignalsFromPeer(
- std::function<void(uint32_t*)> signal_handler) {
+ std::function<void(std::atomic<uint32_t>*)> signal_handler) {
const vsoc_signal_table_layout& table = incoming_signal_table();
const size_t num_offsets = (1 << table.num_nodes_lg2);
std::atomic<uint32_t>* offsets =
@@ -93,8 +93,9 @@
uint32_t offset = offsets[i].exchange(0);
if (offset) {
bool round_trip = offset & UADDR_OFFSET_ROUND_TRIP_FLAG;
- uint32_t* uaddr =
- region_offset_to_pointer<uint32_t>(offset & UADDR_OFFSET_MASK);
+ std::atomic<uint32_t>* uaddr =
+ region_offset_to_pointer<std::atomic<uint32_t>>(offset &
+ UADDR_OFFSET_MASK);
signal_handler(uaddr);
if (round_trip) {
SendSignalToPeer(uaddr, false);
@@ -103,12 +104,13 @@
}
}
-void vsoc::RegionView::SendSignal(Sides sides_to_signal, uint32_t* uaddr) {
+void vsoc::RegionView::SendSignal(Sides sides_to_signal,
+ std::atomic<uint32_t>* uaddr) {
if (sides_to_signal.value_ & Sides::Peer) {
// If we should also be signalling our side set the round trip flag on
// the futex signal.
bool round_trip = sides_to_signal.value_ & Sides::OurSide;
- SendSignalToPeer(reinterpret_cast<uint32_t*>(uaddr), round_trip);
+ SendSignalToPeer(uaddr, round_trip);
// Return without signaling our waiters to give the other side a chance
// to run.
return;
@@ -119,7 +121,8 @@
}
}
-void vsoc::RegionView::SendSignalToPeer(uint32_t* uaddr, bool round_trip) {
+void vsoc::RegionView::SendSignalToPeer(std::atomic<uint32_t>* uaddr,
+ bool round_trip) {
const vsoc_signal_table_layout& table = outgoing_signal_table();
std::atomic<uint32_t>* offsets =
region_offset_to_pointer<std::atomic<uint32_t>>(
@@ -180,6 +183,7 @@
return std::unique_ptr<vsoc::RegionWorker>(new vsoc::RegionWorker(this));
}
-void vsoc::RegionView::WaitForSignal(uint32_t* uaddr, uint32_t expected_value) {
+void vsoc::RegionView::WaitForSignal(std::atomic<uint32_t>* uaddr,
+ uint32_t expected_value) {
syscall(SYS_futex, uaddr, FUTEX_WAIT, expected_value, nullptr, nullptr, 0);
}
diff --git a/common/vsoc/lib/region_view.h b/common/vsoc/lib/region_view.h
index 0c366ee..0f82ace 100644
--- a/common/vsoc/lib/region_view.h
+++ b/common/vsoc/lib/region_view.h
@@ -100,7 +100,8 @@
// signal_handler: An action to perform on every offset signalled by our
// peer, usually a FUTEX_WAKE call, but can be customized for other
// purposes.
- void ProcessSignalsFromPeer(std::function<void(uint32_t*)> signal_handler);
+ void ProcessSignalsFromPeer(
+ std::function<void(std::atomic<uint32_t>*)> signal_handler);
// Post a signal to the guest, the host, or both.
// See futex(2) FUTEX_WAKE for details.
@@ -108,7 +109,8 @@
// sides_to_signal: controls where the signal is sent
//
// signal_addr: the memory location to signal. Must be within the region.
- void SendSignal(layout::Sides sides_to_signal, uint32_t* signal_addr);
+ void SendSignal(layout::Sides sides_to_signal,
+ std::atomic<uint32_t>* signal_addr);
// Post a signal to our peer for a specific memeory location.
// See futex(2) FUTEX_WAKE for details.
@@ -117,7 +119,7 @@
//
// round_trip: true if there may be waiters on both sides of the shared
// memory.
- void SendSignalToPeer(uint32_t* signal_addr, bool round_trip);
+ void SendSignalToPeer(std::atomic<uint32_t>* signal_addr, bool round_trip);
// Waits until an interrupt appears on this region, then clears the
// interrupted flag and returns.
@@ -134,7 +136,8 @@
// signal_addr: the memory that will be signaled. Must be within the region.
//
// last_observed_value: the value that motivated the calling code to wait.
- void WaitForSignal(uint32_t* signal_addr, uint32_t last_observed_value);
+ void WaitForSignal(std::atomic<uint32_t>* signal_addr,
+ uint32_t last_observed_value);
// Starts the signal table scanner. This must be invoked by subclasses, which
// must store the returned unique_ptr as a class member.
diff --git a/common/vsoc/shm/circqueue.h b/common/vsoc/shm/circqueue.h
index 8c5b966..c8ee3af 100644
--- a/common/vsoc/shm/circqueue.h
+++ b/common/vsoc/shm/circqueue.h
@@ -87,9 +87,9 @@
// they should be interpreted modulo the buffer size. This fact along with the
// buffer size being a power of two greatly simplyfies the index calculations.
// Advances when a reader has finished with buffer space
- uint32_t r_released_;
+ std::atomic<uint32_t> r_released_;
// Advances when buffer space is filled and ready for a reader
- uint32_t w_pub_;
+ std::atomic<uint32_t> w_pub_;
// Spinlock that protects the region. 0 means unlocked
SpinLock lock_;
// The actual memory in the buffer
diff --git a/common/vsoc/shm/e2e_test_region_layout.h b/common/vsoc/shm/e2e_test_region_layout.h
index f0a24e9..7227726 100644
--- a/common/vsoc/shm/e2e_test_region_layout.h
+++ b/common/vsoc/shm/e2e_test_region_layout.h
@@ -15,6 +15,7 @@
* limitations under the License.
*/
+#include <atomic>
#include <cstdint>
#include "common/vsoc/shm/base.h"
#include "common/vsoc/shm/version.h"
@@ -117,8 +118,8 @@
// Later guest tests will wait on this
E2ETestStageRegister host_status;
// These fields are used to test the signaling mechanism.
- uint32_t host_to_guest_signal;
- uint32_t guest_to_host_signal;
+ std::atomic<uint32_t> host_to_guest_signal;
+ std::atomic<uint32_t> guest_to_host_signal;
// There rest of the region will be filled by guest_host_strings.
// We actually use more than one of these, but we can't know how many
// until we examine the region.
diff --git a/common/vsoc/shm/fb_bcast_layout.h b/common/vsoc/shm/fb_bcast_layout.h
index 67c229b..21985c0 100644
--- a/common/vsoc/shm/fb_bcast_layout.h
+++ b/common/vsoc/shm/fb_bcast_layout.h
@@ -35,7 +35,7 @@
uint16_t refresh_rate_hz;
// The frame sequential number
- uint32_t seq_num;
+ std::atomic<uint32_t> seq_num;
// The offset in the gralloc buffer region of the current frame buffer.
uint32_t frame_offset;
// Protects access to the frame offset and sequential number.
diff --git a/guest/vsoc/lib/guest_region_e2e_test.cpp b/guest/vsoc/lib/guest_region_e2e_test.cpp
index b91c1c1..62d98f6 100644
--- a/guest/vsoc/lib/guest_region_e2e_test.cpp
+++ b/guest/vsoc/lib/guest_region_e2e_test.cpp
@@ -118,20 +118,22 @@
LOG(INFO) << "Signal sent. Waiting for first signal from peer";
primary.WaitForInterrupt();
int count = 0; // counts the number of signals received.
- primary.ProcessSignalsFromPeer([&primary, &count](uint32_t* uaddr) {
- ++count;
- EXPECT_TRUE(uaddr == &primary.data()->host_to_guest_signal);
- });
+ primary.ProcessSignalsFromPeer(
+ [&primary, &count](std::atomic<uint32_t>* uaddr) {
+ ++count;
+ EXPECT_TRUE(uaddr == &primary.data()->host_to_guest_signal);
+ });
EXPECT_TRUE(count == 1);
LOG(INFO) << "Signal received on primary region";
secondary.SendSignal(side, &secondary.data()->guest_to_host_signal);
LOG(INFO) << "Signal sent. Waiting for second signal from peer";
secondary.WaitForInterrupt();
count = 0;
- secondary.ProcessSignalsFromPeer([&secondary, &count](uint32_t* uaddr) {
- ++count;
- EXPECT_TRUE(uaddr == &secondary.data()->host_to_guest_signal);
- });
+ secondary.ProcessSignalsFromPeer(
+ [&secondary, &count](std::atomic<uint32_t>* uaddr) {
+ ++count;
+ EXPECT_TRUE(uaddr == &secondary.data()->host_to_guest_signal);
+ });
EXPECT_TRUE(count == 1);
LOG(INFO) << "Signal received on secondary region";
diff --git a/host/vsoc/lib/host_region_e2e_test.cpp b/host/vsoc/lib/host_region_e2e_test.cpp
index dc87f46..a707838 100644
--- a/host/vsoc/lib/host_region_e2e_test.cpp
+++ b/host/vsoc/lib/host_region_e2e_test.cpp
@@ -87,20 +87,22 @@
LOG(INFO) << "Signal sent. Waiting for first signal from peer";
primary.WaitForInterrupt();
int count = 0; // counts the number of signals received.
- primary.ProcessSignalsFromPeer([&primary, &count](uint32_t* uaddr) {
- ++count;
- EXPECT_TRUE(uaddr == &primary.data()->guest_to_host_signal);
- });
+ primary.ProcessSignalsFromPeer(
+ [&primary, &count](std::atomic<uint32_t>* uaddr) {
+ ++count;
+ EXPECT_TRUE(uaddr == &primary.data()->guest_to_host_signal);
+ });
EXPECT_TRUE(count == 1);
LOG(INFO) << "Signal received on primary region";
secondary.SendSignal(side, &secondary.data()->host_to_guest_signal);
LOG(INFO) << "Signal sent. Waiting for second signal from peer";
secondary.WaitForInterrupt();
count = 0;
- secondary.ProcessSignalsFromPeer([&secondary, &count](uint32_t* uaddr) {
- ++count;
- EXPECT_TRUE(uaddr == &secondary.data()->guest_to_host_signal);
- });
+ secondary.ProcessSignalsFromPeer(
+ [&secondary, &count](std::atomic<uint32_t>* uaddr) {
+ ++count;
+ EXPECT_TRUE(uaddr == &secondary.data()->guest_to_host_signal);
+ });
EXPECT_TRUE(count == 1);
LOG(INFO) << "Signal received on secondary region";
