Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit e2cdec98 authored by Tianyu Jiang's avatar Tianyu Jiang
Browse files

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
parent eb3e8440

libs/vr/libbufferhub/buffer_hub_base.cpp

+3 −3
Original line number Diff line number Diff line
@@ -123,17 +123,17 @@ int BufferHubBase::ImportBuffer() { 
  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;

}

libs/vr/libbufferhub/consumer_buffer.cpp

+3 −3
Original line number Diff line number Diff line
@@ -38,7 +38,7 @@ int ConsumerBuffer::LocalAcquire(DvrNativeBufferMetadata* out_meta, 
  // 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 @@ int ConsumerBuffer::LocalAcquire(DvrNativeBufferMetadata* out_meta, 
    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 @@ int ConsumerBuffer::LocalRelease(const DvrNativeBufferMetadata* meta, 
    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);

libs/vr/libbufferhub/include/private/dvr/buffer_hub_base.h

+3 −1
Original line number Diff line number Diff line
@@ -90,7 +90,9 @@ class BufferHubBase : public pdx::Client { 
  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.

libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h

+1 −1
Original line number Diff line number Diff line
@@ -40,7 +40,7 @@ static inline void ModifyBufferState(std::atomic<uint64_t>* buffer_state, 
  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));

}

libs/vr/libbufferhub/producer_buffer.cpp

+4 −4
Original line number Diff line number Diff line
@@ -80,7 +80,7 @@ int ProducerBuffer::LocalPost(const DvrNativeBufferMetadata* meta, 
    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::PostAsync(const DvrNativeBufferMetadata* meta, 


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 @@ int ProducerBuffer::LocalGain(DvrNativeBufferMetadata* out_meta, 
    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 @@ Status<LocalChannelHandle> ProducerBuffer::Detach() { 


  // 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