From af0be8dbc5df319bb65ae62cb9924d7344847a73 Mon Sep 17 00:00:00 2001 From: Augustin Cavalier Date: Mon, 5 Aug 2019 22:58:28 -0400 Subject: [PATCH] kernel/condition_variable: Clean up comments and reduce needless unlocks. This "race prevention" code does not seem to be really hit at all in practice, at least from testing, so no need to do a full unlock/lock universally for it. I'm still not sure why the previous fixes here removed 80% of the performance benefits of the original change; I need to investigate that more. --- src/system/kernel/condition_variable.cpp | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/system/kernel/condition_variable.cpp b/src/system/kernel/condition_variable.cpp index db8bf76f63..1f0c19a3be 100644 --- a/src/system/kernel/condition_variable.cpp +++ b/src/system/kernel/condition_variable.cpp @@ -159,18 +159,13 @@ ConditionVariableEntry::Wait(uint32 flags, bigtime_t timeout) entryLocker.Lock(); - // Remove entry from variable, if not done yet. Since we are going to - // lock ourselves and recheck fVariable if it is not NULL, we don't need - // to acquire our own lock before doing this first NULL check. + // Remove entry from variable, if not done yet. if (fVariable != NULL) { - if (fVariable == NULL) - return error; SpinLocker conditionLocker(fVariable->fLock); - entryLocker.Unlock(); - if (fVariable->fEntries.Contains(this)) { fVariable->fEntries.Remove(this); } else { + entryLocker.Unlock(); // The variable's fEntries did not contain us, but we currently // have the variable's lock acquired. This must mean we are in // a race with the variable's Notify. It is possible we will be @@ -178,8 +173,8 @@ ConditionVariableEntry::Wait(uint32 flags, bigtime_t timeout) // spin until our fVariable member is unset by the Notify thread // and then re-acquire our own lock to avoid a use-after-free. while (atomic_pointer_get(&fVariable) != NULL) {} + entryLocker.Lock(); } - entryLocker.Lock(); fVariable = NULL; }