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

Commit 5fbd8a3a authored by TreeHugger Robot's avatar TreeHugger Robot Committed by Android (Google) Code Review
Browse files

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

parents 7371fedb 6705349c
Loading
Loading
Loading
Loading
+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;
}
+1 −0
Original line number Diff line number Diff line
@@ -134,6 +134,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_;
+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
+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};
+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) ||