Load with memory_order_acquire
std::atomic::load operation is with std::memory_order_seq_cst by
default. However, load operation in bufferhubd and buffer hub client
does not need sequentially-consistent ordering which is provided by
std::memory_order_seq_cst. This change changes our load operation with
std::memory_order_acquire so that no reads or writes in the current
thread can be reordered before this load, all writes in other threads
that release the same atomic variable are visible in the current thread.
Test: all tests are still passing.
Test: vega still working.
Bug: 112007999
Bug: 118718713
Change-Id: I2ac75cc306c3de35bf3d953b353f9a9442bdebbc
diff --git a/libs/vr/libbufferhub/buffer_hub_base.cpp b/libs/vr/libbufferhub/buffer_hub_base.cpp
index b4c5d42..68cc766 100644
--- a/libs/vr/libbufferhub/buffer_hub_base.cpp
+++ b/libs/vr/libbufferhub/buffer_hub_base.cpp
@@ -123,17 +123,17 @@
buffer_state_ = &metadata_header_->buffer_state;
ALOGD_IF(TRACE,
"BufferHubBase::ImportBuffer: id=%d, buffer_state=%" PRIx64 ".",
- id(), buffer_state_->load());
+ id(), buffer_state_->load(std::memory_order_acquire));
fence_state_ = &metadata_header_->fence_state;
ALOGD_IF(TRACE,
"BufferHubBase::ImportBuffer: id=%d, fence_state=%" PRIx64 ".", id(),
- fence_state_->load());
+ fence_state_->load(std::memory_order_acquire));
active_clients_bit_mask_ = &metadata_header_->active_clients_bit_mask;
ALOGD_IF(
TRACE,
"BufferHubBase::ImportBuffer: id=%d, active_clients_bit_mask=%" PRIx64
".",
- id(), active_clients_bit_mask_->load());
+ id(), active_clients_bit_mask_->load(std::memory_order_acquire));
return 0;
}
diff --git a/libs/vr/libbufferhub/consumer_buffer.cpp b/libs/vr/libbufferhub/consumer_buffer.cpp
index a91e842..83d6951 100644
--- a/libs/vr/libbufferhub/consumer_buffer.cpp
+++ b/libs/vr/libbufferhub/consumer_buffer.cpp
@@ -38,7 +38,7 @@
// Only check producer bit and this consumer buffer's particular consumer bit.
// The buffer is can be acquired iff: 1) producer bit is set; 2) consumer bit
// is not set.
- uint64_t buffer_state = buffer_state_->load();
+ uint64_t buffer_state = buffer_state_->load(std::memory_order_acquire);
if (!BufferHubDefs::IsBufferPosted(buffer_state, client_state_mask())) {
ALOGE("ConsumerBuffer::LocalAcquire: not posted, id=%d state=%" PRIx64
" client_state_mask=%" PRIx64 ".",
@@ -57,7 +57,7 @@
out_meta->user_metadata_ptr = 0;
}
- uint64_t fence_state = fence_state_->load();
+ uint64_t fence_state = fence_state_->load(std::memory_order_acquire);
// If there is an acquire fence from producer, we need to return it.
if (fence_state & BufferHubDefs::kProducerStateBit) {
*out_fence = shared_acquire_fence_.Duplicate();
@@ -118,7 +118,7 @@
return error;
// Check invalid state transition.
- uint64_t buffer_state = buffer_state_->load();
+ uint64_t buffer_state = buffer_state_->load(std::memory_order_acquire);
if (!BufferHubDefs::IsBufferAcquired(buffer_state)) {
ALOGE("ConsumerBuffer::LocalRelease: not acquired id=%d state=%" PRIx64 ".",
id(), buffer_state);
diff --git a/libs/vr/libbufferhub/include/private/dvr/buffer_hub_base.h b/libs/vr/libbufferhub/include/private/dvr/buffer_hub_base.h
index 1ea8302..09feb73 100644
--- a/libs/vr/libbufferhub/include/private/dvr/buffer_hub_base.h
+++ b/libs/vr/libbufferhub/include/private/dvr/buffer_hub_base.h
@@ -90,7 +90,9 @@
int cid() const { return cid_; }
// Returns the buffer buffer state.
- uint64_t buffer_state() { return buffer_state_->load(); };
+ uint64_t buffer_state() {
+ return buffer_state_->load(std::memory_order_acquire);
+ };
// A state mask which is unique to a buffer hub client among all its siblings
// sharing the same concrete graphic buffer.
diff --git a/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h b/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h
index 489692b..80a00be 100644
--- a/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h
+++ b/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h
@@ -40,7 +40,7 @@
uint64_t old_state;
uint64_t new_state;
do {
- old_state = buffer_state->load();
+ old_state = buffer_state->load(std::memory_order_acquire);
new_state = (old_state & ~clear_mask) | set_mask;
} while (!buffer_state->compare_exchange_weak(old_state, new_state));
}
diff --git a/libs/vr/libbufferhub/producer_buffer.cpp b/libs/vr/libbufferhub/producer_buffer.cpp
index 3730e7d..10517c5 100644
--- a/libs/vr/libbufferhub/producer_buffer.cpp
+++ b/libs/vr/libbufferhub/producer_buffer.cpp
@@ -80,7 +80,7 @@
return error;
// Check invalid state transition.
- uint64_t buffer_state = buffer_state_->load();
+ uint64_t buffer_state = buffer_state_->load(std::memory_order_acquire);
if (!BufferHubDefs::IsBufferGained(buffer_state)) {
ALOGE("ProducerBuffer::LocalPost: not gained, id=%d state=%" PRIx64 ".",
id(), buffer_state);
@@ -135,7 +135,7 @@
int ProducerBuffer::LocalGain(DvrNativeBufferMetadata* out_meta,
LocalHandle* out_fence, bool gain_posted_buffer) {
- uint64_t buffer_state = buffer_state_->load();
+ uint64_t buffer_state = buffer_state_->load(std::memory_order_acquire);
ALOGD_IF(TRACE, "ProducerBuffer::LocalGain: buffer=%d, state=%" PRIx64 ".",
id(), buffer_state);
@@ -168,7 +168,7 @@
out_meta->user_metadata_ptr = 0;
}
- uint64_t fence_state = fence_state_->load();
+ uint64_t fence_state = fence_state_->load(std::memory_order_acquire);
// If there is an release fence from consumer, we need to return it.
if (fence_state & BufferHubDefs::kConsumerStateMask) {
*out_fence = shared_release_fence_.Duplicate();
@@ -230,7 +230,7 @@
// TODO(b/112338294) Keep here for reference. Remove it after new logic is
// written.
- /* uint64_t buffer_state = buffer_state_->load();
+ /* uint64_t buffer_state = buffer_state_->load(std::memory_order_acquire);
if (!BufferHubDefs::IsBufferGained(buffer_state)) {
// Can only detach a ProducerBuffer when it's in gained state.
ALOGW("ProducerBuffer::Detach: The buffer (id=%d, state=0x%" PRIx64