| Message ID | 20180417085030.32650-2-horms+renesas@verge.net.au (mailing list archive) |
|---|---|
| State | Under Review |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
On Tue, Apr 17, 2018 at 10:50:26AM +0200, Simon Horman wrote: > From: Masaru Nagai <masaru.nagai.vx@renesas.com> > > [ 58.490829] ================================= > [ 58.495205] [ INFO: inconsistent lock state ] > [ 58.499583] 4.9.0-yocto-standard-00007-g2ef7caf #57 Not tainted > [ 58.505529] --------------------------------- > [ 58.509904] inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage. > [ 58.515939] swapper/0/0 [HC1[1]:SC1[1]:HE0:SE0] takes: > [ 58.521099] (&(&list->lock)->rlock#2){?.-...}, at: [<ffff00000899f474>] skb_queue_tail+0x2c/0x68 > {HARDIRQ-ON-W} state was registered at: Maybe add a short text to the log to describe the approach of this fix?
Hi Simon, On Tue, Apr 17, 2018 at 10:50 AM, Simon Horman <horms+renesas@verge.net.au> wrote: > From: Masaru Nagai <masaru.nagai.vx@renesas.com> > > [ 58.490829] ================================= > [ 58.495205] [ INFO: inconsistent lock state ] > [ 58.499583] 4.9.0-yocto-standard-00007-g2ef7caf #57 Not tainted Can this be triggered on contemporary kernels, too? > [ 58.505529] --------------------------------- > [ 58.509904] inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage. Gr{oetje,eeting}s, Geert
From: Simon Horman <horms+renesas@verge.net.au> Date: Tue, 17 Apr 2018 10:50:26 +0200 > From: Masaru Nagai <masaru.nagai.vx@renesas.com> > > [ 58.490829] ================================= > [ 58.495205] [ INFO: inconsistent lock state ] > [ 58.499583] 4.9.0-yocto-standard-00007-g2ef7caf #57 Not tainted ... > Fixes: f51bdc236b6c ("ravb: Add dma queue interrupt support") > Signed-off-by: Masaru Nagai <masaru.nagai.vx@renesas.com> > Signed-off-by: Kazuya Mizuguchi <kazuya.mizuguchi.ks@renesas.com> > Signed-off-by: Simon Horman <horms+renesas@verge.net.au> This really needs more than the lockdep dump in the commit message, explaining what the problem is and how it was corrected. Are the wrong interrupt types enabled? Are they handled improperly? It definitely isn't clear from just reading the patch.
diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c index 68f122140966..b311b1ac1286 100644 --- a/drivers/net/ethernet/renesas/ravb_main.c +++ b/drivers/net/ethernet/renesas/ravb_main.c @@ -481,7 +481,7 @@ static int ravb_dmac_init(struct net_device *ndev) /* Receive FIFO full error, descriptor empty */ ravb_write(ndev, RIC2_QFE0 | RIC2_QFE1 | RIC2_RFFE, RIC2); /* Frame transmitted, timestamp FIFO updated */ - ravb_write(ndev, TIC_FTE0 | TIC_FTE1 | TIC_TFUE, TIC); + ravb_write(ndev, TIC_FTE0 | TIC_FTE1, TIC); /* Setting the control will start the AVB-DMAC process. */ ravb_modify(ndev, CCC, CCC_OPC, CCC_OPC_OPERATION); @@ -793,18 +793,6 @@ static bool ravb_queue_interrupt(struct net_device *ndev, int q) return false; } -static bool ravb_timestamp_interrupt(struct net_device *ndev) -{ - u32 tis = ravb_read(ndev, TIS); - - if (tis & TIS_TFUF) { - ravb_write(ndev, ~TIS_TFUF, TIS); - ravb_get_tx_tstamp(ndev); - return true; - } - return false; -} - static irqreturn_t ravb_interrupt(int irq, void *dev_id) { struct net_device *ndev = dev_id; @@ -817,13 +805,9 @@ static irqreturn_t ravb_interrupt(int irq, void *dev_id) iss = ravb_read(ndev, ISS); /* Received and transmitted interrupts */ - if (iss & (ISS_FRS | ISS_FTS | ISS_TFUS)) { + if (iss & (ISS_FRS | ISS_FTS)) { int q; - /* Timestamp updated */ - if (ravb_timestamp_interrupt(ndev)) - result = IRQ_HANDLED; - /* Network control and best effort queue RX/TX */ for (q = RAVB_NC; q >= RAVB_BE; q--) { if (ravb_queue_interrupt(ndev, q)) @@ -854,7 +838,7 @@ static irqreturn_t ravb_interrupt(int irq, void *dev_id) return result; } -/* Timestamp/Error/gPTP interrupt handler */ +/* Error/gPTP interrupt handler */ static irqreturn_t ravb_multi_interrupt(int irq, void *dev_id) { struct net_device *ndev = dev_id; @@ -866,10 +850,6 @@ static irqreturn_t ravb_multi_interrupt(int irq, void *dev_id) /* Get interrupt status */ iss = ravb_read(ndev, ISS); - /* Timestamp updated */ - if ((iss & ISS_TFUS) && ravb_timestamp_interrupt(ndev)) - result = IRQ_HANDLED; - /* Error status summary */ if (iss & ISS_ES) { ravb_error_interrupt(ndev); @@ -939,6 +919,10 @@ static int ravb_poll(struct napi_struct *napi, int budget) } /* Processing TX Descriptor Ring */ if (tis & mask) { + /* Timestamp updated */ + if (q == RAVB_NC) + ravb_get_tx_tstamp(ndev); + spin_lock_irqsave(&priv->lock, flags); /* Clear TX interrupt */ ravb_write(ndev, ~mask, TIS);