[haiku-commits] Change in haiku[master]: kernel/locks: Remove ignore_unlock_count.

  • From: Gerrit <review@xxxxxxxxxxxxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Sat, 16 May 2020 21:18:27 +0000

From waddlesplash <waddlesplash@xxxxxxxxx>:

waddlesplash has uploaded this change for review. ( 
https://review.haiku-os.org/c/haiku/+/2716 ;)


Change subject: kernel/locks: Remove ignore_unlock_count.
......................................................................

kernel/locks: Remove ignore_unlock_count.

This was actually just plain wrong: if ignore_unlock_count was > 0
and the holding thread called unlock(), MUTEX_FLAG_RELEASED
would never be set and the lock would never be able to be acquired again.

As far as I can tell, there is no reason to ignore unlocks, ever;
if no threads are waiting, then mutex_unlock() will act appropriately.
So all we need to do is increment the lock's count here,
as we are relinquishing our request for locking.

On the other hand, if we did not find our structure in the lock,
that means we own the lock; so to return with an error from here
would also result in a deadlock, as nothing would ever set
MUTEX_FLAG_RELEASED.

The first of these problems would only occur under non-KDEBUG
(i.e. non-nightly) kernels, as ignore_unlock_count was only
set from there. But the latter could occur on any kernel.

May fix #16044, as it appears there is a case where ACPICA
calls this with a timeout of 0 (we should make this be
a mutex_trylock, anyway.)
---
M headers/private/kernel/lock.h
M src/system/kernel/locks/lock.cpp
2 files changed, 9 insertions(+), 18 deletions(-)



  git pull ssh://git.haiku-os.org:22/haiku refs/changes/16/2716/1

diff --git a/headers/private/kernel/lock.h b/headers/private/kernel/lock.h
index 500118b..2343cf5 100644
--- a/headers/private/kernel/lock.h
+++ b/headers/private/kernel/lock.h
@@ -24,11 +24,10 @@
        spinlock                                lock;
 #if KDEBUG
        thread_id                               holder;
-       uint16                                  _unused;
 #else
        int32                                   count;
-       uint16                                  ignore_unlock_count;
 #endif
+       uint16                                  _unused;
        uint8                                   flags;
 } mutex;

diff --git a/src/system/kernel/locks/lock.cpp b/src/system/kernel/locks/lock.cpp
index 488d413..1f31d80 100644
--- a/src/system/kernel/locks/lock.cpp
+++ b/src/system/kernel/locks/lock.cpp
@@ -609,7 +609,6 @@
        lock->holder = -1;
 #else
        lock->count = 0;
-       lock->ignore_unlock_count = 0;
 #endif
        lock->flags = flags & MUTEX_FLAG_CLONE_NAME;

@@ -801,11 +800,6 @@
                        thread_get_current_thread_id(), lock, lock->holder);
                return;
        }
-#else
-       if (lock->ignore_unlock_count > 0) {
-               lock->ignore_unlock_count--;
-               return;
-       }
 #endif

        mutex_waiter* waiter = lock->waiters;
@@ -931,17 +925,15 @@

 #if !KDEBUG
                        // we need to fix the lock count
-                       if (atomic_add(&lock->count, 1) == -1) {
-                               // This means we were the only thread waiting 
for the lock and
-                               // the lock owner has already called 
atomic_add() in
-                               // mutex_unlock(). That is we probably would 
get the lock very
-                               // soon (if the lock holder has a low priority, 
that might
-                               // actually take rather long, though), but the 
timeout already
-                               // occurred, so we don't try to wait. Just 
increment the ignore
-                               // unlock count.
-                               lock->ignore_unlock_count++;
-                       }
+                       atomic_add(&lock->count, 1);
 #endif
+               } else {
+                       // the structure is not in the list -- even though the 
timeout
+                       // occurred, this means we own the lock now
+#if KDEBUG
+                       ASSERT(lock->holder == waiter.thread->id);
+#endif
+                       return B_OK;
                }
        }


--
To view, visit https://review.haiku-os.org/c/haiku/+/2716
To unsubscribe, or for help writing mail filters, visit 
https://review.haiku-os.org/settings

Gerrit-Project: haiku
Gerrit-Branch: master
Gerrit-Change-Id: I98215df218514c70ac1922bc3a6f10e01087e44b
Gerrit-Change-Number: 2716
Gerrit-PatchSet: 1
Gerrit-Owner: waddlesplash <waddlesplash@xxxxxxxxx>
Gerrit-MessageType: newchange

Other related posts:

  • » [haiku-commits] Change in haiku[master]: kernel/locks: Remove ignore_unlock_count. - Gerrit