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

Commit 6705349c authored by Tianyu's avatar Tianyu
Browse files

Move active clients mask to an atomic uint64_t in shared memory. 

Previously, the active_consumer_bit_mask_ was a private member of producer
channel for creating new consumer buffers with unique buffer_state_bit_
so that newly added consumer can be uniquely identified amoung its
siblings. It was a union of all consumer's buffer_state_bit_.
In the re-design of buffer state, every consumer/producer/client buffers need
to know the location of every other consumer/producer/client in
buffer_state in order to post a buffer: when a client posts a buffer, it
changes its own buffer_state to 00 and others' buffer_state to 10. Thus, it
need to know where the other siblings locates in buffer_state atomic uint64_t.
active_consumer_bit_mask_ suffices this need.
This change moves the active_consumer_bit_mask_ from producer channel to an
uint64_t atomic variable in shared memory, and rename it as
active_clients_bit_mask_ (because it contain both consumers and
producers buffer_state_bit).

Test: marlin-eng on master branch
Test: vega_xr-eng on oc-dr1-daydream-dev branch
Test: buffer_hub-test buffer_hub_queue-test buffer_hub_queue_producer-test
Bug: 112007999
Change-Id: I1ae562701545c7504fd9367c8c8c63a2fd609264
parent 696d3d46

libs/vr/libbufferhub/buffer_hub_base.cpp

