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

Commit 6c902ef9 authored by Lajos Molnar's avatar Lajos Molnar
Browse files

codec2_vndk: don't spin during sync variable locking/unlocking 

Removed spinning and sched_yield for the user-space locking section
of lock/unlock and added comments to the code.

TODO: remove the spinning code in unlock as it does not seem to be
necessary.

Bug: 287432155
Change-Id: Ie283402b3177013f918e3facb287c23a13df0550
parent 098db221

media/codec2/vndk/platform/C2SurfaceSyncObj.cpp

+45 −11
Original line number Diff line number Diff line
@@ -114,8 +114,8 @@ C2SyncVariables *C2SurfaceSyncMemory::mem() { 
}



namespace {

    constexpr int kSpinNumForLock = 100;

    constexpr int kSpinNumForUnlock = 200;

    constexpr int kSpinNumForLock = 0;

    constexpr int kSpinNumForUnlock = 0;



    enum : uint32_t {

        FUTEX_UNLOCKED = 0,
@@ -125,32 +125,65 @@ namespace { 
}



int C2SyncVariables::lock() {

    uint32_t old;

    for (int i = 0; i < kSpinNumForLock; i++) {

        old = 0;

    uint32_t old = FUTEX_UNLOCKED;



    // see if we can lock uncontended immediately (if previously unlocked)

    if (mLock.compare_exchange_strong(old, FUTEX_LOCKED_UNCONTENDED)) {

        return 0;

    }



    // spin to see if we can get it with a short wait without involving kernel

    for (int i = 0; i < kSpinNumForLock; i++) {

        sched_yield();



        old = FUTEX_UNLOCKED;

        if (mLock.compare_exchange_strong(old, FUTEX_LOCKED_UNCONTENDED)) {

            return 0;

        }

    }



    if (old == FUTEX_LOCKED_UNCONTENDED)

    // still locked, if other side thinks it was uncontended, now it is contended, so let them

    // know that they need to wake us up.

    if (old == FUTEX_LOCKED_UNCONTENDED) {

        old = mLock.exchange(FUTEX_LOCKED_CONTENDED);

        // It is possible that the other holder released the lock at this very moment (and old

        // becomes UNLOCKED), If so, we will not involve the kernel to wait for the lock to be

        // released, but are still marking our lock contended (even though we are the only

        // holders.)

    }



    while (old) {

    // while the futex is still locked by someone else

    while (old != FUTEX_UNLOCKED) {

        // wait until other side releases the lock (and still contented)

        (void)syscall(__NR_futex, &mLock, FUTEX_WAIT, FUTEX_LOCKED_CONTENDED, NULL, NULL, 0);

        // try to relock

        old = mLock.exchange(FUTEX_LOCKED_CONTENDED);

    }

    return 0;

}



int C2SyncVariables::unlock() {

    if (mLock.exchange(FUTEX_UNLOCKED) == FUTEX_LOCKED_UNCONTENDED) return 0;

    // TRICKY: here we assume that we are holding this lock



    // unlock the lock immediately (since we were holding it)

    // If it is (still) locked uncontested, we are done (no need to involve the kernel)

    if (mLock.exchange(FUTEX_UNLOCKED) == FUTEX_LOCKED_UNCONTENDED) {

        return 0;

    }



    // We don't need to spin for unlock as here we know already we have a waiter who we need to

    // wake up. This code was here in case someone just happened to lock this lock (uncontested)

    // before we would wake up other waiters to avoid a syscall. It is unsure if this ever gets

    // exercised or if this is the behavior we want. (Note that if this code is removed, the same

    // situation is still handled in lock() by the woken up waiter that realizes that the lock is

    // now taken.)

    for (int i = 0; i < kSpinNumForUnlock; i++) {

        if (mLock.load()) {

        // here we seem to check if someone relocked this lock, and if they relocked uncontested,

        // we up it to contested (since there are other waiters.)

        if (mLock.load() != FUTEX_UNLOCKED) {

            uint32_t old = FUTEX_LOCKED_UNCONTENDED;

            mLock.compare_exchange_strong(old, FUTEX_LOCKED_CONTENDED);

            // this is always true here so we return immediately

            if (old) {

                return 0;

            }
@@ -158,6 +191,7 @@ int C2SyncVariables::unlock() { 
        sched_yield();

    }



    // wake up one waiter

    (void)syscall(__NR_futex, &mLock, FUTEX_WAKE, 1, NULL, NULL, 0);

    return 0;

}