lkml.org 
[lkml]   [2005]   [Mar]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[PATCH 6/5] timers: enable irqs in __mod_timer()
    On top of "[PATCH 5/5] timers: cleanup, kill __get_base()", see
    http://marc.theaimsgroup.com/?l=linux-kernel&m=111125359121372

    If the timer is currently running on another CPU, __mod_timer()
    spins with interrupts disabled and timer->lock held. I think it
    is better to spin_unlock_irqrestore(&timer->lock) in __mod_timer's
    retry path.

    This patch is unneccessary long. It is because it tries to cleanup
    the code a bit. I do not like the fact that lock+test+unlock pattern
    is duplicated in the code.

    If you think that this patch uglifies the code or does not match
    kernel's coding style - just say nack :)

    Signed-off-by: Oleg Nesterov <oleg@tv-sign.ru>

    --- 2.6.12-rc1/kernel/timer.c~6_LOCK 2005-03-19 23:34:23.000000000 +0300
    +++ 2.6.12-rc1/kernel/timer.c 2005-03-21 18:55:25.000000000 +0300
    @@ -174,64 +174,60 @@ int __mod_timer(struct timer_list *timer
    {
    tvec_base_t *old_base, *new_base;
    unsigned long flags;
    - int ret = 0;
    -
    - BUG_ON(!timer->function);
    -
    - check_timer(timer);
    -
    - spin_lock_irqsave(&timer->lock, flags);
    - new_base = &__get_cpu_var(tvec_bases);
    -repeat:
    - old_base = timer_base(timer);
    -
    - /*
    - * Prevent deadlocks via ordering by old_base < new_base.
    - */
    - if (old_base && (new_base != old_base)) {
    - if (old_base < new_base) {
    - spin_lock(&new_base->lock);
    - spin_lock(&old_base->lock);
    - } else {
    - spin_lock(&old_base->lock);
    - spin_lock(&new_base->lock);
    - }
    - /*
    - * The timer base might have been cancelled while we were
    - * trying to take the lock(s), or can still be running on
    - * old_base's CPU.
    - */
    - if (timer_base(timer) != old_base
    - || old_base->running_timer == timer) {
    - spin_unlock(&new_base->lock);
    - spin_unlock(&old_base->lock);
    - goto repeat;
    - }
    - } else {
    - spin_lock(&new_base->lock);
    - if (timer_base(timer) != old_base) {
    - spin_unlock(&new_base->lock);
    - goto repeat;
    - }
    - }
    -
    - /*
    - * Delete the previous timeout (if there was any).
    - * We hold timer->lock, no need to check old_base != 0.
    - */
    - if (timer_pending(timer)) {
    - list_del(&timer->entry);
    - ret = 1;
    - }
    -
    - timer->expires = expires;
    - internal_add_timer(new_base, timer);
    - __set_base(timer, new_base, 1);
    -
    - if (old_base && (new_base != old_base))
    - spin_unlock(&old_base->lock);
    - spin_unlock(&new_base->lock);
    - spin_unlock_irqrestore(&timer->lock, flags);
    + int new_lock, ret;
    +
    + BUG_ON(!timer->function);
    + check_timer(timer);
    +
    + for (ret = -1; ret < 0; ) {
    + spin_lock_irqsave(&timer->lock, flags);
    + new_base = &__get_cpu_var(tvec_bases);
    + old_base = timer_base(timer);
    +
    + /* Prevent AB-BA deadlocks. */
    + new_lock = old_base < new_base;
    + if (new_lock)
    + spin_lock(&new_base->lock);
    +
    + /* Note:
    + * (old_base == NULL) => (new_lock == 1)
    + * (old_base == new_base) => (new_lock == 0)
    + */
    + if (old_base) {
    + spin_lock(&old_base->lock);
    +
    + if (timer_base(timer) != old_base)
    + goto unlock;
    +
    + if (old_base != new_base) {
    + /* Ensure the timer is serialized. */
    + if (old_base->running_timer == timer)
    + goto unlock;
    +
    + if (!new_lock) {
    + spin_lock(&new_base->lock);
    + new_lock = 1;
    + }
    + }
    + }
    +
    + ret = 0;
    + /* We hold timer->lock, no need to check old_base != 0. */
    + if (timer_pending(timer)) {
    + list_del(&timer->entry);
    + ret = 1;
    + }
    +
    + timer->expires = expires;
    + internal_add_timer(new_base, timer);
    + __set_base(timer, new_base, 1);
    +unlock:
    + if (old_base)
    + spin_unlock(&old_base->lock);
    + if (new_lock)
    + spin_unlock(&new_base->lock);
    + spin_unlock_irqrestore(&timer->lock, flags);
    + }

    return ret;
    }
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/
    \
     
     \ /
      Last update: 2005-03-22 14:11    [W:0.027 / U:33.092 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site