Messages in this thread | | | Date | Wed, 21 Sep 2005 15:28:01 +0400 | From | Oleg Nesterov <> | Subject | Re: [PATCH] ktimers subsystem |
| |
Thomas Gleixner wrote: > > +static int internal_restart_ktimer(struct ktimer *timer, nsec_t *tim, > + int mode) > +{ > + struct ktimer_base *base, *new_base; > + unsigned long flags; > + int ret; > + > + BUG_ON(!timer->function); > + > + retry: > + base = lock_ktimer_base(timer, &flags); > + > + /* Remove an active timer from the queue */ > + ret = remove_ktimer(timer, base); > + > + /* Switch the timer base, if necessary */ > + new_base = switch_ktimer_base(timer, base); > + > + /* SMP */ > + if (ktimer_base_can_change && unlikely(!new_base)){ > + spin_unlock_irqrestore(&base->lock, flags); > + wait_for_ktimer(timer); > + goto retry; > + }
This is deadlockable.
The caller of modify_ktimer()->internal_restart_ktimer() can hold locks which would prevent the completion of the ktimer->function().
Also, I don't understand why the timer is deleted _before_ checking that new_base != NULL. This way you can delete the timer (ret == 1), then goto retry, then get ret == 0.
Oleg. - 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/
| |