Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a99f9114 authored by Tianyu Jiang's avatar Tianyu Jiang
Browse files

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
parent 7bd483a2

libs/gui/BufferHubProducer.cpp

+1 −1
Original line number Diff line number Diff line
@@ -520,7 +520,7 @@ status_t BufferHubProducer::cancelBuffer(int slot, const sp<Fence>& fence) { 
    }



    auto buffer_producer = buffers_[slot].mBufferProducer;

    queue_->Enqueue(buffer_producer, size_t(slot), 0ULL);

    queue_->Enqueue(buffer_producer, size_t(slot), 0U);

    buffers_[slot].mBufferState.cancel();

    buffers_[slot].mFence = fence;

    ALOGV("cancelBuffer: slot %d", slot);

libs/ui/BufferHubBuffer.cpp

+15 −15
Original line number Diff line number Diff line
@@ -191,22 +191,22 @@ int BufferHubBuffer::ImportGraphicBuffer() { 
    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::Gain() { 
}



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::Post() { 
}



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::Acquire() { 
}



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,

libs/ui/include/ui/BufferHubBuffer.h

+6 −6
Original line number Diff line number Diff line
@@ -86,13 +86,13 @@ public: 
    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 @@ private: 


    // 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 @@ private: 
    // 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;

libs/ui/include/ui/BufferHubDefs.h

+52 −49
Original line number Diff line number Diff line
@@ -29,10 +29,10 @@ namespace android { 


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 @@ namespace BufferHubDefs { 
//  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 @@ struct __attribute__((aligned(8))) MetadataHeader { 


    // 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 @@ struct __attribute__((aligned(8))) MetadataHeader { 
    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

libs/ui/tests/BufferHubBuffer_test.cpp

+6 −6
Original line number Diff line number Diff line
@@ -67,9 +67,9 @@ protected: 
    }



    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 @@ private: 
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 @@ TEST_F(BufferHubBufferTest, DuplicateBufferHubBuffer) { 
                                      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 @@ TEST_F(BufferHubBufferTest, DuplicateBufferHubBuffer) { 


    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.