Remove pending_consumers_ from producer channel
pending_consumers_ was created before the creation of buffer state atomics.
Producer channel need to check and issue signal if pending_consumers_ is
zero. pending_consumers_ == 0 is the same as buffer_state of
non-orphaned buffers are fully released. Thus, removing
pending_consumers_ as it gives redundant information and lower the
complexity of producer channel.
Bug: 119331650
Test: all tests are passing
on Blueline:
AHardwareBufferTest BufferHubBuffer_test BufferHubMetadata_test
buffer_hub_binder_service-test buffer_hub_queue_producer-test
libgui_test libsensor_test vrflinger_test buffer_hub-test
buffer_hub_queue-test dvr_buffer_queue-test dvr_api-test
dvr_display-test
on Vega:
AHardwareBufferTest BufferHubBuffer_test BufferHubMetadata_test
buffer_hub_queue_producer-test buffer_hub-test buffer_hub_queue-test
dvr_buffer_queue-test dvr_api-test libdvrtracking-test
Change-Id: I5ccf3e6b3a2a304e238c272bda443e6d180babc3
diff --git a/services/vr/bufferhubd/buffer_channel.cpp b/services/vr/bufferhubd/buffer_channel.cpp
index 6c64bcd..cf072b6 100644
--- a/services/vr/bufferhubd/buffer_channel.cpp
+++ b/services/vr/bufferhubd/buffer_channel.cpp
@@ -52,8 +52,7 @@
buffer_id(), /*consumer_count=*/0, buffer_node_->buffer_desc().width,
buffer_node_->buffer_desc().height, buffer_node_->buffer_desc().layers,
buffer_node_->buffer_desc().format, buffer_node_->buffer_desc().usage,
- /*pending_count=*/0, /*state=*/0, /*signaled_mask=*/0,
- /*index=*/0);
+ /*state=*/0, /*signaled_mask=*/0, /*index=*/0);
}
void BufferChannel::HandleImpulse(Message& /*message*/) {
diff --git a/services/vr/bufferhubd/buffer_hub.cpp b/services/vr/bufferhubd/buffer_hub.cpp
index b73c47d..f50d292 100644
--- a/services/vr/bufferhubd/buffer_hub.cpp
+++ b/services/vr/bufferhubd/buffer_hub.cpp
@@ -56,8 +56,6 @@
stream << " ";
stream << std::setw(10) << "Usage";
stream << " ";
- stream << std::setw(9) << "Pending";
- stream << " ";
stream << std::setw(18) << "State";
stream << " ";
stream << std::setw(18) << "Signaled";
@@ -90,8 +88,6 @@
stream << std::setw(8) << info.usage;
stream << std::dec << std::setfill(' ');
stream << " ";
- stream << std::setw(9) << info.pending_count;
- stream << " ";
stream << "0x" << std::hex << std::setfill('0');
stream << std::setw(16) << info.state;
stream << " ";
diff --git a/services/vr/bufferhubd/consumer_channel.cpp b/services/vr/bufferhubd/consumer_channel.cpp
index 98ef917..f936e95 100644
--- a/services/vr/bufferhubd/consumer_channel.cpp
+++ b/services/vr/bufferhubd/consumer_channel.cpp
@@ -153,11 +153,10 @@
}
}
-bool ConsumerChannel::OnProducerPosted() {
+void ConsumerChannel::OnProducerPosted() {
acquired_ = false;
released_ = false;
SignalAvailable();
- return true;
}
void ConsumerChannel::OnProducerClosed() {
diff --git a/services/vr/bufferhubd/include/private/dvr/buffer_hub.h b/services/vr/bufferhubd/include/private/dvr/buffer_hub.h
index e47ffa3..676617d 100644
--- a/services/vr/bufferhubd/include/private/dvr/buffer_hub.h
+++ b/services/vr/bufferhubd/include/private/dvr/buffer_hub.h
@@ -52,7 +52,6 @@
uint32_t layer_count = 0;
uint32_t format = 0;
uint64_t usage = 0;
- size_t pending_count = 0;
uint64_t state = 0;
uint64_t signaled_mask = 0;
uint64_t index = 0;
@@ -63,8 +62,7 @@
BufferInfo(int id, size_t consumer_count, uint32_t width, uint32_t height,
uint32_t layer_count, uint32_t format, uint64_t usage,
- size_t pending_count, uint64_t state, uint64_t signaled_mask,
- uint64_t index)
+ uint64_t state, uint64_t signaled_mask, uint64_t index)
: id(id),
type(kProducerType),
consumer_count(consumer_count),
@@ -73,7 +71,6 @@
layer_count(layer_count),
format(format),
usage(usage),
- pending_count(pending_count),
state(state),
signaled_mask(signaled_mask),
index(index) {}
diff --git a/services/vr/bufferhubd/include/private/dvr/consumer_channel.h b/services/vr/bufferhubd/include/private/dvr/consumer_channel.h
index 3298529..de0f23c 100644
--- a/services/vr/bufferhubd/include/private/dvr/consumer_channel.h
+++ b/services/vr/bufferhubd/include/private/dvr/consumer_channel.h
@@ -26,7 +26,7 @@
uint64_t client_state_mask() const { return client_state_mask_; }
BufferInfo GetBufferInfo() const override;
- bool OnProducerPosted();
+ void OnProducerPosted();
void OnProducerClosed();
private:
diff --git a/services/vr/bufferhubd/include/private/dvr/producer_channel.h b/services/vr/bufferhubd/include/private/dvr/producer_channel.h
index 9aead79..4734439 100644
--- a/services/vr/bufferhubd/include/private/dvr/producer_channel.h
+++ b/services/vr/bufferhubd/include/private/dvr/producer_channel.h
@@ -62,7 +62,6 @@
pdx::Status<void> OnConsumerRelease(Message& message,
LocalFence release_fence);
- void DecrementPendingConsumers();
void OnConsumerOrphaned(const uint64_t& consumer_state_mask);
void AddConsumer(ConsumerChannel* channel);
@@ -74,9 +73,6 @@
private:
std::vector<ConsumerChannel*> consumer_channels_;
- // This counts the number of consumers left to process this buffer. If this is
- // zero then the producer can re-acquire ownership.
- int pending_consumers_;
IonBuffer buffer_;
diff --git a/services/vr/bufferhubd/producer_channel.cpp b/services/vr/bufferhubd/producer_channel.cpp
index 4b4301f..ab1d94c 100644
--- a/services/vr/bufferhubd/producer_channel.cpp
+++ b/services/vr/bufferhubd/producer_channel.cpp
@@ -56,7 +56,6 @@
uint64_t usage, size_t user_metadata_size,
int* error)
: BufferHubChannel(service, channel_id, channel_id, kProducerType),
- pending_consumers_(0),
user_metadata_size_(user_metadata_size),
metadata_buf_size_(BufferHubDefs::kMetadataHeaderSize +
user_metadata_size) {
@@ -183,7 +182,7 @@
return BufferInfo(buffer_id(), consumer_channels_.size(), buffer_.width(),
buffer_.height(), buffer_.layer_count(), buffer_.format(),
- buffer_.usage(), pending_consumers_,
+ buffer_.usage(),
buffer_state_->load(std::memory_order_acquire),
signaled_mask, metadata_header_->queue_index);
}
@@ -370,8 +369,9 @@
(consumer_state_mask & BufferHubDefs::kHighBitsMask);
}
}
- if (update_buffer_state && consumer->OnProducerPosted())
- pending_consumers_++;
+ if (update_buffer_state) {
+ consumer->OnProducerPosted();
+ }
return {status.take()};
}
@@ -424,13 +424,9 @@
// Signal any interested consumers. If there are none, the buffer will stay
// in posted state until a consumer comes online. This behavior guarantees
// that no frame is silently dropped.
- pending_consumers_ = 0;
for (auto consumer : consumer_channels_) {
- if (consumer->OnProducerPosted())
- pending_consumers_++;
+ consumer->OnProducerPosted();
}
- ALOGD_IF(TRACE, "ProducerChannel::OnProducerPost: %d pending consumers",
- pending_consumers_);
return {};
}
@@ -439,18 +435,6 @@
ATRACE_NAME("ProducerChannel::OnGain");
ALOGD_IF(TRACE, "ProducerChannel::OnGain: buffer_id=%d", buffer_id());
- // There are still pending consumers, return busy.
- if (pending_consumers_ > 0) {
- ALOGE(
- "ProducerChannel::OnGain: Producer (id=%d) is gaining a buffer that "
- "still has %d pending consumer(s).",
- buffer_id(), pending_consumers_);
- // TODO(b/77153033): add gain_posted_buffer to the impulse args, and
- // return busy if the function does not allow gaining posted buffer.
- // TODO(b/119331650): remove this if check if pending_consumers_ is removed.
- // return ErrorStatus(EBUSY);
- }
-
ClearAvailable();
post_fence_.close();
return {std::move(returned_fence_)};
@@ -547,48 +531,25 @@
}
}
- DecrementPendingConsumers();
- if (pending_consumers_ == 0 && orphaned_consumer_bit_mask_) {
- ALOGW(
- "%s: orphaned buffer detected during the this acquire/release cycle: "
- "id=%d orphaned=0x%" PRIx64 " queue_index=%" PRIx64 ".",
- __FUNCTION__, buffer_id(), orphaned_consumer_bit_mask_,
- metadata_header_->queue_index);
- orphaned_consumer_bit_mask_ = 0;
+ uint64_t current_buffer_state =
+ buffer_state_->load(std::memory_order_acquire);
+ if (BufferHubDefs::IsClientReleased(current_buffer_state,
+ ~orphaned_consumer_bit_mask_)) {
+ SignalAvailable();
+ if (orphaned_consumer_bit_mask_) {
+ ALOGW(
+ "%s: orphaned buffer detected during the this acquire/release cycle: "
+ "id=%d orphaned=0x%" PRIx64 " queue_index=%" PRIx64 ".",
+ __FUNCTION__, buffer_id(), orphaned_consumer_bit_mask_,
+ metadata_header_->queue_index);
+ orphaned_consumer_bit_mask_ = 0;
+ }
}
- ALOGE_IF(
- pending_consumers_ && BufferHubDefs::IsBufferReleased(
- buffer_state_->load(std::memory_order_acquire)),
- "ProducerChannel::OnConsumerRelease: buffer state inconsistent: "
- "pending_consumers=%d, buffer buffer is in releaed state.",
- pending_consumers_);
return {};
}
-void ProducerChannel::DecrementPendingConsumers() {
- if (pending_consumers_ == 0) {
- ALOGE("ProducerChannel::DecrementPendingConsumers: no pending consumer.");
- return;
- }
-
- --pending_consumers_;
- ALOGD_IF(TRACE, "%s: buffer_id=%d %d consumers left", __FUNCTION__,
- buffer_id(), pending_consumers_);
-
- if (pending_consumers_ == 0) {
- ALOGD_IF(TRACE,
- "%s: releasing last consumer: buffer_id=%d state=%" PRIx64 ".",
- __FUNCTION__, buffer_id(),
- buffer_state_->load(std::memory_order_acquire));
- SignalAvailable();
- }
-}
-
void ProducerChannel::OnConsumerOrphaned(const uint64_t& consumer_state_mask) {
- // Ignore the orphaned consumer.
- DecrementPendingConsumers();
-
// Remember the ignored consumer so that newly added consumer won't be
// taking the same state mask as this orphaned consumer.
ALOGE_IF(orphaned_consumer_bit_mask_ & consumer_state_mask,
@@ -597,6 +558,13 @@
__FUNCTION__, consumer_state_mask);
orphaned_consumer_bit_mask_ |= consumer_state_mask;
+ uint64_t current_buffer_state =
+ buffer_state_->load(std::memory_order_acquire);
+ if (BufferHubDefs::IsClientReleased(current_buffer_state,
+ ~orphaned_consumer_bit_mask_)) {
+ SignalAvailable();
+ }
+
// Atomically clear the fence state bit as an orphaned consumer will never
// signal a release fence.
fence_state_->fetch_and(~consumer_state_mask, std::memory_order_release);