diff --git a/headers/private/kernel/timer.h b/headers/private/kernel/timer.h index 2789b56493..a1c91af824 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 8c0cabec15..70e6082a2b 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 7a90a8f930..87f564a5a3 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 20bf8f79ad..9fd94c9776 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 3f626c9eeb..afea3a79d2 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) {