On Tue, 2015-08-18 at 11:04 +0000, David Laight wrote: > From: Eric Dumazet > > Sent: 13 August 2015 23:45 > > When replacing del_timer() with del_timer_sync(), I introduced > > a deadlock condition : > > > > reqsk_queue_unlink() is called from inet_csk_reqsk_queue_drop() > > > > inet_csk_reqsk_queue_drop() can be called from many contexts, > > one being the timer handler itself (reqsk_timer_handler()). > > > > In this case, del_timer_sync() loops forever. > > > > Simple fix is to test if timer is pending. > > Doesn't that mean you fail to wait for the timer function > to finish if you are calling from a different context?
No, this is the purpose of del_timer_sync() > > What you need to know is whether the current context > is that running the timer itself. > In which case you need to mark the timer 'to be deleted' > and actually delete it when the timer function returns. > (so other code can still wait for completion.) Please read del_timer_sync() documentation and/or implementation if you have doubts. /** * del_timer_sync - deactivate a timer and wait for the handler to finish. * @timer: the timer to be deactivated * * This function only differs from del_timer() on SMP: besides deactivating * the timer it also makes sure the handler has finished executing on other * CPUs. * * Synchronization rules: Callers must prevent restarting of the timer, * otherwise this function is meaningless. It must not be called from * interrupt contexts unless the timer is an irqsafe one. The caller must * not hold locks which would prevent completion of the timer's * handler. The timer's handler must not call add_timer_on(). Upon exit the * timer is not queued and the handler is not running on any CPU. * * Note: For !irqsafe timers, you must not hold locks that are held in * interrupt context while calling this function. Even if the lock has * nothing to do with the timer in question. Here's why: * * CPU0 CPU1 * ---- ---- * <SOFTIRQ> * call_timer_fn(); * base->running_timer = mytimer; * spin_lock_irq(somelock); * <IRQ> * spin_lock(somelock); * del_timer_sync(mytimer); * while (base->running_timer == mytimer); * * Now del_timer_sync() will never return and never release somelock. * The interrupt on the other CPU is waiting to grab somelock but * it has interrupted the softirq that CPU0 is waiting to finish. * * The function returns whether it has deactivated a pending timer or not. */ -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html