From 11cacd0c1345cd3296f12e07d18996a2c7572a64 Mon Sep 17 00:00:00 2001 From: Pawel Dziepak Date: Mon, 4 Nov 2013 23:45:14 +0100 Subject: [PATCH] kernel: Remove thread_block_with_timeout_locked() --- headers/private/kernel/thread.h | 18 +++++++-------- src/system/kernel/condition_variable.cpp | 8 ++----- src/system/kernel/sem.cpp | 6 +---- src/system/kernel/signal.cpp | 10 ++++++-- src/system/kernel/thread.cpp | 29 +++++++----------------- 5 files changed, 27 insertions(+), 44 deletions(-) diff --git a/headers/private/kernel/thread.h b/headers/private/kernel/thread.h index ecb6245dbd..9cc12499c2 100644 --- a/headers/private/kernel/thread.h +++ b/headers/private/kernel/thread.h @@ -130,8 +130,6 @@ status_t deselect_thread(int32 object, struct select_info *info, bool kernel); status_t thread_block(); status_t thread_block_with_timeout(uint32 timeoutFlags, bigtime_t timeout); -status_t thread_block_with_timeout_locked(uint32 timeoutFlags, - bigtime_t timeout); void thread_unblock(Thread* thread, status_t status); // used in syscalls.c @@ -230,22 +228,22 @@ thread_is_blocked(Thread* thread) If a client lock other than the scheduler lock is used, this function must be called with that lock being held. Afterwards that lock should be dropped and the function that actually blocks the thread shall be invoked - (thread_block[_locked]() or thread_block_with_timeout[_locked]()). In - between these two steps no functionality that uses the thread blocking API - for this thread shall be used. + (thread_block[_locked]() or thread_block_with_timeout()). In between these + two steps no functionality that uses the thread blocking API for this thread + shall be used. When the caller determines that the condition for unblocking the thread occurred, it calls thread_unblock_locked() to unblock the thread. At that time one of locks that are held when calling thread_prepare_to_block() must be held. Usually that would be the client lock. In two cases it generally isn't, however, since the unblocking code doesn't know about the client - lock: 1. When thread_block_with_timeout[_locked]() had been used and the - timeout occurs. 2. When thread_prepare_to_block() had been called with one - or both of the \c B_CAN_INTERRUPT or \c B_KILL_CAN_INTERRUPT flags specified - and someone calls thread_interrupt() that is supposed to wake up the thread. + lock: 1. When thread_block_with_timeout() had been used and the timeout + occurs. 2. When thread_prepare_to_block() had been called with one or both + of the \c B_CAN_INTERRUPT or \c B_KILL_CAN_INTERRUPT flags specified and + someone calls thread_interrupt() that is supposed to wake up the thread. In either of these two cases only the scheduler lock is held by the unblocking code. A timeout can only happen after - thread_block_with_timeout_locked() has been called, but an interruption is + thread_block_with_timeout() has been called, but an interruption is possible at any time. The client code must deal with those situations. Generally blocking and unblocking threads proceed in the following manner: diff --git a/src/system/kernel/condition_variable.cpp b/src/system/kernel/condition_variable.cpp index 47215606b1..7f6152ebce 100644 --- a/src/system/kernel/condition_variable.cpp +++ b/src/system/kernel/condition_variable.cpp @@ -134,15 +134,11 @@ ConditionVariableEntry::Wait(uint32 flags, bigtime_t timeout) conditionLocker.Unlock(); - SpinLocker schedulerLocker(gSchedulerLock); - status_t error; if ((flags & (B_RELATIVE_TIMEOUT | B_ABSOLUTE_TIMEOUT)) != 0) - error = thread_block_with_timeout_locked(flags, timeout); + error = thread_block_with_timeout(flags, timeout); else - error = thread_block_locked(thread_get_current_thread()); - - schedulerLocker.Unlock(); + error = thread_block(); conditionLocker.Lock(); diff --git a/src/system/kernel/sem.cpp b/src/system/kernel/sem.cpp index 7f01069291..7980ed29e2 100644 --- a/src/system/kernel/sem.cpp +++ b/src/system/kernel/sem.cpp @@ -852,13 +852,9 @@ switch_sem_etc(sem_id semToBeReleased, sem_id id, int32 count, semToBeReleased = -1; } - schedulerLocker.Lock(); - status_t acquireStatus = timeout == B_INFINITE_TIMEOUT - ? thread_block_locked(thread) - : thread_block_with_timeout_locked(flags, timeout); + ? thread_block() : thread_block_with_timeout(flags, timeout); - schedulerLocker.Unlock(); GRAB_SEM_LOCK(sSems[slot]); // If we're still queued, this means the acquiration failed, and we diff --git a/src/system/kernel/signal.cpp b/src/system/kernel/signal.cpp index 1d61b15202..751d763f6e 100644 --- a/src/system/kernel/signal.cpp +++ b/src/system/kernel/signal.cpp @@ -2041,16 +2041,22 @@ sigwait_internal(const sigset_t* set, siginfo_t* info, uint32 flags, thread_prepare_to_block(thread, flags, THREAD_BLOCK_TYPE_SIGNAL, NULL); + schedulerLocker.Unlock(); + if ((flags & B_ABSOLUTE_TIMEOUT) != 0) { - error = thread_block_with_timeout_locked(flags, timeout); + error = thread_block_with_timeout(flags, timeout); if (error == B_WOULD_BLOCK || error == B_TIMED_OUT) { error = B_WOULD_BLOCK; // POSIX requires EAGAIN (B_WOULD_BLOCK) on timeout timedOut = true; + + schedulerLocker.Lock(); break; } } else - thread_block_locked(thread); + thread_block(); + + schedulerLocker.Lock(); } // restore the original block mask diff --git a/src/system/kernel/thread.cpp b/src/system/kernel/thread.cpp index e11fea41ed..d36b212a09 100644 --- a/src/system/kernel/thread.cpp +++ b/src/system/kernel/thread.cpp @@ -1364,11 +1364,9 @@ common_snooze_etc(bigtime_t timeout, clockid_t clockID, uint32 flags, Thread* thread = thread_get_current_thread(); - InterruptsSpinLocker schedulerLocker(gSchedulerLock); - thread_prepare_to_block(thread, flags, THREAD_BLOCK_TYPE_SNOOZE, NULL); - status_t status = thread_block_with_timeout_locked(flags, timeout); + status_t status = thread_block_with_timeout(flags, timeout); if (status == B_TIMED_OUT || status == B_WOULD_BLOCK) return B_OK; @@ -2800,19 +2798,6 @@ thread_block() } -/*! Blocks the current thread with a timeout. - - Acquires the scheduler lock and calls thread_block_with_timeout_locked(). - See there for more information. -*/ -status_t -thread_block_with_timeout(uint32 timeoutFlags, bigtime_t timeout) -{ - InterruptsSpinLocker _(gSchedulerLock); - return thread_block_with_timeout_locked(timeoutFlags, timeout); -} - - /*! Blocks the current thread with a timeout. The thread is blocked until someone else unblock it or the specified timeout @@ -2821,7 +2806,7 @@ thread_block_with_timeout(uint32 timeoutFlags, bigtime_t timeout) thread_prepare_to_block(), this function will return immediately. See thread_prepare_to_block() for more details. - The caller must hold the scheduler lock. + The caller must not hold the scheduler lock. \param thread The current thread. \param timeoutFlags The standard timeout flags: @@ -2841,10 +2826,12 @@ thread_block_with_timeout(uint32 timeoutFlags, bigtime_t timeout) client code). */ status_t -thread_block_with_timeout_locked(uint32 timeoutFlags, bigtime_t timeout) +thread_block_with_timeout(uint32 timeoutFlags, bigtime_t timeout) { Thread* thread = thread_get_current_thread(); + InterruptsSpinLocker locker(gSchedulerLock); + if (thread->wait.status != 1) return thread->wait.status; @@ -2871,6 +2858,8 @@ thread_block_with_timeout_locked(uint32 timeoutFlags, bigtime_t timeout) // block status_t error = thread_block_locked(thread); + locker.Unlock(); + // cancel timer, if it didn't fire if (error != B_TIMED_OUT && useTimer) cancel_timer(&thread->wait.unblock_timer); @@ -3584,11 +3573,9 @@ _user_block_thread(uint32 flags, bigtime_t timeout) thread_prepare_to_block(thread, flags, THREAD_BLOCK_TYPE_OTHER, "user"); threadLocker.Unlock(); - InterruptsSpinLocker schedulerLocker(gSchedulerLock); - status_t status = thread_block_with_timeout_locked(flags, timeout); + status_t status = thread_block_with_timeout(flags, timeout); - schedulerLocker.Unlock(); threadLocker.Lock(); // Interruptions or timeouts can race with other threads unblocking us.