sched, x86: clean up hrtick implementation
random uvesafb failures were reported against Gentoo:
http://bugs.gentoo.org/show_bug.cgi?id=222799
and Mihai Moldovan bisected it back to:
> 8f4d37ec073c17e2d4aa8851df5837d798606d6f is first bad commit
> commit 8f4d37ec073c17e2d4aa8851df5837d798606d6f
> Author: Peter Zijlstra <a.p.zijlstra@chello.nl>
> Date: Fri Jan 25 21:08:29 2008 +0100
>
> sched: high-res preemption tick
Linus suspected it to be hrtick + vm86 interaction and observed:
> Btw, Peter, Ingo: I think that commit is doing bad things. They aren't
> _incorrect_ per se, but they are definitely bad.
>
> Why?
>
> Using random _TIF_WORK_MASK flags is really impolite for doing
> "scheduling" work. There's a reason that arch/x86/kernel/entry_32.S
> special-cases the _TIF_NEED_RESCHED flag: we don't want to exit out of
> vm86 mode unnecessarily.
>
> See the "work_notifysig_v86" label, and how it does that
> "save_v86_state()" thing etc etc.
Right, I never liked having to fiddle with those TIF flags. Initially I
needed it because the hrtimer base lock could not nest in the rq lock.
That however is fixed these days.
Currently the only reason left to fiddle with the TIF flags is remote
wakeups. We cannot program a remote cpu's hrtimer. I've been thinking
about using the new and improved IPI function call stuff to implement
hrtimer_start_on().
However that does require that smp_call_function_single(.wait=0) works
from interrupt context - /me looks at the latest series from Jens - Yes
that does seem to be supported, good.
Here's a stab at cleaning this stuff up ...
Mihai reported test success as well.
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
Tested-by: Mihai Moldovan <ionic@ionic.de>
Cc: Michal Januszewski <spock@gentoo.org>
Cc: Antonino Daplas <adaplas@gmail.com>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
diff --git a/kernel/sched.c b/kernel/sched.c
index 1ee18db..c13c75e9 100644
--- a/kernel/sched.c
+++ b/kernel/sched.c
@@ -571,8 +571,10 @@
#endif
#ifdef CONFIG_SCHED_HRTICK
- unsigned long hrtick_flags;
- ktime_t hrtick_expire;
+#ifdef CONFIG_SMP
+ int hrtick_csd_pending;
+ struct call_single_data hrtick_csd;
+#endif
struct hrtimer hrtick_timer;
#endif
@@ -983,13 +985,6 @@
return rq;
}
-static void __resched_task(struct task_struct *p, int tif_bit);
-
-static inline void resched_task(struct task_struct *p)
-{
- __resched_task(p, TIF_NEED_RESCHED);
-}
-
#ifdef CONFIG_SCHED_HRTICK
/*
* Use HR-timers to deliver accurate preemption points.
@@ -1001,25 +996,6 @@
* When we get rescheduled we reprogram the hrtick_timer outside of the
* rq->lock.
*/
-static inline void resched_hrt(struct task_struct *p)
-{
- __resched_task(p, TIF_HRTICK_RESCHED);
-}
-
-static inline void resched_rq(struct rq *rq)
-{
- unsigned long flags;
-
- spin_lock_irqsave(&rq->lock, flags);
- resched_task(rq->curr);
- spin_unlock_irqrestore(&rq->lock, flags);
-}
-
-enum {
- HRTICK_SET, /* re-programm hrtick_timer */
- HRTICK_RESET, /* not a new slice */
- HRTICK_BLOCK, /* stop hrtick operations */
-};
/*
* Use hrtick when:
@@ -1030,40 +1006,11 @@
{
if (!sched_feat(HRTICK))
return 0;
- if (unlikely(test_bit(HRTICK_BLOCK, &rq->hrtick_flags)))
+ if (!cpu_online(cpu_of(rq)))
return 0;
return hrtimer_is_hres_active(&rq->hrtick_timer);
}
-/*
- * Called to set the hrtick timer state.
- *
- * called with rq->lock held and irqs disabled
- */
-static void hrtick_start(struct rq *rq, u64 delay, int reset)
-{
- assert_spin_locked(&rq->lock);
-
- /*
- * preempt at: now + delay
- */
- rq->hrtick_expire =
- ktime_add_ns(rq->hrtick_timer.base->get_time(), delay);
- /*
- * indicate we need to program the timer
- */
- __set_bit(HRTICK_SET, &rq->hrtick_flags);
- if (reset)
- __set_bit(HRTICK_RESET, &rq->hrtick_flags);
-
- /*
- * New slices are called from the schedule path and don't need a
- * forced reschedule.
- */
- if (reset)
- resched_hrt(rq->curr);
-}
-
static void hrtick_clear(struct rq *rq)
{
if (hrtimer_active(&rq->hrtick_timer))
@@ -1071,32 +1018,6 @@
}
/*
- * Update the timer from the possible pending state.
- */
-static void hrtick_set(struct rq *rq)
-{
- ktime_t time;
- int set, reset;
- unsigned long flags;
-
- WARN_ON_ONCE(cpu_of(rq) != smp_processor_id());
-
- spin_lock_irqsave(&rq->lock, flags);
- set = __test_and_clear_bit(HRTICK_SET, &rq->hrtick_flags);
- reset = __test_and_clear_bit(HRTICK_RESET, &rq->hrtick_flags);
- time = rq->hrtick_expire;
- clear_thread_flag(TIF_HRTICK_RESCHED);
- spin_unlock_irqrestore(&rq->lock, flags);
-
- if (set) {
- hrtimer_start(&rq->hrtick_timer, time, HRTIMER_MODE_ABS);
- if (reset && !hrtimer_active(&rq->hrtick_timer))
- resched_rq(rq);
- } else
- hrtick_clear(rq);
-}
-
-/*
* High-resolution timer tick.
* Runs from hardirq context with interrupts disabled.
*/
@@ -1115,27 +1036,37 @@
}
#ifdef CONFIG_SMP
-static void hotplug_hrtick_disable(int cpu)
+/*
+ * called from hardirq (IPI) context
+ */
+static void __hrtick_start(void *arg)
{
- struct rq *rq = cpu_rq(cpu);
- unsigned long flags;
+ struct rq *rq = arg;
- spin_lock_irqsave(&rq->lock, flags);
- rq->hrtick_flags = 0;
- __set_bit(HRTICK_BLOCK, &rq->hrtick_flags);
- spin_unlock_irqrestore(&rq->lock, flags);
-
- hrtick_clear(rq);
+ spin_lock(&rq->lock);
+ hrtimer_restart(&rq->hrtick_timer);
+ rq->hrtick_csd_pending = 0;
+ spin_unlock(&rq->lock);
}
-static void hotplug_hrtick_enable(int cpu)
+/*
+ * Called to set the hrtick timer state.
+ *
+ * called with rq->lock held and irqs disabled
+ */
+static void hrtick_start(struct rq *rq, u64 delay)
{
- struct rq *rq = cpu_rq(cpu);
- unsigned long flags;
+ struct hrtimer *timer = &rq->hrtick_timer;
+ ktime_t time = ktime_add_ns(timer->base->get_time(), delay);
- spin_lock_irqsave(&rq->lock, flags);
- __clear_bit(HRTICK_BLOCK, &rq->hrtick_flags);
- spin_unlock_irqrestore(&rq->lock, flags);
+ timer->expires = time;
+
+ if (rq == this_rq()) {
+ hrtimer_restart(timer);
+ } else if (!rq->hrtick_csd_pending) {
+ __smp_call_function_single(cpu_of(rq), &rq->hrtick_csd);
+ rq->hrtick_csd_pending = 1;
+ }
}
static int
@@ -1150,16 +1081,7 @@
case CPU_DOWN_PREPARE_FROZEN:
case CPU_DEAD:
case CPU_DEAD_FROZEN:
- hotplug_hrtick_disable(cpu);
- return NOTIFY_OK;
-
- case CPU_UP_PREPARE:
- case CPU_UP_PREPARE_FROZEN:
- case CPU_DOWN_FAILED:
- case CPU_DOWN_FAILED_FROZEN:
- case CPU_ONLINE:
- case CPU_ONLINE_FROZEN:
- hotplug_hrtick_enable(cpu);
+ hrtick_clear(cpu_rq(cpu));
return NOTIFY_OK;
}
@@ -1170,46 +1092,45 @@
{
hotcpu_notifier(hotplug_hrtick, 0);
}
+#else
+/*
+ * Called to set the hrtick timer state.
+ *
+ * called with rq->lock held and irqs disabled
+ */
+static void hrtick_start(struct rq *rq, u64 delay)
+{
+ hrtimer_start(&rq->hrtick_timer, ns_to_ktime(delay), HRTIMER_MODE_REL);
+}
+
+static void init_hrtick(void)
+{
+}
#endif /* CONFIG_SMP */
static void init_rq_hrtick(struct rq *rq)
{
- rq->hrtick_flags = 0;
+#ifdef CONFIG_SMP
+ rq->hrtick_csd_pending = 0;
+
+ rq->hrtick_csd.flags = 0;
+ rq->hrtick_csd.func = __hrtick_start;
+ rq->hrtick_csd.info = rq;
+#endif
+
hrtimer_init(&rq->hrtick_timer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
rq->hrtick_timer.function = hrtick;
rq->hrtick_timer.cb_mode = HRTIMER_CB_IRQSAFE_NO_SOFTIRQ;
}
-
-void hrtick_resched(void)
-{
- struct rq *rq;
- unsigned long flags;
-
- if (!test_thread_flag(TIF_HRTICK_RESCHED))
- return;
-
- local_irq_save(flags);
- rq = cpu_rq(smp_processor_id());
- hrtick_set(rq);
- local_irq_restore(flags);
-}
#else
static inline void hrtick_clear(struct rq *rq)
{
}
-static inline void hrtick_set(struct rq *rq)
-{
-}
-
static inline void init_rq_hrtick(struct rq *rq)
{
}
-void hrtick_resched(void)
-{
-}
-
static inline void init_hrtick(void)
{
}
@@ -1228,16 +1149,16 @@
#define tsk_is_polling(t) test_tsk_thread_flag(t, TIF_POLLING_NRFLAG)
#endif
-static void __resched_task(struct task_struct *p, int tif_bit)
+static void resched_task(struct task_struct *p)
{
int cpu;
assert_spin_locked(&task_rq(p)->lock);
- if (unlikely(test_tsk_thread_flag(p, tif_bit)))
+ if (unlikely(test_tsk_thread_flag(p, TIF_NEED_RESCHED)))
return;
- set_tsk_thread_flag(p, tif_bit);
+ set_tsk_thread_flag(p, TIF_NEED_RESCHED);
cpu = task_cpu(p);
if (cpu == smp_processor_id())
@@ -1303,10 +1224,10 @@
#endif /* CONFIG_NO_HZ */
#else /* !CONFIG_SMP */
-static void __resched_task(struct task_struct *p, int tif_bit)
+static void resched_task(struct task_struct *p)
{
assert_spin_locked(&task_rq(p)->lock);
- set_tsk_thread_flag(p, tif_bit);
+ set_tsk_need_resched(p);
}
#endif /* CONFIG_SMP */
@@ -4395,7 +4316,7 @@
struct task_struct *prev, *next;
unsigned long *switch_count;
struct rq *rq;
- int cpu, hrtick = sched_feat(HRTICK);
+ int cpu;
need_resched:
preempt_disable();
@@ -4410,7 +4331,7 @@
schedule_debug(prev);
- if (hrtick)
+ if (sched_feat(HRTICK))
hrtick_clear(rq);
/*
@@ -4457,9 +4378,6 @@
} else
spin_unlock_irq(&rq->lock);
- if (hrtick)
- hrtick_set(rq);
-
if (unlikely(reacquire_kernel_lock(current) < 0))
goto need_resched_nonpreemptible;