+10 −3
Original line number Diff line number Diff line
@@ -116,9 +116,10 @@ int BufferHubBase::ImportBuffer() { 
  cid_ = buffer_desc.buffer_cid();

  buffer_state_bit_ = buffer_desc.buffer_state_bit();



  // Note that here the buffer state is mapped from shared memory as an atomic

  // object. The std::atomic's constructor will not be called so that the

  // original value stored in the memory region will be preserved.

  // Note that here the buffer_state, fence_state and active_clients_bit_mask

  // are mapped from shared memory as an atomic object. The std::atomic's

  // constructor will not be called so that the original value stored in the

  // memory region will be preserved.

  buffer_state_ = &metadata_header_->buffer_state;

  ALOGD_IF(TRACE,

           "BufferHubBase::ImportBuffer: id=%d, buffer_state=%" PRIx64 ".",
@@ -127,6 +128,12 @@ int BufferHubBase::ImportBuffer() { 
  ALOGD_IF(TRACE,

           "BufferHubBase::ImportBuffer: id=%d, fence_state=%" PRIx64 ".", id(),

           fence_state_->load());

  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());



  return 0;

}

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

+1 −0
Original line number Diff line number Diff line
@@ -139,6 +139,7 @@ class BufferHubBase : public pdx::Client { 
  void* user_metadata_ptr_{nullptr};

  std::atomic<uint64_t>* buffer_state_{nullptr};

  std::atomic<uint64_t>* fence_state_{nullptr};

  std::atomic<uint64_t>* active_clients_bit_mask_{nullptr};



  LocalHandle shared_acquire_fence_;

  LocalHandle shared_release_fence_;

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

+2 −1
Original line number Diff line number Diff line
@@ -75,6 +75,7 @@ struct __attribute__((packed, aligned(8))) MetadataHeader { 
  // and vendor HAL).

  std::atomic<uint64_t> buffer_state;

  std::atomic<uint64_t> fence_state;

  std::atomic<uint64_t> active_clients_bit_mask;

  uint64_t queue_index;



  // Public data format, which should be updated with caution. See more details
@@ -82,7 +83,7 @@ struct __attribute__((packed, aligned(8))) MetadataHeader { 
  DvrNativeBufferMetadata metadata;

};



static_assert(sizeof(MetadataHeader) == 128, "Unexpected MetadataHeader size");

static_assert(sizeof(MetadataHeader) == 136, "Unexpected MetadataHeader size");

static constexpr size_t kMetadataHeaderSize = sizeof(MetadataHeader);



}  // namespace BufferHubDefs

services/vr/bufferhubd/include/private/dvr/producer_channel.h

+2 −4
Original line number Diff line number Diff line
@@ -8,8 +8,8 @@ 
#include <pdx/channel_handle.h>

#include <pdx/file_handle.h>

#include <pdx/rpc/buffer_wrapper.h>

#include <private/dvr/bufferhub_rpc.h>

#include <private/dvr/buffer_hub.h>

#include <private/dvr/bufferhub_rpc.h>

#include <private/dvr/ion_buffer.h>



namespace android {
@@ -81,10 +81,8 @@ class ProducerChannel : public BufferHubChannel { 
  BufferHubDefs::MetadataHeader* metadata_header_ = nullptr;

  std::atomic<uint64_t>* buffer_state_ = nullptr;

  std::atomic<uint64_t>* fence_state_ = nullptr;

  std::atomic<uint64_t>* active_clients_bit_mask_ = nullptr;



  // All active consumer bits. Valid bits are the lower 63 bits, while the

  // highest bit is reserved for the producer and should not be set.

  uint64_t active_consumer_bit_mask_{0ULL};

  // All orphaned consumer bits. Valid bits are the lower 63 bits, while the

  // highest bit is reserved for the producer and should not be set.

  uint64_t orphaned_consumer_bit_mask_{0ULL};

services/vr/bufferhubd/producer_channel.cpp

+32 −3
Original line number Diff line number Diff line
@@ -97,6 +97,8 @@ int ProducerChannel::InitializeBuffer() { 
  buffer_state_ =

      new (&metadata_header_->buffer_state) std::atomic<uint64_t>(0);

  fence_state_ = new (&metadata_header_->fence_state) std::atomic<uint64_t>(0);

  active_clients_bit_mask_ =

      new (&metadata_header_->active_clients_bit_mask) std::atomic<uint64_t>(0);



  acquire_fence_fd_.Reset(epoll_create1(EPOLL_CLOEXEC));

  release_fence_fd_.Reset(epoll_create1(EPOLL_CLOEXEC));
@@ -260,8 +262,12 @@ Status<RemoteChannelHandle> ProducerChannel::CreateConsumer(Message& message) { 


  // Try find the next consumer state bit which has not been claimed by any

  // consumer yet.

  // memory_order_acquire is chosen here because all writes in other threads

  // that release active_clients_bit_mask_ need to be visible here.

  uint64_t current_active_clients_bit_mask =

      active_clients_bit_mask_->load(std::memory_order_acquire);

  uint64_t consumer_state_bit = BufferHubDefs::FindNextClearedBit(

      active_consumer_bit_mask_ | orphaned_consumer_bit_mask_ |

      current_active_clients_bit_mask | orphaned_consumer_bit_mask_ |

      BufferHubDefs::kProducerStateBit);

  if (consumer_state_bit == 0ULL) {

    ALOGE(
@@ -270,6 +276,23 @@ Status<RemoteChannelHandle> ProducerChannel::CreateConsumer(Message& message) { 
    return ErrorStatus(E2BIG);

  }



  uint64_t updated_active_clients_bit_mask =

      current_active_clients_bit_mask | consumer_state_bit;

  // Set the updated value only if the current value stays the same as what was

  // read before. If the comparison succeeds, update the value without

  // reordering anything before or after this read-modify-write in the current

  // thread, and the modification will be visible in other threads that acquire

  // active_clients_bit_mask_. If the comparison fails, load the result of

  // all writes from all threads to updated_active_clients_bit_mask.

  if (!active_clients_bit_mask_->compare_exchange_weak(

          current_active_clients_bit_mask, updated_active_clients_bit_mask,

          std::memory_order_acq_rel, std::memory_order_acquire)) {

    ALOGE("Current active clients bit mask is changed to %" PRIu64

          ", which was expected to be %" PRIu64 ".",

          updated_active_clients_bit_mask, current_active_clients_bit_mask);

    return ErrorStatus(EBUSY);

  }



  auto consumer =

      std::make_shared<ConsumerChannel>(service(), buffer_id(), channel_id,

                                        consumer_state_bit, shared_from_this());
@@ -279,6 +302,10 @@ Status<RemoteChannelHandle> ProducerChannel::CreateConsumer(Message& message) { 
        "ProducerChannel::CreateConsumer: failed to set new consumer channel: "

        "%s",

        channel_status.GetErrorMessage().c_str());

    // Restore the consumer state bit and make it visible in other threads that

    // acquire the active_clients_bit_mask_.

    active_clients_bit_mask_->fetch_and(~consumer_state_bit,

                                        std::memory_order_release);

    return ErrorStatus(ENOMEM);

  }
@@ -289,7 +316,6 @@ Status<RemoteChannelHandle> ProducerChannel::CreateConsumer(Message& message) { 
      pending_consumers_++;

  }



  active_consumer_bit_mask_ |= consumer_state_bit;

  return {status.take()};

}
@@ -551,7 +577,10 @@ void ProducerChannel::AddConsumer(ConsumerChannel* channel) { 
void ProducerChannel::RemoveConsumer(ConsumerChannel* channel) {

  consumer_channels_.erase(

      std::find(consumer_channels_.begin(), consumer_channels_.end(), channel));

  active_consumer_bit_mask_ &= ~channel->consumer_state_bit();

  // Restore the consumer state bit and make it visible in other threads that

  // acquire the active_clients_bit_mask_.

  active_clients_bit_mask_->fetch_and(~channel->consumer_state_bit(),

                                      std::memory_order_release);



  const uint64_t buffer_state = buffer_state_->load();

  if (BufferHubDefs::IsBufferPosted(buffer_state) ||