Add shared memory based buffer metadata
This CLs reduces BufferHub CPU consumption by adding asynchronous
state transition so that out-of-process VR composition can run on 2016
pixel devices smoothly. In addition, this CL addresses a couple corner
cases in the existing bufferhub logic, which fixes various blackscreen
issues.
1/ Tracks buffer transition states (gained, posted, acquired, released)
from the client side via atomic shared memory and adds
PostAsync/AcquireAsync/ReleaseAsync/GainAsync with metadata and
fence support.
2/ Adds dequeue order guarantee for buffers enqueued with
dvrWriteBufferQueuePostBuffer.
3/ Synchronous BuffeHub operations are still supported.
4/ Bump up the bufferhubd's soft limit of open file descriptor.
5/ Handle orphaned consumer in acquired state. This is a corner case
that consumer process goes aways (most likely due to a crash) leaving
buffer stuck in acquired state with inconsistent buffer state.
6/ Fixes a race condition for released buffer to be Gain'ed and
Acquire'd when a new consumer is created in released state.
7/ Improve silent consumer queue efficiency: Silent queues no longer
import buffers or receive signals about new buffers and they are
limited to only spawning other consumers and notifications about
producers hanging up.
8/ Modify PDX/UDS channel event signaling to work around epoll
behavior. PDX UDS uses a combination of an eventfd and an epoll set
to simulate the original PDX transport channel events. An odd
behavior discovered in the kernel implementation of epoll was found
that causes the epoll fd to "unsignal" itself whenever epoll_wait()
is called on it, regardless of whether it should still be
pending. This breaks the edge triggerd behavior in nested epoll sets
that channel events depend on. Since this is unlikely to ever be
fixed in the kernel we work around the behavior by using the epoll
set only as a logical OR of two eventfds and never calling
epoll_wait() on it. When polling is required we use regluar poll()
with the eventfds and data fd to avoid the bad behavior in
epoll_wait().
9/ Keep reading data after PDX hangup signal. UDS will signal hangup
when the other end of the socket closes. However, data could still be
in the kerenl buffer and should be consumed. Fix an issue where the
service misses an impulse sent right before the socket is closed.
Bug: 65455724
Bug: 65458354
Bug: 65458312
Bug: 64027135
Bug: 67424527
Test: libpdx_uds_tests
bufferhub_tests
buffer_hub_queue-test
buffer_hub_queue_producer-test
dvr_api-test
Change-Id: Id07db1f206ccf4e06f7ee3c671193334408971ca
diff --git a/libs/vr/libbufferhub/bufferhub_tests.cpp b/libs/vr/libbufferhub/bufferhub_tests.cpp
index 1daa5d6..c4b9a8c 100644
--- a/libs/vr/libbufferhub/bufferhub_tests.cpp
+++ b/libs/vr/libbufferhub/bufferhub_tests.cpp
@@ -1,5 +1,9 @@
#include <gtest/gtest.h>
+#include <poll.h>
#include <private/dvr/buffer_hub_client.h>
+#include <private/dvr/bufferhub_rpc.h>
+#include <sys/epoll.h>
+#include <sys/eventfd.h>
#include <mutex>
#include <thread>
@@ -13,8 +17,10 @@
return result; \
})()
-using android::dvr::BufferProducer;
using android::dvr::BufferConsumer;
+using android::dvr::BufferHubDefs::kConsumerStateMask;
+using android::dvr::BufferHubDefs::kProducerStateBit;
+using android::dvr::BufferProducer;
using android::pdx::LocalHandle;
const int kWidth = 640;
@@ -37,29 +43,149 @@
BufferConsumer::Import(c->CreateConsumer());
ASSERT_TRUE(c2.get() != nullptr);
+ // Producer state mask is unique, i.e. 1.
+ EXPECT_EQ(p->buffer_state_bit(), kProducerStateBit);
+ // Consumer state mask cannot have producer bit on.
+ EXPECT_EQ(c->buffer_state_bit() & kProducerStateBit, 0);
+ // Consumer state mask must be a single, i.e. power of 2.
+ EXPECT_NE(c->buffer_state_bit(), 0);
+ EXPECT_EQ(c->buffer_state_bit() & (c->buffer_state_bit() - 1), 0);
+ // Consumer state mask cannot have producer bit on.
+ EXPECT_EQ(c2->buffer_state_bit() & kProducerStateBit, 0);
+ // Consumer state mask must be a single, i.e. power of 2.
+ EXPECT_NE(c2->buffer_state_bit(), 0);
+ EXPECT_EQ(c2->buffer_state_bit() & (c2->buffer_state_bit() - 1), 0);
+ // Each consumer should have unique bit.
+ EXPECT_EQ(c->buffer_state_bit() & c2->buffer_state_bit(), 0);
+
+ // Initial state: producer not available, consumers not available.
+ EXPECT_EQ(0, RETRY_EINTR(p->Poll(100)));
+ EXPECT_EQ(0, RETRY_EINTR(c->Poll(100)));
+ EXPECT_EQ(0, RETRY_EINTR(c2->Poll(100)));
+
EXPECT_EQ(0, p->Post(LocalHandle(), kContext));
- // Both consumers should be triggered.
- EXPECT_GE(0, RETRY_EINTR(p->Poll(0)));
- EXPECT_LT(0, RETRY_EINTR(c->Poll(10)));
- EXPECT_LT(0, RETRY_EINTR(c2->Poll(10)));
+
+ // New state: producer not available, consumers available.
+ EXPECT_EQ(0, RETRY_EINTR(p->Poll(100)));
+ EXPECT_EQ(1, RETRY_EINTR(c->Poll(100)));
+ EXPECT_EQ(1, RETRY_EINTR(c2->Poll(100)));
uint64_t context;
LocalHandle fence;
- EXPECT_LE(0, c->Acquire(&fence, &context));
+ EXPECT_EQ(0, c->Acquire(&fence, &context));
EXPECT_EQ(kContext, context);
- EXPECT_GE(0, RETRY_EINTR(c->Poll(0)));
+ EXPECT_EQ(0, RETRY_EINTR(c->Poll(100)));
+ EXPECT_EQ(1, RETRY_EINTR(c2->Poll(100)));
- EXPECT_LE(0, c2->Acquire(&fence, &context));
+ EXPECT_EQ(0, c2->Acquire(&fence, &context));
EXPECT_EQ(kContext, context);
- EXPECT_GE(0, RETRY_EINTR(c2->Poll(0)));
+ EXPECT_EQ(0, RETRY_EINTR(c2->Poll(100)));
+ EXPECT_EQ(0, RETRY_EINTR(c->Poll(100)));
EXPECT_EQ(0, c->Release(LocalHandle()));
- EXPECT_GE(0, RETRY_EINTR(p->Poll(0)));
+ EXPECT_EQ(0, RETRY_EINTR(p->Poll(100)));
EXPECT_EQ(0, c2->Discard());
- EXPECT_LE(0, RETRY_EINTR(p->Poll(0)));
+ EXPECT_EQ(1, RETRY_EINTR(p->Poll(100)));
EXPECT_EQ(0, p->Gain(&fence));
- EXPECT_GE(0, RETRY_EINTR(p->Poll(0)));
+ EXPECT_EQ(0, RETRY_EINTR(p->Poll(100)));
+ EXPECT_EQ(0, RETRY_EINTR(c->Poll(100)));
+ EXPECT_EQ(0, RETRY_EINTR(c2->Poll(100)));
+}
+
+TEST_F(LibBufferHubTest, TestEpoll) {
+ std::unique_ptr<BufferProducer> p = BufferProducer::Create(
+ kWidth, kHeight, kFormat, kUsage, sizeof(uint64_t));
+ ASSERT_TRUE(p.get() != nullptr);
+ std::unique_ptr<BufferConsumer> c =
+ BufferConsumer::Import(p->CreateConsumer());
+ ASSERT_TRUE(c.get() != nullptr);
+
+ LocalHandle epoll_fd{epoll_create1(EPOLL_CLOEXEC)};
+ ASSERT_TRUE(epoll_fd.IsValid());
+
+ epoll_event event;
+ std::array<epoll_event, 64> events;
+
+ auto event_sources = p->GetEventSources();
+ ASSERT_LT(event_sources.size(), events.size());
+
+ for (const auto& event_source : event_sources) {
+ event = {.events = event_source.event_mask | EPOLLET,
+ .data = {.fd = p->event_fd()}};
+ ASSERT_EQ(0, epoll_ctl(epoll_fd.Get(), EPOLL_CTL_ADD, event_source.event_fd,
+ &event));
+ }
+
+ event_sources = c->GetEventSources();
+ ASSERT_LT(event_sources.size(), events.size());
+
+ for (const auto& event_source : event_sources) {
+ event = {.events = event_source.event_mask | EPOLLET,
+ .data = {.fd = c->event_fd()}};
+ ASSERT_EQ(0, epoll_ctl(epoll_fd.Get(), EPOLL_CTL_ADD, event_source.event_fd,
+ &event));
+ }
+
+ // 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.
+ EXPECT_EQ(0, p->Post({}, kContext));
+ ASSERT_EQ(1, epoll_wait(epoll_fd.Get(), events.data(), events.size(), 100));
+ ASSERT_TRUE(events[0].events & EPOLLIN);
+ ASSERT_EQ(c->event_fd(), events[0].data.fd);
+
+ // Save the event bits to translate later.
+ event = events[0];
+
+ // Check for events again. Edge-triggered mode should prevent any.
+ EXPECT_EQ(0, epoll_wait(epoll_fd.Get(), events.data(), events.size(), 100));
+ EXPECT_EQ(0, epoll_wait(epoll_fd.Get(), events.data(), events.size(), 100));
+ EXPECT_EQ(0, epoll_wait(epoll_fd.Get(), events.data(), events.size(), 100));
+ EXPECT_EQ(0, epoll_wait(epoll_fd.Get(), events.data(), events.size(), 100));
+
+ // Translate the events.
+ auto event_status = c->GetEventMask(event.events);
+ ASSERT_TRUE(event_status);
+ ASSERT_TRUE(event_status.get() & EPOLLIN);
+
+ // Check for events again. Edge-triggered mode should prevent any.
+ EXPECT_EQ(0, epoll_wait(epoll_fd.Get(), events.data(), events.size(), 100));
+}
+
+TEST_F(LibBufferHubTest, TestStateMask) {
+ std::unique_ptr<BufferProducer> p = BufferProducer::Create(
+ kWidth, kHeight, kFormat, kUsage, sizeof(uint64_t));
+ ASSERT_TRUE(p.get() != nullptr);
+
+ // It's ok to create up to 63 consumer buffers.
+ uint64_t buffer_state_bits = p->buffer_state_bit();
+ std::array<std::unique_ptr<BufferConsumer>, 63> cs;
+ for (size_t i = 0; i < 63; i++) {
+ cs[i] = BufferConsumer::Import(p->CreateConsumer());
+ ASSERT_TRUE(cs[i].get() != nullptr);
+ // Expect all buffers have unique state mask.
+ EXPECT_EQ(buffer_state_bits & cs[i]->buffer_state_bit(), 0);
+ buffer_state_bits |= cs[i]->buffer_state_bit();
+ }
+ EXPECT_EQ(buffer_state_bits, kProducerStateBit | kConsumerStateMask);
+
+ // The 64th creation will fail with out-of-memory error.
+ auto state = p->CreateConsumer();
+ EXPECT_EQ(state.error(), E2BIG);
+
+ // Release any consumer should allow us to re-create.
+ for (size_t i = 0; i < 63; i++) {
+ buffer_state_bits &= ~cs[i]->buffer_state_bit();
+ cs[i] = nullptr;
+ cs[i] = BufferConsumer::Import(p->CreateConsumer());
+ ASSERT_TRUE(cs[i].get() != nullptr);
+ // The released state mask will be reused.
+ EXPECT_EQ(buffer_state_bits & cs[i]->buffer_state_bit(), 0);
+ buffer_state_bits |= cs[i]->buffer_state_bit();
+ EXPECT_EQ(buffer_state_bits, kProducerStateBit | kConsumerStateMask);
+ }
}
TEST_F(LibBufferHubTest, TestStateTransitions) {
@@ -98,6 +224,7 @@
// Release in acquired state should succeed.
EXPECT_EQ(0, c->Release(LocalHandle()));
+ EXPECT_LT(0, RETRY_EINTR(p->Poll(10)));
// Release, acquire, and post in released state should fail.
EXPECT_EQ(-EBUSY, c->Release(LocalHandle()));
@@ -144,6 +271,11 @@
int64_t field1;
int64_t field2;
};
+ struct OverSizedMetadata {
+ int64_t field1;
+ int64_t field2;
+ int64_t field3;
+ };
std::unique_ptr<BufferProducer> p = BufferProducer::Create(
kWidth, kHeight, kFormat, kUsage, sizeof(Metadata));
ASSERT_TRUE(p.get() != nullptr);
@@ -151,9 +283,16 @@
BufferConsumer::Import(p->CreateConsumer());
ASSERT_TRUE(c.get() != nullptr);
- int64_t sequence = 3;
- EXPECT_NE(0, p->Post(LocalHandle(), sequence));
+ // It is illegal to post metadata larger than originally requested during
+ // buffer allocation.
+ OverSizedMetadata evil_meta = {};
+ EXPECT_NE(0, p->Post(LocalHandle(), evil_meta));
EXPECT_GE(0, RETRY_EINTR(c->Poll(10)));
+
+ // It is ok to post metadata smaller than originally requested during
+ // buffer allocation.
+ int64_t sequence = 42;
+ EXPECT_EQ(0, p->Post(LocalHandle(), sequence));
}
TEST_F(LibBufferHubTest, TestAcquireWithWrongMetaSize) {
@@ -161,6 +300,11 @@
int64_t field1;
int64_t field2;
};
+ struct OverSizedMetadata {
+ int64_t field1;
+ int64_t field2;
+ int64_t field3;
+ };
std::unique_ptr<BufferProducer> p = BufferProducer::Create(
kWidth, kHeight, kFormat, kUsage, sizeof(Metadata));
ASSERT_TRUE(p.get() != nullptr);
@@ -173,7 +317,16 @@
LocalHandle fence;
int64_t sequence;
- EXPECT_NE(0, c->Acquire(&fence, &sequence));
+ OverSizedMetadata e;
+
+ // It is illegal to acquire metadata larger than originally requested during
+ // buffer allocation.
+ EXPECT_NE(0, c->Acquire(&fence, &e));
+
+ // It is ok to acquire metadata smaller than originally requested during
+ // buffer allocation.
+ EXPECT_EQ(0, c->Acquire(&fence, &sequence));
+ EXPECT_EQ(m.field1, sequence);
}
TEST_F(LibBufferHubTest, TestAcquireWithNoMeta) {
@@ -266,12 +419,140 @@
LocalHandle fence;
auto c = BufferConsumer::Import(p->CreateConsumer());
ASSERT_NE(nullptr, c);
- EXPECT_NE(-EPIPE, c->Acquire(&fence));
+ EXPECT_EQ(0, p->Post<void>(LocalHandle()));
+ EXPECT_EQ(0, c->Acquire(&fence));
+ EXPECT_EQ(0, c->Release(LocalHandle()));
+ EXPECT_LT(0, RETRY_EINTR(p->Poll(10)));
// Test that removing persistence and closing the producer orphans the
// consumer.
+ EXPECT_EQ(0, p->Gain(&fence));
+ EXPECT_EQ(0, p->Post<void>(LocalHandle()));
EXPECT_EQ(0, p->RemovePersistence());
p = nullptr;
+ // Orphaned consumer can acquire the posted buffer one more time in
+ // asynchronous manner. But synchronous call will fail.
+ DvrNativeBufferMetadata meta;
+ EXPECT_EQ(0, c->AcquireAsync(&meta, &fence));
EXPECT_EQ(-EPIPE, c->Release(LocalHandle()));
}
+
+namespace {
+
+int PollFd(int fd, int timeout_ms) {
+ pollfd p = {fd, POLLIN, 0};
+ return poll(&p, 1, timeout_ms);
+}
+
+} // namespace
+
+TEST_F(LibBufferHubTest, TestAcquireFence) {
+ std::unique_ptr<BufferProducer> p = BufferProducer::Create(
+ kWidth, kHeight, kFormat, kUsage, /*metadata_size=*/0);
+ ASSERT_TRUE(p.get() != nullptr);
+ std::unique_ptr<BufferConsumer> c =
+ BufferConsumer::Import(p->CreateConsumer());
+ ASSERT_TRUE(c.get() != nullptr);
+
+ DvrNativeBufferMetadata meta;
+ LocalHandle f1(eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK));
+
+ // Post with unsignaled fence.
+ EXPECT_EQ(0, p->PostAsync(&meta, f1));
+
+ // Should acquire a valid fence.
+ LocalHandle f2;
+ EXPECT_LT(0, RETRY_EINTR(c->Poll(10)));
+ EXPECT_EQ(0, c->AcquireAsync(&meta, &f2));
+ EXPECT_TRUE(f2.IsValid());
+ // The original fence and acquired fence should have different fd number.
+ EXPECT_NE(f1.Get(), f2.Get());
+ EXPECT_GE(0, PollFd(f2.Get(), 0));
+
+ // Signal the original fence will trigger the new fence.
+ eventfd_write(f1.Get(), 1);
+ // Now the original FD has been signaled.
+ EXPECT_LT(0, PollFd(f2.Get(), 10));
+
+ // Release the consumer with an invalid fence.
+ EXPECT_EQ(0, c->ReleaseAsync(&meta, LocalHandle()));
+
+ // Should gain an invalid fence.
+ LocalHandle f3;
+ EXPECT_LT(0, RETRY_EINTR(p->Poll(10)));
+ EXPECT_EQ(0, p->GainAsync(&meta, &f3));
+ EXPECT_FALSE(f3.IsValid());
+
+ // Post with a signaled fence.
+ EXPECT_EQ(0, p->PostAsync(&meta, f1));
+
+ // Should acquire a valid fence and it's already signalled.
+ LocalHandle f4;
+ EXPECT_LT(0, RETRY_EINTR(c->Poll(10)));
+ EXPECT_EQ(0, c->AcquireAsync(&meta, &f4));
+ EXPECT_TRUE(f4.IsValid());
+ EXPECT_LT(0, PollFd(f4.Get(), 10));
+
+ // Release with an unsignalled fence and signal it immediately after release
+ // without producer gainning.
+ LocalHandle f5(eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK));
+ EXPECT_EQ(0, c->ReleaseAsync(&meta, f5));
+ eventfd_write(f5.Get(), 1);
+
+ // Should gain a valid fence, which is already signaled.
+ LocalHandle f6;
+ EXPECT_LT(0, RETRY_EINTR(p->Poll(10)));
+ EXPECT_EQ(0, p->GainAsync(&meta, &f6));
+ EXPECT_TRUE(f6.IsValid());
+ EXPECT_LT(0, PollFd(f6.Get(), 10));
+}
+
+TEST_F(LibBufferHubTest, TestOrphanedAcquire) {
+ std::unique_ptr<BufferProducer> p = BufferProducer::Create(
+ kWidth, kHeight, kFormat, kUsage, sizeof(uint64_t));
+ ASSERT_TRUE(p.get() != nullptr);
+ std::unique_ptr<BufferConsumer> c1 =
+ BufferConsumer::Import(p->CreateConsumer());
+ ASSERT_TRUE(c1.get() != nullptr);
+ const uint64_t consumer_state_bit1 = c1->buffer_state_bit();
+
+ DvrNativeBufferMetadata meta;
+ EXPECT_EQ(0, p->PostAsync(&meta, LocalHandle()));
+
+ LocalHandle fence;
+ EXPECT_LT(0, RETRY_EINTR(c1->Poll(10)));
+ 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(10)));
+
+ std::unique_ptr<BufferConsumer> c2 =
+ BufferConsumer::Import(p->CreateConsumer());
+ ASSERT_TRUE(c2.get() != nullptr);
+ const uint64_t consumer_state_bit2 = c2->buffer_state_bit();
+ EXPECT_NE(consumer_state_bit1, consumer_state_bit2);
+
+ // The new consumer is available for acquire.
+ EXPECT_LT(0, RETRY_EINTR(c2->Poll(10)));
+ EXPECT_LE(0, c2->AcquireAsync(&meta, &fence));
+ // Releasing the consumer makes the buffer gainable.
+ EXPECT_EQ(0, c2->ReleaseAsync(&meta, LocalHandle()));
+
+ // The buffer is now available for the producer to gain.
+ EXPECT_LT(0, RETRY_EINTR(p->Poll(10)));
+
+ // But if another consumer is created in released state.
+ std::unique_ptr<BufferConsumer> c3 =
+ BufferConsumer::Import(p->CreateConsumer());
+ ASSERT_TRUE(c3.get() != nullptr);
+ const uint64_t consumer_state_bit3 = c3->buffer_state_bit();
+ EXPECT_NE(consumer_state_bit2, consumer_state_bit3);
+ // The consumer buffer is not acquirable.
+ EXPECT_GE(0, RETRY_EINTR(c3->Poll(10)));
+ EXPECT_EQ(-EBUSY, c3->AcquireAsync(&meta, &fence));
+
+ // Producer should be able to gain no matter what.
+ EXPECT_EQ(0, p->GainAsync(&meta, &fence));
+}