Change atomics in ashmem from uint64_t to uint32_t
Fix: 117849512
Test: Blueline: atest AHardwareBufferTest BufferHub_test
buffer_hub_queue_producer-test libgui_test
libsensor_test vrflinger_test buffer_hub-test buffer_hub_queue-test
dvr_buffer_queue-test dvr_api-test dvr_display-test
Test: in libui_test InputSurfacesTest are segfault on top of master already.
Test: Vega: AHardwareBufferTest BufferHubBuffer_test
BufferHubMetadata_test buffer_hub_queue_producer-test buffer_hub-test
dvr_buffer_queue-test buffer_hub_queue-test dvr_api-test
libdvrtracking-test
Change-Id: I55f91c21f7ac07615b5451b5413521d7938cf591
diff --git a/libs/ui/BufferHubBuffer.cpp b/libs/ui/BufferHubBuffer.cpp
index 1464e48..5bc113f 100644
--- a/libs/ui/BufferHubBuffer.cpp
+++ b/libs/ui/BufferHubBuffer.cpp
@@ -191,22 +191,22 @@
mClientStateMask = bufferTraits.client_state_mask();
// TODO(b/112012161) Set up shared fences.
- ALOGD("BufferHubBuffer::ImportGraphicBuffer: id=%d, buffer_state=%" PRIx64 ".", id(),
+ ALOGD("BufferHubBuffer::ImportGraphicBuffer: id=%d, buffer_state=%" PRIx32 ".", id(),
buffer_state_->load(std::memory_order_acquire));
return 0;
}
int BufferHubBuffer::Gain() {
- uint64_t current_buffer_state = buffer_state_->load(std::memory_order_acquire);
+ uint32_t current_buffer_state = buffer_state_->load(std::memory_order_acquire);
if (IsClientGained(current_buffer_state, mClientStateMask)) {
- ALOGV("%s: Buffer is already gained by this client %" PRIx64 ".", __FUNCTION__,
+ ALOGV("%s: Buffer is already gained by this client %" PRIx32 ".", __FUNCTION__,
mClientStateMask);
return 0;
}
do {
if (AnyClientGained(current_buffer_state & (~mClientStateMask)) ||
AnyClientAcquired(current_buffer_state)) {
- ALOGE("%s: Buffer is in use, id=%d mClientStateMask=%" PRIx64 " state=%" PRIx64 ".",
+ ALOGE("%s: Buffer is in use, id=%d mClientStateMask=%" PRIx32 " state=%" PRIx32 ".",
__FUNCTION__, mId, mClientStateMask, current_buffer_state);
return -EBUSY;
}
@@ -220,13 +220,13 @@
}
int BufferHubBuffer::Post() {
- uint64_t current_buffer_state = buffer_state_->load(std::memory_order_acquire);
- uint64_t current_active_clients_bit_mask = 0ULL;
- uint64_t updated_buffer_state = 0ULL;
+ uint32_t current_buffer_state = buffer_state_->load(std::memory_order_acquire);
+ uint32_t current_active_clients_bit_mask = 0U;
+ uint32_t updated_buffer_state = 0U;
do {
if (!IsClientGained(current_buffer_state, mClientStateMask)) {
ALOGE("%s: Cannot post a buffer that is not gained by this client. buffer_id=%d "
- "mClientStateMask=%" PRIx64 " state=%" PRIx64 ".",
+ "mClientStateMask=%" PRIx32 " state=%" PRIx32 ".",
__FUNCTION__, mId, mClientStateMask, current_buffer_state);
return -EBUSY;
}
@@ -242,17 +242,17 @@
}
int BufferHubBuffer::Acquire() {
- uint64_t current_buffer_state = buffer_state_->load(std::memory_order_acquire);
+ uint32_t current_buffer_state = buffer_state_->load(std::memory_order_acquire);
if (IsClientAcquired(current_buffer_state, mClientStateMask)) {
- ALOGV("%s: Buffer is already acquired by this client %" PRIx64 ".", __FUNCTION__,
+ ALOGV("%s: Buffer is already acquired by this client %" PRIx32 ".", __FUNCTION__,
mClientStateMask);
return 0;
}
- uint64_t updated_buffer_state = 0ULL;
+ uint32_t updated_buffer_state = 0U;
do {
if (!IsClientPosted(current_buffer_state, mClientStateMask)) {
ALOGE("%s: Cannot acquire a buffer that is not in posted state. buffer_id=%d "
- "mClientStateMask=%" PRIx64 " state=%" PRIx64 ".",
+ "mClientStateMask=%" PRIx32 " state=%" PRIx32 ".",
__FUNCTION__, mId, mClientStateMask, current_buffer_state);
return -EBUSY;
}
@@ -266,13 +266,13 @@
}
int BufferHubBuffer::Release() {
- uint64_t current_buffer_state = buffer_state_->load(std::memory_order_acquire);
+ uint32_t current_buffer_state = buffer_state_->load(std::memory_order_acquire);
if (IsClientReleased(current_buffer_state, mClientStateMask)) {
- ALOGV("%s: Buffer is already released by this client %" PRIx64 ".", __FUNCTION__,
+ ALOGV("%s: Buffer is already released by this client %" PRIx32 ".", __FUNCTION__,
mClientStateMask);
return 0;
}
- uint64_t updated_buffer_state = 0ULL;
+ uint32_t updated_buffer_state = 0U;
do {
updated_buffer_state = current_buffer_state & (~mClientStateMask);
} while (!buffer_state_->compare_exchange_weak(current_buffer_state, updated_buffer_state,
diff --git a/libs/ui/include/ui/BufferHubBuffer.h b/libs/ui/include/ui/BufferHubBuffer.h
index 11e9b5c..90dd391 100644
--- a/libs/ui/include/ui/BufferHubBuffer.h
+++ b/libs/ui/include/ui/BufferHubBuffer.h
@@ -86,13 +86,13 @@
const native_handle_t* DuplicateHandle() { return mBufferHandle.DuplicateHandle(); }
// Returns the current value of MetadataHeader::buffer_state.
- uint64_t buffer_state() {
+ uint32_t buffer_state() {
return mMetadata.metadata_header()->buffer_state.load(std::memory_order_acquire);
}
// A state mask which is unique to a buffer hub client among all its siblings sharing the same
// concrete graphic buffer.
- uint64_t client_state_mask() const { return mClientStateMask; }
+ uint32_t client_state_mask() const { return mClientStateMask; }
size_t user_metadata_size() const { return mMetadata.user_metadata_size(); }
@@ -154,7 +154,7 @@
// Client state mask of this BufferHubBuffer object. It is unique amoung all
// clients/users of the buffer.
- uint64_t mClientStateMask = 0;
+ uint32_t mClientStateMask = 0U;
// Stores ground truth of the buffer.
AHardwareBuffer_Desc mBufferDesc;
@@ -166,9 +166,9 @@
// bufferhubd daemon and all buffer clients.
BufferHubMetadata mMetadata;
// Shortcuts to the atomics inside the header of mMetadata.
- std::atomic<uint64_t>* buffer_state_{nullptr};
- std::atomic<uint64_t>* fence_state_{nullptr};
- std::atomic<uint64_t>* active_clients_bit_mask_{nullptr};
+ std::atomic<uint32_t>* buffer_state_ = nullptr;
+ std::atomic<uint32_t>* fence_state_ = nullptr;
+ std::atomic<uint32_t>* active_clients_bit_mask_ = nullptr;
// PDX backend.
BufferHubClient mClient;
diff --git a/libs/ui/include/ui/BufferHubDefs.h b/libs/ui/include/ui/BufferHubDefs.h
index ef6668b..d259fef 100644
--- a/libs/ui/include/ui/BufferHubDefs.h
+++ b/libs/ui/include/ui/BufferHubDefs.h
@@ -29,10 +29,10 @@
namespace BufferHubDefs {
-// 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.
+// Single buffer clients (up to 16) ownership signal.
+// 32-bit atomic unsigned int.
+// Each client takes 2 bits. The first bit locates in the first 16 bits of
+// buffer_state; the second bit locates in the last 16 bits of buffer_state.
// Client states:
// Gained state 11. Exclusive write state.
// Posted state 10.
@@ -42,88 +42,88 @@
// MSB LSB
// | |
// v v
-// [C31|...|C1|C0|C31| ... |C1|C0]
+// [C15|...|C1|C0|C15| ... |C1|C0]
// Maximum number of clients a buffer can have.
-static constexpr int kMaxNumberOfClients = 32;
+static constexpr int kMaxNumberOfClients = 16;
// Definition of bit masks.
// MSB LSB
// | kHighBitsMask | kLowbitsMask |
// v v v
-// [b63| ... |b32|b31| ... |b0]
+// [b31| ... |b16|b15| ... |b0]
-// The location of lower 32 bits in the 64-bit buffer state.
-static constexpr uint64_t kLowbitsMask = (1ULL << kMaxNumberOfClients) - 1ULL;
+// The location of lower 16 bits in the 32-bit buffer state.
+static constexpr uint32_t kLowbitsMask = (1U << kMaxNumberOfClients) - 1U;
-// The location of higher 32 bits in the 64-bit buffer state.
-static constexpr uint64_t kHighBitsMask = ~kLowbitsMask;
+// The location of higher 16 bits in the 32-bit buffer state.
+static constexpr uint32_t kHighBitsMask = ~kLowbitsMask;
// The client bit mask of the first client.
-static constexpr uint64_t kFirstClientBitMask = (1ULL << kMaxNumberOfClients) + 1ULL;
+static constexpr uint32_t kFirstClientBitMask = (1U << kMaxNumberOfClients) + 1U;
// 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 AnyClientGained(uint32_t state) {
+ uint32_t high_bits = state >> kMaxNumberOfClients;
+ uint32_t low_bits = state & kLowbitsMask;
+ return high_bits == low_bits && low_bits != 0U;
}
// Returns true if the input client is in gained state.
-static inline bool IsClientGained(uint64_t state, uint64_t client_bit_mask) {
+static inline bool IsClientGained(uint32_t state, uint32_t client_bit_mask) {
return state == client_bit_mask;
}
// 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;
+static inline bool AnyClientPosted(uint32_t state) {
+ uint32_t high_bits = state >> kMaxNumberOfClients;
+ uint32_t low_bits = state & kLowbitsMask;
+ uint32_t posted_or_acquired = high_bits ^ low_bits;
return posted_or_acquired & high_bits;
}
// 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 bool IsClientPosted(uint32_t state, uint32_t client_bit_mask) {
+ uint32_t client_bits = state & client_bit_mask;
+ if (client_bits == 0U) return false;
+ uint32_t low_bits = client_bits & kLowbitsMask;
+ return low_bits == 0U;
}
// 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;
+static inline bool AnyClientAcquired(uint32_t state) {
+ uint32_t high_bits = state >> kMaxNumberOfClients;
+ uint32_t low_bits = state & kLowbitsMask;
+ uint32_t posted_or_acquired = high_bits ^ low_bits;
return posted_or_acquired & low_bits;
}
// 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;
+static inline bool IsClientAcquired(uint32_t state, uint32_t client_bit_mask) {
+ uint32_t client_bits = state & client_bit_mask;
+ if (client_bits == 0U) return false;
+ uint32_t high_bits = client_bits & kHighBitsMask;
+ return high_bits == 0U;
}
// Returns true if all clients are in released state.
-static inline bool IsBufferReleased(uint64_t state) {
- return state == 0ULL;
+static inline bool IsBufferReleased(uint32_t state) {
+ return state == 0U;
}
// 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;
+static inline bool IsClientReleased(uint32_t state, uint32_t client_bit_mask) {
+ return (state & client_bit_mask) == 0U;
}
// 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;
+static inline uint32_t FindNextAvailableClientStateMask(uint32_t union_bits) {
+ uint32_t low_union = union_bits & kLowbitsMask;
+ if (low_union == kLowbitsMask) return 0U;
+ uint32_t incremented = low_union + 1U;
+ uint32_t difference = incremented ^ low_union;
+ uint32_t new_low_bit = (difference + 1U) >> 1;
return new_low_bit + (new_low_bit << kMaxNumberOfClients);
}
@@ -135,15 +135,18 @@
// 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;
+ std::atomic<uint32_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;
+ std::atomic<uint32_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;
+ std::atomic<uint32_t> active_clients_bit_mask;
+
+ // Explicit padding 4 bytes.
+ uint32_t padding;
// The index of the buffer queue where the buffer belongs to.
uint64_t queue_index;
@@ -152,7 +155,7 @@
DvrNativeBufferMetadata metadata;
};
-static_assert(sizeof(MetadataHeader) == 136, "Unexpected MetadataHeader size");
+static_assert(sizeof(MetadataHeader) == 128, "Unexpected MetadataHeader size");
static constexpr size_t kMetadataHeaderSize = sizeof(MetadataHeader);
} // namespace BufferHubDefs
diff --git a/libs/ui/tests/BufferHubBuffer_test.cpp b/libs/ui/tests/BufferHubBuffer_test.cpp
index f616785..a894f20 100644
--- a/libs/ui/tests/BufferHubBuffer_test.cpp
+++ b/libs/ui/tests/BufferHubBuffer_test.cpp
@@ -67,9 +67,9 @@
}
std::unique_ptr<BufferHubBuffer> b1;
- uint64_t b1ClientMask = 0ULL;
+ uint64_t b1ClientMask = 0U;
std::unique_ptr<BufferHubBuffer> b2;
- uint64_t b2ClientMask = 0ULL;
+ uint64_t b2ClientMask = 0U;
private:
// Creates b1 and b2 as the clients of the same buffer for testing.
@@ -79,13 +79,13 @@
void BufferHubBufferStateTransitionTest::CreateTwoClientsOfABuffer() {
b1 = BufferHubBuffer::Create(kWidth, kHeight, kLayerCount, kFormat, kUsage, kUserMetadataSize);
b1ClientMask = b1->client_state_mask();
- ASSERT_NE(b1ClientMask, 0ULL);
+ ASSERT_NE(b1ClientMask, 0U);
auto statusOrHandle = b1->Duplicate();
ASSERT_TRUE(statusOrHandle);
LocalChannelHandle h2 = statusOrHandle.take();
b2 = BufferHubBuffer::Import(std::move(h2));
b2ClientMask = b2->client_state_mask();
- ASSERT_NE(b2ClientMask, 0ULL);
+ ASSERT_NE(b2ClientMask, 0U);
ASSERT_NE(b2ClientMask, b1ClientMask);
}
@@ -126,7 +126,7 @@
kUserMetadataSize);
int id1 = b1->id();
uint64_t bufferStateMask1 = b1->client_state_mask();
- EXPECT_NE(bufferStateMask1, 0ULL);
+ EXPECT_NE(bufferStateMask1, 0U);
EXPECT_TRUE(b1->IsValid());
EXPECT_EQ(b1->user_metadata_size(), kUserMetadataSize);
@@ -149,7 +149,7 @@
int id2 = b2->id();
uint64_t bufferStateMask2 = b2->client_state_mask();
- EXPECT_NE(bufferStateMask2, 0ULL);
+ EXPECT_NE(bufferStateMask2, 0U);
// These two buffer instances are based on the same physical buffer under the
// hood, so they should share the same id.