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/libbufferhubqueue/tests/Android.bp b/libs/vr/libbufferhubqueue/tests/Android.bp
index 865573c..8bd1ef1 100644
--- a/libs/vr/libbufferhubqueue/tests/Android.bp
+++ b/libs/vr/libbufferhubqueue/tests/Android.bp
@@ -1,4 +1,7 @@
+header_libraries = [
+ "libdvr_headers",
+]
shared_libraries = [
"libbase",
@@ -21,6 +24,7 @@
cc_test {
srcs: ["buffer_hub_queue-test.cpp"],
+ header_libs: header_libraries,
static_libs: static_libraries,
shared_libs: shared_libraries,
cflags: [
@@ -35,6 +39,7 @@
cc_test {
srcs: ["buffer_hub_queue_producer-test.cpp"],
+ header_libs: header_libraries,
static_libs: static_libraries,
shared_libs: shared_libraries,
cflags: [
diff --git a/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp b/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp
index 7581a06..8a72531 100644
--- a/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp
+++ b/libs/vr/libbufferhubqueue/tests/buffer_hub_queue-test.cpp
@@ -3,6 +3,8 @@
#include <private/dvr/buffer_hub_queue_client.h>
#include <gtest/gtest.h>
+#include <poll.h>
+#include <sys/eventfd.h>
#include <vector>
@@ -46,9 +48,9 @@
void AllocateBuffer(size_t* slot_out = nullptr) {
// Create producer buffer.
- auto status = producer_queue_->AllocateBuffer(
- kBufferWidth, kBufferHeight, kBufferLayerCount, kBufferFormat,
- kBufferUsage);
+ auto status = producer_queue_->AllocateBuffer(kBufferWidth, kBufferHeight,
+ kBufferLayerCount,
+ kBufferFormat, kBufferUsage);
ASSERT_TRUE(status.ok());
size_t slot = status.take();
@@ -56,6 +58,23 @@
*slot_out = slot;
}
+ bool WaitAndHandleOnce(BufferHubQueue* queue, int timeout_ms) {
+ pollfd pfd{queue->queue_fd(), POLLIN, 0};
+ int ret;
+ do {
+ ret = poll(&pfd, 1, timeout_ms);
+ } while (ret == -1 && errno == EINTR);
+
+ if (ret < 0) {
+ ALOGW("Failed to poll queue %d's event fd, error: %s.", queue->id(),
+ strerror(errno));
+ return false;
+ } else if (ret == 0) {
+ return false;
+ }
+ return queue->HandleQueueEvents();
+ }
+
protected:
ProducerQueueConfigBuilder config_builder_;
std::unique_ptr<ProducerQueue> producer_queue_;
@@ -75,7 +94,7 @@
for (size_t i = 0; i < nb_dequeue_times; i++) {
size_t slot;
LocalHandle fence;
- auto p1_status = producer_queue_->Dequeue(0, &slot, &fence);
+ auto p1_status = producer_queue_->Dequeue(100, &slot, &fence);
ASSERT_TRUE(p1_status.ok());
auto p1 = p1_status.take();
ASSERT_NE(nullptr, p1);
@@ -113,31 +132,26 @@
// Dequeue returns timeout since no buffer is ready to consumer, but
// this implicitly triggers buffer import and bump up |capacity|.
LocalHandle fence;
- auto status = consumer_queue_->Dequeue(0, &slot, &seq, &fence);
+ auto status = consumer_queue_->Dequeue(100, &slot, &seq, &fence);
ASSERT_FALSE(status.ok());
ASSERT_EQ(ETIMEDOUT, status.error());
ASSERT_EQ(consumer_queue_->capacity(), i + 1);
}
- // Use /dev/zero as a stand-in for a fence. As long as BufferHub does not need
- // to merge fences, which only happens when multiple consumers release the
- // same buffer with release fences, the file object should simply pass
- // through.
- LocalHandle post_fence("/dev/zero", O_RDONLY);
- struct stat post_fence_stat;
- ASSERT_EQ(0, fstat(post_fence.Get(), &post_fence_stat));
+ // Use eventfd as a stand-in for a fence.
+ LocalHandle post_fence(eventfd(0, EFD_CLOEXEC | EFD_NONBLOCK));
for (size_t i = 0; i < kBufferCount; i++) {
LocalHandle fence;
// First time there is no buffer available to dequeue.
- auto consumer_status = consumer_queue_->Dequeue(0, &slot, &seq, &fence);
+ auto consumer_status = consumer_queue_->Dequeue(100, &slot, &seq, &fence);
ASSERT_FALSE(consumer_status.ok());
ASSERT_EQ(ETIMEDOUT, consumer_status.error());
// Make sure Producer buffer is POSTED so that it's ready to Accquire
// in the consumer's Dequeue() function.
- auto producer_status = producer_queue_->Dequeue(0, &slot, &fence);
+ auto producer_status = producer_queue_->Dequeue(100, &slot, &fence);
ASSERT_TRUE(producer_status.ok());
auto producer = producer_status.take();
ASSERT_NE(nullptr, producer);
@@ -147,20 +161,10 @@
// Second time the just the POSTED buffer should be dequeued.
uint64_t seq_out = 0;
- consumer_status = consumer_queue_->Dequeue(0, &slot, &seq_out, &fence);
+ consumer_status = consumer_queue_->Dequeue(100, &slot, &seq_out, &fence);
ASSERT_TRUE(consumer_status.ok());
EXPECT_TRUE(fence.IsValid());
- struct stat acquire_fence_stat;
- ASSERT_EQ(0, fstat(fence.Get(), &acquire_fence_stat));
-
- // The file descriptors should refer to the same file object. Testing the
- // device id and inode is a proxy for testing that the fds refer to the same
- // file object.
- EXPECT_NE(post_fence.Get(), fence.Get());
- EXPECT_EQ(post_fence_stat.st_dev, acquire_fence_stat.st_dev);
- EXPECT_EQ(post_fence_stat.st_ino, acquire_fence_stat.st_ino);
-
auto consumer = consumer_status.take();
ASSERT_NE(nullptr, consumer);
ASSERT_EQ(seq_in, seq_out);
@@ -196,12 +200,11 @@
for (size_t i = 0; i < kBufferCount; i++) {
Entry* entry = &buffers[i];
- auto producer_status =
- producer_queue_->Dequeue(0, &entry->slot, &entry->fence);
+ auto producer_status = producer_queue_->Dequeue(
+ /*timeout_ms=*/100, &entry->slot, &entry->fence);
ASSERT_TRUE(producer_status.ok());
entry->buffer = producer_status.take();
ASSERT_NE(nullptr, entry->buffer);
- EXPECT_EQ(i, entry->slot);
}
// Remove a buffer and make sure both queues reflect the change.
@@ -218,8 +221,8 @@
buffers[0].buffer = nullptr;
// Now the consumer queue should know it's gone.
- EXPECT_FALSE(consumer_queue_->HandleQueueEvents());
- EXPECT_EQ(kBufferCount - 1, consumer_queue_->capacity());
+ EXPECT_FALSE(WaitAndHandleOnce(consumer_queue_.get(), /*timeout_ms=*/100));
+ ASSERT_EQ(kBufferCount - 1, consumer_queue_->capacity());
// Allocate a new buffer. This should take the first empty slot.
size_t slot;
@@ -286,17 +289,20 @@
auto silent_queue = producer_queue_->CreateSilentConsumerQueue();
ASSERT_NE(nullptr, silent_queue);
- // Check that buffers are correctly imported on construction.
- EXPECT_EQ(kBufferCount, silent_queue->capacity());
+ // Check that silent queue doesn't import buffers on creation.
+ EXPECT_EQ(0, silent_queue->capacity());
// Dequeue and post a buffer.
size_t slot;
LocalHandle fence;
- auto producer_status = producer_queue_->Dequeue(0, &slot, &fence);
+ auto producer_status =
+ producer_queue_->Dequeue(/*timeout_ms=*/100, &slot, &fence);
ASSERT_TRUE(producer_status.ok());
auto producer_buffer = producer_status.take();
ASSERT_NE(nullptr, producer_buffer);
ASSERT_EQ(0, producer_buffer->Post<void>({}));
+ // After post, check the number of remaining available buffers.
+ EXPECT_EQ(kBufferCount - 1, producer_queue_->count());
// Currently we expect no buffer to be available prior to calling
// WaitForBuffers/HandleQueueEvents.
@@ -314,23 +320,30 @@
EXPECT_EQ(1u, consumer_queue_->count());
// Reclaim released/ignored buffers.
- producer_queue_->HandleQueueEvents();
+ ASSERT_EQ(kBufferCount - 1, producer_queue_->count());
+
+ usleep(10000);
+ WaitAndHandleOnce(producer_queue_.get(), /*timeout_ms=*/100);
ASSERT_EQ(kBufferCount - 1, producer_queue_->count());
// Post another buffer.
- producer_status = producer_queue_->Dequeue(0, &slot, &fence);
+ producer_status = producer_queue_->Dequeue(/*timeout_ms=*/100, &slot, &fence);
ASSERT_TRUE(producer_status.ok());
producer_buffer = producer_status.take();
ASSERT_NE(nullptr, producer_buffer);
ASSERT_EQ(0, producer_buffer->Post<void>({}));
// Verify that the consumer queue receives it.
- EXPECT_EQ(1u, consumer_queue_->count());
- EXPECT_TRUE(consumer_queue_->HandleQueueEvents());
- EXPECT_EQ(2u, consumer_queue_->count());
+ size_t consumer_queue_count = consumer_queue_->count();
+ WaitAndHandleOnce(consumer_queue_.get(), /*timeout_ms=*/100);
+ EXPECT_LT(consumer_queue_count, consumer_queue_->count());
+
+ // Save the current consumer queue buffer count to compare after the dequeue.
+ consumer_queue_count = consumer_queue_->count();
// Dequeue and acquire/release (discard) buffers on the consumer end.
- auto consumer_status = consumer_queue_->Dequeue(0, &slot, &fence);
+ auto consumer_status =
+ consumer_queue_->Dequeue(/*timeout_ms=*/100, &slot, &fence);
ASSERT_TRUE(consumer_status.ok());
auto consumer_buffer = consumer_status.take();
ASSERT_NE(nullptr, consumer_buffer);
@@ -338,7 +351,7 @@
// Buffer should be returned to the producer queue without being handled by
// the silent consumer queue.
- EXPECT_EQ(1u, consumer_queue_->count());
+ EXPECT_GT(consumer_queue_count, consumer_queue_->count());
EXPECT_EQ(kBufferCount - 2, producer_queue_->count());
EXPECT_TRUE(producer_queue_->HandleQueueEvents());
EXPECT_EQ(kBufferCount - 1, producer_queue_->count());
@@ -362,13 +375,13 @@
for (auto mi : ms) {
size_t slot;
LocalHandle fence;
- auto p1_status = producer_queue_->Dequeue(0, &slot, &fence);
+ auto p1_status = producer_queue_->Dequeue(100, &slot, &fence);
ASSERT_TRUE(p1_status.ok());
auto p1 = p1_status.take();
ASSERT_NE(nullptr, p1);
ASSERT_EQ(p1->Post(LocalHandle(-1), &mi, sizeof(mi)), 0);
TestMetadata mo;
- auto c1_status = consumer_queue_->Dequeue(0, &slot, &mo, &fence);
+ auto c1_status = consumer_queue_->Dequeue(100, &slot, &mo, &fence);
ASSERT_TRUE(c1_status.ok());
auto c1 = c1_status.take();
ASSERT_EQ(mi.a, mo.a);
@@ -387,7 +400,7 @@
int64_t mi = 3;
size_t slot;
LocalHandle fence;
- auto p1_status = producer_queue_->Dequeue(0, &slot, &fence);
+ auto p1_status = producer_queue_->Dequeue(100, &slot, &fence);
ASSERT_TRUE(p1_status.ok());
auto p1 = p1_status.take();
ASSERT_NE(nullptr, p1);
@@ -395,7 +408,7 @@
int32_t mo;
// Acquire a buffer with mismatched metadata is not OK.
- auto c1_status = consumer_queue_->Dequeue(0, &slot, &mo, &fence);
+ auto c1_status = consumer_queue_->Dequeue(100, &slot, &mo, &fence);
ASSERT_FALSE(c1_status.ok());
}
@@ -406,14 +419,14 @@
size_t slot;
LocalHandle fence;
- auto p1_status = producer_queue_->Dequeue(0, &slot, &fence);
+ auto p1_status = producer_queue_->Dequeue(100, &slot, &fence);
ASSERT_TRUE(p1_status.ok());
auto p1 = p1_status.take();
ASSERT_NE(nullptr, p1);
int64_t mo;
- producer_queue_->Enqueue(p1, slot);
- auto c1_status = consumer_queue_->Dequeue(0, &slot, &mo, &fence);
+ producer_queue_->Enqueue(p1, slot, 0ULL);
+ auto c1_status = consumer_queue_->Dequeue(100, &slot, &mo, &fence);
ASSERT_FALSE(c1_status.ok());
}
@@ -424,14 +437,14 @@
size_t s1;
AllocateBuffer();
LocalHandle fence;
- auto p1_status = producer_queue_->Dequeue(0, &s1, &fence);
+ auto p1_status = producer_queue_->Dequeue(100, &s1, &fence);
ASSERT_TRUE(p1_status.ok());
auto p1 = p1_status.take();
ASSERT_NE(nullptr, p1);
// producer queue is exhausted
size_t s2;
- auto p2_status = producer_queue_->Dequeue(0, &s2, &fence);
+ auto p2_status = producer_queue_->Dequeue(100, &s2, &fence);
ASSERT_FALSE(p2_status.ok());
ASSERT_EQ(ETIMEDOUT, p2_status.error());
@@ -441,7 +454,7 @@
ASSERT_EQ(producer_queue_->capacity(), 2U);
// now we can dequeue again
- p2_status = producer_queue_->Dequeue(0, &s2, &fence);
+ p2_status = producer_queue_->Dequeue(100, &s2, &fence);
ASSERT_TRUE(p2_status.ok());
auto p2 = p2_status.take();
ASSERT_NE(nullptr, p2);
@@ -456,7 +469,7 @@
int64_t seq = 1;
ASSERT_EQ(p1->Post(LocalHandle(), seq), 0);
size_t cs1, cs2;
- auto c1_status = consumer_queue_->Dequeue(0, &cs1, &seq, &fence);
+ auto c1_status = consumer_queue_->Dequeue(100, &cs1, &seq, &fence);
ASSERT_TRUE(c1_status.ok());
auto c1 = c1_status.take();
ASSERT_NE(nullptr, c1);
@@ -465,7 +478,7 @@
ASSERT_EQ(cs1, s1);
ASSERT_EQ(p2->Post(LocalHandle(), seq), 0);
- auto c2_status = consumer_queue_->Dequeue(0, &cs2, &seq, &fence);
+ auto c2_status = consumer_queue_->Dequeue(100, &cs2, &seq, &fence);
ASSERT_TRUE(c2_status.ok());
auto c2 = c2_status.take();
ASSERT_NE(nullptr, c2);
@@ -485,7 +498,7 @@
LocalHandle fence;
size_t slot;
- auto p1_status = producer_queue_->Dequeue(0, &slot, &fence);
+ auto p1_status = producer_queue_->Dequeue(100, &slot, &fence);
ASSERT_TRUE(p1_status.ok());
auto p1 = p1_status.take();
ASSERT_EQ(p1->usage() & set_mask, set_mask);
@@ -504,7 +517,7 @@
LocalHandle fence;
size_t slot;
- auto p1_status = producer_queue_->Dequeue(0, &slot, &fence);
+ auto p1_status = producer_queue_->Dequeue(100, &slot, &fence);
ASSERT_TRUE(p1_status.ok());
auto p1 = p1_status.take();
ASSERT_EQ(0u, p1->usage() & clear_mask);
@@ -543,9 +556,9 @@
ASSERT_TRUE(status.ok());
// While allocation without those bits should fail.
- status = producer_queue_->AllocateBuffer(
- kBufferWidth, kBufferHeight, kBufferLayerCount, kBufferFormat,
- kBufferUsage & ~deny_clear_mask);
+ status = producer_queue_->AllocateBuffer(kBufferWidth, kBufferHeight,
+ kBufferLayerCount, kBufferFormat,
+ kBufferUsage & ~deny_clear_mask);
ASSERT_FALSE(status.ok());
ASSERT_EQ(EINVAL, status.error());
}
@@ -603,7 +616,7 @@
// Free all buffers when one buffer is dequeued.
CHECK_NO_BUFFER_THEN_ALLOCATE(kBufferCount);
- producer_status = producer_queue_->Dequeue(0, &slot, &fence);
+ producer_status = producer_queue_->Dequeue(100, &slot, &fence);
ASSERT_TRUE(producer_status.ok());
status = producer_queue_->FreeAllBuffers();
EXPECT_TRUE(status.ok());
@@ -611,7 +624,7 @@
// Free all buffers when all buffers are dequeued.
CHECK_NO_BUFFER_THEN_ALLOCATE(kBufferCount);
for (size_t i = 0; i < kBufferCount; i++) {
- producer_status = producer_queue_->Dequeue(0, &slot, &fence);
+ producer_status = producer_queue_->Dequeue(100, &slot, &fence);
ASSERT_TRUE(producer_status.ok());
}
status = producer_queue_->FreeAllBuffers();
@@ -619,7 +632,7 @@
// Free all buffers when one buffer is posted.
CHECK_NO_BUFFER_THEN_ALLOCATE(kBufferCount);
- producer_status = producer_queue_->Dequeue(0, &slot, &fence);
+ producer_status = producer_queue_->Dequeue(100, &slot, &fence);
ASSERT_TRUE(producer_status.ok());
producer_buffer = producer_status.take();
ASSERT_NE(nullptr, producer_buffer);
@@ -630,7 +643,7 @@
// Free all buffers when all buffers are posted.
CHECK_NO_BUFFER_THEN_ALLOCATE(kBufferCount);
for (size_t i = 0; i < kBufferCount; i++) {
- producer_status = producer_queue_->Dequeue(0, &slot, &fence);
+ producer_status = producer_queue_->Dequeue(100, &slot, &fence);
ASSERT_TRUE(producer_status.ok());
producer_buffer = producer_status.take();
ASSERT_NE(nullptr, producer_buffer);
@@ -642,12 +655,12 @@
// Free all buffers when all buffers are acquired.
CHECK_NO_BUFFER_THEN_ALLOCATE(kBufferCount);
for (size_t i = 0; i < kBufferCount; i++) {
- producer_status = producer_queue_->Dequeue(0, &slot, &fence);
+ producer_status = producer_queue_->Dequeue(100, &slot, &fence);
ASSERT_TRUE(producer_status.ok());
producer_buffer = producer_status.take();
ASSERT_NE(nullptr, producer_buffer);
ASSERT_EQ(0, producer_buffer->Post(fence, &seq, sizeof(seq)));
- consumer_status = consumer_queue_->Dequeue(0, &slot, &seq, &fence);
+ consumer_status = consumer_queue_->Dequeue(100, &slot, &seq, &fence);
ASSERT_TRUE(consumer_status.ok());
}