aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorStephen Boyd <sboyd@codeaurora.org>2012-04-18 13:25:58 -0400
committerDavid S. Miller <davem@davemloft.net>2012-04-21 15:33:14 -0400
commitc5a99937a9cf74a623384023201a7d98b51e7e3b (patch)
tree48f114446b6e0c6dde76a6b0bd0aa7fae9af76bf
parent3adadc08cc1e2cbcc15a640d639297ef5fcb17f5 (diff)
ks8851: Fix mutex deadlock in ks8851_net_stop()
There is a potential deadlock scenario when the ks8851 driver is removed. The interrupt handler schedules a workqueue which acquires a mutex that ks8851_net_stop() also acquires before flushing the workqueue. Previously lockdep wouldn't be able to find this problem but now that it has the support we can trigger this lockdep warning by rmmoding the driver after an ifconfig up. Fix the possible deadlock by disabling the interrupts in the chip and then release the lock across the workqueue flushing. The mutex is only there to proect the registers anyway so this should be ok. ======================================================= [ INFO: possible circular locking dependency detected ] 3.0.21-00021-g8b33780-dirty #2911 ------------------------------------------------------- rmmod/125 is trying to acquire lock: ((&ks->irq_work)){+.+...}, at: [<c019e0b8>] flush_work+0x0/0xac but task is already holding lock: (&ks->lock){+.+...}, at: [<bf00b850>] ks8851_net_stop+0x64/0x138 [ks8851] which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (&ks->lock){+.+...}: [<c01b89c8>] __lock_acquire+0x940/0x9f8 [<c01b9058>] lock_acquire+0x10c/0x130 [<c083dbec>] mutex_lock_nested+0x68/0x3dc [<bf00bd48>] ks8851_irq_work+0x24/0x46c [ks8851] [<c019c580>] process_one_work+0x2d8/0x518 [<c019cb98>] worker_thread+0x220/0x3a0 [<c01a2ad4>] kthread+0x88/0x94 [<c0107008>] kernel_thread_exit+0x0/0x8 -> #0 ((&ks->irq_work)){+.+...}: [<c01b7984>] validate_chain+0x914/0x1018 [<c01b89c8>] __lock_acquire+0x940/0x9f8 [<c01b9058>] lock_acquire+0x10c/0x130 [<c019e104>] flush_work+0x4c/0xac [<bf00b858>] ks8851_net_stop+0x6c/0x138 [ks8851] [<c06b209c>] __dev_close_many+0x98/0xcc [<c06b2174>] dev_close_many+0x68/0xd0 [<c06b22ec>] rollback_registered_many+0xcc/0x2b8 [<c06b2554>] rollback_registered+0x28/0x34 [<c06b25b8>] unregister_netdevice_queue+0x58/0x7c [<c06b25f4>] unregister_netdev+0x18/0x20 [<bf00c1f4>] ks8851_remove+0x64/0xb4 [ks8851] [<c049ddf0>] spi_drv_remove+0x18/0x1c [<c0468e98>] __device_release_driver+0x7c/0xbc [<c0468f64>] driver_detach+0x8c/0xb4 [<c0467f00>] bus_remove_driver+0xb8/0xe8 [<c01c1d20>] sys_delete_module+0x1e8/0x27c [<c0105ec0>] ret_fast_syscall+0x0/0x3c other info that might help us debug this: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(&ks->lock); lock((&ks->irq_work)); lock(&ks->lock); lock((&ks->irq_work)); *** DEADLOCK *** 4 locks held by rmmod/125: #0: (&__lockdep_no_validate__){+.+.+.}, at: [<c0468f44>] driver_detach+0x6c/0xb4 #1: (&__lockdep_no_validate__){+.+.+.}, at: [<c0468f50>] driver_detach+0x78/0xb4 #2: (rtnl_mutex){+.+.+.}, at: [<c06b25e8>] unregister_netdev+0xc/0x20 #3: (&ks->lock){+.+...}, at: [<bf00b850>] ks8851_net_stop+0x64/0x138 [ks8851] Cc: Ben Dooks <ben-linux@fluff.org> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org> Signed-off-by: David S. Miller <davem@davemloft.net>
-rw-r--r--drivers/net/ethernet/micrel/ks8851.c11
1 files changed, 6 insertions, 5 deletions
diff --git a/drivers/net/ethernet/micrel/ks8851.c b/drivers/net/ethernet/micrel/ks8851.c
index e5dc0757f077..ad10759e3459 100644
--- a/drivers/net/ethernet/micrel/ks8851.c
+++ b/drivers/net/ethernet/micrel/ks8851.c
@@ -889,16 +889,17 @@ static int ks8851_net_stop(struct net_device *dev)
889 netif_stop_queue(dev); 889 netif_stop_queue(dev);
890 890
891 mutex_lock(&ks->lock); 891 mutex_lock(&ks->lock);
892 /* turn off the IRQs and ack any outstanding */
893 ks8851_wrreg16(ks, KS_IER, 0x0000);
894 ks8851_wrreg16(ks, KS_ISR, 0xffff);
895 mutex_unlock(&ks->lock);
892 896
893 /* stop any outstanding work */ 897 /* stop any outstanding work */
894 flush_work(&ks->irq_work); 898 flush_work(&ks->irq_work);
895 flush_work(&ks->tx_work); 899 flush_work(&ks->tx_work);
896 flush_work(&ks->rxctrl_work); 900 flush_work(&ks->rxctrl_work);
897 901
898 /* turn off the IRQs and ack any outstanding */ 902 mutex_lock(&ks->lock);
899 ks8851_wrreg16(ks, KS_IER, 0x0000);
900 ks8851_wrreg16(ks, KS_ISR, 0xffff);
901
902 /* shutdown RX process */ 903 /* shutdown RX process */
903 ks8851_wrreg16(ks, KS_RXCR1, 0x0000); 904 ks8851_wrreg16(ks, KS_RXCR1, 0x0000);
904 905
@@ -907,6 +908,7 @@ static int ks8851_net_stop(struct net_device *dev)
907 908
908 /* set powermode to soft power down to save power */ 909 /* set powermode to soft power down to save power */
909 ks8851_set_powermode(ks, PMECR_PM_SOFTDOWN); 910 ks8851_set_powermode(ks, PMECR_PM_SOFTDOWN);
911 mutex_unlock(&ks->lock);
910 912
911 /* ensure any queued tx buffers are dumped */ 913 /* ensure any queued tx buffers are dumped */
912 while (!skb_queue_empty(&ks->txq)) { 914 while (!skb_queue_empty(&ks->txq)) {
@@ -918,7 +920,6 @@ static int ks8851_net_stop(struct net_device *dev)
918 dev_kfree_skb(txb); 920 dev_kfree_skb(txb);
919 } 921 }
920 922
921 mutex_unlock(&ks->lock);
922 return 0; 923 return 0;
923} 924}
924 925