Change the definition of buffer state and client state bits.
Please refer to go/bufferhub-buffer-state-redesign for more information.
In this change:
1. Every clients takes up two bits in the buffer_state.
One from the higher 32 bits, one from the lower 32 bits. For details:
go/bufferhub-buffer-state-redesign
2. Upon the creation of a new buffer, the buffer is in released state.
Previously, only producer creates buffer, and upon creation, the buffer
was in gained state. Now, producer needs to specifically gain the buffer
before trying to produce and post it.
3. If there is no other clients when a client post a buffer, the buffer
will actually be in released state instead of posted state. This is
because the posted buffer does not have readers and can be reused
immediately.
4. If a new client is added to the buffer when the buffer is in acquired
or posted state, the buffer state of the new client will be set to posted
state and able to acquire the same buffer content as posted.
In the next change:
variables of type std::atomic<uint64_t> in metadata header in shared memory
will be replaced by std::atomic<uint32_t>
Test: marlin-eng passing AHardwareBufferTest BufferHubBuffer_test
BufferHubMetadata_test buffer_hub_binder_service-test
buffer_hub_queue_producer-test dvr_api-test libgui_test
libsensor_test vrflinger_test buffer_hub-test
dvr_buffer_queue-test dvr_display-test buffer_hub_queue-test
Test: smartphone VR works on blueline-eng
Test: vega_xr passing AHardwareBufferTest BufferHubBuffer_test
BufferHubMetadata_test buffer_hub_queue_producer-test buffer_hub-test
buffer_hub_queue-test dvr_buffer_queue-test dvr_api-test
Cherrypicking this changelist to oc-dr1-daydream-dev branch requires
ag/5514563 to be merged at the same time to make Vega actually work.
Bug: 112007999
Change-Id: I86393818ad922a91c709fe22f8e99b0667d2e9ef
diff --git a/libs/ui/tests/BufferHubBuffer_test.cpp b/libs/ui/tests/BufferHubBuffer_test.cpp
index d30636f..260ecf9 100644
--- a/libs/ui/tests/BufferHubBuffer_test.cpp
+++ b/libs/ui/tests/BufferHubBuffer_test.cpp
@@ -35,7 +35,7 @@
const int kUsage = 0;
const size_t kUserMetadataSize = 0;
-using dvr::BufferHubDefs::IsBufferGained;
+using dvr::BufferHubDefs::IsBufferReleased;
using dvr::BufferHubDefs::kFirstClientBitMask;
using dvr::BufferHubDefs::kMetadataHeaderSize;
using frameworks::bufferhub::V1_0::BufferHubStatus;
@@ -118,9 +118,9 @@
// We use client_state_mask() to tell those two instances apart.
EXPECT_NE(bufferStateMask1, bufferStateMask2);
- // Both buffer instances should be in gained state.
- EXPECT_TRUE(IsBufferGained(b1->buffer_state()));
- EXPECT_TRUE(IsBufferGained(b2->buffer_state()));
+ // Both buffer instances should be in released state currently.
+ EXPECT_TRUE(IsBufferReleased(b1->buffer_state()));
+ EXPECT_TRUE(IsBufferReleased(b2->buffer_state()));
// TODO(b/112338294): rewrite test after migration
return;
diff --git a/libs/ui/tests/BufferHubMetadata_test.cpp b/libs/ui/tests/BufferHubMetadata_test.cpp
index 70f86b3..14422bf 100644
--- a/libs/ui/tests/BufferHubMetadata_test.cpp
+++ b/libs/ui/tests/BufferHubMetadata_test.cpp
@@ -17,7 +17,7 @@
#include <gtest/gtest.h>
#include <ui/BufferHubMetadata.h>
-using android::dvr::BufferHubDefs::IsBufferGained;
+using android::dvr::BufferHubDefs::IsBufferReleased;
namespace android {
namespace dvr {
@@ -52,19 +52,13 @@
BufferHubDefs::MetadataHeader* mh1 = m1.metadata_header();
EXPECT_NE(mh1, nullptr);
- // TODO(b/111976433): Update this test once BufferHub state machine gets
- // updated. In the old model, buffer starts in the gained state (i.e.
- // valued 0). In the new model, buffer states in the released state.
- EXPECT_TRUE(IsBufferGained(mh1->fence_state.load()));
+ EXPECT_TRUE(IsBufferReleased(mh1->buffer_state.load()));
EXPECT_TRUE(m2.IsValid());
BufferHubDefs::MetadataHeader* mh2 = m2.metadata_header();
EXPECT_NE(mh2, nullptr);
- // TODO(b/111976433): Update this test once BufferHub state machine gets
- // updated. In the old model, buffer starts in the gained state (i.e.
- // valued 0). In the new model, buffer states in the released state.
- EXPECT_TRUE(IsBufferGained(mh2->fence_state.load()));
+ EXPECT_TRUE(IsBufferReleased(mh2->buffer_state.load()));
}
TEST_F(BufferHubMetadataTest, MoveMetadataInvalidatesOldOne) {
diff --git a/libs/vr/libbufferhub/buffer_hub-test.cpp b/libs/vr/libbufferhub/buffer_hub-test.cpp
index 73ca69b..2d9a42b 100644
--- a/libs/vr/libbufferhub/buffer_hub-test.cpp
+++ b/libs/vr/libbufferhub/buffer_hub-test.cpp
@@ -23,11 +23,13 @@
using android::sp;
using android::dvr::ConsumerBuffer;
using android::dvr::ProducerBuffer;
-using android::dvr::BufferHubDefs::IsBufferAcquired;
-using android::dvr::BufferHubDefs::IsBufferGained;
-using android::dvr::BufferHubDefs::IsBufferPosted;
+using android::dvr::BufferHubDefs::AnyClientAcquired;
+using android::dvr::BufferHubDefs::AnyClientGained;
+using android::dvr::BufferHubDefs::AnyClientPosted;
using android::dvr::BufferHubDefs::IsBufferReleased;
-using android::dvr::BufferHubDefs::kConsumerStateMask;
+using android::dvr::BufferHubDefs::IsClientAcquired;
+using android::dvr::BufferHubDefs::IsClientPosted;
+using android::dvr::BufferHubDefs::IsClientReleased;
using android::dvr::BufferHubDefs::kFirstClientBitMask;
using android::dvr::BufferHubDefs::kMetadataHeaderSize;
using android::pdx::LocalChannelHandle;
@@ -52,58 +54,49 @@
std::unique_ptr<ProducerBuffer> p = ProducerBuffer::Create(
kWidth, kHeight, kFormat, kUsage, sizeof(uint64_t));
ASSERT_TRUE(p.get() != nullptr);
- std::unique_ptr<ConsumerBuffer> c =
+ std::unique_ptr<ConsumerBuffer> c1 =
ConsumerBuffer::Import(p->CreateConsumer());
- ASSERT_TRUE(c.get() != nullptr);
+ ASSERT_TRUE(c1.get() != nullptr);
// Check that consumers can spawn other consumers.
std::unique_ptr<ConsumerBuffer> c2 =
- ConsumerBuffer::Import(c->CreateConsumer());
+ ConsumerBuffer::Import(c1->CreateConsumer());
ASSERT_TRUE(c2.get() != nullptr);
- // Producer state mask is unique, i.e. 1.
+ // Checks the state masks of client p, c1 and c2.
EXPECT_EQ(p->client_state_mask(), kFirstClientBitMask);
- // Consumer state mask cannot have producer bit on.
- EXPECT_EQ(c->client_state_mask() & kFirstClientBitMask, 0U);
- // Consumer state mask must be a single, i.e. power of 2.
- EXPECT_NE(c->client_state_mask(), 0U);
- EXPECT_EQ(c->client_state_mask() & (c->client_state_mask() - 1), 0U);
- // Consumer state mask cannot have producer bit on.
- EXPECT_EQ(c2->client_state_mask() & kFirstClientBitMask, 0U);
- // Consumer state mask must be a single, i.e. power of 2.
- EXPECT_NE(c2->client_state_mask(), 0U);
- EXPECT_EQ(c2->client_state_mask() & (c2->client_state_mask() - 1), 0U);
- // Each consumer should have unique bit.
- EXPECT_EQ(c->client_state_mask() & c2->client_state_mask(), 0U);
+ EXPECT_EQ(c1->client_state_mask(), kFirstClientBitMask << 1);
+ EXPECT_EQ(c2->client_state_mask(), kFirstClientBitMask << 2);
// Initial state: producer not available, consumers not available.
EXPECT_EQ(0, RETRY_EINTR(p->Poll(kPollTimeoutMs)));
- EXPECT_EQ(0, RETRY_EINTR(c->Poll(kPollTimeoutMs)));
+ EXPECT_EQ(0, RETRY_EINTR(c1->Poll(kPollTimeoutMs)));
EXPECT_EQ(0, RETRY_EINTR(c2->Poll(kPollTimeoutMs)));
+ EXPECT_EQ(0, p->GainAsync());
EXPECT_EQ(0, p->Post(LocalHandle()));
// New state: producer not available, consumers available.
EXPECT_EQ(0, RETRY_EINTR(p->Poll(kPollTimeoutMs)));
- EXPECT_EQ(1, RETRY_EINTR(c->Poll(kPollTimeoutMs)));
+ EXPECT_EQ(1, RETRY_EINTR(c1->Poll(kPollTimeoutMs)));
EXPECT_EQ(1, RETRY_EINTR(c2->Poll(kPollTimeoutMs)));
LocalHandle fence;
- EXPECT_EQ(0, c->Acquire(&fence));
- EXPECT_EQ(0, RETRY_EINTR(c->Poll(kPollTimeoutMs)));
+ EXPECT_EQ(0, c1->Acquire(&fence));
+ EXPECT_EQ(0, RETRY_EINTR(c1->Poll(kPollTimeoutMs)));
EXPECT_EQ(1, RETRY_EINTR(c2->Poll(kPollTimeoutMs)));
EXPECT_EQ(0, c2->Acquire(&fence));
EXPECT_EQ(0, RETRY_EINTR(c2->Poll(kPollTimeoutMs)));
- EXPECT_EQ(0, RETRY_EINTR(c->Poll(kPollTimeoutMs)));
+ EXPECT_EQ(0, RETRY_EINTR(c1->Poll(kPollTimeoutMs)));
- EXPECT_EQ(0, c->Release(LocalHandle()));
+ EXPECT_EQ(0, c1->Release(LocalHandle()));
EXPECT_EQ(0, RETRY_EINTR(p->Poll(kPollTimeoutMs)));
EXPECT_EQ(0, c2->Discard());
-
EXPECT_EQ(1, RETRY_EINTR(p->Poll(kPollTimeoutMs)));
+
EXPECT_EQ(0, p->Gain(&fence));
EXPECT_EQ(0, RETRY_EINTR(p->Poll(kPollTimeoutMs)));
- EXPECT_EQ(0, RETRY_EINTR(c->Poll(kPollTimeoutMs)));
+ EXPECT_EQ(0, RETRY_EINTR(c1->Poll(kPollTimeoutMs)));
EXPECT_EQ(0, RETRY_EINTR(c2->Poll(kPollTimeoutMs)));
}
@@ -144,7 +137,8 @@
// No events should be signaled initially.
ASSERT_EQ(0, epoll_wait(epoll_fd.Get(), events.data(), events.size(), 0));
- // Post the producer and check for consumer signal.
+ // Gain and post the producer and check for consumer signal.
+ EXPECT_EQ(0, p->GainAsync());
EXPECT_EQ(0, p->Post({}));
ASSERT_EQ(1, epoll_wait(epoll_fd.Get(), events.data(), events.size(),
kPollTimeoutMs));
@@ -189,7 +183,7 @@
EXPECT_EQ(client_state_masks & cs[i]->client_state_mask(), 0U);
client_state_masks |= cs[i]->client_state_mask();
}
- EXPECT_EQ(client_state_masks, kFirstClientBitMask | kConsumerStateMask);
+ EXPECT_EQ(client_state_masks, ~0ULL);
// The 64th creation will fail with out-of-memory error.
auto state = p->CreateConsumer();
@@ -204,7 +198,6 @@
// The released state mask will be reused.
EXPECT_EQ(client_state_masks & cs[i]->client_state_mask(), 0U);
client_state_masks |= cs[i]->client_state_mask();
- EXPECT_EQ(client_state_masks, kFirstClientBitMask | kConsumerStateMask);
}
}
@@ -217,24 +210,21 @@
ASSERT_TRUE(c.get() != nullptr);
LocalHandle fence;
+ EXPECT_EQ(0, p->GainAsync());
- // The producer buffer starts in gained state.
-
- // Acquire, release, and gain in gained state should fail.
+ // Acquire and gain in gained state should fail.
EXPECT_EQ(-EBUSY, c->Acquire(&fence));
- EXPECT_EQ(-EBUSY, c->Release(LocalHandle()));
EXPECT_EQ(-EALREADY, p->Gain(&fence));
// Post in gained state should succeed.
EXPECT_EQ(0, p->Post(LocalHandle()));
- // Post, release, and gain in posted state should fail.
+ // Post and gain in posted state should fail.
EXPECT_EQ(-EBUSY, p->Post(LocalHandle()));
- EXPECT_EQ(-EBUSY, c->Release(LocalHandle()));
EXPECT_EQ(-EBUSY, p->Gain(&fence));
// Acquire in posted state should succeed.
- EXPECT_LE(0, c->Acquire(&fence));
+ EXPECT_EQ(0, c->Acquire(&fence));
// Acquire, post, and gain in acquired state should fail.
EXPECT_EQ(-EBUSY, c->Acquire(&fence));
@@ -245,17 +235,15 @@
EXPECT_EQ(0, c->Release(LocalHandle()));
EXPECT_LT(0, RETRY_EINTR(p->Poll(kPollTimeoutMs)));
- // Release, acquire, and post in released state should fail.
- EXPECT_EQ(-EBUSY, c->Release(LocalHandle()));
+ // Acquire and post in released state should fail.
EXPECT_EQ(-EBUSY, c->Acquire(&fence));
EXPECT_EQ(-EBUSY, p->Post(LocalHandle()));
// Gain in released state should succeed.
EXPECT_EQ(0, p->Gain(&fence));
- // Acquire, release, and gain in gained state should fail.
+ // Acquire and gain in gained state should fail.
EXPECT_EQ(-EBUSY, c->Acquire(&fence));
- EXPECT_EQ(-EBUSY, c->Release(LocalHandle()));
EXPECT_EQ(-EALREADY, p->Gain(&fence));
}
@@ -269,24 +257,21 @@
DvrNativeBufferMetadata metadata;
LocalHandle invalid_fence;
+ EXPECT_EQ(0, p->GainAsync());
- // The producer buffer starts in gained state.
-
- // Acquire, release, and gain in gained state should fail.
+ // Acquire and gain in gained state should fail.
EXPECT_EQ(-EBUSY, c->AcquireAsync(&metadata, &invalid_fence));
EXPECT_FALSE(invalid_fence.IsValid());
- EXPECT_EQ(-EBUSY, c->ReleaseAsync(&metadata, invalid_fence));
EXPECT_EQ(-EALREADY, p->GainAsync(&metadata, &invalid_fence));
EXPECT_FALSE(invalid_fence.IsValid());
// Post in gained state should succeed.
EXPECT_EQ(0, p->PostAsync(&metadata, invalid_fence));
EXPECT_EQ(p->buffer_state(), c->buffer_state());
- EXPECT_TRUE(IsBufferPosted(p->buffer_state()));
+ EXPECT_TRUE(AnyClientPosted(p->buffer_state()));
- // Post, release, and gain in posted state should fail.
+ // Post and gain in posted state should fail.
EXPECT_EQ(-EBUSY, p->PostAsync(&metadata, invalid_fence));
- EXPECT_EQ(-EBUSY, c->ReleaseAsync(&metadata, invalid_fence));
EXPECT_EQ(-EBUSY, p->GainAsync(&metadata, &invalid_fence));
EXPECT_FALSE(invalid_fence.IsValid());
@@ -295,7 +280,7 @@
EXPECT_EQ(0, c->AcquireAsync(&metadata, &invalid_fence));
EXPECT_FALSE(invalid_fence.IsValid());
EXPECT_EQ(p->buffer_state(), c->buffer_state());
- EXPECT_TRUE(IsBufferAcquired(p->buffer_state()));
+ EXPECT_TRUE(AnyClientAcquired(p->buffer_state()));
// Acquire, post, and gain in acquired state should fail.
EXPECT_EQ(-EBUSY, c->AcquireAsync(&metadata, &invalid_fence));
@@ -310,8 +295,7 @@
EXPECT_EQ(p->buffer_state(), c->buffer_state());
EXPECT_TRUE(IsBufferReleased(p->buffer_state()));
- // Release, acquire, and post in released state should fail.
- EXPECT_EQ(-EBUSY, c->ReleaseAsync(&metadata, invalid_fence));
+ // Acquire and post in released state should fail.
EXPECT_EQ(-EBUSY, c->AcquireAsync(&metadata, &invalid_fence));
EXPECT_FALSE(invalid_fence.IsValid());
EXPECT_EQ(-EBUSY, p->PostAsync(&metadata, invalid_fence));
@@ -320,12 +304,11 @@
EXPECT_EQ(0, p->GainAsync(&metadata, &invalid_fence));
EXPECT_FALSE(invalid_fence.IsValid());
EXPECT_EQ(p->buffer_state(), c->buffer_state());
- EXPECT_TRUE(IsBufferGained(p->buffer_state()));
+ EXPECT_TRUE(AnyClientGained(p->buffer_state()));
- // Acquire, release, and gain in gained state should fail.
+ // Acquire and gain in gained state should fail.
EXPECT_EQ(-EBUSY, c->AcquireAsync(&metadata, &invalid_fence));
EXPECT_FALSE(invalid_fence.IsValid());
- EXPECT_EQ(-EBUSY, c->ReleaseAsync(&metadata, invalid_fence));
EXPECT_EQ(-EALREADY, p->GainAsync(&metadata, &invalid_fence));
EXPECT_FALSE(invalid_fence.IsValid());
}
@@ -334,9 +317,12 @@
std::unique_ptr<ProducerBuffer> p = ProducerBuffer::Create(
kWidth, kHeight, kFormat, kUsage, sizeof(uint64_t));
ASSERT_TRUE(p.get() != nullptr);
-
- // The producer buffer starts in gained state. Post the buffer.
+ std::unique_ptr<ConsumerBuffer> c =
+ ConsumerBuffer::Import(p->CreateConsumer());
+ ASSERT_TRUE(c.get() != nullptr);
+ ASSERT_EQ(0, p->GainAsync());
ASSERT_EQ(0, p->Post(LocalHandle()));
+ ASSERT_TRUE(AnyClientPosted(p->buffer_state()));
// Gain in posted state should only succeed with gain_posted_buffer = true.
LocalHandle invalid_fence;
@@ -348,9 +334,12 @@
std::unique_ptr<ProducerBuffer> p = ProducerBuffer::Create(
kWidth, kHeight, kFormat, kUsage, sizeof(uint64_t));
ASSERT_TRUE(p.get() != nullptr);
-
- // The producer buffer starts in gained state. Post the buffer.
+ std::unique_ptr<ConsumerBuffer> c =
+ ConsumerBuffer::Import(p->CreateConsumer());
+ ASSERT_TRUE(c.get() != nullptr);
+ ASSERT_EQ(0, p->GainAsync());
ASSERT_EQ(0, p->Post(LocalHandle()));
+ ASSERT_TRUE(AnyClientPosted(p->buffer_state()));
// GainAsync in posted state should only succeed with gain_posted_buffer
// equals true.
@@ -360,54 +349,49 @@
EXPECT_EQ(0, p->GainAsync(&metadata, &invalid_fence, true));
}
-TEST_F(LibBufferHubTest, TestZeroConsumer) {
+TEST_F(LibBufferHubTest, TestGainPostedBuffer_noConsumer) {
std::unique_ptr<ProducerBuffer> p = ProducerBuffer::Create(
kWidth, kHeight, kFormat, kUsage, sizeof(uint64_t));
ASSERT_TRUE(p.get() != nullptr);
+ ASSERT_EQ(0, p->GainAsync());
+ ASSERT_EQ(0, p->Post(LocalHandle()));
+ // Producer state bit is in released state after post. The overall state of
+ // the buffer is also released because there is no consumer of this buffer.
+ ASSERT_TRUE(IsBufferReleased(p->buffer_state()));
- DvrNativeBufferMetadata metadata;
+ // Gain in released state should succeed.
LocalHandle invalid_fence;
-
- // Newly created.
- EXPECT_TRUE(IsBufferGained(p->buffer_state()));
- EXPECT_EQ(0, p->PostAsync(&metadata, invalid_fence));
- EXPECT_TRUE(IsBufferPosted(p->buffer_state()));
-
- // The buffer should stay in posted stay until a consumer picks it up.
- EXPECT_GE(0, RETRY_EINTR(p->Poll(kPollTimeoutMs)));
-
- // A new consumer should still be able to acquire the buffer immediately.
- std::unique_ptr<ConsumerBuffer> c =
- ConsumerBuffer::Import(p->CreateConsumer());
- ASSERT_TRUE(c.get() != nullptr);
- EXPECT_EQ(0, c->AcquireAsync(&metadata, &invalid_fence));
- EXPECT_TRUE(IsBufferAcquired(c->buffer_state()));
+ EXPECT_EQ(0, p->Gain(&invalid_fence, false));
}
TEST_F(LibBufferHubTest, TestMaxConsumers) {
std::unique_ptr<ProducerBuffer> p = ProducerBuffer::Create(
kWidth, kHeight, kFormat, kUsage, sizeof(uint64_t));
ASSERT_TRUE(p.get() != nullptr);
+ uint64_t producer_state_mask = p->client_state_mask();
std::array<std::unique_ptr<ConsumerBuffer>, kMaxConsumerCount> cs;
- for (size_t i = 0; i < kMaxConsumerCount; i++) {
+ for (size_t i = 0; i < kMaxConsumerCount; ++i) {
cs[i] = ConsumerBuffer::Import(p->CreateConsumer());
ASSERT_TRUE(cs[i].get() != nullptr);
- EXPECT_TRUE(IsBufferGained(cs[i]->buffer_state()));
+ EXPECT_TRUE(IsBufferReleased(cs[i]->buffer_state()));
+ EXPECT_NE(producer_state_mask, cs[i]->client_state_mask());
}
+ EXPECT_EQ(0, p->GainAsync());
DvrNativeBufferMetadata metadata;
LocalHandle invalid_fence;
// Post the producer should trigger all consumers to be available.
EXPECT_EQ(0, p->PostAsync(&metadata, invalid_fence));
- EXPECT_TRUE(IsBufferPosted(p->buffer_state()));
- for (size_t i = 0; i < kMaxConsumerCount; i++) {
+ EXPECT_TRUE(IsClientReleased(p->buffer_state(), p->client_state_mask()));
+ for (size_t i = 0; i < kMaxConsumerCount; ++i) {
EXPECT_TRUE(
- IsBufferPosted(cs[i]->buffer_state(), cs[i]->client_state_mask()));
+ IsClientPosted(cs[i]->buffer_state(), cs[i]->client_state_mask()));
EXPECT_LT(0, RETRY_EINTR(cs[i]->Poll(kPollTimeoutMs)));
EXPECT_EQ(0, cs[i]->AcquireAsync(&metadata, &invalid_fence));
- EXPECT_TRUE(IsBufferAcquired(p->buffer_state()));
+ EXPECT_TRUE(
+ IsClientAcquired(p->buffer_state(), cs[i]->client_state_mask()));
}
// All consumers have to release before the buffer is considered to be
@@ -430,44 +414,57 @@
std::unique_ptr<ProducerBuffer> p = ProducerBuffer::Create(
kWidth, kHeight, kFormat, kUsage, sizeof(uint64_t));
ASSERT_TRUE(p.get() != nullptr);
- EXPECT_TRUE(IsBufferGained(p->buffer_state()));
+ EXPECT_EQ(0, p->GainAsync());
+ EXPECT_TRUE(AnyClientGained(p->buffer_state()));
std::unique_ptr<ConsumerBuffer> c =
ConsumerBuffer::Import(p->CreateConsumer());
ASSERT_TRUE(c.get() != nullptr);
- EXPECT_TRUE(IsBufferGained(c->buffer_state()));
+ EXPECT_TRUE(AnyClientGained(c->buffer_state()));
DvrNativeBufferMetadata metadata;
LocalHandle invalid_fence;
// Post the gained buffer should signal already created consumer.
EXPECT_EQ(0, p->PostAsync(&metadata, invalid_fence));
- EXPECT_TRUE(IsBufferPosted(p->buffer_state()));
+ EXPECT_TRUE(AnyClientPosted(p->buffer_state()));
EXPECT_LT(0, RETRY_EINTR(c->Poll(kPollTimeoutMs)));
EXPECT_EQ(0, c->AcquireAsync(&metadata, &invalid_fence));
- EXPECT_TRUE(IsBufferAcquired(c->buffer_state()));
+ EXPECT_TRUE(AnyClientAcquired(c->buffer_state()));
}
-TEST_F(LibBufferHubTest, TestCreateConsumerWhenBufferPosted) {
+TEST_F(LibBufferHubTest, TestCreateTheFirstConsumerAfterPostingBuffer) {
std::unique_ptr<ProducerBuffer> p = ProducerBuffer::Create(
kWidth, kHeight, kFormat, kUsage, sizeof(uint64_t));
ASSERT_TRUE(p.get() != nullptr);
- EXPECT_TRUE(IsBufferGained(p->buffer_state()));
+ EXPECT_EQ(0, p->GainAsync());
+ EXPECT_TRUE(AnyClientGained(p->buffer_state()));
DvrNativeBufferMetadata metadata;
LocalHandle invalid_fence;
// Post the gained buffer before any consumer gets created.
+ // The buffer should be in released state because it is not expected to be
+ // read by any clients.
EXPECT_EQ(0, p->PostAsync(&metadata, invalid_fence));
- EXPECT_TRUE(IsBufferPosted(p->buffer_state()));
+ EXPECT_TRUE(IsBufferReleased(p->buffer_state()));
+ EXPECT_EQ(0, RETRY_EINTR(p->Poll(kPollTimeoutMs)));
- // Newly created consumer should be automatically sigalled.
+ // Newly created consumer will not be signalled for the posted buffer before
+ // its creation. It cannot acquire the buffer immediately.
std::unique_ptr<ConsumerBuffer> c =
ConsumerBuffer::Import(p->CreateConsumer());
ASSERT_TRUE(c.get() != nullptr);
- EXPECT_TRUE(IsBufferPosted(c->buffer_state()));
+ EXPECT_FALSE(IsClientPosted(c->buffer_state(), c->client_state_mask()));
+ EXPECT_EQ(-EBUSY, c->AcquireAsync(&metadata, &invalid_fence));
+
+ // Producer should be able to gain back and post the buffer
+ EXPECT_EQ(0, p->GainAsync());
+ EXPECT_EQ(0, p->PostAsync(&metadata, invalid_fence));
+
+ // Consumer should be able to pick up the buffer this time.
EXPECT_EQ(0, c->AcquireAsync(&metadata, &invalid_fence));
- EXPECT_TRUE(IsBufferAcquired(c->buffer_state()));
+ EXPECT_TRUE(IsClientAcquired(c->buffer_state(), c->client_state_mask()));
}
TEST_F(LibBufferHubTest, TestCreateConsumerWhenBufferReleased) {
@@ -479,6 +476,7 @@
ConsumerBuffer::Import(p->CreateConsumer());
ASSERT_TRUE(c1.get() != nullptr);
+ EXPECT_EQ(0, p->GainAsync());
DvrNativeBufferMetadata metadata;
LocalHandle invalid_fence;
@@ -503,7 +501,7 @@
EXPECT_TRUE(IsBufferReleased(p->buffer_state()));
EXPECT_EQ(0, p->GainAsync(&metadata, &invalid_fence));
- EXPECT_TRUE(IsBufferGained(p->buffer_state()));
+ EXPECT_TRUE(AnyClientGained(p->buffer_state()));
}
TEST_F(LibBufferHubTest, TestWithCustomMetadata) {
@@ -517,6 +515,7 @@
std::unique_ptr<ConsumerBuffer> c =
ConsumerBuffer::Import(p->CreateConsumer());
ASSERT_TRUE(c.get() != nullptr);
+ EXPECT_EQ(0, p->GainAsync());
Metadata m = {1, 3};
EXPECT_EQ(0, p->Post(LocalHandle(), &m, sizeof(Metadata)));
EXPECT_LE(0, RETRY_EINTR(c->Poll(kPollTimeoutMs)));
@@ -545,6 +544,7 @@
std::unique_ptr<ConsumerBuffer> c =
ConsumerBuffer::Import(p->CreateConsumer());
ASSERT_TRUE(c.get() != nullptr);
+ EXPECT_EQ(0, p->GainAsync());
// It is illegal to post metadata larger than originally requested during
// buffer allocation.
@@ -573,6 +573,7 @@
std::unique_ptr<ConsumerBuffer> c =
ConsumerBuffer::Import(p->CreateConsumer());
ASSERT_TRUE(c.get() != nullptr);
+ EXPECT_EQ(0, p->GainAsync());
Metadata m = {1, 3};
EXPECT_EQ(0, p->Post(LocalHandle(), &m, sizeof(m)));
@@ -598,6 +599,7 @@
std::unique_ptr<ConsumerBuffer> c =
ConsumerBuffer::Import(p->CreateConsumer());
ASSERT_TRUE(c.get() != nullptr);
+ EXPECT_EQ(0, p->GainAsync());
int64_t sequence = 3;
EXPECT_EQ(0, p->Post(LocalHandle(), &sequence, sizeof(sequence)));
@@ -613,6 +615,7 @@
std::unique_ptr<ConsumerBuffer> c =
ConsumerBuffer::Import(p->CreateConsumer());
ASSERT_TRUE(c.get() != nullptr);
+ EXPECT_EQ(0, p->GainAsync());
LocalHandle fence;
@@ -627,6 +630,7 @@
std::unique_ptr<ConsumerBuffer> c =
ConsumerBuffer::Import(p->CreateConsumer());
ASSERT_TRUE(c.get() != nullptr);
+ EXPECT_EQ(0, p->GainAsync());
int64_t sequence = 3;
EXPECT_NE(0, p->Post(LocalHandle(), &sequence, sizeof(sequence)));
@@ -648,6 +652,7 @@
std::unique_ptr<ConsumerBuffer> c =
ConsumerBuffer::Import(p->CreateConsumer());
ASSERT_TRUE(c.get() != nullptr);
+ EXPECT_EQ(0, p->GainAsync());
DvrNativeBufferMetadata meta;
LocalHandle f1(eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK));
@@ -711,44 +716,94 @@
ASSERT_TRUE(c1.get() != nullptr);
const uint64_t client_state_mask1 = c1->client_state_mask();
+ EXPECT_EQ(0, p->GainAsync());
DvrNativeBufferMetadata meta;
EXPECT_EQ(0, p->PostAsync(&meta, LocalHandle()));
LocalHandle fence;
EXPECT_LT(0, RETRY_EINTR(c1->Poll(kPollTimeoutMs)));
- EXPECT_LE(0, c1->AcquireAsync(&meta, &fence));
- // Destroy the consumer now will make it orphaned and the buffer is still
- // acquired.
- c1 = nullptr;
- EXPECT_GE(0, RETRY_EINTR(p->Poll(kPollTimeoutMs)));
+ EXPECT_EQ(0, c1->AcquireAsync(&meta, &fence));
+ // Destroy the consumer who has acquired but not released the buffer.
+ c1 = nullptr;
+
+ // The buffer is now available for the producer to gain.
+ EXPECT_LT(0, RETRY_EINTR(p->Poll(kPollTimeoutMs)));
+
+ // Newly added consumer is not able to acquire the buffer.
std::unique_ptr<ConsumerBuffer> c2 =
ConsumerBuffer::Import(p->CreateConsumer());
ASSERT_TRUE(c2.get() != nullptr);
const uint64_t client_state_mask2 = c2->client_state_mask();
EXPECT_NE(client_state_mask1, client_state_mask2);
+ EXPECT_EQ(0, RETRY_EINTR(c2->Poll(kPollTimeoutMs)));
+ EXPECT_EQ(-EBUSY, c2->AcquireAsync(&meta, &fence));
- // The new consumer is available for acquire.
+ // Producer should be able to gain.
+ EXPECT_EQ(0, p->GainAsync(&meta, &fence, false));
+}
+
+TEST_F(LibBufferHubTest, TestAcquireLastPosted) {
+ std::unique_ptr<ProducerBuffer> p = ProducerBuffer::Create(
+ kWidth, kHeight, kFormat, kUsage, sizeof(uint64_t));
+ ASSERT_TRUE(p.get() != nullptr);
+ std::unique_ptr<ConsumerBuffer> c1 =
+ ConsumerBuffer::Import(p->CreateConsumer());
+ ASSERT_TRUE(c1.get() != nullptr);
+ const uint64_t client_state_mask1 = c1->client_state_mask();
+
+ EXPECT_EQ(0, p->GainAsync());
+ DvrNativeBufferMetadata meta;
+ EXPECT_EQ(0, p->PostAsync(&meta, LocalHandle()));
+ EXPECT_LT(0, RETRY_EINTR(c1->Poll(kPollTimeoutMs)));
+
+ // c2 is created when the buffer is in posted state. buffer state for c1 is
+ // posted. Thus, c2 should be automatically set to posted and able to acquire.
+ std::unique_ptr<ConsumerBuffer> c2 =
+ ConsumerBuffer::Import(p->CreateConsumer());
+ ASSERT_TRUE(c2.get() != nullptr);
+ const uint64_t client_state_mask2 = c2->client_state_mask();
+ EXPECT_NE(client_state_mask1, client_state_mask2);
EXPECT_LT(0, RETRY_EINTR(c2->Poll(kPollTimeoutMs)));
- EXPECT_LE(0, c2->AcquireAsync(&meta, &fence));
- // Releasing the consumer makes the buffer gainable.
- EXPECT_EQ(0, c2->ReleaseAsync(&meta, LocalHandle()));
+ LocalHandle invalid_fence;
+ EXPECT_EQ(0, c2->AcquireAsync(&meta, &invalid_fence));
- // The buffer is now available for the producer to gain.
- EXPECT_LT(0, RETRY_EINTR(p->Poll(kPollTimeoutMs)));
+ EXPECT_EQ(0, c1->AcquireAsync(&meta, &invalid_fence));
- // But if another consumer is created in released state.
+ // c3 is created when the buffer is in acquired state. buffer state for c1 and
+ // c2 are acquired. Thus, c3 should be automatically set to posted and able to
+ // acquire.
std::unique_ptr<ConsumerBuffer> c3 =
ConsumerBuffer::Import(p->CreateConsumer());
ASSERT_TRUE(c3.get() != nullptr);
const uint64_t client_state_mask3 = c3->client_state_mask();
+ EXPECT_NE(client_state_mask1, client_state_mask3);
EXPECT_NE(client_state_mask2, client_state_mask3);
- // The consumer buffer is not acquirable.
- EXPECT_GE(0, RETRY_EINTR(c3->Poll(kPollTimeoutMs)));
- EXPECT_EQ(-EBUSY, c3->AcquireAsync(&meta, &fence));
+ EXPECT_LT(0, RETRY_EINTR(c3->Poll(kPollTimeoutMs)));
+ EXPECT_EQ(0, c3->AcquireAsync(&meta, &invalid_fence));
- // Producer should be able to gain no matter what.
- EXPECT_EQ(0, p->GainAsync(&meta, &fence));
+ // Releasing c2 and c3 in normal ways.
+ EXPECT_EQ(0, c2->Release(LocalHandle()));
+ EXPECT_EQ(0, c3->ReleaseAsync(&meta, LocalHandle()));
+
+ // Destroy the c1 who has not released the buffer.
+ c1 = nullptr;
+
+ // The buffer is now available for the producer to gain.
+ EXPECT_LT(0, RETRY_EINTR(p->Poll(kPollTimeoutMs)));
+
+ // C4 is created in released state. Thus, it cannot gain the just posted
+ // buffer.
+ std::unique_ptr<ConsumerBuffer> c4 =
+ ConsumerBuffer::Import(p->CreateConsumer());
+ ASSERT_TRUE(c4.get() != nullptr);
+ const uint64_t client_state_mask4 = c4->client_state_mask();
+ EXPECT_NE(client_state_mask3, client_state_mask4);
+ EXPECT_GE(0, RETRY_EINTR(c3->Poll(kPollTimeoutMs)));
+ EXPECT_EQ(-EBUSY, c3->AcquireAsync(&meta, &invalid_fence));
+
+ // Producer should be able to gain.
+ EXPECT_EQ(0, p->GainAsync(&meta, &invalid_fence));
}
TEST_F(LibBufferHubTest, TestDetachBufferFromProducer) {
@@ -767,6 +822,7 @@
int p_id = p->id();
// Detach in posted state should fail.
+ EXPECT_EQ(0, p->GainAsync());
EXPECT_EQ(0, p->PostAsync(&metadata, invalid_fence));
EXPECT_GT(RETRY_EINTR(c->Poll(kPollTimeoutMs)), 0);
auto s1 = p->Detach();
@@ -869,7 +925,8 @@
ASSERT_TRUE(p1.get() != nullptr);
int p1_id = p1->id();
- // Detached the producer.
+ // Detached the producer from gained state.
+ EXPECT_EQ(0, p1->GainAsync());
auto status_or_handle = p1->Detach();
EXPECT_TRUE(status_or_handle.ok());
LocalChannelHandle h1 = status_or_handle.take();
@@ -919,8 +976,8 @@
EXPECT_NE(b1->client_state_mask(), b2->client_state_mask());
// Both buffer instances should be in gained state.
- EXPECT_TRUE(IsBufferGained(b1->buffer_state()));
- EXPECT_TRUE(IsBufferGained(b2->buffer_state()));
+ EXPECT_TRUE(IsBufferReleased(b1->buffer_state()));
+ EXPECT_TRUE(IsBufferReleased(b2->buffer_state()));
// TODO(b/112338294) rewrite test after migration
return;
diff --git a/libs/vr/libbufferhub/buffer_hub_base.cpp b/libs/vr/libbufferhub/buffer_hub_base.cpp
index 68cc766..2dc427a 100644
--- a/libs/vr/libbufferhub/buffer_hub_base.cpp
+++ b/libs/vr/libbufferhub/buffer_hub_base.cpp
@@ -26,6 +26,8 @@
cid_(-1) {}
BufferHubBase::~BufferHubBase() {
+ // buffer_state and fence_state are not reset here. They will be used to
+ // clean up epoll fd if necessary in ProducerChannel::RemoveConsumer method.
if (metadata_header_ != nullptr) {
metadata_buffer_.Unlock();
}
diff --git a/libs/vr/libbufferhub/consumer_buffer.cpp b/libs/vr/libbufferhub/consumer_buffer.cpp
index 8e630ec..62fb5fd 100644
--- a/libs/vr/libbufferhub/consumer_buffer.cpp
+++ b/libs/vr/libbufferhub/consumer_buffer.cpp
@@ -35,17 +35,41 @@
if (!out_meta)
return -EINVAL;
- // 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(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 ".",
- id(), buffer_state, client_state_mask());
+ // The buffer can be acquired iff the buffer state for this client is posted.
+ uint64_t current_buffer_state =
+ buffer_state_->load(std::memory_order_acquire);
+ if (!BufferHubDefs::IsClientPosted(current_buffer_state,
+ client_state_mask())) {
+ ALOGE(
+ "%s: Failed to acquire the buffer. The buffer is not posted, id=%d "
+ "state=%" PRIx64 " client_state_mask=%" PRIx64 ".",
+ __FUNCTION__, id(), current_buffer_state, client_state_mask());
return -EBUSY;
}
+ // Change the buffer state for this consumer from posted to acquired.
+ uint64_t updated_buffer_state = current_buffer_state ^ client_state_mask();
+ while (!buffer_state_->compare_exchange_weak(
+ current_buffer_state, updated_buffer_state, std::memory_order_acq_rel,
+ std::memory_order_acquire)) {
+ ALOGD(
+ "%s Failed to acquire the buffer. Current buffer state was changed to "
+ "%" PRIx64
+ " when trying to acquire the buffer and modify the buffer state to "
+ "%" PRIx64 ". About to try again if the buffer is still posted.",
+ __FUNCTION__, current_buffer_state, updated_buffer_state);
+ if (!BufferHubDefs::IsClientPosted(current_buffer_state,
+ client_state_mask())) {
+ ALOGE(
+ "%s: Failed to acquire the buffer. The buffer is no longer posted, "
+ "id=%d state=%" PRIx64 " client_state_mask=%" PRIx64 ".",
+ __FUNCTION__, id(), current_buffer_state, client_state_mask());
+ return -EBUSY;
+ }
+ // The failure of compare_exchange_weak updates current_buffer_state.
+ updated_buffer_state = current_buffer_state ^ client_state_mask();
+ }
+
// Copy the canonical metadata.
void* metadata_ptr = reinterpret_cast<void*>(&metadata_header_->metadata);
memcpy(out_meta, metadata_ptr, sizeof(DvrNativeBufferMetadata));
@@ -64,8 +88,6 @@
*out_fence = shared_acquire_fence_.Duplicate();
}
- // Set the consumer bit unique to this consumer.
- BufferHubDefs::ModifyBufferState(buffer_state_, 0ULL, client_state_mask());
return 0;
}
@@ -118,12 +140,26 @@
if (const int error = CheckMetadata(meta->user_metadata_size))
return error;
- // Check invalid state transition.
- 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);
- return -EBUSY;
+ // Set the buffer state of this client to released if it is not already in
+ // released state.
+ uint64_t current_buffer_state =
+ buffer_state_->load(std::memory_order_acquire);
+ if (BufferHubDefs::IsClientReleased(current_buffer_state,
+ client_state_mask())) {
+ return 0;
+ }
+ uint64_t updated_buffer_state = current_buffer_state & (~client_state_mask());
+ while (!buffer_state_->compare_exchange_weak(
+ current_buffer_state, updated_buffer_state, std::memory_order_acq_rel,
+ std::memory_order_acquire)) {
+ ALOGD(
+ "%s: Failed to release the buffer. Current buffer state was changed to "
+ "%" PRIx64
+ " when trying to release the buffer and modify the buffer state to "
+ "%" PRIx64 ". About to try again.",
+ __FUNCTION__, current_buffer_state, updated_buffer_state);
+ // The failure of compare_exchange_weak updates current_buffer_state.
+ updated_buffer_state = current_buffer_state & (~client_state_mask());
}
// On release, only the user requested metadata is copied back into the shared
@@ -141,8 +177,6 @@
if (const int error = UpdateSharedFence(release_fence, shared_release_fence_))
return error;
- // For release operation, the client don't need to change the state as it's
- // bufferhubd's job to flip the produer bit once all consumers are released.
return 0;
}
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 650da97..400def7 100644
--- a/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h
+++ b/libs/vr/libbufferhub/include/private/dvr/buffer_hub_defs.h
@@ -20,52 +20,104 @@
static constexpr uint32_t kMetadataUsage =
GRALLOC_USAGE_SW_READ_OFTEN | GRALLOC_USAGE_SW_WRITE_OFTEN;
-// Single producuer multiple (up to 63) consumers ownership signal.
+// Single buffer clients (up to 32) ownership signal.
// 64-bit atomic unsigned int.
+// Each client takes 2 bits. The first bit locates in the first 32 bits of
+// buffer_state; the second bit locates in the last 32 bits of buffer_state.
+// Client states:
+// Gained state 11. Exclusive write state.
+// Posted state 10.
+// Acquired state 01. Shared read state.
+// Released state 00.
//
-// MSB LSB
-// | |
-// v v
-// [C62|...|C1|C0|P]
-// Gain'ed state: [..|0|0|0] -> Exclusively Writable.
-// Post'ed state: [..|0|0|1]
-// Acquired'ed state: [..|X|X|1] -> At least one bit is set in higher 63 bits
-// Released'ed state: [..|X|X|0] -> At least one bit is set in higher 63 bits
-static constexpr int kMaxNumberOfClients = 64;
-static constexpr uint64_t kFirstClientBitMask = 1ULL;
-static constexpr uint64_t kConsumerStateMask = ~kFirstClientBitMask;
+// MSB LSB
+// | |
+// v v
+// [C31|...|C1|C0|C31| ... |C1|C0]
-static inline void ModifyBufferState(std::atomic<uint64_t>* buffer_state,
- uint64_t clear_mask, uint64_t set_mask) {
- uint64_t old_state;
- uint64_t new_state;
- do {
- 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));
+// Maximum number of clients a buffer can have.
+static constexpr int kMaxNumberOfClients = 32;
+
+// Definition of bit masks.
+// MSB LSB
+// | kHighBitsMask | kLowbitsMask |
+// v v v
+// [b63| ... |b32|b31| ... |b0]
+
+// The location of lower 32 bits in the 64-bit buffer state.
+static constexpr uint64_t kLowbitsMask = (1ULL << kMaxNumberOfClients) - 1ULL;
+
+// The location of higher 32 bits in the 64-bit buffer state.
+static constexpr uint64_t kHighBitsMask = ~kLowbitsMask;
+
+// The client bit mask of the first client.
+static constexpr uint64_t kFirstClientBitMask =
+ (1ULL << kMaxNumberOfClients) + 1ULL;
+
+// Returns true if any of the client is in gained state.
+static inline bool AnyClientGained(uint64_t state) {
+ uint64_t high_bits = state >> kMaxNumberOfClients;
+ uint64_t low_bits = state & kLowbitsMask;
+ return high_bits == low_bits && low_bits != 0ULL;
}
-static inline bool IsBufferGained(uint64_t state) { return state == 0; }
-
-static inline bool IsBufferPosted(uint64_t state,
- uint64_t consumer_bit = kConsumerStateMask) {
- return (state & kFirstClientBitMask) && !(state & consumer_bit);
+// Returns true if the input client is in gained state.
+static inline bool IsClientGained(uint64_t state, uint64_t client_bit_mask) {
+ return state == client_bit_mask;
}
-static inline bool IsBufferAcquired(uint64_t state) {
- return (state & kFirstClientBitMask) && (state & kConsumerStateMask);
+// Returns true if any of the client is in posted state.
+static inline bool AnyClientPosted(uint64_t state) {
+ uint64_t high_bits = state >> kMaxNumberOfClients;
+ uint64_t low_bits = state & kLowbitsMask;
+ uint64_t posted_or_acquired = high_bits ^ low_bits;
+ return posted_or_acquired & high_bits;
}
-static inline bool IsBufferReleased(uint64_t state) {
- return !(state & kFirstClientBitMask) && (state & kConsumerStateMask);
+// Returns true if the input client is in posted state.
+static inline bool IsClientPosted(uint64_t state, uint64_t client_bit_mask) {
+ uint64_t client_bits = state & client_bit_mask;
+ if (client_bits == 0ULL)
+ return false;
+ uint64_t low_bits = client_bits & kLowbitsMask;
+ return low_bits == 0ULL;
}
-static inline uint64_t FindNextAvailableClientStateMask(uint64_t bits) {
- return ~bits - (~bits & (~bits - 1));
+// Return true if any of the client is in acquired state.
+static inline bool AnyClientAcquired(uint64_t state) {
+ uint64_t high_bits = state >> kMaxNumberOfClients;
+ uint64_t low_bits = state & kLowbitsMask;
+ uint64_t posted_or_acquired = high_bits ^ low_bits;
+ return posted_or_acquired & low_bits;
}
-static inline uint64_t FindFirstClearedBit() {
- return FindNextAvailableClientStateMask(kFirstClientBitMask);
+// Return true if the input client is in acquired state.
+static inline bool IsClientAcquired(uint64_t state, uint64_t client_bit_mask) {
+ uint64_t client_bits = state & client_bit_mask;
+ if (client_bits == 0ULL)
+ return false;
+ uint64_t high_bits = client_bits & kHighBitsMask;
+ return high_bits == 0ULL;
+}
+
+// Returns true if all clients are in released state.
+static inline bool IsBufferReleased(uint64_t state) { return state == 0ULL; }
+
+// Returns true if the input client is in released state.
+static inline bool IsClientReleased(uint64_t state, uint64_t client_bit_mask) {
+ return (state & client_bit_mask) == 0ULL;
+}
+
+// Returns the next available buffer client's client_state_masks.
+// @params union_bits. Union of all existing clients' client_state_masks.
+static inline uint64_t FindNextAvailableClientStateMask(uint64_t union_bits) {
+ uint64_t low_union = union_bits & kLowbitsMask;
+ if (low_union == kLowbitsMask)
+ return 0ULL;
+ uint64_t incremented = low_union + 1ULL;
+ uint64_t difference = incremented ^ low_union;
+ uint64_t new_low_bit = (difference + 1ULL) >> 1;
+ return new_low_bit + (new_low_bit << kMaxNumberOfClients);
}
struct __attribute__((packed, aligned(8))) MetadataHeader {
@@ -74,9 +126,21 @@
// part is subject for future updates, it's not stable cross Android version,
// so don't have it visible from outside of the Android platform (include Apps
// and vendor HAL).
+
+ // Every client takes up one bit from the higher 32 bits and one bit from the
+ // lower 32 bits in buffer_state.
std::atomic<uint64_t> buffer_state;
+
+ // Every client takes up one bit in fence_state. Only the lower 32 bits are
+ // valid. The upper 32 bits are there for easier manipulation, but the value
+ // should be ignored.
std::atomic<uint64_t> fence_state;
+
+ // Every client takes up one bit from the higher 32 bits and one bit from the
+ // lower 32 bits in active_clients_bit_mask.
std::atomic<uint64_t> active_clients_bit_mask;
+
+ // The index of the buffer queue where the buffer belongs to.
uint64_t queue_index;
// Public data format, which should be updated with caution. See more details
diff --git a/libs/vr/libbufferhub/include/private/dvr/consumer_buffer.h b/libs/vr/libbufferhub/include/private/dvr/consumer_buffer.h
index 7349779..7aa50b1 100644
--- a/libs/vr/libbufferhub/include/private/dvr/consumer_buffer.h
+++ b/libs/vr/libbufferhub/include/private/dvr/consumer_buffer.h
@@ -48,9 +48,8 @@
// Asynchronously acquires a bufer.
int AcquireAsync(DvrNativeBufferMetadata* out_meta, LocalHandle* out_fence);
- // This should be called after a successful Acquire call. If the fence is
- // valid the fence determines the buffer usage, otherwise the buffer is
- // released immediately.
+ // Releases the buffer from any buffer state. If the fence is valid the fence
+ // determines the buffer usage, otherwise the buffer is released immediately.
// This returns zero or a negative unix error code.
int Release(const LocalHandle& release_fence);
int ReleaseAsync();
diff --git a/libs/vr/libbufferhub/producer_buffer.cpp b/libs/vr/libbufferhub/producer_buffer.cpp
index f36e169..1bfdc8f 100644
--- a/libs/vr/libbufferhub/producer_buffer.cpp
+++ b/libs/vr/libbufferhub/producer_buffer.cpp
@@ -79,14 +79,43 @@
if (const int error = CheckMetadata(meta->user_metadata_size))
return error;
- // Check invalid state transition.
- 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);
+ // The buffer can be posted iff the buffer state for this client is gained.
+ uint64_t current_buffer_state =
+ buffer_state_->load(std::memory_order_acquire);
+ if (!BufferHubDefs::IsClientGained(current_buffer_state,
+ client_state_mask())) {
+ ALOGE("%s: not gained, id=%d state=%" PRIx64 ".", __FUNCTION__, id(),
+ current_buffer_state);
return -EBUSY;
}
+ // Set the producer client buffer state to released, other clients' buffer
+ // state to posted.
+ uint64_t current_active_clients_bit_mask =
+ active_clients_bit_mask_->load(std::memory_order_acquire);
+ uint64_t updated_buffer_state = current_active_clients_bit_mask &
+ (~client_state_mask()) &
+ BufferHubDefs::kHighBitsMask;
+ while (!buffer_state_->compare_exchange_weak(
+ current_buffer_state, updated_buffer_state, std::memory_order_acq_rel,
+ std::memory_order_acquire)) {
+ ALOGD(
+ "%s: Failed to post the buffer. Current buffer state was changed to "
+ "%" PRIx64
+ " when trying to post the buffer and modify the buffer state to "
+ "%" PRIx64
+ ". About to try again if the buffer is still gained by this client.",
+ __FUNCTION__, current_buffer_state, updated_buffer_state);
+ if (!BufferHubDefs::IsClientGained(current_buffer_state,
+ client_state_mask())) {
+ ALOGE(
+ "%s: Failed to post the buffer. The buffer is no longer gained, "
+ "id=%d state=%" PRIx64 ".",
+ __FUNCTION__, id(), current_buffer_state);
+ return -EBUSY;
+ }
+ }
+
// Copy the canonical metadata.
void* metadata_ptr = reinterpret_cast<void*>(&metadata_header_->metadata);
memcpy(metadata_ptr, meta, sizeof(DvrNativeBufferMetadata));
@@ -101,10 +130,6 @@
if (const int error = UpdateSharedFence(ready_fence, shared_acquire_fence_))
return error;
- // Set the producer bit atomically to transit into posted state.
- // The producer state bit mask is kFirstClientBitMask for now.
- BufferHubDefs::ModifyBufferState(buffer_state_, 0ULL,
- BufferHubDefs::kFirstClientBitMask);
return 0;
}
@@ -136,25 +161,50 @@
int ProducerBuffer::LocalGain(DvrNativeBufferMetadata* out_meta,
LocalHandle* out_fence, bool gain_posted_buffer) {
- uint64_t buffer_state = buffer_state_->load(std::memory_order_acquire);
- ALOGD_IF(TRACE, "ProducerBuffer::LocalGain: buffer=%d, state=%" PRIx64 ".",
- id(), buffer_state);
-
if (!out_meta)
return -EINVAL;
- if (BufferHubDefs::IsBufferGained(buffer_state)) {
- // We don't want to log error when gaining a newly allocated
- // buffer.
- ALOGI("ProducerBuffer::LocalGain: already gained id=%d.", id());
+ uint64_t current_buffer_state =
+ buffer_state_->load(std::memory_order_acquire);
+ ALOGD_IF(TRACE, "%s: buffer=%d, state=%" PRIx64 ".", __FUNCTION__, id(),
+ current_buffer_state);
+
+ if (BufferHubDefs::IsClientGained(current_buffer_state,
+ client_state_mask())) {
+ ALOGI("%s: already gained id=%d.", __FUNCTION__, id());
return -EALREADY;
}
- if (BufferHubDefs::IsBufferAcquired(buffer_state) ||
- (BufferHubDefs::IsBufferPosted(buffer_state) && !gain_posted_buffer)) {
- ALOGE("ProducerBuffer::LocalGain: not released id=%d state=%" PRIx64 ".",
- id(), buffer_state);
+ if (BufferHubDefs::AnyClientAcquired(current_buffer_state) ||
+ (BufferHubDefs::AnyClientPosted(current_buffer_state) &&
+ !gain_posted_buffer)) {
+ ALOGE("%s: not released id=%d state=%" PRIx64 ".", __FUNCTION__, id(),
+ current_buffer_state);
return -EBUSY;
}
+ // Change the buffer state to gained state.
+ uint64_t updated_buffer_state = client_state_mask();
+ while (!buffer_state_->compare_exchange_weak(
+ current_buffer_state, updated_buffer_state, std::memory_order_acq_rel,
+ std::memory_order_acquire)) {
+ ALOGD(
+ "%s: Failed to gain the buffer. Current buffer state was changed to "
+ "%" PRIx64
+ " when trying to gain the buffer and modify the buffer state to "
+ "%" PRIx64
+ ". About to try again if the buffer is still not read by other "
+ "clients.",
+ __FUNCTION__, current_buffer_state, updated_buffer_state);
+
+ if (BufferHubDefs::AnyClientAcquired(current_buffer_state) ||
+ (BufferHubDefs::AnyClientPosted(current_buffer_state) &&
+ !gain_posted_buffer)) {
+ ALOGE(
+ "%s: Failed to gain the buffer. The buffer is no longer released. "
+ "id=%d state=%" PRIx64 ".",
+ __FUNCTION__, id(), current_buffer_state);
+ return -EBUSY;
+ }
+ }
// Canonical metadata is undefined on Gain. Except for user_metadata and
// release_fence_mask. Fill in the user_metadata_ptr in address space of the
@@ -169,16 +219,20 @@
out_meta->user_metadata_ptr = 0;
}
- uint64_t fence_state = fence_state_->load(std::memory_order_acquire);
+ uint64_t current_fence_state = fence_state_->load(std::memory_order_acquire);
+ uint64_t current_active_clients_bit_mask =
+ active_clients_bit_mask_->load(std::memory_order_acquire);
// If there is an release fence from consumer, we need to return it.
- if (fence_state & BufferHubDefs::kConsumerStateMask) {
+ // TODO(b/112007999) add an atomic variable in metadata header in shared
+ // memory to indicate which client is the last producer of the buffer.
+ // Currently, assume the first client is the only producer to the buffer.
+ if (current_fence_state & current_active_clients_bit_mask &
+ (~BufferHubDefs::kFirstClientBitMask)) {
*out_fence = shared_release_fence_.Duplicate();
out_meta->release_fence_mask =
- fence_state & BufferHubDefs::kConsumerStateMask;
+ current_fence_state & current_active_clients_bit_mask;
}
- // Clear out all bits and the buffer is now back to gained state.
- buffer_state_->store(0ULL);
return 0;
}
@@ -232,7 +286,8 @@
// TODO(b/112338294) Keep here for reference. Remove it after new logic is
// written.
/* uint64_t buffer_state = buffer_state_->load(std::memory_order_acquire);
- if (!BufferHubDefs::IsBufferGained(buffer_state)) {
+ if (!BufferHubDefs::IsClientGained(
+ buffer_state, BufferHubDefs::kFirstClientStateMask)) {
// Can only detach a ProducerBuffer when it's in gained state.
ALOGW("ProducerBuffer::Detach: The buffer (id=%d, state=0x%" PRIx64
") is not in gained state.",
diff --git a/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp b/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp
index f7942d0..9c4f73f 100644
--- a/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp
+++ b/libs/vr/libbufferhubqueue/buffer_hub_queue_client.cpp
@@ -532,7 +532,8 @@
Status<size_t> ProducerQueue::InsertBuffer(
const std::shared_ptr<BufferProducer>& buffer) {
if (buffer == nullptr ||
- !BufferHubDefs::IsBufferGained(buffer->buffer_state())) {
+ !BufferHubDefs::IsClientGained(buffer->buffer_state(),
+ buffer->client_state_mask())) {
ALOGE(
"ProducerQueue::InsertBuffer: Can only insert a buffer when it's in "
"gained state.");
@@ -637,7 +638,7 @@
static_cast<int>(*slot));
return ErrorStatus(EIO);
}
- if (!BufferHubDefs::IsBufferAcquired(buffer->buffer_state())) {
+ if (!BufferHubDefs::AnyClientAcquired(buffer->buffer_state())) {
*slot = *iter;
unavailable_buffers_slot_.erase(iter);
unavailable_buffers_slot_.push_back(*slot);
diff --git a/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp b/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp
index 874eb3a..fd6ca43 100644
--- a/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp
+++ b/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp
@@ -112,7 +112,7 @@
// Consumer acquires a buffer.
auto c1_status = consumer_queue_->Dequeue(kTimeoutMs, &slot, &mo, &fence);
- EXPECT_TRUE(c1_status.ok());
+ EXPECT_TRUE(c1_status.ok()) << c1_status.GetErrorMessage();
auto c1 = c1_status.take();
ASSERT_NE(c1, nullptr);
EXPECT_EQ(mi.index, i);
@@ -334,6 +334,7 @@
std::shared_ptr<BufferProducer> p1 = BufferProducer::Create(
kBufferWidth, kBufferHeight, kBufferFormat, kBufferUsage, 0);
ASSERT_TRUE(p1 != nullptr);
+ ASSERT_EQ(p1->GainAsync(), 0);
// Inserting a posted buffer will fail.
DvrNativeBufferMetadata meta;
@@ -345,9 +346,10 @@
// Inserting a gained buffer will succeed.
std::shared_ptr<BufferProducer> p2 = BufferProducer::Create(
kBufferWidth, kBufferHeight, kBufferFormat, kBufferUsage);
+ ASSERT_EQ(p2->GainAsync(), 0);
ASSERT_TRUE(p2 != nullptr);
status_or_slot = producer_queue_->InsertBuffer(p2);
- EXPECT_TRUE(status_or_slot.ok());
+ EXPECT_TRUE(status_or_slot.ok()) << status_or_slot.GetErrorMessage();
// This is the first buffer inserted, should take slot 0.
size_t slot = status_or_slot.get();
EXPECT_EQ(slot, 0);
@@ -585,7 +587,7 @@
mi.user_metadata_ptr = reinterpret_cast<uint64_t>(&user_metadata);
EXPECT_EQ(p1->PostAsync(&mi, {}), 0);
auto c1_status = consumer_queue_->Dequeue(kTimeoutMs, &slot, &mo, &fence);
- EXPECT_TRUE(c1_status.ok());
+ EXPECT_TRUE(c1_status.ok()) << c1_status.GetErrorMessage();
auto c1 = c1_status.take();
ASSERT_NE(c1, nullptr);
@@ -689,7 +691,7 @@
size_t cs1, cs2;
auto c1_status = consumer_queue_->Dequeue(kTimeoutMs, &cs1, &mo, &fence);
- ASSERT_TRUE(c1_status.ok());
+ ASSERT_TRUE(c1_status.ok()) << c1_status.GetErrorMessage();
auto c1 = c1_status.take();
ASSERT_NE(c1, nullptr);
ASSERT_EQ(consumer_queue_->count(), 0U);
@@ -905,7 +907,7 @@
ASSERT_NE(producer_buffer, nullptr);
ASSERT_EQ(producer_buffer->PostAsync(&mi, fence), 0);
consumer_status = consumer_queue_->Dequeue(kTimeoutMs, &slot, &mo, &fence);
- ASSERT_TRUE(consumer_status.ok());
+ ASSERT_TRUE(consumer_status.ok()) << consumer_status.GetErrorMessage();
}
status = producer_queue_->FreeAllBuffers();
@@ -999,7 +1001,7 @@
// Make sure the buffer can be dequeued from consumer side.
auto s4 = consumer_queue_->Dequeue(kTimeoutMs, &slot, &consumer_meta, &fence);
- EXPECT_TRUE(s4.ok());
+ EXPECT_TRUE(s4.ok()) << s4.GetErrorMessage();
EXPECT_EQ(consumer_queue_->capacity(), 1U);
auto consumer = s4.take();
@@ -1066,7 +1068,7 @@
// Make sure the buffer can be dequeued from consumer side.
auto s3 = consumer_queue_->Dequeue(kTimeoutMs, &slot, &consumer_meta, &fence);
- EXPECT_TRUE(s3.ok());
+ EXPECT_TRUE(s3.ok()) << s3.GetErrorMessage();
EXPECT_EQ(consumer_queue_->capacity(), 1U);
auto consumer = s3.take();
diff --git a/services/vr/bufferhubd/Android.bp b/services/vr/bufferhubd/Android.bp
index b5e6bb4..0a59f29 100644
--- a/services/vr/bufferhubd/Android.bp
+++ b/services/vr/bufferhubd/Android.bp
@@ -75,5 +75,3 @@
name: "bufferhubd",
init_rc: ["bufferhubd.rc"],
}
-
-subdirs = ["tests"]
\ No newline at end of file
diff --git a/services/vr/bufferhubd/include/private/dvr/producer_channel.h b/services/vr/bufferhubd/include/private/dvr/producer_channel.h
index 3ad9c70..6239886 100644
--- a/services/vr/bufferhubd/include/private/dvr/producer_channel.h
+++ b/services/vr/bufferhubd/include/private/dvr/producer_channel.h
@@ -63,7 +63,7 @@
LocalFence release_fence);
void DecrementPendingConsumers();
- void OnConsumerOrphaned(ConsumerChannel* channel);
+ void OnConsumerOrphaned(const uint64_t& consumer_state_mask);
void AddConsumer(ConsumerChannel* channel);
void RemoveConsumer(ConsumerChannel* channel);
diff --git a/services/vr/bufferhubd/producer_channel.cpp b/services/vr/bufferhubd/producer_channel.cpp
index c6e8ea9..55034b3 100644
--- a/services/vr/bufferhubd/producer_channel.cpp
+++ b/services/vr/bufferhubd/producer_channel.cpp
@@ -255,16 +255,16 @@
// 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 client_state_mask = BufferHubDefs::FindNextAvailableClientStateMask(
- current_active_clients_bit_mask | orphaned_consumer_bit_mask_);
- if (client_state_mask == 0ULL) {
- ALOGE(
- "ProducerChannel::CreateConsumer: reached the maximum mumber of "
- "consumers per producer: 63.");
+ uint64_t consumer_state_mask =
+ BufferHubDefs::FindNextAvailableClientStateMask(
+ current_active_clients_bit_mask | orphaned_consumer_bit_mask_);
+ if (consumer_state_mask == 0ULL) {
+ ALOGE("%s: reached the maximum mumber of consumers per producer: 63.",
+ __FUNCTION__);
return ErrorStatus(E2BIG);
}
uint64_t updated_active_clients_bit_mask =
- current_active_clients_bit_mask | client_state_mask;
+ current_active_clients_bit_mask | consumer_state_mask;
// 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
@@ -276,24 +276,24 @@
while (!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 %" PRIx64
+ ALOGE("%s: Current active clients bit mask is changed to %" PRIx64
", which was expected to be %" PRIx64
". Trying to generate a new client state mask to resolve race "
"condition.",
- updated_active_clients_bit_mask, current_active_clients_bit_mask);
- client_state_mask = BufferHubDefs::FindNextAvailableClientStateMask(
+ __FUNCTION__, updated_active_clients_bit_mask,
+ current_active_clients_bit_mask);
+ consumer_state_mask = BufferHubDefs::FindNextAvailableClientStateMask(
current_active_clients_bit_mask | orphaned_consumer_bit_mask_);
- if (client_state_mask == 0ULL) {
- ALOGE(
- "ProducerChannel::CreateConsumer: reached the maximum mumber of "
- "consumers per producer: 63.");
+ if (consumer_state_mask == 0ULL) {
+ ALOGE("%s: reached the maximum mumber of consumers per producer: %d.",
+ __FUNCTION__, (BufferHubDefs::kMaxNumberOfClients - 1));
return ErrorStatus(E2BIG);
}
updated_active_clients_bit_mask =
- current_active_clients_bit_mask | client_state_mask;
+ current_active_clients_bit_mask | consumer_state_mask;
}
- return {client_state_mask};
+ return {consumer_state_mask};
}
void ProducerChannel::RemoveConsumerClientMask(uint64_t consumer_state_mask) {
@@ -311,17 +311,15 @@
Status<RemoteChannelHandle> ProducerChannel::CreateConsumer(
Message& message, uint64_t consumer_state_mask) {
- ATRACE_NAME("ProducerChannel::CreateConsumer");
- ALOGD_IF(TRACE,
- "ProducerChannel::CreateConsumer: buffer_id=%d, producer_owns=%d",
+ ATRACE_NAME(__FUNCTION__);
+ ALOGD_IF(TRACE, "%s: buffer_id=%d, producer_owns=%d", __FUNCTION__,
buffer_id(), producer_owns_);
int channel_id;
auto status = message.PushChannel(0, nullptr, &channel_id);
if (!status) {
- ALOGE(
- "ProducerChannel::CreateConsumer: Failed to push consumer channel: %s",
- status.GetErrorMessage().c_str());
+ ALOGE("%s: Failed to push consumer channel: %s", __FUNCTION__,
+ status.GetErrorMessage().c_str());
RemoveConsumerClientMask(consumer_state_mask);
return ErrorStatus(ENOMEM);
}
@@ -331,24 +329,52 @@
shared_from_this());
const auto channel_status = service()->SetChannel(channel_id, consumer);
if (!channel_status) {
- ALOGE(
- "ProducerChannel::CreateConsumer: failed to set new consumer channel: "
- "%s",
- channel_status.GetErrorMessage().c_str());
+ ALOGE("%s: failed to set new consumer channel: %s.", __FUNCTION__,
+ channel_status.GetErrorMessage().c_str());
RemoveConsumerClientMask(consumer_state_mask);
return ErrorStatus(ENOMEM);
}
uint64_t current_buffer_state =
buffer_state_->load(std::memory_order_acquire);
- if (!producer_owns_ &&
- (BufferHubDefs::IsBufferPosted(current_buffer_state) ||
- BufferHubDefs::IsBufferAcquired(current_buffer_state))) {
- // Signal the new consumer when adding it to a posted producer.
- if (consumer->OnProducerPosted())
- pending_consumers_++;
+ if (BufferHubDefs::IsBufferReleased(current_buffer_state) ||
+ BufferHubDefs::AnyClientGained(current_buffer_state)) {
+ return {status.take()};
}
+ // Signal the new consumer when adding it to a posted producer.
+ bool update_buffer_state = true;
+ if (!BufferHubDefs::IsClientPosted(current_buffer_state,
+ consumer_state_mask)) {
+ uint64_t updated_buffer_state =
+ current_buffer_state ^
+ (consumer_state_mask & BufferHubDefs::kHighBitsMask);
+ while (!buffer_state_->compare_exchange_weak(
+ current_buffer_state, updated_buffer_state, std::memory_order_acq_rel,
+ std::memory_order_acquire)) {
+ ALOGI(
+ "%s: Failed to post to the new consumer. "
+ "Current buffer state was changed to %" PRIx64
+ " when trying to acquire the buffer and modify the buffer state to "
+ "%" PRIx64
+ ". About to try again if the buffer is still not gained nor fully "
+ "released.",
+ __FUNCTION__, current_buffer_state, updated_buffer_state);
+ if (BufferHubDefs::IsBufferReleased(current_buffer_state) ||
+ BufferHubDefs::AnyClientGained(current_buffer_state)) {
+ ALOGI("%s: buffer is gained or fully released, state=%" PRIx64 ".",
+ __FUNCTION__, current_buffer_state);
+ update_buffer_state = false;
+ break;
+ }
+ updated_buffer_state =
+ current_buffer_state ^
+ (consumer_state_mask & BufferHubDefs::kHighBitsMask);
+ }
+ }
+ if (update_buffer_state && consumer->OnProducerPosted())
+ pending_consumers_++;
+
return {status.take()};
}
@@ -422,7 +448,8 @@
if (producer_owns_) {
ALOGE("ProducerChanneL::OnGain: Already in gained state: channel=%d",
channel_id());
- return ErrorStatus(EALREADY);
+ // TODO(b/119331650): remove this if check if producer_owns_ is removed.
+ // return ErrorStatus(EALREADY);
}
// There are still pending consumers, return busy.
@@ -431,7 +458,10 @@
"ProducerChannel::OnGain: Producer (id=%d) is gaining a buffer that "
"still has %d pending consumer(s).",
buffer_id(), pending_consumers_);
- return ErrorStatus(EBUSY);
+ // 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();
@@ -449,10 +479,12 @@
buffer_id());
uint64_t buffer_state = buffer_state_->load(std::memory_order_acquire);
- if (!BufferHubDefs::IsBufferGained(buffer_state)) {
+ if (!BufferHubDefs::IsClientGained(
+ buffer_state, BufferHubDefs::kFirstClientStateMask)) {
// Can only detach a BufferProducer when it's in gained state.
ALOGW(
- "ProducerChannel::OnProducerDetach: The buffer (id=%d, state=0x%" PRIx64
+ "ProducerChannel::OnProducerDetach: The buffer (id=%d, state=%"
+ PRIx64
") is not in gained state.",
buffer_id(), buffer_state);
return {};
@@ -485,12 +517,11 @@
const auto channel_status =
service()->SetChannel(channel_id, std::move(channel));
if (!channel_status) {
- // Technically, this should never fail, as we just pushed the channel. Note
- // that LOG_FATAL will be stripped out in non-debug build.
+ // Technically, this should never fail, as we just pushed the channel.
+ // Note that LOG_FATAL will be stripped out in non-debug build.
LOG_FATAL(
- "ProducerChannel::OnProducerDetach: Failed to set new detached buffer "
- "channel: %s.",
- channel_status.GetErrorMessage().c_str());
+ "ProducerChannel::OnProducerDetach: Failed to set new detached "
+ "buffer channel: %s.", channel_status.GetErrorMessage().c_str());
}
return status;
@@ -505,8 +536,8 @@
return ErrorStatus(EBUSY);
}
- // Return a borrowed fd to avoid unnecessary duplication of the underlying fd.
- // Serialization just needs to read the handle.
+ // Return a borrowed fd to avoid unnecessary duplication of the underlying
+ // fd. Serialization just needs to read the handle.
return {std::move(post_fence_)};
}
@@ -539,28 +570,13 @@
}
DecrementPendingConsumers();
- if (pending_consumers_ == 0) {
- // Clear the producer bit atomically to transit into released state. This
- // has to done by BufferHub as it requries synchronization among all
- // consumers.
- BufferHubDefs::ModifyBufferState(buffer_state_,
- BufferHubDefs::kFirstClientBitMask, 0ULL);
- ALOGD_IF(TRACE,
- "ProducerChannel::OnConsumerRelease: releasing last consumer: "
- "buffer_id=%d state=%" PRIx64 ".",
- buffer_id(), buffer_state_->load(std::memory_order_acquire));
-
- if (orphaned_consumer_bit_mask_) {
- ALOGW(
- "ProducerChannel::OnConsumerRelease: orphaned buffer detected "
- "during the this acquire/release cycle: id=%d orphaned=0x%" PRIx64
- " queue_index=%" PRIu64 ".",
- buffer_id(), orphaned_consumer_bit_mask_,
- metadata_header_->queue_index);
- orphaned_consumer_bit_mask_ = 0;
- }
-
- SignalAvailable();
+ 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;
}
ALOGE_IF(
@@ -579,34 +595,43 @@
}
--pending_consumers_;
- ALOGD_IF(TRACE,
- "ProducerChannel::DecrementPendingConsumers: buffer_id=%d %d "
- "consumers left",
+ 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(ConsumerChannel* channel) {
+void ProducerChannel::OnConsumerOrphaned(const uint64_t& consumer_state_mask) {
// Ignore the orphaned consumer.
DecrementPendingConsumers();
- const uint64_t client_state_mask = channel->client_state_mask();
- ALOGE_IF(orphaned_consumer_bit_mask_ & client_state_mask,
- "ProducerChannel::OnConsumerOrphaned: Consumer "
- "(client_state_mask=%" PRIx64 ") is already orphaned.",
- client_state_mask);
- orphaned_consumer_bit_mask_ |= client_state_mask;
+ // 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,
+ "%s: Consumer (consumer_state_mask=%" PRIx64
+ ") is already orphaned.",
+ __FUNCTION__, consumer_state_mask);
+ orphaned_consumer_bit_mask_ |= consumer_state_mask;
// Atomically clear the fence state bit as an orphaned consumer will never
- // signal a release fence. Also clear the buffer state as it won't be released
- // as well.
- fence_state_->fetch_and(~client_state_mask);
- BufferHubDefs::ModifyBufferState(buffer_state_, client_state_mask, 0ULL);
+ // signal a release fence.
+ fence_state_->fetch_and(~consumer_state_mask, std::memory_order_release);
+
+ // Atomically set the buffer state of this consumer to released state.
+ buffer_state_->fetch_and(~consumer_state_mask, std::memory_order_release);
ALOGW(
- "ProducerChannel::OnConsumerOrphaned: detected new orphaned consumer "
- "buffer_id=%d client_state_mask=%" PRIx64 " queue_index=%" PRIu64
+ "%s: detected new orphaned consumer buffer_id=%d "
+ "consumer_state_mask=%" PRIx64 " queue_index=%" PRIx64
" buffer_state=%" PRIx64 " fence_state=%" PRIx64 ".",
- buffer_id(), client_state_mask, metadata_header_->queue_index,
+ __FUNCTION__, buffer_id(), consumer_state_mask,
+ metadata_header_->queue_index,
buffer_state_->load(std::memory_order_acquire),
fence_state_->load(std::memory_order_acquire));
}
@@ -620,45 +645,64 @@
std::find(consumer_channels_.begin(), consumer_channels_.end(), channel));
// 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->client_state_mask(),
- std::memory_order_release);
+ uint64_t consumer_state_mask = channel->client_state_mask();
+ uint64_t current_active_clients_bit_mask =
+ active_clients_bit_mask_->load(std::memory_order_acquire);
+ uint64_t updated_active_clients_bit_mask =
+ current_active_clients_bit_mask & (~consumer_state_mask);
+ while (!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)) {
+ ALOGI(
+ "%s: Failed to remove consumer state mask. Current active clients bit "
+ "mask is changed to %" PRIu64
+ " when trying to acquire and modify it to %" PRIu64
+ ". About to try again.",
+ __FUNCTION__, current_active_clients_bit_mask,
+ updated_active_clients_bit_mask);
+ updated_active_clients_bit_mask =
+ current_active_clients_bit_mask & (~consumer_state_mask);
+ }
- const uint64_t buffer_state = buffer_state_->load(std::memory_order_acquire);
- if (BufferHubDefs::IsBufferPosted(buffer_state) ||
- BufferHubDefs::IsBufferAcquired(buffer_state)) {
+ const uint64_t current_buffer_state =
+ buffer_state_->load(std::memory_order_acquire);
+ if (BufferHubDefs::IsClientPosted(current_buffer_state,
+ consumer_state_mask) ||
+ BufferHubDefs::IsClientAcquired(current_buffer_state,
+ consumer_state_mask)) {
// The consumer client is being destoryed without releasing. This could
// happen in corner cases when the consumer crashes. Here we mark it
// orphaned before remove it from producer.
- OnConsumerOrphaned(channel);
+ OnConsumerOrphaned(consumer_state_mask);
+ return;
}
- if (BufferHubDefs::IsBufferReleased(buffer_state) ||
- BufferHubDefs::IsBufferGained(buffer_state)) {
+ if (BufferHubDefs::IsClientReleased(current_buffer_state,
+ consumer_state_mask) ||
+ BufferHubDefs::AnyClientGained(current_buffer_state)) {
// The consumer is being close while it is suppose to signal a release
// fence. Signal the dummy fence here.
- if (fence_state_->load(std::memory_order_acquire) &
- channel->client_state_mask()) {
+ if (fence_state_->load(std::memory_order_acquire) & consumer_state_mask) {
epoll_event event;
event.events = EPOLLIN;
- event.data.u64 = channel->client_state_mask();
+ event.data.u64 = consumer_state_mask;
if (epoll_ctl(release_fence_fd_.Get(), EPOLL_CTL_MOD,
dummy_fence_fd_.Get(), &event) < 0) {
ALOGE(
- "ProducerChannel::RemoveConsumer: Failed to modify the shared "
- "release fence to include the dummy fence: %s",
- strerror(errno));
+ "%s: Failed to modify the shared release fence to include the "
+ "dummy fence: %s",
+ __FUNCTION__, strerror(errno));
return;
}
- ALOGW(
- "ProducerChannel::RemoveConsumer: signal dummy release fence "
- "buffer_id=%d",
- buffer_id());
+ ALOGW("%s: signal dummy release fence buffer_id=%d", __FUNCTION__,
+ buffer_id());
eventfd_write(dummy_fence_fd_.Get(), 1);
}
}
}
-// Returns true if the given parameters match the underlying buffer parameters.
+// Returns true if the given parameters match the underlying buffer
+// parameters.
bool ProducerChannel::CheckParameters(uint32_t width, uint32_t height,
uint32_t layer_count, uint32_t format,
uint64_t usage,
diff --git a/services/vr/bufferhubd/producer_queue_channel.cpp b/services/vr/bufferhubd/producer_queue_channel.cpp
index 6b5027c..6b33f50 100644
--- a/services/vr/bufferhubd/producer_queue_channel.cpp
+++ b/services/vr/bufferhubd/producer_queue_channel.cpp
@@ -319,7 +319,12 @@
return ErrorStatus(EINVAL);
}
uint64_t buffer_state = producer_channel->buffer_state();
- if (!BufferHubDefs::IsBufferGained(buffer_state)) {
+ // TODO(b/112007999) add an atomic variable in metadata header in shared
+ // memory to indicate which client is the last producer of the buffer.
+ // Currently, the first client is the only producer to the buffer.
+ // Thus, it checks whether the first client gains the buffer below.
+ if (!BufferHubDefs::IsClientGained(buffer_state,
+ BufferHubDefs::kFirstClientBitMask)) {
// Rejects the request if the requested buffer is not in Gained state.
ALOGE(
"ProducerQueueChannel::InsertBuffer: The buffer (cid=%d, "