[haiku-commits] BRANCH pdziepak-github.scheduler [d8fcc8a] src/system/kernel headers/private/kernel src/system/kernel/scheduler
- From: pdziepak-github.scheduler <community@xxxxxxxxxxxx>
- To: haiku-commits@xxxxxxxxxxxxx
- Date: Thu, 31 Oct 2013 02:15:35 +0100 (CET)
added 1 changeset to branch 'refs/remotes/pdziepak-github/scheduler'
old head: c8dd9f7780c426e592a3ccb231e6bfab51f15eb9
new head: d8fcc8a82519cef977c689cee497316be6f1531f
overview: https://github.com/pdziepak/Haiku/compare/c8dd9f7...d8fcc8a
----------------------------------------------------------------------------
d8fcc8a: kernel: Remove B_TIMER_ACQUIRE_SCHEDULER_LOCK flag
The flag main purpose is to avoid race conditions between event handler
and cancel_timer(). However, cancel_timer() is safe even without
using gSchedulerLock.
If the event is scheduled to happen on other CPU than the CPU that
invokes cancel_timer() then cancel_timer() either disables the event
before its handler starts executing or waits until the event handler
is done.
If the event is scheduled on the same CPU that calls cancel_timer()
then, since cancel_timer() disables interrupts, the event is either
executed before cancel_timer() or when the timer interrupt handler
starts running the event is already disabled.
[ Pawel Dziepak <pdziepak@xxxxxxxxxxx> ]
----------------------------------------------------------------------------
Commit: d8fcc8a82519cef977c689cee497316be6f1531f
Author: Pawel Dziepak <pdziepak@xxxxxxxxxxx>
Date: Thu Oct 31 00:49:43 2013 UTC
----------------------------------------------------------------------------
5 files changed, 12 insertions(+), 56 deletions(-)
headers/private/kernel/timer.h | 7 +------
src/system/kernel/UserTimer.cpp | 16 ++++------------
src/system/kernel/scheduler/scheduler.cpp | 2 +-
src/system/kernel/thread.cpp | 19 +++----------------
src/system/kernel/timer.cpp | 24 +++---------------------
----------------------------------------------------------------------------
diff --git a/headers/private/kernel/timer.h b/headers/private/kernel/timer.h
index 2789b56..a1c91af 100644
--- a/headers/private/kernel/timer.h
+++ b/headers/private/kernel/timer.h
@@ -23,13 +23,8 @@ struct kernel_args;
#define B_TIMER_USE_TIMER_STRUCT_TIMES 0x4000
// For add_timer(): Use the timer::schedule_time (absolute time) and
// timer::period values instead of the period parameter.
-#define B_TIMER_ACQUIRE_SCHEDULER_LOCK 0x8000
- // The timer hook is invoked with the scheduler lock held. When invoking
- // cancel_timer() with the scheduler lock held, too, this helps to avoid
- // race conditions.
#define B_TIMER_FLAGS \
- (B_TIMER_USE_TIMER_STRUCT_TIMES | B_TIMER_ACQUIRE_SCHEDULER_LOCK
\
- | B_TIMER_REAL_TIME_BASE)
+ (B_TIMER_USE_TIMER_STRUCT_TIMES | B_TIMER_REAL_TIME_BASE)
/* Timer info structure */
struct timer_info {
diff --git a/src/system/kernel/UserTimer.cpp b/src/system/kernel/UserTimer.cpp
index 8c0cabe..70e6082 100644
--- a/src/system/kernel/UserTimer.cpp
+++ b/src/system/kernel/UserTimer.cpp
@@ -191,6 +191,7 @@ UserTimer::Cancel()
/*static*/ int32
UserTimer::HandleTimerHook(struct timer* timer)
{
+ InterruptsSpinLocker _(gSchedulerLock);
((UserTimer*)timer->user_data)->HandleTimer();
return B_HANDLED_INTERRUPT;
}
@@ -352,9 +353,7 @@ SystemTimeUserTimer::ScheduleKernelTimer(bigtime_t now,
CheckPeriodicOverrun(now);
uint32 timerFlags = B_ONE_SHOT_ABSOLUTE_TIMER
- | B_TIMER_USE_TIMER_STRUCT_TIMES |
B_TIMER_ACQUIRE_SCHEDULER_LOCK;
- // We use B_TIMER_ACQUIRE_SCHEDULER_LOCK to avoid race
conditions
- // between setting/canceling the timer and the event handler.
+ | B_TIMER_USE_TIMER_STRUCT_TIMES;
fTimer.schedule_time = std::max(fNextTime, (bigtime_t)0);
fTimer.period = 0;
@@ -692,10 +691,7 @@ TeamTimeUserTimer::_Update(bool unscheduling)
// rounding errors.
add_timer(&fTimer, &HandleTimerHook, fTimer.schedule_time,
- B_ONE_SHOT_ABSOLUTE_TIMER | B_TIMER_USE_TIMER_STRUCT_TIMES
- | B_TIMER_ACQUIRE_SCHEDULER_LOCK);
- // We use B_TIMER_ACQUIRE_SCHEDULER_LOCK to avoid race
conditions
- // between setting/canceling the timer and the event handler.
+ B_ONE_SHOT_ABSOLUTE_TIMER | B_TIMER_USE_TIMER_STRUCT_TIMES);
// We use B_TIMER_USE_TIMER_STRUCT_TIMES, so period remains 0,
which
// our base class expects.
@@ -989,11 +985,7 @@ ThreadTimeUserTimer::Start()
fTimer.schedule_time = 0;
fTimer.period = 0;
- uint32 flags = B_ONE_SHOT_ABSOLUTE_TIMER
- | B_TIMER_USE_TIMER_STRUCT_TIMES |
B_TIMER_ACQUIRE_SCHEDULER_LOCK;
- // We use B_TIMER_ACQUIRE_SCHEDULER_LOCK to avoid race
conditions
- // between setting/canceling the timer and the event handler.
-
+ uint32 flags = B_ONE_SHOT_ABSOLUTE_TIMER |
B_TIMER_USE_TIMER_STRUCT_TIMES;
add_timer(&fTimer, &HandleTimerHook, fTimer.schedule_time, flags);
fScheduled = true;
diff --git a/src/system/kernel/scheduler/scheduler.cpp
b/src/system/kernel/scheduler/scheduler.cpp
index 7a90a8f..87f564a 100644
--- a/src/system/kernel/scheduler/scheduler.cpp
+++ b/src/system/kernel/scheduler/scheduler.cpp
@@ -1507,7 +1507,7 @@ _scheduler_reschedule(void)
if (!thread_is_idle_thread(nextThread)) {
bigtime_t quantum = compute_quantum(oldThread);
add_timer(quantumTimer, &reschedule_event, quantum,
- B_ONE_SHOT_RELATIVE_TIMER |
B_TIMER_ACQUIRE_SCHEDULER_LOCK);
+ B_ONE_SHOT_RELATIVE_TIMER);
update_cpu_performance(nextThread, thisCore);
} else
diff --git a/src/system/kernel/thread.cpp b/src/system/kernel/thread.cpp
index 20bf8f7..9fd94c9 100644
--- a/src/system/kernel/thread.cpp
+++ b/src/system/kernel/thread.cpp
@@ -1923,13 +1923,8 @@ thread_exit(void)
}
if (team != kernelTeam) {
- // Cancel previously installed alarm timer, if any. Hold the
scheduler
- // lock to make sure that when cancel_timer() returns, the
alarm timer
- // hook will not be invoked anymore (since
- // B_TIMER_ACQUIRE_SCHEDULER_LOCK is used).
- InterruptsSpinLocker schedulerLocker(gSchedulerLock);
+ // Cancel previously installed alarm timer, if any.
cancel_timer(&thread->alarm);
- schedulerLocker.Unlock();
// Delete all user timers associated with the thread.
ThreadLocker threadLocker(thread);
@@ -2782,12 +2777,8 @@ thread_preboot_init_percpu(struct kernel_args *args,
int32 cpuNum)
static status_t
thread_block_timeout(timer* timer)
{
- // The timer has been installed with B_TIMER_ACQUIRE_SCHEDULER_LOCK, so
- // we're holding the scheduler lock already. This makes things
comfortably
- // easy.
-
Thread* thread = (Thread*)timer->user_data;
- thread_unblock_locked(thread, B_TIMED_OUT);
+ thread_unblock(thread, B_TIMED_OUT);
return B_HANDLED_INTERRUPT;
}
@@ -2858,10 +2849,7 @@ thread_block_with_timeout_locked(uint32 timeoutFlags,
bigtime_t timeout)
&& timeout != B_INFINITE_TIMEOUT;
if (useTimer) {
- // Timer flags: absolute/relative + "acquire thread lock". The
latter
- // avoids nasty race conditions and deadlock problems that could
- // otherwise occur between our cancel_timer() and a concurrently
- // executing thread_block_timeout().
+ // Timer flags: absolute/relative.
uint32 timerFlags;
if ((timeoutFlags & B_RELATIVE_TIMEOUT) != 0) {
timerFlags = B_ONE_SHOT_RELATIVE_TIMER;
@@ -2870,7 +2858,6 @@ thread_block_with_timeout_locked(uint32 timeoutFlags,
bigtime_t timeout)
if ((timeoutFlags & B_TIMEOUT_REAL_TIME_BASE) != 0)
timerFlags |= B_TIMER_REAL_TIME_BASE;
}
- timerFlags |= B_TIMER_ACQUIRE_SCHEDULER_LOCK;
// install the timer
thread->wait.unblock_timer.user_data = thread;
diff --git a/src/system/kernel/timer.cpp b/src/system/kernel/timer.cpp
index 3f626c9..afea3a7 100644
--- a/src/system/kernel/timer.cpp
+++ b/src/system/kernel/timer.cpp
@@ -274,25 +274,8 @@ timer_interrupt()
// call the callback
// note: if the event is not periodic, it is ok
// to delete the event structure inside the callback
- if (event->hook) {
- bool callHook = true;
-
- // we may need to acquire the scheduler lock
- if ((mode & B_TIMER_ACQUIRE_SCHEDULER_LOCK) != 0) {
- acquire_spinlock(&gSchedulerLock);
-
- // If the event has been cancelled in the
meantime, we don't
- // call the hook anymore.
- if (cpuData.current_event == NULL)
- callHook = false;
- }
-
- if (callHook)
- rc = event->hook(event);
-
- if ((mode & B_TIMER_ACQUIRE_SCHEDULER_LOCK) != 0)
- release_spinlock(&gSchedulerLock);
- }
+ if (event->hook)
+ rc = event->hook(event);
cpuData.current_event_in_progress = 0;
@@ -461,8 +444,7 @@ cancel_timer(timer* event)
// lock to be held while calling the event hook, we'll have to wait
// for the hook to complete. When called from the timer hook we don't
// wait either, of course.
- if ((event->flags & B_TIMER_ACQUIRE_SCHEDULER_LOCK) == 0
- && cpu != smp_get_current_cpu()) {
+ if (cpu != smp_get_current_cpu()) {
spinLocker.Unlock();
while (cpuData.current_event_in_progress == 1) {
Other related posts:
