From 5b85840320151f61e04d83a23ef2567a07094503 Mon Sep 17 00:00:00 2001 From: Michael Grzeschik Date: Wed, 28 Jun 2017 18:28:33 +0200 Subject: arcnet: change irq handler to lock irqsave This patch prevents the arcnet driver from the following deadlock. [ 41.273910] ====================================================== [ 41.280397] [ INFO: SOFTIRQ-safe -> SOFTIRQ-unsafe lock order detected ] [ 41.287433] 4.4.0-00034-gc0ae784 #536 Not tainted [ 41.292366] ------------------------------------------------------ [ 41.298863] arcecho/233 [HC0[0]:SC0[2]:HE0:SE0] is trying to acquire: [ 41.305628] (&(&lp->lock)->rlock){+.+...}, at: [] arcnet_send_packet+0x60/0x1c0 [arcnet] [ 41.315199] [ 41.315199] and this task is already holding: [ 41.321324] (_xmit_ARCNET#2){+.-...}, at: [] packet_direct_xmit+0xfc/0x1c8 [ 41.329593] which would create a new lock dependency: [ 41.334893] (_xmit_ARCNET#2){+.-...} -> (&(&lp->lock)->rlock){+.+...} [ 41.341801] [ 41.341801] but this new dependency connects a SOFTIRQ-irq-safe lock: [ 41.350108] (_xmit_ARCNET#2){+.-...} ... which became SOFTIRQ-irq-safe at: [ 41.357539] [] _raw_spin_lock+0x30/0x40 [ 41.362677] [] dev_watchdog+0x5c/0x264 [ 41.367723] [] call_timer_fn+0x6c/0xf4 [ 41.372759] [] run_timer_softirq+0x154/0x210 [ 41.378340] [] __do_softirq+0x144/0x298 [ 41.383469] [] irq_exit+0xcc/0x130 [ 41.388138] [] __handle_domain_irq+0x60/0xb4 [ 41.393728] [] __irq_svc+0x58/0x78 [ 41.398402] [] arch_cpu_idle+0x24/0x3c [ 41.403443] [] cpu_startup_entry+0x1f8/0x25c [ 41.409029] [] start_kernel+0x3c0/0x3cc [ 41.414170] [ 41.414170] to a SOFTIRQ-irq-unsafe lock: [ 41.419931] (&(&lp->lock)->rlock){+.+...} ... which became SOFTIRQ-irq-unsafe at: [ 41.427996] ... [] _raw_spin_lock+0x30/0x40 [ 41.433409] [] arcnet_interrupt+0x2c/0x800 [arcnet] [ 41.439646] [] handle_nested_irq+0x8c/0xec [ 41.445063] [] regmap_irq_thread+0x190/0x314 [ 41.450661] [] irq_thread_fn+0x1c/0x34 [ 41.455700] [] irq_thread+0x13c/0x1dc [ 41.460649] [] kthread+0xe4/0xf8 [ 41.465158] [] ret_from_fork+0x14/0x24 [ 41.470207] [ 41.470207] other info that might help us debug this: [ 41.470207] [ 41.478627] Possible interrupt unsafe locking scenario: [ 41.478627] [ 41.485763] CPU0 CPU1 [ 41.490521] ---- ---- [ 41.495279] lock(&(&lp->lock)->rlock); [ 41.499414] local_irq_disable(); [ 41.505636] lock(_xmit_ARCNET#2); [ 41.511967] lock(&(&lp->lock)->rlock); [ 41.518741] [ 41.521490] lock(_xmit_ARCNET#2); [ 41.525356] [ 41.525356] *** DEADLOCK *** [ 41.525356] [ 41.531587] 1 lock held by arcecho/233: [ 41.535617] #0: (_xmit_ARCNET#2){+.-...}, at: [] packet_direct_xmit+0xfc/0x1c8 [ 41.544355] the dependencies between SOFTIRQ-irq-safe lock and the holding lock: [ 41.552362] -> (_xmit_ARCNET#2){+.-...} ops: 27 { [ 41.557357] HARDIRQ-ON-W at: [ 41.560664] [] _raw_spin_lock+0x30/0x40 [ 41.567445] [] dev_deactivate_many+0x114/0x304 [ 41.574866] [] dev_deactivate+0x24/0x38 [ 41.581646] [] linkwatch_do_dev+0x40/0x74 [ 41.588613] [] __linkwatch_run_queue+0xec/0x140 [ 41.596120] [] linkwatch_event+0x2c/0x34 [ 41.602991] [] process_one_work+0x188/0x40c [ 41.610131] [] worker_thread+0x4c/0x480 [ 41.616912] [] kthread+0xe4/0xf8 [ 41.623048] [] ret_from_fork+0x14/0x24 [ 41.629735] IN-SOFTIRQ-W at: [ 41.633039] [] _raw_spin_lock+0x30/0x40 [ 41.639820] [] dev_watchdog+0x5c/0x264 [ 41.646508] [] call_timer_fn+0x6c/0xf4 [ 41.653190] [] run_timer_softirq+0x154/0x210 [ 41.660425] [] __do_softirq+0x144/0x298 [ 41.667201] [] irq_exit+0xcc/0x130 [ 41.673518] [] __handle_domain_irq+0x60/0xb4 [ 41.680754] [] __irq_svc+0x58/0x78 [ 41.687077] [] arch_cpu_idle+0x24/0x3c [ 41.693769] [] cpu_startup_entry+0x1f8/0x25c [ 41.701006] [] start_kernel+0x3c0/0x3cc [ 41.707791] INITIAL USE at: [ 41.711003] [] _raw_spin_lock+0x30/0x40 [ 41.717696] [] dev_deactivate_many+0x114/0x304 [ 41.725026] [] dev_deactivate+0x24/0x38 [ 41.731718] [] linkwatch_do_dev+0x40/0x74 [ 41.738593] [] __linkwatch_run_queue+0xec/0x140 [ 41.746011] [] linkwatch_event+0x2c/0x34 [ 41.752789] [] process_one_work+0x188/0x40c [ 41.759847] [] worker_thread+0x4c/0x480 [ 41.766541] [] kthread+0xe4/0xf8 [ 41.772596] [] ret_from_fork+0x14/0x24 [ 41.779198] } [ 41.780945] ... key at: [] netdev_xmit_lock_key+0x38/0x1c8 [ 41.788192] ... acquired at: [ 41.791309] [] lock_acquire+0x70/0x90 [ 41.796361] [] _raw_spin_lock_irqsave+0x40/0x54 [ 41.802324] [] arcnet_send_packet+0x60/0x1c0 [arcnet] [ 41.808844] [] packet_direct_xmit+0x130/0x1c8 [ 41.814622] [] packet_sendmsg+0x3b8/0x680 [ 41.820034] [] sock_sendmsg+0x14/0x24 [ 41.825091] [] SyS_sendto+0xb8/0xe0 [ 41.829956] [] SyS_send+0x18/0x20 [ 41.834638] [] ret_fast_syscall+0x0/0x1c [ 41.839954] [ 41.841514] the dependencies between the lock to be acquired and SOFTIRQ-irq-unsafe lock: [ 41.850302] -> (&(&lp->lock)->rlock){+.+...} ops: 5 { [ 41.855644] HARDIRQ-ON-W at: [ 41.858945] [] _raw_spin_lock+0x30/0x40 [ 41.865726] [] arcnet_interrupt+0x2c/0x800 [arcnet] [ 41.873607] [] handle_nested_irq+0x8c/0xec [ 41.880666] [] regmap_irq_thread+0x190/0x314 [ 41.887901] [] irq_thread_fn+0x1c/0x34 [ 41.894593] [] irq_thread+0x13c/0x1dc [ 41.901195] [] kthread+0xe4/0xf8 [ 41.907338] [] ret_from_fork+0x14/0x24 [ 41.914025] SOFTIRQ-ON-W at: [ 41.917328] [] _raw_spin_lock+0x30/0x40 [ 41.924106] [] arcnet_interrupt+0x2c/0x800 [arcnet] [ 41.931981] [] handle_nested_irq+0x8c/0xec [ 41.939028] [] regmap_irq_thread+0x190/0x314 [ 41.946264] [] irq_thread_fn+0x1c/0x34 [ 41.952954] [] irq_thread+0x13c/0x1dc [ 41.959548] [] kthread+0xe4/0xf8 [ 41.965689] [] ret_from_fork+0x14/0x24 [ 41.972379] INITIAL USE at: [ 41.975595] [] _raw_spin_lock+0x30/0x40 [ 41.982283] [] arcnet_interrupt+0x2c/0x800 [arcnet] [ 41.990063] [] handle_nested_irq+0x8c/0xec [ 41.997027] [] regmap_irq_thread+0x190/0x314 [ 42.004172] [] irq_thread_fn+0x1c/0x34 [ 42.010766] [] irq_thread+0x13c/0x1dc [ 42.017267] [] kthread+0xe4/0xf8 [ 42.023314] [] ret_from_fork+0x14/0x24 [ 42.029903] } [ 42.031648] ... key at: [] __key.42091+0x0/0xfffff0f8 [arcnet] [ 42.039255] ... acquired at: [ 42.042372] [] lock_acquire+0x70/0x90 [ 42.047413] [] _raw_spin_lock_irqsave+0x40/0x54 [ 42.053364] [] arcnet_send_packet+0x60/0x1c0 [arcnet] [ 42.059872] [] packet_direct_xmit+0x130/0x1c8 [ 42.065634] [] packet_sendmsg+0x3b8/0x680 [ 42.071030] [] sock_sendmsg+0x14/0x24 [ 42.076069] [] SyS_sendto+0xb8/0xe0 [ 42.080926] [] SyS_send+0x18/0x20 [ 42.085601] [] ret_fast_syscall+0x0/0x1c [ 42.090918] [ 42.092481] [ 42.092481] stack backtrace: [ 42.097065] CPU: 0 PID: 233 Comm: arcecho Not tainted 4.4.0-00034-gc0ae784 #536 [ 42.104751] Hardware name: Generic AM33XX (Flattened Device Tree) [ 42.111183] [] (unwind_backtrace) from [] (show_stack+0x10/0x14) [ 42.119337] [] (show_stack) from [] (dump_stack+0x8c/0x9c) [ 42.126937] [] (dump_stack) from [] (check_usage+0x4bc/0x63c) [ 42.134815] [] (check_usage) from [] (check_irq_usage+0x58/0xb0) [ 42.142964] [] (check_irq_usage) from [] (__lock_acquire+0x1524/0x20b0) [ 42.151740] [] (__lock_acquire) from [] (lock_acquire+0x70/0x90) [ 42.159886] [] (lock_acquire) from [] (_raw_spin_lock_irqsave+0x40/0x54) [ 42.168768] [] (_raw_spin_lock_irqsave) from [] (arcnet_send_packet+0x60/0x1c0 [arcnet]) [ 42.179115] [] (arcnet_send_packet [arcnet]) from [] (packet_direct_xmit+0x130/0x1c8) [ 42.189182] [] (packet_direct_xmit) from [] (packet_sendmsg+0x3b8/0x680) [ 42.198059] [] (packet_sendmsg) from [] (sock_sendmsg+0x14/0x24) [ 42.206199] [] (sock_sendmsg) from [] (SyS_sendto+0xb8/0xe0) [ 42.213978] [] (SyS_sendto) from [] (SyS_send+0x18/0x20) [ 42.221388] [] (SyS_send) from [] (ret_fast_syscall+0x0/0x1c) Signed-off-by: Michael Grzeschik --- v1 -> v2: removed unneeded zero assignment of flags Signed-off-by: David S. Miller --- drivers/net/arcnet/arcnet.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'drivers/net/arcnet') diff --git a/drivers/net/arcnet/arcnet.c b/drivers/net/arcnet/arcnet.c index 62ee439d5882..53a1cb551def 100644 --- a/drivers/net/arcnet/arcnet.c +++ b/drivers/net/arcnet/arcnet.c @@ -756,6 +756,7 @@ irqreturn_t arcnet_interrupt(int irq, void *dev_id) struct net_device *dev = dev_id; struct arcnet_local *lp; int recbuf, status, diagstatus, didsomething, boguscount; + unsigned long flags; int retval = IRQ_NONE; arc_printk(D_DURING, dev, "\n"); @@ -765,7 +766,7 @@ irqreturn_t arcnet_interrupt(int irq, void *dev_id) lp = netdev_priv(dev); BUG_ON(!lp); - spin_lock(&lp->lock); + spin_lock_irqsave(&lp->lock, flags); /* RESET flag was enabled - if device is not running, we must * clear it right away (but nothing else). @@ -774,7 +775,7 @@ irqreturn_t arcnet_interrupt(int irq, void *dev_id) if (lp->hw.status(dev) & RESETflag) lp->hw.command(dev, CFLAGScmd | RESETclear); lp->hw.intmask(dev, 0); - spin_unlock(&lp->lock); + spin_unlock_irqrestore(&lp->lock, flags); return retval; } @@ -998,7 +999,7 @@ irqreturn_t arcnet_interrupt(int irq, void *dev_id) udelay(1); lp->hw.intmask(dev, lp->intmask); - spin_unlock(&lp->lock); + spin_unlock_irqrestore(&lp->lock, flags); return retval; } EXPORT_SYMBOL(arcnet_interrupt); -- cgit v1.2.3 From 06908d7aee8d62a80cabfd134d0354dc4d2794bc Mon Sep 17 00:00:00 2001 From: Colin Ian King Date: Wed, 28 Jun 2017 18:28:34 +0200 Subject: Trivial fix to spelling mistake in arc_printk message Signed-off-by: Colin Ian King Signed-off-by: Michael Grzeschik Signed-off-by: David S. Miller --- drivers/net/arcnet/capmode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/net/arcnet') diff --git a/drivers/net/arcnet/capmode.c b/drivers/net/arcnet/capmode.c index 2056878fb087..4fa2e46b48d3 100644 --- a/drivers/net/arcnet/capmode.c +++ b/drivers/net/arcnet/capmode.c @@ -212,7 +212,7 @@ static int ack_tx(struct net_device *dev, int acked) ackpkt->soft.cap.proto = 0; /* using protocol 0 for acknowledge */ ackpkt->soft.cap.mes.ack = acked; - arc_printk(D_PROTO, dev, "Ackknowledge for cap packet %x.\n", + arc_printk(D_PROTO, dev, "Acknowledge for cap packet %x.\n", *((int *)&ackpkt->soft.cap.cookie[0])); ackskb->protocol = cpu_to_be16(ETH_P_ARCNET); -- cgit v1.2.3 From 0d494fcf867f95040b5b67e4bc5af739bcda37da Mon Sep 17 00:00:00 2001 From: Michael Grzeschik Date: Wed, 28 Jun 2017 18:28:35 +0200 Subject: arcnet: com20020: remove needless base_addr assignment The assignment is superfluous. Signed-off-by: Michael Grzeschik Signed-off-by: David S. Miller --- drivers/net/arcnet/com20020.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'drivers/net/arcnet') diff --git a/drivers/net/arcnet/com20020.c b/drivers/net/arcnet/com20020.c index 13d9ad4b3f5c..78043a9c5981 100644 --- a/drivers/net/arcnet/com20020.c +++ b/drivers/net/arcnet/com20020.c @@ -246,8 +246,6 @@ int com20020_found(struct net_device *dev, int shared) return -ENODEV; } - dev->base_addr = ioaddr; - arc_printk(D_NORMAL, dev, "%s: station %02Xh found at %03lXh, IRQ %d.\n", lp->card_name, dev->dev_addr[0], dev->base_addr, dev->irq); -- cgit v1.2.3 From cb108619f2fc77846bf7a7543517f3487f455b24 Mon Sep 17 00:00:00 2001 From: Michael Grzeschik Date: Wed, 28 Jun 2017 18:28:36 +0200 Subject: arcnet: com20020-pci: fix dev_id calculation The dev_id was miscalculated. Only the two bits 4-5 are relevant for the MA1 card. PCIARC1 and PCIFB2 use the four bits 4-7 for id selection. Signed-off-by: Michael Grzeschik Signed-off-by: David S. Miller --- drivers/net/arcnet/com20020-pci.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'drivers/net/arcnet') diff --git a/drivers/net/arcnet/com20020-pci.c b/drivers/net/arcnet/com20020-pci.c index 239de38fbd6a..fec2df2c869f 100644 --- a/drivers/net/arcnet/com20020-pci.c +++ b/drivers/net/arcnet/com20020-pci.c @@ -135,6 +135,7 @@ static int com20020pci_probe(struct pci_dev *pdev, for (i = 0; i < ci->devcount; i++) { struct com20020_pci_channel_map *cm = &ci->chan_map_tbl[i]; struct com20020_dev *card; + int dev_id_mask = 0xf; dev = alloc_arcdev(device); if (!dev) { @@ -179,8 +180,8 @@ static int com20020pci_probe(struct pci_dev *pdev, /* Get the dev_id from the PLX rotary coder */ if (!strncmp(ci->name, "EAE PLX-PCI MA1", 15)) - dev->dev_id = 0xc; - dev->dev_id ^= inb(priv->misc + ci->rotary) >> 4; + dev_id_mask = 0x3; + dev->dev_id = (inb(priv->misc + ci->rotary) >> 4) & dev_id_mask; snprintf(dev->name, sizeof(dev->name), "arc%d-%d", dev->dev_id, i); -- cgit v1.2.3 From 2a0ea04c83ab82c3852c9171d2fa5cd9a1432c9b Mon Sep 17 00:00:00 2001 From: Michael Grzeschik Date: Wed, 28 Jun 2017 18:28:37 +0200 Subject: arcnet: com20020-pci: add missing pdev setup in netdev structure We add the pdev data to the pci devices netdev structure. This way the interface get consistent device names in the userspace (udev). Signed-off-by: Michael Grzeschik Signed-off-by: David S. Miller --- drivers/net/arcnet/com20020-pci.c | 1 + 1 file changed, 1 insertion(+) (limited to 'drivers/net/arcnet') diff --git a/drivers/net/arcnet/com20020-pci.c b/drivers/net/arcnet/com20020-pci.c index fec2df2c869f..47f80b83dcf4 100644 --- a/drivers/net/arcnet/com20020-pci.c +++ b/drivers/net/arcnet/com20020-pci.c @@ -167,6 +167,7 @@ static int com20020pci_probe(struct pci_dev *pdev, arcnet_outb(0x00, ioaddr, COM20020_REG_W_COMMAND); arcnet_inb(ioaddr, COM20020_REG_R_DIAGSTAT); + SET_NETDEV_DEV(dev, &pdev->dev); dev->base_addr = ioaddr; dev->dev_addr[0] = node; dev->irq = pdev->irq; -- cgit v1.2.3