lkml.org 
[lkml]   [2008]   [Sep]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [GIT pull] timer fixes for .27
    On Tue, Sep 30, 2008 at 12:15:11AM +0200, Thomas Gleixner wrote:
    > Linus,
    >
    > Please pull the latest timers-fixes-for-linus git tree from:
    >
    > git://git.kernel.org/pub/scm/linux/kernel/git/tip/linux-2.6-tip.git timers-fixes-for-linus
    >
    > The patches fix hard to trigger bugs in the CPU offline code of
    > hrtimers which were noticed by Paul McKenney recently. In the worst
    > case they can leave migrated hrtimers in a stale state.

    Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
    Tested-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>

    > Thanks,
    >
    > tglx
    >
    > ------------------>
    > Thomas Gleixner (4):
    > hrtimer: migrate pending list on cpu offline
    > hrtimer: fix migration of CB_IRQSAFE_NO_SOFTIRQ hrtimers
    > hrtimer: mark migration state
    > hrtimer: prevent migration of per CPU hrtimers
    >
    >
    > include/linux/hrtimer.h | 18 ++++++--
    > kernel/hrtimer.c | 95 +++++++++++++++++++++++++++++++++++++----
    > kernel/sched.c | 4 +-
    > kernel/time/tick-sched.c | 2 +-
    > kernel/trace/trace_sysprof.c | 2 +-
    > 5 files changed, 103 insertions(+), 18 deletions(-)
    >
    > diff --git a/include/linux/hrtimer.h b/include/linux/hrtimer.h
    > index 6d93dce..2f245fe 100644
    > --- a/include/linux/hrtimer.h
    > +++ b/include/linux/hrtimer.h
    > @@ -47,14 +47,22 @@ enum hrtimer_restart {
    > * HRTIMER_CB_IRQSAFE: Callback may run in hardirq context
    > * HRTIMER_CB_IRQSAFE_NO_RESTART: Callback may run in hardirq context and
    > * does not restart the timer
    > - * HRTIMER_CB_IRQSAFE_NO_SOFTIRQ: Callback must run in hardirq context
    > - * Special mode for tick emultation
    > + * HRTIMER_CB_IRQSAFE_PERCPU: Callback must run in hardirq context
    > + * Special mode for tick emulation and
    > + * scheduler timer. Such timers are per
    > + * cpu and not allowed to be migrated on
    > + * cpu unplug.
    > + * HRTIMER_CB_IRQSAFE_UNLOCKED: Callback should run in hardirq context
    > + * with timer->base lock unlocked
    > + * used for timers which call wakeup to
    > + * avoid lock order problems with rq->lock
    > */
    > enum hrtimer_cb_mode {
    > HRTIMER_CB_SOFTIRQ,
    > HRTIMER_CB_IRQSAFE,
    > HRTIMER_CB_IRQSAFE_NO_RESTART,
    > - HRTIMER_CB_IRQSAFE_NO_SOFTIRQ,
    > + HRTIMER_CB_IRQSAFE_PERCPU,
    > + HRTIMER_CB_IRQSAFE_UNLOCKED,
    > };
    >
    > /*
    > @@ -67,9 +75,10 @@ enum hrtimer_cb_mode {
    > * 0x02 callback function running
    > * 0x04 callback pending (high resolution mode)
    > *
    > - * Special case:
    > + * Special cases:
    > * 0x03 callback function running and enqueued
    > * (was requeued on another CPU)
    > + * 0x09 timer was migrated on CPU hotunplug
    > * The "callback function running and enqueued" status is only possible on
    > * SMP. It happens for example when a posix timer expired and the callback
    > * queued a signal. Between dropping the lock which protects the posix timer
    > @@ -87,6 +96,7 @@ enum hrtimer_cb_mode {
    > #define HRTIMER_STATE_ENQUEUED 0x01
    > #define HRTIMER_STATE_CALLBACK 0x02
    > #define HRTIMER_STATE_PENDING 0x04
    > +#define HRTIMER_STATE_MIGRATE 0x08
    >
    > /**
    > * struct hrtimer - the basic hrtimer structure
    > diff --git a/kernel/hrtimer.c b/kernel/hrtimer.c
    > index b8e4dce..cdec83e 100644
    > --- a/kernel/hrtimer.c
    > +++ b/kernel/hrtimer.c
    > @@ -672,13 +672,14 @@ static inline int hrtimer_enqueue_reprogram(struct hrtimer *timer,
    > */
    > BUG_ON(timer->function(timer) != HRTIMER_NORESTART);
    > return 1;
    > - case HRTIMER_CB_IRQSAFE_NO_SOFTIRQ:
    > + case HRTIMER_CB_IRQSAFE_PERCPU:
    > + case HRTIMER_CB_IRQSAFE_UNLOCKED:
    > /*
    > * This is solely for the sched tick emulation with
    > * dynamic tick support to ensure that we do not
    > * restart the tick right on the edge and end up with
    > * the tick timer in the softirq ! The calling site
    > - * takes care of this.
    > + * takes care of this. Also used for hrtimer sleeper !
    > */
    > debug_hrtimer_deactivate(timer);
    > return 1;
    > @@ -1245,7 +1246,8 @@ static void __run_hrtimer(struct hrtimer *timer)
    > timer_stats_account_hrtimer(timer);
    >
    > fn = timer->function;
    > - if (timer->cb_mode == HRTIMER_CB_IRQSAFE_NO_SOFTIRQ) {
    > + if (timer->cb_mode == HRTIMER_CB_IRQSAFE_PERCPU ||
    > + timer->cb_mode == HRTIMER_CB_IRQSAFE_UNLOCKED) {
    > /*
    > * Used for scheduler timers, avoid lock inversion with
    > * rq->lock and tasklist_lock.
    > @@ -1452,7 +1454,7 @@ void hrtimer_init_sleeper(struct hrtimer_sleeper *sl, struct task_struct *task)
    > sl->timer.function = hrtimer_wakeup;
    > sl->task = task;
    > #ifdef CONFIG_HIGH_RES_TIMERS
    > - sl->timer.cb_mode = HRTIMER_CB_IRQSAFE_NO_SOFTIRQ;
    > + sl->timer.cb_mode = HRTIMER_CB_IRQSAFE_UNLOCKED;
    > #endif
    > }
    >
    > @@ -1591,29 +1593,95 @@ static void __cpuinit init_hrtimers_cpu(int cpu)
    >
    > #ifdef CONFIG_HOTPLUG_CPU
    >
    > -static void migrate_hrtimer_list(struct hrtimer_clock_base *old_base,
    > - struct hrtimer_clock_base *new_base)
    > +static int migrate_hrtimer_list(struct hrtimer_clock_base *old_base,
    > + struct hrtimer_clock_base *new_base, int dcpu)
    > {
    > struct hrtimer *timer;
    > struct rb_node *node;
    > + int raise = 0;
    >
    > while ((node = rb_first(&old_base->active))) {
    > timer = rb_entry(node, struct hrtimer, node);
    > BUG_ON(hrtimer_callback_running(timer));
    > debug_hrtimer_deactivate(timer);
    > - __remove_hrtimer(timer, old_base, HRTIMER_STATE_INACTIVE, 0);
    > +
    > + /*
    > + * Should not happen. Per CPU timers should be
    > + * canceled _before_ the migration code is called
    > + */
    > + if (timer->cb_mode == HRTIMER_CB_IRQSAFE_PERCPU) {
    > + __remove_hrtimer(timer, old_base,
    > + HRTIMER_STATE_INACTIVE, 0);
    > + WARN(1, "hrtimer (%p %p)active but cpu %d dead\n",
    > + timer, timer->function, dcpu);
    > + continue;
    > + }
    > +
    > + /*
    > + * Mark it as STATE_MIGRATE not INACTIVE otherwise the
    > + * timer could be seen as !active and just vanish away
    > + * under us on another CPU
    > + */
    > + __remove_hrtimer(timer, old_base, HRTIMER_STATE_MIGRATE, 0);
    > timer->base = new_base;
    > /*
    > * Enqueue the timer. Allow reprogramming of the event device
    > */
    > enqueue_hrtimer(timer, new_base, 1);
    > +
    > +#ifdef CONFIG_HIGH_RES_TIMERS
    > + /*
    > + * Happens with high res enabled when the timer was
    > + * already expired and the callback mode is
    > + * HRTIMER_CB_IRQSAFE_UNLOCKED (hrtimer_sleeper). The
    > + * enqueue code does not move them to the soft irq
    > + * pending list for performance/latency reasons, but
    > + * in the migration state, we need to do that
    > + * otherwise we end up with a stale timer.
    > + */
    > + if (timer->state == HRTIMER_STATE_MIGRATE) {
    > + timer->state = HRTIMER_STATE_PENDING;
    > + list_add_tail(&timer->cb_entry,
    > + &new_base->cpu_base->cb_pending);
    > + raise = 1;
    > + }
    > +#endif
    > + /* Clear the migration state bit */
    > + timer->state &= ~HRTIMER_STATE_MIGRATE;
    > + }
    > + return raise;
    > +}
    > +
    > +#ifdef CONFIG_HIGH_RES_TIMERS
    > +static int migrate_hrtimer_pending(struct hrtimer_cpu_base *old_base,
    > + struct hrtimer_cpu_base *new_base)
    > +{
    > + struct hrtimer *timer;
    > + int raise = 0;
    > +
    > + while (!list_empty(&old_base->cb_pending)) {
    > + timer = list_entry(old_base->cb_pending.next,
    > + struct hrtimer, cb_entry);
    > +
    > + __remove_hrtimer(timer, timer->base, HRTIMER_STATE_PENDING, 0);
    > + timer->base = &new_base->clock_base[timer->base->index];
    > + list_add_tail(&timer->cb_entry, &new_base->cb_pending);
    > + raise = 1;
    > }
    > + return raise;
    > +}
    > +#else
    > +static int migrate_hrtimer_pending(struct hrtimer_cpu_base *old_base,
    > + struct hrtimer_cpu_base *new_base)
    > +{
    > + return 0;
    > }
    > +#endif
    >
    > static void migrate_hrtimers(int cpu)
    > {
    > struct hrtimer_cpu_base *old_base, *new_base;
    > - int i;
    > + int i, raise = 0;
    >
    > BUG_ON(cpu_online(cpu));
    > old_base = &per_cpu(hrtimer_bases, cpu);
    > @@ -1626,14 +1694,21 @@ static void migrate_hrtimers(int cpu)
    > spin_lock_nested(&old_base->lock, SINGLE_DEPTH_NESTING);
    >
    > for (i = 0; i < HRTIMER_MAX_CLOCK_BASES; i++) {
    > - migrate_hrtimer_list(&old_base->clock_base[i],
    > - &new_base->clock_base[i]);
    > + if (migrate_hrtimer_list(&old_base->clock_base[i],
    > + &new_base->clock_base[i], cpu))
    > + raise = 1;
    > }
    >
    > + if (migrate_hrtimer_pending(old_base, new_base))
    > + raise = 1;
    > +
    > spin_unlock(&old_base->lock);
    > spin_unlock(&new_base->lock);
    > local_irq_enable();
    > put_cpu_var(hrtimer_bases);
    > +
    > + if (raise)
    > + hrtimer_raise_softirq();
    > }
    > #endif /* CONFIG_HOTPLUG_CPU */
    >
    > diff --git a/kernel/sched.c b/kernel/sched.c
    > index 13dd2db..ad1962d 100644
    > --- a/kernel/sched.c
    > +++ b/kernel/sched.c
    > @@ -201,7 +201,7 @@ void init_rt_bandwidth(struct rt_bandwidth *rt_b, u64 period, u64 runtime)
    > hrtimer_init(&rt_b->rt_period_timer,
    > CLOCK_MONOTONIC, HRTIMER_MODE_REL);
    > rt_b->rt_period_timer.function = sched_rt_period_timer;
    > - rt_b->rt_period_timer.cb_mode = HRTIMER_CB_IRQSAFE_NO_SOFTIRQ;
    > + rt_b->rt_period_timer.cb_mode = HRTIMER_CB_IRQSAFE_UNLOCKED;
    > }
    >
    > static void start_rt_bandwidth(struct rt_bandwidth *rt_b)
    > @@ -1119,7 +1119,7 @@ static void init_rq_hrtick(struct rq *rq)
    >
    > 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;
    > + rq->hrtick_timer.cb_mode = HRTIMER_CB_IRQSAFE_PERCPU;
    > }
    > #else
    > static inline void hrtick_clear(struct rq *rq)
    > diff --git a/kernel/time/tick-sched.c b/kernel/time/tick-sched.c
    > index 39019b3..cb02324 100644
    > --- a/kernel/time/tick-sched.c
    > +++ b/kernel/time/tick-sched.c
    > @@ -625,7 +625,7 @@ void tick_setup_sched_timer(void)
    > */
    > hrtimer_init(&ts->sched_timer, CLOCK_MONOTONIC, HRTIMER_MODE_ABS);
    > ts->sched_timer.function = tick_sched_timer;
    > - ts->sched_timer.cb_mode = HRTIMER_CB_IRQSAFE_NO_SOFTIRQ;
    > + ts->sched_timer.cb_mode = HRTIMER_CB_IRQSAFE_PERCPU;
    >
    > /* Get the next period (per cpu) */
    > ts->sched_timer.expires = tick_init_jiffy_update();
    > diff --git a/kernel/trace/trace_sysprof.c b/kernel/trace/trace_sysprof.c
    > index bb948e5..db58fb6 100644
    > --- a/kernel/trace/trace_sysprof.c
    > +++ b/kernel/trace/trace_sysprof.c
    > @@ -202,7 +202,7 @@ static void start_stack_timer(int cpu)
    >
    > hrtimer_init(hrtimer, CLOCK_MONOTONIC, HRTIMER_MODE_REL);
    > hrtimer->function = stack_trace_timer_fn;
    > - hrtimer->cb_mode = HRTIMER_CB_IRQSAFE_NO_SOFTIRQ;
    > + hrtimer->cb_mode = HRTIMER_CB_IRQSAFE_PERCPU;
    >
    > hrtimer_start(hrtimer, ns_to_ktime(sample_period), HRTIMER_MODE_REL);
    > }


    \
     
     \ /
      Last update: 2008-09-30 00:47    [W:7.587 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site