diff options
| author | Patrick McHardy <kaber@trash.net> | 2007-07-18 02:32:39 -0700 |
|---|---|---|
| committer | Greg Kroah-Hartman <gregkh@suse.de> | 2007-08-09 14:27:26 -0700 |
| commit | c1bb818242296c121ff574cb1fec2bc0127d6b31 (patch) | |
| tree | bb70695302462e8564c08655f8b94a6feb91ce9d /net | |
| parent | 2c92c72b750cb92566b7e6031f84bc9a35699e73 (diff) | |
Fix TC deadlock.
[NET_SCHED]: Revert "avoid transmit softirq on watchdog wakeup" optimization
As noticed by Ranko Zivojnovic <ranko@spidernet.net>, calling qdisc_run
from the timer handler can result in deadlock:
> CPU#0
>
> qdisc_watchdog() fires and gets dev->queue_lock
> qdisc_run()...qdisc_restart()...
> -> releases dev->queue_lock and enters dev_hard_start_xmit()
>
> CPU#1
>
> tc del qdisc dev ...
> qdisc_graft()...dev_graft_qdisc()...dev_deactivate()...
> -> grabs dev->queue_lock ...
>
> qdisc_reset()...{cbq,hfsc,htb,netem,tbf}_reset()...qdisc_watchdog_cancel()...
> -> hrtimer_cancel() - waiting for the qdisc_watchdog() to exit, while still
> holding dev->queue_lock
>
> CPU#0
>
> dev_hard_start_xmit() returns ...
> -> wants to get dev->queue_lock(!)
>
> DEADLOCK!
The entire optimization is a bit questionable IMO, it moves potentially
large parts of NET_TX_SOFTIRQ work to TIMER_SOFTIRQ/HRTIMER_SOFTIRQ,
which kind of defeats the separation of them.
Signed-off-by: Patrick McHardy <kaber@trash.net>
Acked-by: Ranko Zivojnovic <ranko@spidernet.net>
Signed-off-by: David S. Miller <davem@davemloft.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Diffstat (limited to 'net')
| -rw-r--r-- | net/sched/sch_api.c | 6 |
1 files changed, 1 insertions, 5 deletions
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c index bec600af03ca..7a6b0b7cc68a 100644 --- a/net/sched/sch_api.c +++ b/net/sched/sch_api.c @@ -290,11 +290,7 @@ static enum hrtimer_restart qdisc_watchdog(struct hrtimer *timer) wd->qdisc->flags &= ~TCQ_F_THROTTLED; smp_wmb(); - if (spin_trylock(&dev->queue_lock)) { - qdisc_run(dev); - spin_unlock(&dev->queue_lock); - } else - netif_schedule(dev); + netif_schedule(dev); return HRTIMER_NORESTART; } |