From 20953ad68ee522f6420b63c200ac9b23f96d937a Mon Sep 17 00:00:00 2001 From: David Kilroy Date: Wed, 7 Jan 2009 00:23:55 +0000 Subject: orinoco: take the driver lock in the rx tasklet Fix the warning reproduced below. We add to rx_list in interrupt context and remove elements in tasklet context. While removing elements we need to prevent the interrupt modifying the list. Note that "orinoco: Process bulk of receive interrupt in a tasklet" did not preserve locking semantics on what is now orinoco_rx. This patch reinstates the locking semantics and ensures it covers rx_list as well. This leads to additional cleanup required in free_orinocodev. [89479.105038] WARNING: at lib/list_debug.c:30 __list_add+0x8f/0xa0() [89479.105058] list_add corruption. prev->next should be next (dddb3568), but was cbc28978. (prev=dddb3568). [89479.106002] Pid: 15746, comm: X Not tainted 2.6.28-1avb #26 [89479.106020] Call Trace: [89479.106062] [] warn_slowpath+0x60/0x80 [89479.106104] [] ? native_sched_clock+0x20/0x70 [89479.106194] [] ? lock_release_holdtime+0x35/0x200 [89479.106218] [] ? __slab_alloc+0x550/0x560 [89479.106254] [] ? _spin_unlock+0x1d/0x20 [89479.106270] [] ? __slab_alloc+0x550/0x560 [89479.106302] [] ? delay_tsc+0x17/0x24 [89479.106319] [] ? __const_udelay+0x21/0x30 [89479.106376] [] ? hermes_bap_seek+0x112/0x1e0 [hermes] [89479.106396] [] ? trace_hardirqs_off+0xb/0x10 [89479.106418] [] ? __kmalloc_track_caller+0xb7/0x110 [89479.106448] [] ? dev_alloc_skb+0x1c/0x30 [89479.106465] [] ? dev_alloc_skb+0x1c/0x30 [89479.106482] [] __list_add+0x8f/0xa0 [89479.106551] [] orinoco_interrupt+0xcae/0x16c0 [orinoco] [89479.106574] [] ? tick_dev_program_event+0x33/0xb0 [89479.106594] [] ? native_sched_clock+0x20/0x70 [89479.106613] [] ? lock_release_holdtime+0x35/0x200 [89479.106662] [] ? trace_hardirqs_off+0xb/0x10 [89479.106892] [] ? usb_hcd_irq+0x97/0xa0 [usbcore] [89479.106926] [] handle_IRQ_event+0x29/0x60 [89479.106947] [] handle_level_irq+0x69/0xe0 [89479.106963] [] ? handle_level_irq+0x0/0xe0 [89479.106977] [] ? tcp_v4_rcv+0x633/0x6e0 [89479.107025] [] ? common_interrupt+0x28/0x30 [89479.107057] [] ? sk_run_filter+0x320/0x7a0 [89479.107078] [] ? list_del+0x21/0x90 [89479.107106] [] ? orinoco_rx_isr_tasklet+0x2ce/0x480 [orinoco] [89479.107131] [] ? __lock_acquire+0x160/0x1650 [89479.107151] [] ? native_sched_clock+0x20/0x70 [89479.107169] [] ? lock_release_holdtime+0x35/0x200 [89479.107200] [] ? irq_enter+0xa/0x60 [89479.107217] [] ? do_IRQ+0xd2/0x130 [89479.107518] [] ? restore_nocheck_notrace+0x0/0xe [89479.107542] [] ? __do_softirq+0x0/0x110 [89479.107561] [] ? trace_hardirqs_on_caller+0x74/0x140 [89479.107583] [] ? trace_hardirqs_on_thunk+0xc/0x10 [89479.107602] [] ? tasklet_action+0x27/0x90 [89479.107620] [] ? trace_hardirqs_on_caller+0x74/0x140 [89479.107638] [] ? tasklet_action+0x43/0x90 [89479.107655] [] ? __do_softirq+0x6f/0x110 [89479.107674] [] ? __do_softirq+0x0/0x110 [89479.107685] [] ? handle_level_irq+0x0/0xe0 [89479.107715] [] ? irq_exit+0x5d/0x80 [89479.107732] [] ? do_IRQ+0xd2/0x130 [89479.107747] [] ? sysenter_exit+0xf/0x16 [89479.107765] [] ? trace_hardirqs_on_caller+0xfd/0x140 [89479.107782] [] ? common_interrupt+0x28/0x30 [89479.107797] ---[ end trace a1fc0a52df4a729d ]--- Reported-by: Andrey Borzenkov Signed-off-by: David Kilroy Signed-off-by: John W. Linville --- drivers/net/wireless/orinoco/orinoco.c | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) (limited to 'drivers/net/wireless/orinoco') diff --git a/drivers/net/wireless/orinoco/orinoco.c b/drivers/net/wireless/orinoco/orinoco.c index bc84e2792f8..c3bb85e0251 100644 --- a/drivers/net/wireless/orinoco/orinoco.c +++ b/drivers/net/wireless/orinoco/orinoco.c @@ -1610,6 +1610,16 @@ static void orinoco_rx_isr_tasklet(unsigned long data) struct orinoco_rx_data *rx_data, *temp; struct hermes_rx_descriptor *desc; struct sk_buff *skb; + unsigned long flags; + + /* orinoco_rx requires the driver lock, and we also need to + * protect priv->rx_list, so just hold the lock over the + * lot. + * + * If orinoco_lock fails, we've unplugged the card. In this + * case just abort. */ + if (orinoco_lock(priv, &flags) != 0) + return; /* extract desc and skb from queue */ list_for_each_entry_safe(rx_data, temp, &priv->rx_list, list) { @@ -1622,6 +1632,8 @@ static void orinoco_rx_isr_tasklet(unsigned long data) kfree(desc); } + + orinoco_unlock(priv, &flags); } /********************************************************************/ @@ -3645,12 +3657,22 @@ struct net_device void free_orinocodev(struct net_device *dev) { struct orinoco_private *priv = netdev_priv(dev); + struct orinoco_rx_data *rx_data, *temp; - /* No need to empty priv->rx_list: if the tasklet is scheduled - * when we call tasklet_kill it will run one final time, - * emptying the list */ + /* If the tasklet is scheduled when we call tasklet_kill it + * will run one final time. However the tasklet will only + * drain priv->rx_list if the hw is still available. */ tasklet_kill(&priv->rx_tasklet); + /* Explicitly drain priv->rx_list */ + list_for_each_entry_safe(rx_data, temp, &priv->rx_list, list) { + list_del(&rx_data->list); + + dev_kfree_skb(rx_data->skb); + kfree(rx_data->desc); + kfree(rx_data); + } + unregister_pm_notifier(&priv->pm_notifier); orinoco_uncache_fw(priv); -- cgit v1.2.3