* [PATCH v3 1/1 net-next] net: fec: put tx to napi poll function to fix dead lock
@ 2013-03-04 3:34 ` Frank Li
0 siblings, 0 replies; 14+ messages in thread
From: Frank Li @ 2013-03-04 3:34 UTC (permalink / raw)
To: lznuaa, shawn.guo, B38611, davem, linux-arm-kernel, netdev
Cc: s.hauer, Frank Li
up stack ndo_start_xmit already hold lock.
fec_enet_start_xmit needn't spin lock.
stat_xmit just update fep->cur_tx
fec_enet_tx just update fep->dirty_tx
Reserve a empty bdb to check full or empty
cur_tx == dirty_tx means full
cur_tx == dirty_tx +1 means empty
So needn't is_full variable.
Fix spin lock deadlock
=================================
[ INFO: inconsistent lock state ]
3.8.0-rc5+ #107 Not tainted
---------------------------------
inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage.
ptp4l/615 [HC1[1]:SC0[0]:HE0:SE1] takes:
(&(&list->lock)->rlock#3){?.-...}, at: [<8042c3c4>] skb_queue_tail+0x20/0x50
{HARDIRQ-ON-W} state was registered at:
[<80067250>] mark_lock+0x154/0x4e8
[<800676f4>] mark_irqflags+0x110/0x1a4
[<80069208>] __lock_acquire+0x494/0x9c0
[<80069ce8>] lock_acquire+0x90/0xa4
[<80527ad0>] _raw_spin_lock_bh+0x44/0x54
[<804877e0>] first_packet_length+0x38/0x1f0
[<804879e4>] udp_poll+0x4c/0x5c
[<804231f8>] sock_poll+0x24/0x28
[<800d27f0>] do_poll.isra.10+0x120/0x254
[<800d36e4>] do_sys_poll+0x15c/0x1e8
[<800d3828>] sys_poll+0x60/0xc8
[<8000e780>] ret_fast_syscall+0x0/0x3c
*** DEADLOCK ***
1 lock held by ptp4l/615:
#0: (&(&fep->hw_lock)->rlock){-.-...}, at: [<80355f9c>] fec_enet_tx+0x24/0x268
stack backtrace:
Backtrace:
[<800121e0>] (dump_backtrace+0x0/0x10c) from [<80516210>] (dump_stack+0x18/0x1c)
r6:8063b1fc r5:bf38b2f8 r4:bf38b000 r3:bf38b000
[<805161f8>] (dump_stack+0x0/0x1c) from [<805189d0>] (print_usage_bug.part.34+0x164/0x1a4)
[<8051886c>] (print_usage_bug.part.34+0x0/0x1a4) from [<80518a88>] (print_usage_bug+0x78/0x88)
r8:80065664 r7:bf38b2f8 r6:00000002 r5:00000000 r4:bf38b000
[<80518a10>] (print_usage_bug+0x0/0x88) from [<80518b58>] (mark_lock_irq+0xc0/0x270)
r7:bf38b000 r6:00000002 r5:bf38b2f8 r4:00000000
[<80518a98>] (mark_lock_irq+0x0/0x270) from [<80067270>] (mark_lock+0x174/0x4e8)
[<800670fc>] (mark_lock+0x0/0x4e8) from [<80067744>] (mark_irqflags+0x160/0x1a4)
[<800675e4>] (mark_irqflags+0x0/0x1a4) from [<80069208>] (__lock_acquire+0x494/0x9c0)
r5:00000002 r4:bf38b2f8
[<80068d74>] (__lock_acquire+0x0/0x9c0) from [<80069ce8>] (lock_acquire+0x90/0xa4)
[<80069c58>] (lock_acquire+0x0/0xa4) from [<805278d8>] (_raw_spin_lock_irqsave+0x4c/0x60)
[<8052788c>] (_raw_spin_lock_irqsave+0x0/0x60) from [<8042c3c4>] (skb_queue_tail+0x20/0x50)
r6:bfbb2180 r5:bf1d0190 r4:bf1d0184
[<8042c3a4>] (skb_queue_tail+0x0/0x50) from [<8042c4cc>] (sock_queue_err_skb+0xd8/0x188)
r6:00000056 r5:bfbb2180 r4:bf1d0000 r3:00000000
[<8042c3f4>] (sock_queue_err_skb+0x0/0x188) from [<8042d15c>] (skb_tstamp_tx+0x70/0xa0)
r6:bf0dddb0 r5:bf1d0000 r4:bfbb2180 r3:00000004
[<8042d0ec>] (skb_tstamp_tx+0x0/0xa0) from [<803561d0>] (fec_enet_tx+0x258/0x268)
r6:c089d260 r5:00001c00 r4:bfbd0000
[<80355f78>] (fec_enet_tx+0x0/0x268) from [<803562cc>] (fec_enet_interrupt+0xec/0xf8)
[<803561e0>] (fec_enet_interrupt+0x0/0xf8) from [<8007d5b0>] (handle_irq_event_percpu+0x54/0x1a0)
[<8007d55c>] (handle_irq_event_percpu+0x0/0x1a0) from [<8007d740>] (handle_irq_event+0x44/0x64)
[<8007d6fc>] (handle_irq_event+0x0/0x64) from [<80080690>] (handle_fasteoi_irq+0xc4/0x15c)
r6:bf0dc000 r5:bf811290 r4:bf811240 r3:00000000
[<800805cc>] (handle_fasteoi_irq+0x0/0x15c) from [<8007ceec>] (generic_handle_irq+0x28/0x38)
r5:807130c8 r4:00000096
[<8007cec4>] (generic_handle_irq+0x0/0x38) from [<8000f16c>] (handle_IRQ+0x54/0xb4)
r4:8071d280 r3:00000180
[<8000f118>] (handle_IRQ+0x0/0xb4) from [<80008544>] (gic_handle_irq+0x30/0x64)
r8:8000e924 r7:f4000100 r6:bf0ddef8 r5:8071c974 r4:f400010c
r3:00000000
[<80008514>] (gic_handle_irq+0x0/0x64) from [<8000e2e4>] (__irq_svc+0x44/0x5c)
Exception stack(0xbf0ddef8 to 0xbf0ddf40)
Signed-off-by: Frank Li <Frank.Li@freescale.com>
---
Change from v3 to v2
* remove return value of fec_enet_tx
Change from v1 to v2
* ignore TX package count in poll function
drivers/net/ethernet/freescale/fec.c | 85 ++++++++++++++++-----------------
drivers/net/ethernet/freescale/fec.h | 3 -
2 files changed, 41 insertions(+), 47 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
index 0fe68c4..bca5a24 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -246,14 +246,13 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
struct bufdesc *bdp;
void *bufaddr;
unsigned short status;
- unsigned long flags;
+ unsigned int index;
if (!fep->link) {
/* Link is down or autonegotiation is in progress. */
return NETDEV_TX_BUSY;
}
- spin_lock_irqsave(&fep->hw_lock, flags);
/* Fill in a Tx ring entry */
bdp = fep->cur_tx;
@@ -264,7 +263,6 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
* This should not happen, since ndev->tbusy should be set.
*/
printk("%s: tx queue full!.\n", ndev->name);
- spin_unlock_irqrestore(&fep->hw_lock, flags);
return NETDEV_TX_BUSY;
}
@@ -280,13 +278,13 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
* 4-byte boundaries. Use bounce buffers to copy data
* and get it aligned. Ugh.
*/
+ if (fep->bufdesc_ex)
+ index = (struct bufdesc_ex *)bdp -
+ (struct bufdesc_ex *)fep->tx_bd_base;
+ else
+ index = bdp - fep->tx_bd_base;
+
if (((unsigned long) bufaddr) & FEC_ALIGNMENT) {
- unsigned int index;
- if (fep->bufdesc_ex)
- index = (struct bufdesc_ex *)bdp -
- (struct bufdesc_ex *)fep->tx_bd_base;
- else
- index = bdp - fep->tx_bd_base;
memcpy(fep->tx_bounce[index], skb->data, skb->len);
bufaddr = fep->tx_bounce[index];
}
@@ -300,10 +298,7 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
swap_buffer(bufaddr, skb->len);
/* Save skb pointer */
- fep->tx_skbuff[fep->skb_cur] = skb;
-
- ndev->stats.tx_bytes += skb->len;
- fep->skb_cur = (fep->skb_cur+1) & TX_RING_MOD_MASK;
+ fep->tx_skbuff[index] = skb;
/* Push the data cache so the CPM does not get stale memory
* data.
@@ -331,26 +326,22 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
ebdp->cbd_esc = BD_ENET_TX_INT;
}
}
- /* Trigger transmission start */
- writel(0, fep->hwp + FEC_X_DES_ACTIVE);
-
/* If this was the last BD in the ring, start at the beginning again. */
if (status & BD_ENET_TX_WRAP)
bdp = fep->tx_bd_base;
else
bdp = fec_enet_get_nextdesc(bdp, fep->bufdesc_ex);
- if (bdp == fep->dirty_tx) {
- fep->tx_full = 1;
+ fep->cur_tx = bdp;
+
+ if (fep->cur_tx == fep->dirty_tx)
netif_stop_queue(ndev);
- }
- fep->cur_tx = bdp;
+ /* Trigger transmission start */
+ writel(0, fep->hwp + FEC_X_DES_ACTIVE);
skb_tx_timestamp(skb);
- spin_unlock_irqrestore(&fep->hw_lock, flags);
-
return NETDEV_TX_OK;
}
@@ -406,11 +397,8 @@ fec_restart(struct net_device *ndev, int duplex)
writel((unsigned long)fep->bd_dma + sizeof(struct bufdesc)
* RX_RING_SIZE, fep->hwp + FEC_X_DES_START);
- fep->dirty_tx = fep->cur_tx = fep->tx_bd_base;
fep->cur_rx = fep->rx_bd_base;
- /* Reset SKB transmit buffers. */
- fep->skb_cur = fep->skb_dirty = 0;
for (i = 0; i <= TX_RING_MOD_MASK; i++) {
if (fep->tx_skbuff[i]) {
dev_kfree_skb_any(fep->tx_skbuff[i]);
@@ -573,20 +561,35 @@ fec_enet_tx(struct net_device *ndev)
struct bufdesc *bdp;
unsigned short status;
struct sk_buff *skb;
+ int index = 0;
fep = netdev_priv(ndev);
- spin_lock(&fep->hw_lock);
bdp = fep->dirty_tx;
+ /* get next bdp of dirty_tx */
+ if (bdp->cbd_sc & BD_ENET_TX_WRAP)
+ bdp = fep->tx_bd_base;
+ else
+ bdp = fec_enet_get_nextdesc(bdp, fep->bufdesc_ex);
+
while (((status = bdp->cbd_sc) & BD_ENET_TX_READY) == 0) {
- if (bdp == fep->cur_tx && fep->tx_full == 0)
+
+ /* current queue is empty */
+ if (bdp == fep->cur_tx)
break;
+ if (fep->bufdesc_ex)
+ index = (struct bufdesc_ex *)bdp -
+ (struct bufdesc_ex *)fep->tx_bd_base;
+ else
+ index = bdp - fep->tx_bd_base;
+
dma_unmap_single(&fep->pdev->dev, bdp->cbd_bufaddr,
FEC_ENET_TX_FRSIZE, DMA_TO_DEVICE);
bdp->cbd_bufaddr = 0;
- skb = fep->tx_skbuff[fep->skb_dirty];
+ skb = fep->tx_skbuff[index];
+
/* Check for errors. */
if (status & (BD_ENET_TX_HB | BD_ENET_TX_LC |
BD_ENET_TX_RL | BD_ENET_TX_UN |
@@ -631,8 +634,9 @@ fec_enet_tx(struct net_device *ndev)
/* Free the sk buffer associated with this last transmit */
dev_kfree_skb_any(skb);
- fep->tx_skbuff[fep->skb_dirty] = NULL;
- fep->skb_dirty = (fep->skb_dirty + 1) & TX_RING_MOD_MASK;
+ fep->tx_skbuff[index] = NULL;
+
+ fep->dirty_tx = bdp;
/* Update pointer to next buffer descriptor to be transmitted */
if (status & BD_ENET_TX_WRAP)
@@ -642,14 +646,12 @@ fec_enet_tx(struct net_device *ndev)
/* Since we have freed up a buffer, the ring is no longer full
*/
- if (fep->tx_full) {
- fep->tx_full = 0;
+ if (fep->dirty_tx != fep->cur_tx) {
if (netif_queue_stopped(ndev))
netif_wake_queue(ndev);
}
}
- fep->dirty_tx = bdp;
- spin_unlock(&fep->hw_lock);
+ return;
}
@@ -816,7 +818,7 @@ fec_enet_interrupt(int irq, void *dev_id)
int_events = readl(fep->hwp + FEC_IEVENT);
writel(int_events, fep->hwp + FEC_IEVENT);
- if (int_events & FEC_ENET_RXF) {
+ if (int_events & (FEC_ENET_RXF | FEC_ENET_TXF)) {
ret = IRQ_HANDLED;
/* Disable the RX interrupt */
@@ -827,15 +829,6 @@ fec_enet_interrupt(int irq, void *dev_id)
}
}
- /* Transmit OK, or non-fatal error. Update the buffer
- * descriptors. FEC handles all errors, we just discover
- * them as part of the transmit process.
- */
- if (int_events & FEC_ENET_TXF) {
- ret = IRQ_HANDLED;
- fec_enet_tx(ndev);
- }
-
if (int_events & FEC_ENET_MII) {
ret = IRQ_HANDLED;
complete(&fep->mdio_done);
@@ -851,6 +844,8 @@ static int fec_enet_rx_napi(struct napi_struct *napi, int budget)
int pkts = fec_enet_rx(ndev, budget);
struct fec_enet_private *fep = netdev_priv(ndev);
+ fec_enet_tx(ndev);
+
if (pkts < budget) {
napi_complete(napi);
writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
@@ -1646,6 +1641,7 @@ static int fec_enet_init(struct net_device *ndev)
/* ...and the same for transmit */
bdp = fep->tx_bd_base;
+ fep->cur_tx = bdp;
for (i = 0; i < TX_RING_SIZE; i++) {
/* Initialize the BD for every fragment in the page. */
@@ -1657,6 +1653,7 @@ static int fec_enet_init(struct net_device *ndev)
/* Set the last buffer to wrap */
bdp = fec_enet_get_prevdesc(bdp, fep->bufdesc_ex);
bdp->cbd_sc |= BD_SC_WRAP;
+ fep->dirty_tx = bdp;
fec_restart(ndev, 0);
diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 01579b8..c0f63be 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -214,8 +214,6 @@ struct fec_enet_private {
unsigned char *tx_bounce[TX_RING_SIZE];
struct sk_buff *tx_skbuff[TX_RING_SIZE];
struct sk_buff *rx_skbuff[RX_RING_SIZE];
- ushort skb_cur;
- ushort skb_dirty;
/* CPM dual port RAM relative addresses */
dma_addr_t bd_dma;
@@ -227,7 +225,6 @@ struct fec_enet_private {
/* The ring entries to be free()ed */
struct bufdesc *dirty_tx;
- uint tx_full;
/* hold while accessing the HW like ringbuffer for tx/rx but not MAC */
spinlock_t hw_lock;
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH v3 1/1 net-next] net: fec: put tx to napi poll function to fix dead lock
@ 2013-03-04 3:34 ` Frank Li
0 siblings, 0 replies; 14+ messages in thread
From: Frank Li @ 2013-03-04 3:34 UTC (permalink / raw)
To: linux-arm-kernel
up stack ndo_start_xmit already hold lock.
fec_enet_start_xmit needn't spin lock.
stat_xmit just update fep->cur_tx
fec_enet_tx just update fep->dirty_tx
Reserve a empty bdb to check full or empty
cur_tx == dirty_tx means full
cur_tx == dirty_tx +1 means empty
So needn't is_full variable.
Fix spin lock deadlock
=================================
[ INFO: inconsistent lock state ]
3.8.0-rc5+ #107 Not tainted
---------------------------------
inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage.
ptp4l/615 [HC1[1]:SC0[0]:HE0:SE1] takes:
(&(&list->lock)->rlock#3){?.-...}, at: [<8042c3c4>] skb_queue_tail+0x20/0x50
{HARDIRQ-ON-W} state was registered at:
[<80067250>] mark_lock+0x154/0x4e8
[<800676f4>] mark_irqflags+0x110/0x1a4
[<80069208>] __lock_acquire+0x494/0x9c0
[<80069ce8>] lock_acquire+0x90/0xa4
[<80527ad0>] _raw_spin_lock_bh+0x44/0x54
[<804877e0>] first_packet_length+0x38/0x1f0
[<804879e4>] udp_poll+0x4c/0x5c
[<804231f8>] sock_poll+0x24/0x28
[<800d27f0>] do_poll.isra.10+0x120/0x254
[<800d36e4>] do_sys_poll+0x15c/0x1e8
[<800d3828>] sys_poll+0x60/0xc8
[<8000e780>] ret_fast_syscall+0x0/0x3c
*** DEADLOCK ***
1 lock held by ptp4l/615:
#0: (&(&fep->hw_lock)->rlock){-.-...}, at: [<80355f9c>] fec_enet_tx+0x24/0x268
stack backtrace:
Backtrace:
[<800121e0>] (dump_backtrace+0x0/0x10c) from [<80516210>] (dump_stack+0x18/0x1c)
r6:8063b1fc r5:bf38b2f8 r4:bf38b000 r3:bf38b000
[<805161f8>] (dump_stack+0x0/0x1c) from [<805189d0>] (print_usage_bug.part.34+0x164/0x1a4)
[<8051886c>] (print_usage_bug.part.34+0x0/0x1a4) from [<80518a88>] (print_usage_bug+0x78/0x88)
r8:80065664 r7:bf38b2f8 r6:00000002 r5:00000000 r4:bf38b000
[<80518a10>] (print_usage_bug+0x0/0x88) from [<80518b58>] (mark_lock_irq+0xc0/0x270)
r7:bf38b000 r6:00000002 r5:bf38b2f8 r4:00000000
[<80518a98>] (mark_lock_irq+0x0/0x270) from [<80067270>] (mark_lock+0x174/0x4e8)
[<800670fc>] (mark_lock+0x0/0x4e8) from [<80067744>] (mark_irqflags+0x160/0x1a4)
[<800675e4>] (mark_irqflags+0x0/0x1a4) from [<80069208>] (__lock_acquire+0x494/0x9c0)
r5:00000002 r4:bf38b2f8
[<80068d74>] (__lock_acquire+0x0/0x9c0) from [<80069ce8>] (lock_acquire+0x90/0xa4)
[<80069c58>] (lock_acquire+0x0/0xa4) from [<805278d8>] (_raw_spin_lock_irqsave+0x4c/0x60)
[<8052788c>] (_raw_spin_lock_irqsave+0x0/0x60) from [<8042c3c4>] (skb_queue_tail+0x20/0x50)
r6:bfbb2180 r5:bf1d0190 r4:bf1d0184
[<8042c3a4>] (skb_queue_tail+0x0/0x50) from [<8042c4cc>] (sock_queue_err_skb+0xd8/0x188)
r6:00000056 r5:bfbb2180 r4:bf1d0000 r3:00000000
[<8042c3f4>] (sock_queue_err_skb+0x0/0x188) from [<8042d15c>] (skb_tstamp_tx+0x70/0xa0)
r6:bf0dddb0 r5:bf1d0000 r4:bfbb2180 r3:00000004
[<8042d0ec>] (skb_tstamp_tx+0x0/0xa0) from [<803561d0>] (fec_enet_tx+0x258/0x268)
r6:c089d260 r5:00001c00 r4:bfbd0000
[<80355f78>] (fec_enet_tx+0x0/0x268) from [<803562cc>] (fec_enet_interrupt+0xec/0xf8)
[<803561e0>] (fec_enet_interrupt+0x0/0xf8) from [<8007d5b0>] (handle_irq_event_percpu+0x54/0x1a0)
[<8007d55c>] (handle_irq_event_percpu+0x0/0x1a0) from [<8007d740>] (handle_irq_event+0x44/0x64)
[<8007d6fc>] (handle_irq_event+0x0/0x64) from [<80080690>] (handle_fasteoi_irq+0xc4/0x15c)
r6:bf0dc000 r5:bf811290 r4:bf811240 r3:00000000
[<800805cc>] (handle_fasteoi_irq+0x0/0x15c) from [<8007ceec>] (generic_handle_irq+0x28/0x38)
r5:807130c8 r4:00000096
[<8007cec4>] (generic_handle_irq+0x0/0x38) from [<8000f16c>] (handle_IRQ+0x54/0xb4)
r4:8071d280 r3:00000180
[<8000f118>] (handle_IRQ+0x0/0xb4) from [<80008544>] (gic_handle_irq+0x30/0x64)
r8:8000e924 r7:f4000100 r6:bf0ddef8 r5:8071c974 r4:f400010c
r3:00000000
[<80008514>] (gic_handle_irq+0x0/0x64) from [<8000e2e4>] (__irq_svc+0x44/0x5c)
Exception stack(0xbf0ddef8 to 0xbf0ddf40)
Signed-off-by: Frank Li <Frank.Li@freescale.com>
---
Change from v3 to v2
* remove return value of fec_enet_tx
Change from v1 to v2
* ignore TX package count in poll function
drivers/net/ethernet/freescale/fec.c | 85 ++++++++++++++++-----------------
drivers/net/ethernet/freescale/fec.h | 3 -
2 files changed, 41 insertions(+), 47 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
index 0fe68c4..bca5a24 100644
--- a/drivers/net/ethernet/freescale/fec.c
+++ b/drivers/net/ethernet/freescale/fec.c
@@ -246,14 +246,13 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
struct bufdesc *bdp;
void *bufaddr;
unsigned short status;
- unsigned long flags;
+ unsigned int index;
if (!fep->link) {
/* Link is down or autonegotiation is in progress. */
return NETDEV_TX_BUSY;
}
- spin_lock_irqsave(&fep->hw_lock, flags);
/* Fill in a Tx ring entry */
bdp = fep->cur_tx;
@@ -264,7 +263,6 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
* This should not happen, since ndev->tbusy should be set.
*/
printk("%s: tx queue full!.\n", ndev->name);
- spin_unlock_irqrestore(&fep->hw_lock, flags);
return NETDEV_TX_BUSY;
}
@@ -280,13 +278,13 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
* 4-byte boundaries. Use bounce buffers to copy data
* and get it aligned. Ugh.
*/
+ if (fep->bufdesc_ex)
+ index = (struct bufdesc_ex *)bdp -
+ (struct bufdesc_ex *)fep->tx_bd_base;
+ else
+ index = bdp - fep->tx_bd_base;
+
if (((unsigned long) bufaddr) & FEC_ALIGNMENT) {
- unsigned int index;
- if (fep->bufdesc_ex)
- index = (struct bufdesc_ex *)bdp -
- (struct bufdesc_ex *)fep->tx_bd_base;
- else
- index = bdp - fep->tx_bd_base;
memcpy(fep->tx_bounce[index], skb->data, skb->len);
bufaddr = fep->tx_bounce[index];
}
@@ -300,10 +298,7 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
swap_buffer(bufaddr, skb->len);
/* Save skb pointer */
- fep->tx_skbuff[fep->skb_cur] = skb;
-
- ndev->stats.tx_bytes += skb->len;
- fep->skb_cur = (fep->skb_cur+1) & TX_RING_MOD_MASK;
+ fep->tx_skbuff[index] = skb;
/* Push the data cache so the CPM does not get stale memory
* data.
@@ -331,26 +326,22 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
ebdp->cbd_esc = BD_ENET_TX_INT;
}
}
- /* Trigger transmission start */
- writel(0, fep->hwp + FEC_X_DES_ACTIVE);
-
/* If this was the last BD in the ring, start at the beginning again. */
if (status & BD_ENET_TX_WRAP)
bdp = fep->tx_bd_base;
else
bdp = fec_enet_get_nextdesc(bdp, fep->bufdesc_ex);
- if (bdp == fep->dirty_tx) {
- fep->tx_full = 1;
+ fep->cur_tx = bdp;
+
+ if (fep->cur_tx == fep->dirty_tx)
netif_stop_queue(ndev);
- }
- fep->cur_tx = bdp;
+ /* Trigger transmission start */
+ writel(0, fep->hwp + FEC_X_DES_ACTIVE);
skb_tx_timestamp(skb);
- spin_unlock_irqrestore(&fep->hw_lock, flags);
-
return NETDEV_TX_OK;
}
@@ -406,11 +397,8 @@ fec_restart(struct net_device *ndev, int duplex)
writel((unsigned long)fep->bd_dma + sizeof(struct bufdesc)
* RX_RING_SIZE, fep->hwp + FEC_X_DES_START);
- fep->dirty_tx = fep->cur_tx = fep->tx_bd_base;
fep->cur_rx = fep->rx_bd_base;
- /* Reset SKB transmit buffers. */
- fep->skb_cur = fep->skb_dirty = 0;
for (i = 0; i <= TX_RING_MOD_MASK; i++) {
if (fep->tx_skbuff[i]) {
dev_kfree_skb_any(fep->tx_skbuff[i]);
@@ -573,20 +561,35 @@ fec_enet_tx(struct net_device *ndev)
struct bufdesc *bdp;
unsigned short status;
struct sk_buff *skb;
+ int index = 0;
fep = netdev_priv(ndev);
- spin_lock(&fep->hw_lock);
bdp = fep->dirty_tx;
+ /* get next bdp of dirty_tx */
+ if (bdp->cbd_sc & BD_ENET_TX_WRAP)
+ bdp = fep->tx_bd_base;
+ else
+ bdp = fec_enet_get_nextdesc(bdp, fep->bufdesc_ex);
+
while (((status = bdp->cbd_sc) & BD_ENET_TX_READY) == 0) {
- if (bdp == fep->cur_tx && fep->tx_full == 0)
+
+ /* current queue is empty */
+ if (bdp == fep->cur_tx)
break;
+ if (fep->bufdesc_ex)
+ index = (struct bufdesc_ex *)bdp -
+ (struct bufdesc_ex *)fep->tx_bd_base;
+ else
+ index = bdp - fep->tx_bd_base;
+
dma_unmap_single(&fep->pdev->dev, bdp->cbd_bufaddr,
FEC_ENET_TX_FRSIZE, DMA_TO_DEVICE);
bdp->cbd_bufaddr = 0;
- skb = fep->tx_skbuff[fep->skb_dirty];
+ skb = fep->tx_skbuff[index];
+
/* Check for errors. */
if (status & (BD_ENET_TX_HB | BD_ENET_TX_LC |
BD_ENET_TX_RL | BD_ENET_TX_UN |
@@ -631,8 +634,9 @@ fec_enet_tx(struct net_device *ndev)
/* Free the sk buffer associated with this last transmit */
dev_kfree_skb_any(skb);
- fep->tx_skbuff[fep->skb_dirty] = NULL;
- fep->skb_dirty = (fep->skb_dirty + 1) & TX_RING_MOD_MASK;
+ fep->tx_skbuff[index] = NULL;
+
+ fep->dirty_tx = bdp;
/* Update pointer to next buffer descriptor to be transmitted */
if (status & BD_ENET_TX_WRAP)
@@ -642,14 +646,12 @@ fec_enet_tx(struct net_device *ndev)
/* Since we have freed up a buffer, the ring is no longer full
*/
- if (fep->tx_full) {
- fep->tx_full = 0;
+ if (fep->dirty_tx != fep->cur_tx) {
if (netif_queue_stopped(ndev))
netif_wake_queue(ndev);
}
}
- fep->dirty_tx = bdp;
- spin_unlock(&fep->hw_lock);
+ return;
}
@@ -816,7 +818,7 @@ fec_enet_interrupt(int irq, void *dev_id)
int_events = readl(fep->hwp + FEC_IEVENT);
writel(int_events, fep->hwp + FEC_IEVENT);
- if (int_events & FEC_ENET_RXF) {
+ if (int_events & (FEC_ENET_RXF | FEC_ENET_TXF)) {
ret = IRQ_HANDLED;
/* Disable the RX interrupt */
@@ -827,15 +829,6 @@ fec_enet_interrupt(int irq, void *dev_id)
}
}
- /* Transmit OK, or non-fatal error. Update the buffer
- * descriptors. FEC handles all errors, we just discover
- * them as part of the transmit process.
- */
- if (int_events & FEC_ENET_TXF) {
- ret = IRQ_HANDLED;
- fec_enet_tx(ndev);
- }
-
if (int_events & FEC_ENET_MII) {
ret = IRQ_HANDLED;
complete(&fep->mdio_done);
@@ -851,6 +844,8 @@ static int fec_enet_rx_napi(struct napi_struct *napi, int budget)
int pkts = fec_enet_rx(ndev, budget);
struct fec_enet_private *fep = netdev_priv(ndev);
+ fec_enet_tx(ndev);
+
if (pkts < budget) {
napi_complete(napi);
writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
@@ -1646,6 +1641,7 @@ static int fec_enet_init(struct net_device *ndev)
/* ...and the same for transmit */
bdp = fep->tx_bd_base;
+ fep->cur_tx = bdp;
for (i = 0; i < TX_RING_SIZE; i++) {
/* Initialize the BD for every fragment in the page. */
@@ -1657,6 +1653,7 @@ static int fec_enet_init(struct net_device *ndev)
/* Set the last buffer to wrap */
bdp = fec_enet_get_prevdesc(bdp, fep->bufdesc_ex);
bdp->cbd_sc |= BD_SC_WRAP;
+ fep->dirty_tx = bdp;
fec_restart(ndev, 0);
diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
index 01579b8..c0f63be 100644
--- a/drivers/net/ethernet/freescale/fec.h
+++ b/drivers/net/ethernet/freescale/fec.h
@@ -214,8 +214,6 @@ struct fec_enet_private {
unsigned char *tx_bounce[TX_RING_SIZE];
struct sk_buff *tx_skbuff[TX_RING_SIZE];
struct sk_buff *rx_skbuff[RX_RING_SIZE];
- ushort skb_cur;
- ushort skb_dirty;
/* CPM dual port RAM relative addresses */
dma_addr_t bd_dma;
@@ -227,7 +225,6 @@ struct fec_enet_private {
/* The ring entries to be free()ed */
struct bufdesc *dirty_tx;
- uint tx_full;
/* hold while accessing the HW like ringbuffer for tx/rx but not MAC */
spinlock_t hw_lock;
--
1.7.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/1 net-next] net: fec: put tx to napi poll function to fix dead lock
2013-03-04 3:34 ` Frank Li
@ 2013-03-04 19:15 ` David Miller
-1 siblings, 0 replies; 14+ messages in thread
From: David Miller @ 2013-03-04 19:15 UTC (permalink / raw)
To: Frank.Li; +Cc: lznuaa, shawn.guo, B38611, linux-arm-kernel, netdev, s.hauer
From: Frank Li <Frank.Li@freescale.com>
Date: Mon, 4 Mar 2013 11:34:25 +0800
> up stack ndo_start_xmit already hold lock.
> fec_enet_start_xmit needn't spin lock.
> stat_xmit just update fep->cur_tx
> fec_enet_tx just update fep->dirty_tx
>
> Reserve a empty bdb to check full or empty
> cur_tx == dirty_tx means full
> cur_tx == dirty_tx +1 means empty
>
> So needn't is_full variable.
...
> Signed-off-by: Frank Li <Frank.Li@freescale.com>
Applied.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 1/1 net-next] net: fec: put tx to napi poll function to fix dead lock
@ 2013-03-04 19:15 ` David Miller
0 siblings, 0 replies; 14+ messages in thread
From: David Miller @ 2013-03-04 19:15 UTC (permalink / raw)
To: linux-arm-kernel
From: Frank Li <Frank.Li@freescale.com>
Date: Mon, 4 Mar 2013 11:34:25 +0800
> up stack ndo_start_xmit already hold lock.
> fec_enet_start_xmit needn't spin lock.
> stat_xmit just update fep->cur_tx
> fec_enet_tx just update fep->dirty_tx
>
> Reserve a empty bdb to check full or empty
> cur_tx == dirty_tx means full
> cur_tx == dirty_tx +1 means empty
>
> So needn't is_full variable.
...
> Signed-off-by: Frank Li <Frank.Li@freescale.com>
Applied.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/1 net-next] net: fec: put tx to napi poll function to fix dead lock
2013-03-04 3:34 ` Frank Li
@ 2013-03-20 11:48 ` Shawn Guo
-1 siblings, 0 replies; 14+ messages in thread
From: Shawn Guo @ 2013-03-20 11:48 UTC (permalink / raw)
To: Frank Li; +Cc: B38611, netdev, s.hauer, lznuaa, davem, linux-arm-kernel
Frank,
I'm running v3.9-rc3 image with NFS on imx6q, and seeing system fail
to resume back. The resume seems being stopped by fec driver. If you
wait long enough, you might see the repeated "eth0: tx queue full!."
error message.
Reverting the patch gets resume back to work. Can you please
investigate?
Shawn
On Mon, Mar 04, 2013 at 11:34:25AM +0800, Frank Li wrote:
> up stack ndo_start_xmit already hold lock.
> fec_enet_start_xmit needn't spin lock.
> stat_xmit just update fep->cur_tx
> fec_enet_tx just update fep->dirty_tx
>
> Reserve a empty bdb to check full or empty
> cur_tx == dirty_tx means full
> cur_tx == dirty_tx +1 means empty
>
> So needn't is_full variable.
>
> Fix spin lock deadlock
> =================================
> [ INFO: inconsistent lock state ]
> 3.8.0-rc5+ #107 Not tainted
> ---------------------------------
> inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage.
> ptp4l/615 [HC1[1]:SC0[0]:HE0:SE1] takes:
> (&(&list->lock)->rlock#3){?.-...}, at: [<8042c3c4>] skb_queue_tail+0x20/0x50
> {HARDIRQ-ON-W} state was registered at:
> [<80067250>] mark_lock+0x154/0x4e8
> [<800676f4>] mark_irqflags+0x110/0x1a4
> [<80069208>] __lock_acquire+0x494/0x9c0
> [<80069ce8>] lock_acquire+0x90/0xa4
> [<80527ad0>] _raw_spin_lock_bh+0x44/0x54
> [<804877e0>] first_packet_length+0x38/0x1f0
> [<804879e4>] udp_poll+0x4c/0x5c
> [<804231f8>] sock_poll+0x24/0x28
> [<800d27f0>] do_poll.isra.10+0x120/0x254
> [<800d36e4>] do_sys_poll+0x15c/0x1e8
> [<800d3828>] sys_poll+0x60/0xc8
> [<8000e780>] ret_fast_syscall+0x0/0x3c
>
> *** DEADLOCK ***
>
> 1 lock held by ptp4l/615:
> #0: (&(&fep->hw_lock)->rlock){-.-...}, at: [<80355f9c>] fec_enet_tx+0x24/0x268
> stack backtrace:
> Backtrace:
> [<800121e0>] (dump_backtrace+0x0/0x10c) from [<80516210>] (dump_stack+0x18/0x1c)
> r6:8063b1fc r5:bf38b2f8 r4:bf38b000 r3:bf38b000
> [<805161f8>] (dump_stack+0x0/0x1c) from [<805189d0>] (print_usage_bug.part.34+0x164/0x1a4)
> [<8051886c>] (print_usage_bug.part.34+0x0/0x1a4) from [<80518a88>] (print_usage_bug+0x78/0x88)
> r8:80065664 r7:bf38b2f8 r6:00000002 r5:00000000 r4:bf38b000
> [<80518a10>] (print_usage_bug+0x0/0x88) from [<80518b58>] (mark_lock_irq+0xc0/0x270)
> r7:bf38b000 r6:00000002 r5:bf38b2f8 r4:00000000
> [<80518a98>] (mark_lock_irq+0x0/0x270) from [<80067270>] (mark_lock+0x174/0x4e8)
> [<800670fc>] (mark_lock+0x0/0x4e8) from [<80067744>] (mark_irqflags+0x160/0x1a4)
> [<800675e4>] (mark_irqflags+0x0/0x1a4) from [<80069208>] (__lock_acquire+0x494/0x9c0)
> r5:00000002 r4:bf38b2f8
> [<80068d74>] (__lock_acquire+0x0/0x9c0) from [<80069ce8>] (lock_acquire+0x90/0xa4)
> [<80069c58>] (lock_acquire+0x0/0xa4) from [<805278d8>] (_raw_spin_lock_irqsave+0x4c/0x60)
> [<8052788c>] (_raw_spin_lock_irqsave+0x0/0x60) from [<8042c3c4>] (skb_queue_tail+0x20/0x50)
> r6:bfbb2180 r5:bf1d0190 r4:bf1d0184
> [<8042c3a4>] (skb_queue_tail+0x0/0x50) from [<8042c4cc>] (sock_queue_err_skb+0xd8/0x188)
> r6:00000056 r5:bfbb2180 r4:bf1d0000 r3:00000000
> [<8042c3f4>] (sock_queue_err_skb+0x0/0x188) from [<8042d15c>] (skb_tstamp_tx+0x70/0xa0)
> r6:bf0dddb0 r5:bf1d0000 r4:bfbb2180 r3:00000004
> [<8042d0ec>] (skb_tstamp_tx+0x0/0xa0) from [<803561d0>] (fec_enet_tx+0x258/0x268)
> r6:c089d260 r5:00001c00 r4:bfbd0000
> [<80355f78>] (fec_enet_tx+0x0/0x268) from [<803562cc>] (fec_enet_interrupt+0xec/0xf8)
> [<803561e0>] (fec_enet_interrupt+0x0/0xf8) from [<8007d5b0>] (handle_irq_event_percpu+0x54/0x1a0)
> [<8007d55c>] (handle_irq_event_percpu+0x0/0x1a0) from [<8007d740>] (handle_irq_event+0x44/0x64)
> [<8007d6fc>] (handle_irq_event+0x0/0x64) from [<80080690>] (handle_fasteoi_irq+0xc4/0x15c)
> r6:bf0dc000 r5:bf811290 r4:bf811240 r3:00000000
> [<800805cc>] (handle_fasteoi_irq+0x0/0x15c) from [<8007ceec>] (generic_handle_irq+0x28/0x38)
> r5:807130c8 r4:00000096
> [<8007cec4>] (generic_handle_irq+0x0/0x38) from [<8000f16c>] (handle_IRQ+0x54/0xb4)
> r4:8071d280 r3:00000180
> [<8000f118>] (handle_IRQ+0x0/0xb4) from [<80008544>] (gic_handle_irq+0x30/0x64)
> r8:8000e924 r7:f4000100 r6:bf0ddef8 r5:8071c974 r4:f400010c
> r3:00000000
> [<80008514>] (gic_handle_irq+0x0/0x64) from [<8000e2e4>] (__irq_svc+0x44/0x5c)
> Exception stack(0xbf0ddef8 to 0xbf0ddf40)
>
> Signed-off-by: Frank Li <Frank.Li@freescale.com>
> ---
> Change from v3 to v2
> * remove return value of fec_enet_tx
> Change from v1 to v2
> * ignore TX package count in poll function
>
> drivers/net/ethernet/freescale/fec.c | 85 ++++++++++++++++-----------------
> drivers/net/ethernet/freescale/fec.h | 3 -
> 2 files changed, 41 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
> index 0fe68c4..bca5a24 100644
> --- a/drivers/net/ethernet/freescale/fec.c
> +++ b/drivers/net/ethernet/freescale/fec.c
> @@ -246,14 +246,13 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> struct bufdesc *bdp;
> void *bufaddr;
> unsigned short status;
> - unsigned long flags;
> + unsigned int index;
>
> if (!fep->link) {
> /* Link is down or autonegotiation is in progress. */
> return NETDEV_TX_BUSY;
> }
>
> - spin_lock_irqsave(&fep->hw_lock, flags);
> /* Fill in a Tx ring entry */
> bdp = fep->cur_tx;
>
> @@ -264,7 +263,6 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> * This should not happen, since ndev->tbusy should be set.
> */
> printk("%s: tx queue full!.\n", ndev->name);
> - spin_unlock_irqrestore(&fep->hw_lock, flags);
> return NETDEV_TX_BUSY;
> }
>
> @@ -280,13 +278,13 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> * 4-byte boundaries. Use bounce buffers to copy data
> * and get it aligned. Ugh.
> */
> + if (fep->bufdesc_ex)
> + index = (struct bufdesc_ex *)bdp -
> + (struct bufdesc_ex *)fep->tx_bd_base;
> + else
> + index = bdp - fep->tx_bd_base;
> +
> if (((unsigned long) bufaddr) & FEC_ALIGNMENT) {
> - unsigned int index;
> - if (fep->bufdesc_ex)
> - index = (struct bufdesc_ex *)bdp -
> - (struct bufdesc_ex *)fep->tx_bd_base;
> - else
> - index = bdp - fep->tx_bd_base;
> memcpy(fep->tx_bounce[index], skb->data, skb->len);
> bufaddr = fep->tx_bounce[index];
> }
> @@ -300,10 +298,7 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> swap_buffer(bufaddr, skb->len);
>
> /* Save skb pointer */
> - fep->tx_skbuff[fep->skb_cur] = skb;
> -
> - ndev->stats.tx_bytes += skb->len;
> - fep->skb_cur = (fep->skb_cur+1) & TX_RING_MOD_MASK;
> + fep->tx_skbuff[index] = skb;
>
> /* Push the data cache so the CPM does not get stale memory
> * data.
> @@ -331,26 +326,22 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> ebdp->cbd_esc = BD_ENET_TX_INT;
> }
> }
> - /* Trigger transmission start */
> - writel(0, fep->hwp + FEC_X_DES_ACTIVE);
> -
> /* If this was the last BD in the ring, start at the beginning again. */
> if (status & BD_ENET_TX_WRAP)
> bdp = fep->tx_bd_base;
> else
> bdp = fec_enet_get_nextdesc(bdp, fep->bufdesc_ex);
>
> - if (bdp == fep->dirty_tx) {
> - fep->tx_full = 1;
> + fep->cur_tx = bdp;
> +
> + if (fep->cur_tx == fep->dirty_tx)
> netif_stop_queue(ndev);
> - }
>
> - fep->cur_tx = bdp;
> + /* Trigger transmission start */
> + writel(0, fep->hwp + FEC_X_DES_ACTIVE);
>
> skb_tx_timestamp(skb);
>
> - spin_unlock_irqrestore(&fep->hw_lock, flags);
> -
> return NETDEV_TX_OK;
> }
>
> @@ -406,11 +397,8 @@ fec_restart(struct net_device *ndev, int duplex)
> writel((unsigned long)fep->bd_dma + sizeof(struct bufdesc)
> * RX_RING_SIZE, fep->hwp + FEC_X_DES_START);
>
> - fep->dirty_tx = fep->cur_tx = fep->tx_bd_base;
> fep->cur_rx = fep->rx_bd_base;
>
> - /* Reset SKB transmit buffers. */
> - fep->skb_cur = fep->skb_dirty = 0;
> for (i = 0; i <= TX_RING_MOD_MASK; i++) {
> if (fep->tx_skbuff[i]) {
> dev_kfree_skb_any(fep->tx_skbuff[i]);
> @@ -573,20 +561,35 @@ fec_enet_tx(struct net_device *ndev)
> struct bufdesc *bdp;
> unsigned short status;
> struct sk_buff *skb;
> + int index = 0;
>
> fep = netdev_priv(ndev);
> - spin_lock(&fep->hw_lock);
> bdp = fep->dirty_tx;
>
> + /* get next bdp of dirty_tx */
> + if (bdp->cbd_sc & BD_ENET_TX_WRAP)
> + bdp = fep->tx_bd_base;
> + else
> + bdp = fec_enet_get_nextdesc(bdp, fep->bufdesc_ex);
> +
> while (((status = bdp->cbd_sc) & BD_ENET_TX_READY) == 0) {
> - if (bdp == fep->cur_tx && fep->tx_full == 0)
> +
> + /* current queue is empty */
> + if (bdp == fep->cur_tx)
> break;
>
> + if (fep->bufdesc_ex)
> + index = (struct bufdesc_ex *)bdp -
> + (struct bufdesc_ex *)fep->tx_bd_base;
> + else
> + index = bdp - fep->tx_bd_base;
> +
> dma_unmap_single(&fep->pdev->dev, bdp->cbd_bufaddr,
> FEC_ENET_TX_FRSIZE, DMA_TO_DEVICE);
> bdp->cbd_bufaddr = 0;
>
> - skb = fep->tx_skbuff[fep->skb_dirty];
> + skb = fep->tx_skbuff[index];
> +
> /* Check for errors. */
> if (status & (BD_ENET_TX_HB | BD_ENET_TX_LC |
> BD_ENET_TX_RL | BD_ENET_TX_UN |
> @@ -631,8 +634,9 @@ fec_enet_tx(struct net_device *ndev)
>
> /* Free the sk buffer associated with this last transmit */
> dev_kfree_skb_any(skb);
> - fep->tx_skbuff[fep->skb_dirty] = NULL;
> - fep->skb_dirty = (fep->skb_dirty + 1) & TX_RING_MOD_MASK;
> + fep->tx_skbuff[index] = NULL;
> +
> + fep->dirty_tx = bdp;
>
> /* Update pointer to next buffer descriptor to be transmitted */
> if (status & BD_ENET_TX_WRAP)
> @@ -642,14 +646,12 @@ fec_enet_tx(struct net_device *ndev)
>
> /* Since we have freed up a buffer, the ring is no longer full
> */
> - if (fep->tx_full) {
> - fep->tx_full = 0;
> + if (fep->dirty_tx != fep->cur_tx) {
> if (netif_queue_stopped(ndev))
> netif_wake_queue(ndev);
> }
> }
> - fep->dirty_tx = bdp;
> - spin_unlock(&fep->hw_lock);
> + return;
> }
>
>
> @@ -816,7 +818,7 @@ fec_enet_interrupt(int irq, void *dev_id)
> int_events = readl(fep->hwp + FEC_IEVENT);
> writel(int_events, fep->hwp + FEC_IEVENT);
>
> - if (int_events & FEC_ENET_RXF) {
> + if (int_events & (FEC_ENET_RXF | FEC_ENET_TXF)) {
> ret = IRQ_HANDLED;
>
> /* Disable the RX interrupt */
> @@ -827,15 +829,6 @@ fec_enet_interrupt(int irq, void *dev_id)
> }
> }
>
> - /* Transmit OK, or non-fatal error. Update the buffer
> - * descriptors. FEC handles all errors, we just discover
> - * them as part of the transmit process.
> - */
> - if (int_events & FEC_ENET_TXF) {
> - ret = IRQ_HANDLED;
> - fec_enet_tx(ndev);
> - }
> -
> if (int_events & FEC_ENET_MII) {
> ret = IRQ_HANDLED;
> complete(&fep->mdio_done);
> @@ -851,6 +844,8 @@ static int fec_enet_rx_napi(struct napi_struct *napi, int budget)
> int pkts = fec_enet_rx(ndev, budget);
> struct fec_enet_private *fep = netdev_priv(ndev);
>
> + fec_enet_tx(ndev);
> +
> if (pkts < budget) {
> napi_complete(napi);
> writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
> @@ -1646,6 +1641,7 @@ static int fec_enet_init(struct net_device *ndev)
>
> /* ...and the same for transmit */
> bdp = fep->tx_bd_base;
> + fep->cur_tx = bdp;
> for (i = 0; i < TX_RING_SIZE; i++) {
>
> /* Initialize the BD for every fragment in the page. */
> @@ -1657,6 +1653,7 @@ static int fec_enet_init(struct net_device *ndev)
> /* Set the last buffer to wrap */
> bdp = fec_enet_get_prevdesc(bdp, fep->bufdesc_ex);
> bdp->cbd_sc |= BD_SC_WRAP;
> + fep->dirty_tx = bdp;
>
> fec_restart(ndev, 0);
>
> diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
> index 01579b8..c0f63be 100644
> --- a/drivers/net/ethernet/freescale/fec.h
> +++ b/drivers/net/ethernet/freescale/fec.h
> @@ -214,8 +214,6 @@ struct fec_enet_private {
> unsigned char *tx_bounce[TX_RING_SIZE];
> struct sk_buff *tx_skbuff[TX_RING_SIZE];
> struct sk_buff *rx_skbuff[RX_RING_SIZE];
> - ushort skb_cur;
> - ushort skb_dirty;
>
> /* CPM dual port RAM relative addresses */
> dma_addr_t bd_dma;
> @@ -227,7 +225,6 @@ struct fec_enet_private {
> /* The ring entries to be free()ed */
> struct bufdesc *dirty_tx;
>
> - uint tx_full;
> /* hold while accessing the HW like ringbuffer for tx/rx but not MAC */
> spinlock_t hw_lock;
>
> --
> 1.7.1
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 1/1 net-next] net: fec: put tx to napi poll function to fix dead lock
@ 2013-03-20 11:48 ` Shawn Guo
0 siblings, 0 replies; 14+ messages in thread
From: Shawn Guo @ 2013-03-20 11:48 UTC (permalink / raw)
To: linux-arm-kernel
Frank,
I'm running v3.9-rc3 image with NFS on imx6q, and seeing system fail
to resume back. The resume seems being stopped by fec driver. If you
wait long enough, you might see the repeated "eth0: tx queue full!."
error message.
Reverting the patch gets resume back to work. Can you please
investigate?
Shawn
On Mon, Mar 04, 2013 at 11:34:25AM +0800, Frank Li wrote:
> up stack ndo_start_xmit already hold lock.
> fec_enet_start_xmit needn't spin lock.
> stat_xmit just update fep->cur_tx
> fec_enet_tx just update fep->dirty_tx
>
> Reserve a empty bdb to check full or empty
> cur_tx == dirty_tx means full
> cur_tx == dirty_tx +1 means empty
>
> So needn't is_full variable.
>
> Fix spin lock deadlock
> =================================
> [ INFO: inconsistent lock state ]
> 3.8.0-rc5+ #107 Not tainted
> ---------------------------------
> inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage.
> ptp4l/615 [HC1[1]:SC0[0]:HE0:SE1] takes:
> (&(&list->lock)->rlock#3){?.-...}, at: [<8042c3c4>] skb_queue_tail+0x20/0x50
> {HARDIRQ-ON-W} state was registered at:
> [<80067250>] mark_lock+0x154/0x4e8
> [<800676f4>] mark_irqflags+0x110/0x1a4
> [<80069208>] __lock_acquire+0x494/0x9c0
> [<80069ce8>] lock_acquire+0x90/0xa4
> [<80527ad0>] _raw_spin_lock_bh+0x44/0x54
> [<804877e0>] first_packet_length+0x38/0x1f0
> [<804879e4>] udp_poll+0x4c/0x5c
> [<804231f8>] sock_poll+0x24/0x28
> [<800d27f0>] do_poll.isra.10+0x120/0x254
> [<800d36e4>] do_sys_poll+0x15c/0x1e8
> [<800d3828>] sys_poll+0x60/0xc8
> [<8000e780>] ret_fast_syscall+0x0/0x3c
>
> *** DEADLOCK ***
>
> 1 lock held by ptp4l/615:
> #0: (&(&fep->hw_lock)->rlock){-.-...}, at: [<80355f9c>] fec_enet_tx+0x24/0x268
> stack backtrace:
> Backtrace:
> [<800121e0>] (dump_backtrace+0x0/0x10c) from [<80516210>] (dump_stack+0x18/0x1c)
> r6:8063b1fc r5:bf38b2f8 r4:bf38b000 r3:bf38b000
> [<805161f8>] (dump_stack+0x0/0x1c) from [<805189d0>] (print_usage_bug.part.34+0x164/0x1a4)
> [<8051886c>] (print_usage_bug.part.34+0x0/0x1a4) from [<80518a88>] (print_usage_bug+0x78/0x88)
> r8:80065664 r7:bf38b2f8 r6:00000002 r5:00000000 r4:bf38b000
> [<80518a10>] (print_usage_bug+0x0/0x88) from [<80518b58>] (mark_lock_irq+0xc0/0x270)
> r7:bf38b000 r6:00000002 r5:bf38b2f8 r4:00000000
> [<80518a98>] (mark_lock_irq+0x0/0x270) from [<80067270>] (mark_lock+0x174/0x4e8)
> [<800670fc>] (mark_lock+0x0/0x4e8) from [<80067744>] (mark_irqflags+0x160/0x1a4)
> [<800675e4>] (mark_irqflags+0x0/0x1a4) from [<80069208>] (__lock_acquire+0x494/0x9c0)
> r5:00000002 r4:bf38b2f8
> [<80068d74>] (__lock_acquire+0x0/0x9c0) from [<80069ce8>] (lock_acquire+0x90/0xa4)
> [<80069c58>] (lock_acquire+0x0/0xa4) from [<805278d8>] (_raw_spin_lock_irqsave+0x4c/0x60)
> [<8052788c>] (_raw_spin_lock_irqsave+0x0/0x60) from [<8042c3c4>] (skb_queue_tail+0x20/0x50)
> r6:bfbb2180 r5:bf1d0190 r4:bf1d0184
> [<8042c3a4>] (skb_queue_tail+0x0/0x50) from [<8042c4cc>] (sock_queue_err_skb+0xd8/0x188)
> r6:00000056 r5:bfbb2180 r4:bf1d0000 r3:00000000
> [<8042c3f4>] (sock_queue_err_skb+0x0/0x188) from [<8042d15c>] (skb_tstamp_tx+0x70/0xa0)
> r6:bf0dddb0 r5:bf1d0000 r4:bfbb2180 r3:00000004
> [<8042d0ec>] (skb_tstamp_tx+0x0/0xa0) from [<803561d0>] (fec_enet_tx+0x258/0x268)
> r6:c089d260 r5:00001c00 r4:bfbd0000
> [<80355f78>] (fec_enet_tx+0x0/0x268) from [<803562cc>] (fec_enet_interrupt+0xec/0xf8)
> [<803561e0>] (fec_enet_interrupt+0x0/0xf8) from [<8007d5b0>] (handle_irq_event_percpu+0x54/0x1a0)
> [<8007d55c>] (handle_irq_event_percpu+0x0/0x1a0) from [<8007d740>] (handle_irq_event+0x44/0x64)
> [<8007d6fc>] (handle_irq_event+0x0/0x64) from [<80080690>] (handle_fasteoi_irq+0xc4/0x15c)
> r6:bf0dc000 r5:bf811290 r4:bf811240 r3:00000000
> [<800805cc>] (handle_fasteoi_irq+0x0/0x15c) from [<8007ceec>] (generic_handle_irq+0x28/0x38)
> r5:807130c8 r4:00000096
> [<8007cec4>] (generic_handle_irq+0x0/0x38) from [<8000f16c>] (handle_IRQ+0x54/0xb4)
> r4:8071d280 r3:00000180
> [<8000f118>] (handle_IRQ+0x0/0xb4) from [<80008544>] (gic_handle_irq+0x30/0x64)
> r8:8000e924 r7:f4000100 r6:bf0ddef8 r5:8071c974 r4:f400010c
> r3:00000000
> [<80008514>] (gic_handle_irq+0x0/0x64) from [<8000e2e4>] (__irq_svc+0x44/0x5c)
> Exception stack(0xbf0ddef8 to 0xbf0ddf40)
>
> Signed-off-by: Frank Li <Frank.Li@freescale.com>
> ---
> Change from v3 to v2
> * remove return value of fec_enet_tx
> Change from v1 to v2
> * ignore TX package count in poll function
>
> drivers/net/ethernet/freescale/fec.c | 85 ++++++++++++++++-----------------
> drivers/net/ethernet/freescale/fec.h | 3 -
> 2 files changed, 41 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
> index 0fe68c4..bca5a24 100644
> --- a/drivers/net/ethernet/freescale/fec.c
> +++ b/drivers/net/ethernet/freescale/fec.c
> @@ -246,14 +246,13 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> struct bufdesc *bdp;
> void *bufaddr;
> unsigned short status;
> - unsigned long flags;
> + unsigned int index;
>
> if (!fep->link) {
> /* Link is down or autonegotiation is in progress. */
> return NETDEV_TX_BUSY;
> }
>
> - spin_lock_irqsave(&fep->hw_lock, flags);
> /* Fill in a Tx ring entry */
> bdp = fep->cur_tx;
>
> @@ -264,7 +263,6 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> * This should not happen, since ndev->tbusy should be set.
> */
> printk("%s: tx queue full!.\n", ndev->name);
> - spin_unlock_irqrestore(&fep->hw_lock, flags);
> return NETDEV_TX_BUSY;
> }
>
> @@ -280,13 +278,13 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> * 4-byte boundaries. Use bounce buffers to copy data
> * and get it aligned. Ugh.
> */
> + if (fep->bufdesc_ex)
> + index = (struct bufdesc_ex *)bdp -
> + (struct bufdesc_ex *)fep->tx_bd_base;
> + else
> + index = bdp - fep->tx_bd_base;
> +
> if (((unsigned long) bufaddr) & FEC_ALIGNMENT) {
> - unsigned int index;
> - if (fep->bufdesc_ex)
> - index = (struct bufdesc_ex *)bdp -
> - (struct bufdesc_ex *)fep->tx_bd_base;
> - else
> - index = bdp - fep->tx_bd_base;
> memcpy(fep->tx_bounce[index], skb->data, skb->len);
> bufaddr = fep->tx_bounce[index];
> }
> @@ -300,10 +298,7 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> swap_buffer(bufaddr, skb->len);
>
> /* Save skb pointer */
> - fep->tx_skbuff[fep->skb_cur] = skb;
> -
> - ndev->stats.tx_bytes += skb->len;
> - fep->skb_cur = (fep->skb_cur+1) & TX_RING_MOD_MASK;
> + fep->tx_skbuff[index] = skb;
>
> /* Push the data cache so the CPM does not get stale memory
> * data.
> @@ -331,26 +326,22 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
> ebdp->cbd_esc = BD_ENET_TX_INT;
> }
> }
> - /* Trigger transmission start */
> - writel(0, fep->hwp + FEC_X_DES_ACTIVE);
> -
> /* If this was the last BD in the ring, start at the beginning again. */
> if (status & BD_ENET_TX_WRAP)
> bdp = fep->tx_bd_base;
> else
> bdp = fec_enet_get_nextdesc(bdp, fep->bufdesc_ex);
>
> - if (bdp == fep->dirty_tx) {
> - fep->tx_full = 1;
> + fep->cur_tx = bdp;
> +
> + if (fep->cur_tx == fep->dirty_tx)
> netif_stop_queue(ndev);
> - }
>
> - fep->cur_tx = bdp;
> + /* Trigger transmission start */
> + writel(0, fep->hwp + FEC_X_DES_ACTIVE);
>
> skb_tx_timestamp(skb);
>
> - spin_unlock_irqrestore(&fep->hw_lock, flags);
> -
> return NETDEV_TX_OK;
> }
>
> @@ -406,11 +397,8 @@ fec_restart(struct net_device *ndev, int duplex)
> writel((unsigned long)fep->bd_dma + sizeof(struct bufdesc)
> * RX_RING_SIZE, fep->hwp + FEC_X_DES_START);
>
> - fep->dirty_tx = fep->cur_tx = fep->tx_bd_base;
> fep->cur_rx = fep->rx_bd_base;
>
> - /* Reset SKB transmit buffers. */
> - fep->skb_cur = fep->skb_dirty = 0;
> for (i = 0; i <= TX_RING_MOD_MASK; i++) {
> if (fep->tx_skbuff[i]) {
> dev_kfree_skb_any(fep->tx_skbuff[i]);
> @@ -573,20 +561,35 @@ fec_enet_tx(struct net_device *ndev)
> struct bufdesc *bdp;
> unsigned short status;
> struct sk_buff *skb;
> + int index = 0;
>
> fep = netdev_priv(ndev);
> - spin_lock(&fep->hw_lock);
> bdp = fep->dirty_tx;
>
> + /* get next bdp of dirty_tx */
> + if (bdp->cbd_sc & BD_ENET_TX_WRAP)
> + bdp = fep->tx_bd_base;
> + else
> + bdp = fec_enet_get_nextdesc(bdp, fep->bufdesc_ex);
> +
> while (((status = bdp->cbd_sc) & BD_ENET_TX_READY) == 0) {
> - if (bdp == fep->cur_tx && fep->tx_full == 0)
> +
> + /* current queue is empty */
> + if (bdp == fep->cur_tx)
> break;
>
> + if (fep->bufdesc_ex)
> + index = (struct bufdesc_ex *)bdp -
> + (struct bufdesc_ex *)fep->tx_bd_base;
> + else
> + index = bdp - fep->tx_bd_base;
> +
> dma_unmap_single(&fep->pdev->dev, bdp->cbd_bufaddr,
> FEC_ENET_TX_FRSIZE, DMA_TO_DEVICE);
> bdp->cbd_bufaddr = 0;
>
> - skb = fep->tx_skbuff[fep->skb_dirty];
> + skb = fep->tx_skbuff[index];
> +
> /* Check for errors. */
> if (status & (BD_ENET_TX_HB | BD_ENET_TX_LC |
> BD_ENET_TX_RL | BD_ENET_TX_UN |
> @@ -631,8 +634,9 @@ fec_enet_tx(struct net_device *ndev)
>
> /* Free the sk buffer associated with this last transmit */
> dev_kfree_skb_any(skb);
> - fep->tx_skbuff[fep->skb_dirty] = NULL;
> - fep->skb_dirty = (fep->skb_dirty + 1) & TX_RING_MOD_MASK;
> + fep->tx_skbuff[index] = NULL;
> +
> + fep->dirty_tx = bdp;
>
> /* Update pointer to next buffer descriptor to be transmitted */
> if (status & BD_ENET_TX_WRAP)
> @@ -642,14 +646,12 @@ fec_enet_tx(struct net_device *ndev)
>
> /* Since we have freed up a buffer, the ring is no longer full
> */
> - if (fep->tx_full) {
> - fep->tx_full = 0;
> + if (fep->dirty_tx != fep->cur_tx) {
> if (netif_queue_stopped(ndev))
> netif_wake_queue(ndev);
> }
> }
> - fep->dirty_tx = bdp;
> - spin_unlock(&fep->hw_lock);
> + return;
> }
>
>
> @@ -816,7 +818,7 @@ fec_enet_interrupt(int irq, void *dev_id)
> int_events = readl(fep->hwp + FEC_IEVENT);
> writel(int_events, fep->hwp + FEC_IEVENT);
>
> - if (int_events & FEC_ENET_RXF) {
> + if (int_events & (FEC_ENET_RXF | FEC_ENET_TXF)) {
> ret = IRQ_HANDLED;
>
> /* Disable the RX interrupt */
> @@ -827,15 +829,6 @@ fec_enet_interrupt(int irq, void *dev_id)
> }
> }
>
> - /* Transmit OK, or non-fatal error. Update the buffer
> - * descriptors. FEC handles all errors, we just discover
> - * them as part of the transmit process.
> - */
> - if (int_events & FEC_ENET_TXF) {
> - ret = IRQ_HANDLED;
> - fec_enet_tx(ndev);
> - }
> -
> if (int_events & FEC_ENET_MII) {
> ret = IRQ_HANDLED;
> complete(&fep->mdio_done);
> @@ -851,6 +844,8 @@ static int fec_enet_rx_napi(struct napi_struct *napi, int budget)
> int pkts = fec_enet_rx(ndev, budget);
> struct fec_enet_private *fep = netdev_priv(ndev);
>
> + fec_enet_tx(ndev);
> +
> if (pkts < budget) {
> napi_complete(napi);
> writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
> @@ -1646,6 +1641,7 @@ static int fec_enet_init(struct net_device *ndev)
>
> /* ...and the same for transmit */
> bdp = fep->tx_bd_base;
> + fep->cur_tx = bdp;
> for (i = 0; i < TX_RING_SIZE; i++) {
>
> /* Initialize the BD for every fragment in the page. */
> @@ -1657,6 +1653,7 @@ static int fec_enet_init(struct net_device *ndev)
> /* Set the last buffer to wrap */
> bdp = fec_enet_get_prevdesc(bdp, fep->bufdesc_ex);
> bdp->cbd_sc |= BD_SC_WRAP;
> + fep->dirty_tx = bdp;
>
> fec_restart(ndev, 0);
>
> diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
> index 01579b8..c0f63be 100644
> --- a/drivers/net/ethernet/freescale/fec.h
> +++ b/drivers/net/ethernet/freescale/fec.h
> @@ -214,8 +214,6 @@ struct fec_enet_private {
> unsigned char *tx_bounce[TX_RING_SIZE];
> struct sk_buff *tx_skbuff[TX_RING_SIZE];
> struct sk_buff *rx_skbuff[RX_RING_SIZE];
> - ushort skb_cur;
> - ushort skb_dirty;
>
> /* CPM dual port RAM relative addresses */
> dma_addr_t bd_dma;
> @@ -227,7 +225,6 @@ struct fec_enet_private {
> /* The ring entries to be free()ed */
> struct bufdesc *dirty_tx;
>
> - uint tx_full;
> /* hold while accessing the HW like ringbuffer for tx/rx but not MAC */
> spinlock_t hw_lock;
>
> --
> 1.7.1
>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/1 net-next] net: fec: put tx to napi poll function to fix dead lock
2013-03-20 11:48 ` Shawn Guo
@ 2013-03-20 13:33 ` Frank Li
-1 siblings, 0 replies; 14+ messages in thread
From: Frank Li @ 2013-03-20 13:33 UTC (permalink / raw)
To: Shawn Guo; +Cc: Frank Li, B38611, davem, linux-arm-kernel, netdev, s.hauer
2013/3/20 Shawn Guo <shawn.guo@linaro.org>:
> Frank,
>
> I'm running v3.9-rc3 image with NFS on imx6q, and seeing system fail
> to resume back. The resume seems being stopped by fec driver. If you
> wait long enough, you might see the repeated "eth0: tx queue full!."
> error message.
>
> Reverting the patch gets resume back to work. Can you please
> investigate?
Okay. I will check tomorrow
>
> Shawn
>
> On Mon, Mar 04, 2013 at 11:34:25AM +0800, Frank Li wrote:
>> up stack ndo_start_xmit already hold lock.
>> fec_enet_start_xmit needn't spin lock.
>> stat_xmit just update fep->cur_tx
>> fec_enet_tx just update fep->dirty_tx
>>
>> Reserve a empty bdb to check full or empty
>> cur_tx == dirty_tx means full
>> cur_tx == dirty_tx +1 means empty
>>
>> So needn't is_full variable.
>>
>> Fix spin lock deadlock
>> =================================
>> [ INFO: inconsistent lock state ]
>> 3.8.0-rc5+ #107 Not tainted
>> ---------------------------------
>> inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage.
>> ptp4l/615 [HC1[1]:SC0[0]:HE0:SE1] takes:
>> (&(&list->lock)->rlock#3){?.-...}, at: [<8042c3c4>] skb_queue_tail+0x20/0x50
>> {HARDIRQ-ON-W} state was registered at:
>> [<80067250>] mark_lock+0x154/0x4e8
>> [<800676f4>] mark_irqflags+0x110/0x1a4
>> [<80069208>] __lock_acquire+0x494/0x9c0
>> [<80069ce8>] lock_acquire+0x90/0xa4
>> [<80527ad0>] _raw_spin_lock_bh+0x44/0x54
>> [<804877e0>] first_packet_length+0x38/0x1f0
>> [<804879e4>] udp_poll+0x4c/0x5c
>> [<804231f8>] sock_poll+0x24/0x28
>> [<800d27f0>] do_poll.isra.10+0x120/0x254
>> [<800d36e4>] do_sys_poll+0x15c/0x1e8
>> [<800d3828>] sys_poll+0x60/0xc8
>> [<8000e780>] ret_fast_syscall+0x0/0x3c
>>
>> *** DEADLOCK ***
>>
>> 1 lock held by ptp4l/615:
>> #0: (&(&fep->hw_lock)->rlock){-.-...}, at: [<80355f9c>] fec_enet_tx+0x24/0x268
>> stack backtrace:
>> Backtrace:
>> [<800121e0>] (dump_backtrace+0x0/0x10c) from [<80516210>] (dump_stack+0x18/0x1c)
>> r6:8063b1fc r5:bf38b2f8 r4:bf38b000 r3:bf38b000
>> [<805161f8>] (dump_stack+0x0/0x1c) from [<805189d0>] (print_usage_bug.part.34+0x164/0x1a4)
>> [<8051886c>] (print_usage_bug.part.34+0x0/0x1a4) from [<80518a88>] (print_usage_bug+0x78/0x88)
>> r8:80065664 r7:bf38b2f8 r6:00000002 r5:00000000 r4:bf38b000
>> [<80518a10>] (print_usage_bug+0x0/0x88) from [<80518b58>] (mark_lock_irq+0xc0/0x270)
>> r7:bf38b000 r6:00000002 r5:bf38b2f8 r4:00000000
>> [<80518a98>] (mark_lock_irq+0x0/0x270) from [<80067270>] (mark_lock+0x174/0x4e8)
>> [<800670fc>] (mark_lock+0x0/0x4e8) from [<80067744>] (mark_irqflags+0x160/0x1a4)
>> [<800675e4>] (mark_irqflags+0x0/0x1a4) from [<80069208>] (__lock_acquire+0x494/0x9c0)
>> r5:00000002 r4:bf38b2f8
>> [<80068d74>] (__lock_acquire+0x0/0x9c0) from [<80069ce8>] (lock_acquire+0x90/0xa4)
>> [<80069c58>] (lock_acquire+0x0/0xa4) from [<805278d8>] (_raw_spin_lock_irqsave+0x4c/0x60)
>> [<8052788c>] (_raw_spin_lock_irqsave+0x0/0x60) from [<8042c3c4>] (skb_queue_tail+0x20/0x50)
>> r6:bfbb2180 r5:bf1d0190 r4:bf1d0184
>> [<8042c3a4>] (skb_queue_tail+0x0/0x50) from [<8042c4cc>] (sock_queue_err_skb+0xd8/0x188)
>> r6:00000056 r5:bfbb2180 r4:bf1d0000 r3:00000000
>> [<8042c3f4>] (sock_queue_err_skb+0x0/0x188) from [<8042d15c>] (skb_tstamp_tx+0x70/0xa0)
>> r6:bf0dddb0 r5:bf1d0000 r4:bfbb2180 r3:00000004
>> [<8042d0ec>] (skb_tstamp_tx+0x0/0xa0) from [<803561d0>] (fec_enet_tx+0x258/0x268)
>> r6:c089d260 r5:00001c00 r4:bfbd0000
>> [<80355f78>] (fec_enet_tx+0x0/0x268) from [<803562cc>] (fec_enet_interrupt+0xec/0xf8)
>> [<803561e0>] (fec_enet_interrupt+0x0/0xf8) from [<8007d5b0>] (handle_irq_event_percpu+0x54/0x1a0)
>> [<8007d55c>] (handle_irq_event_percpu+0x0/0x1a0) from [<8007d740>] (handle_irq_event+0x44/0x64)
>> [<8007d6fc>] (handle_irq_event+0x0/0x64) from [<80080690>] (handle_fasteoi_irq+0xc4/0x15c)
>> r6:bf0dc000 r5:bf811290 r4:bf811240 r3:00000000
>> [<800805cc>] (handle_fasteoi_irq+0x0/0x15c) from [<8007ceec>] (generic_handle_irq+0x28/0x38)
>> r5:807130c8 r4:00000096
>> [<8007cec4>] (generic_handle_irq+0x0/0x38) from [<8000f16c>] (handle_IRQ+0x54/0xb4)
>> r4:8071d280 r3:00000180
>> [<8000f118>] (handle_IRQ+0x0/0xb4) from [<80008544>] (gic_handle_irq+0x30/0x64)
>> r8:8000e924 r7:f4000100 r6:bf0ddef8 r5:8071c974 r4:f400010c
>> r3:00000000
>> [<80008514>] (gic_handle_irq+0x0/0x64) from [<8000e2e4>] (__irq_svc+0x44/0x5c)
>> Exception stack(0xbf0ddef8 to 0xbf0ddf40)
>>
>> Signed-off-by: Frank Li <Frank.Li@freescale.com>
>> ---
>> Change from v3 to v2
>> * remove return value of fec_enet_tx
>> Change from v1 to v2
>> * ignore TX package count in poll function
>>
>> drivers/net/ethernet/freescale/fec.c | 85 ++++++++++++++++-----------------
>> drivers/net/ethernet/freescale/fec.h | 3 -
>> 2 files changed, 41 insertions(+), 47 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
>> index 0fe68c4..bca5a24 100644
>> --- a/drivers/net/ethernet/freescale/fec.c
>> +++ b/drivers/net/ethernet/freescale/fec.c
>> @@ -246,14 +246,13 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>> struct bufdesc *bdp;
>> void *bufaddr;
>> unsigned short status;
>> - unsigned long flags;
>> + unsigned int index;
>>
>> if (!fep->link) {
>> /* Link is down or autonegotiation is in progress. */
>> return NETDEV_TX_BUSY;
>> }
>>
>> - spin_lock_irqsave(&fep->hw_lock, flags);
>> /* Fill in a Tx ring entry */
>> bdp = fep->cur_tx;
>>
>> @@ -264,7 +263,6 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>> * This should not happen, since ndev->tbusy should be set.
>> */
>> printk("%s: tx queue full!.\n", ndev->name);
>> - spin_unlock_irqrestore(&fep->hw_lock, flags);
>> return NETDEV_TX_BUSY;
>> }
>>
>> @@ -280,13 +278,13 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>> * 4-byte boundaries. Use bounce buffers to copy data
>> * and get it aligned. Ugh.
>> */
>> + if (fep->bufdesc_ex)
>> + index = (struct bufdesc_ex *)bdp -
>> + (struct bufdesc_ex *)fep->tx_bd_base;
>> + else
>> + index = bdp - fep->tx_bd_base;
>> +
>> if (((unsigned long) bufaddr) & FEC_ALIGNMENT) {
>> - unsigned int index;
>> - if (fep->bufdesc_ex)
>> - index = (struct bufdesc_ex *)bdp -
>> - (struct bufdesc_ex *)fep->tx_bd_base;
>> - else
>> - index = bdp - fep->tx_bd_base;
>> memcpy(fep->tx_bounce[index], skb->data, skb->len);
>> bufaddr = fep->tx_bounce[index];
>> }
>> @@ -300,10 +298,7 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>> swap_buffer(bufaddr, skb->len);
>>
>> /* Save skb pointer */
>> - fep->tx_skbuff[fep->skb_cur] = skb;
>> -
>> - ndev->stats.tx_bytes += skb->len;
>> - fep->skb_cur = (fep->skb_cur+1) & TX_RING_MOD_MASK;
>> + fep->tx_skbuff[index] = skb;
>>
>> /* Push the data cache so the CPM does not get stale memory
>> * data.
>> @@ -331,26 +326,22 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>> ebdp->cbd_esc = BD_ENET_TX_INT;
>> }
>> }
>> - /* Trigger transmission start */
>> - writel(0, fep->hwp + FEC_X_DES_ACTIVE);
>> -
>> /* If this was the last BD in the ring, start at the beginning again. */
>> if (status & BD_ENET_TX_WRAP)
>> bdp = fep->tx_bd_base;
>> else
>> bdp = fec_enet_get_nextdesc(bdp, fep->bufdesc_ex);
>>
>> - if (bdp == fep->dirty_tx) {
>> - fep->tx_full = 1;
>> + fep->cur_tx = bdp;
>> +
>> + if (fep->cur_tx == fep->dirty_tx)
>> netif_stop_queue(ndev);
>> - }
>>
>> - fep->cur_tx = bdp;
>> + /* Trigger transmission start */
>> + writel(0, fep->hwp + FEC_X_DES_ACTIVE);
>>
>> skb_tx_timestamp(skb);
>>
>> - spin_unlock_irqrestore(&fep->hw_lock, flags);
>> -
>> return NETDEV_TX_OK;
>> }
>>
>> @@ -406,11 +397,8 @@ fec_restart(struct net_device *ndev, int duplex)
>> writel((unsigned long)fep->bd_dma + sizeof(struct bufdesc)
>> * RX_RING_SIZE, fep->hwp + FEC_X_DES_START);
>>
>> - fep->dirty_tx = fep->cur_tx = fep->tx_bd_base;
>> fep->cur_rx = fep->rx_bd_base;
>>
>> - /* Reset SKB transmit buffers. */
>> - fep->skb_cur = fep->skb_dirty = 0;
>> for (i = 0; i <= TX_RING_MOD_MASK; i++) {
>> if (fep->tx_skbuff[i]) {
>> dev_kfree_skb_any(fep->tx_skbuff[i]);
>> @@ -573,20 +561,35 @@ fec_enet_tx(struct net_device *ndev)
>> struct bufdesc *bdp;
>> unsigned short status;
>> struct sk_buff *skb;
>> + int index = 0;
>>
>> fep = netdev_priv(ndev);
>> - spin_lock(&fep->hw_lock);
>> bdp = fep->dirty_tx;
>>
>> + /* get next bdp of dirty_tx */
>> + if (bdp->cbd_sc & BD_ENET_TX_WRAP)
>> + bdp = fep->tx_bd_base;
>> + else
>> + bdp = fec_enet_get_nextdesc(bdp, fep->bufdesc_ex);
>> +
>> while (((status = bdp->cbd_sc) & BD_ENET_TX_READY) == 0) {
>> - if (bdp == fep->cur_tx && fep->tx_full == 0)
>> +
>> + /* current queue is empty */
>> + if (bdp == fep->cur_tx)
>> break;
>>
>> + if (fep->bufdesc_ex)
>> + index = (struct bufdesc_ex *)bdp -
>> + (struct bufdesc_ex *)fep->tx_bd_base;
>> + else
>> + index = bdp - fep->tx_bd_base;
>> +
>> dma_unmap_single(&fep->pdev->dev, bdp->cbd_bufaddr,
>> FEC_ENET_TX_FRSIZE, DMA_TO_DEVICE);
>> bdp->cbd_bufaddr = 0;
>>
>> - skb = fep->tx_skbuff[fep->skb_dirty];
>> + skb = fep->tx_skbuff[index];
>> +
>> /* Check for errors. */
>> if (status & (BD_ENET_TX_HB | BD_ENET_TX_LC |
>> BD_ENET_TX_RL | BD_ENET_TX_UN |
>> @@ -631,8 +634,9 @@ fec_enet_tx(struct net_device *ndev)
>>
>> /* Free the sk buffer associated with this last transmit */
>> dev_kfree_skb_any(skb);
>> - fep->tx_skbuff[fep->skb_dirty] = NULL;
>> - fep->skb_dirty = (fep->skb_dirty + 1) & TX_RING_MOD_MASK;
>> + fep->tx_skbuff[index] = NULL;
>> +
>> + fep->dirty_tx = bdp;
>>
>> /* Update pointer to next buffer descriptor to be transmitted */
>> if (status & BD_ENET_TX_WRAP)
>> @@ -642,14 +646,12 @@ fec_enet_tx(struct net_device *ndev)
>>
>> /* Since we have freed up a buffer, the ring is no longer full
>> */
>> - if (fep->tx_full) {
>> - fep->tx_full = 0;
>> + if (fep->dirty_tx != fep->cur_tx) {
>> if (netif_queue_stopped(ndev))
>> netif_wake_queue(ndev);
>> }
>> }
>> - fep->dirty_tx = bdp;
>> - spin_unlock(&fep->hw_lock);
>> + return;
>> }
>>
>>
>> @@ -816,7 +818,7 @@ fec_enet_interrupt(int irq, void *dev_id)
>> int_events = readl(fep->hwp + FEC_IEVENT);
>> writel(int_events, fep->hwp + FEC_IEVENT);
>>
>> - if (int_events & FEC_ENET_RXF) {
>> + if (int_events & (FEC_ENET_RXF | FEC_ENET_TXF)) {
>> ret = IRQ_HANDLED;
>>
>> /* Disable the RX interrupt */
>> @@ -827,15 +829,6 @@ fec_enet_interrupt(int irq, void *dev_id)
>> }
>> }
>>
>> - /* Transmit OK, or non-fatal error. Update the buffer
>> - * descriptors. FEC handles all errors, we just discover
>> - * them as part of the transmit process.
>> - */
>> - if (int_events & FEC_ENET_TXF) {
>> - ret = IRQ_HANDLED;
>> - fec_enet_tx(ndev);
>> - }
>> -
>> if (int_events & FEC_ENET_MII) {
>> ret = IRQ_HANDLED;
>> complete(&fep->mdio_done);
>> @@ -851,6 +844,8 @@ static int fec_enet_rx_napi(struct napi_struct *napi, int budget)
>> int pkts = fec_enet_rx(ndev, budget);
>> struct fec_enet_private *fep = netdev_priv(ndev);
>>
>> + fec_enet_tx(ndev);
>> +
>> if (pkts < budget) {
>> napi_complete(napi);
>> writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
>> @@ -1646,6 +1641,7 @@ static int fec_enet_init(struct net_device *ndev)
>>
>> /* ...and the same for transmit */
>> bdp = fep->tx_bd_base;
>> + fep->cur_tx = bdp;
>> for (i = 0; i < TX_RING_SIZE; i++) {
>>
>> /* Initialize the BD for every fragment in the page. */
>> @@ -1657,6 +1653,7 @@ static int fec_enet_init(struct net_device *ndev)
>> /* Set the last buffer to wrap */
>> bdp = fec_enet_get_prevdesc(bdp, fep->bufdesc_ex);
>> bdp->cbd_sc |= BD_SC_WRAP;
>> + fep->dirty_tx = bdp;
>>
>> fec_restart(ndev, 0);
>>
>> diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
>> index 01579b8..c0f63be 100644
>> --- a/drivers/net/ethernet/freescale/fec.h
>> +++ b/drivers/net/ethernet/freescale/fec.h
>> @@ -214,8 +214,6 @@ struct fec_enet_private {
>> unsigned char *tx_bounce[TX_RING_SIZE];
>> struct sk_buff *tx_skbuff[TX_RING_SIZE];
>> struct sk_buff *rx_skbuff[RX_RING_SIZE];
>> - ushort skb_cur;
>> - ushort skb_dirty;
>>
>> /* CPM dual port RAM relative addresses */
>> dma_addr_t bd_dma;
>> @@ -227,7 +225,6 @@ struct fec_enet_private {
>> /* The ring entries to be free()ed */
>> struct bufdesc *dirty_tx;
>>
>> - uint tx_full;
>> /* hold while accessing the HW like ringbuffer for tx/rx but not MAC */
>> spinlock_t hw_lock;
>>
>> --
>> 1.7.1
>>
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 1/1 net-next] net: fec: put tx to napi poll function to fix dead lock
@ 2013-03-20 13:33 ` Frank Li
0 siblings, 0 replies; 14+ messages in thread
From: Frank Li @ 2013-03-20 13:33 UTC (permalink / raw)
To: linux-arm-kernel
2013/3/20 Shawn Guo <shawn.guo@linaro.org>:
> Frank,
>
> I'm running v3.9-rc3 image with NFS on imx6q, and seeing system fail
> to resume back. The resume seems being stopped by fec driver. If you
> wait long enough, you might see the repeated "eth0: tx queue full!."
> error message.
>
> Reverting the patch gets resume back to work. Can you please
> investigate?
Okay. I will check tomorrow
>
> Shawn
>
> On Mon, Mar 04, 2013 at 11:34:25AM +0800, Frank Li wrote:
>> up stack ndo_start_xmit already hold lock.
>> fec_enet_start_xmit needn't spin lock.
>> stat_xmit just update fep->cur_tx
>> fec_enet_tx just update fep->dirty_tx
>>
>> Reserve a empty bdb to check full or empty
>> cur_tx == dirty_tx means full
>> cur_tx == dirty_tx +1 means empty
>>
>> So needn't is_full variable.
>>
>> Fix spin lock deadlock
>> =================================
>> [ INFO: inconsistent lock state ]
>> 3.8.0-rc5+ #107 Not tainted
>> ---------------------------------
>> inconsistent {HARDIRQ-ON-W} -> {IN-HARDIRQ-W} usage.
>> ptp4l/615 [HC1[1]:SC0[0]:HE0:SE1] takes:
>> (&(&list->lock)->rlock#3){?.-...}, at: [<8042c3c4>] skb_queue_tail+0x20/0x50
>> {HARDIRQ-ON-W} state was registered at:
>> [<80067250>] mark_lock+0x154/0x4e8
>> [<800676f4>] mark_irqflags+0x110/0x1a4
>> [<80069208>] __lock_acquire+0x494/0x9c0
>> [<80069ce8>] lock_acquire+0x90/0xa4
>> [<80527ad0>] _raw_spin_lock_bh+0x44/0x54
>> [<804877e0>] first_packet_length+0x38/0x1f0
>> [<804879e4>] udp_poll+0x4c/0x5c
>> [<804231f8>] sock_poll+0x24/0x28
>> [<800d27f0>] do_poll.isra.10+0x120/0x254
>> [<800d36e4>] do_sys_poll+0x15c/0x1e8
>> [<800d3828>] sys_poll+0x60/0xc8
>> [<8000e780>] ret_fast_syscall+0x0/0x3c
>>
>> *** DEADLOCK ***
>>
>> 1 lock held by ptp4l/615:
>> #0: (&(&fep->hw_lock)->rlock){-.-...}, at: [<80355f9c>] fec_enet_tx+0x24/0x268
>> stack backtrace:
>> Backtrace:
>> [<800121e0>] (dump_backtrace+0x0/0x10c) from [<80516210>] (dump_stack+0x18/0x1c)
>> r6:8063b1fc r5:bf38b2f8 r4:bf38b000 r3:bf38b000
>> [<805161f8>] (dump_stack+0x0/0x1c) from [<805189d0>] (print_usage_bug.part.34+0x164/0x1a4)
>> [<8051886c>] (print_usage_bug.part.34+0x0/0x1a4) from [<80518a88>] (print_usage_bug+0x78/0x88)
>> r8:80065664 r7:bf38b2f8 r6:00000002 r5:00000000 r4:bf38b000
>> [<80518a10>] (print_usage_bug+0x0/0x88) from [<80518b58>] (mark_lock_irq+0xc0/0x270)
>> r7:bf38b000 r6:00000002 r5:bf38b2f8 r4:00000000
>> [<80518a98>] (mark_lock_irq+0x0/0x270) from [<80067270>] (mark_lock+0x174/0x4e8)
>> [<800670fc>] (mark_lock+0x0/0x4e8) from [<80067744>] (mark_irqflags+0x160/0x1a4)
>> [<800675e4>] (mark_irqflags+0x0/0x1a4) from [<80069208>] (__lock_acquire+0x494/0x9c0)
>> r5:00000002 r4:bf38b2f8
>> [<80068d74>] (__lock_acquire+0x0/0x9c0) from [<80069ce8>] (lock_acquire+0x90/0xa4)
>> [<80069c58>] (lock_acquire+0x0/0xa4) from [<805278d8>] (_raw_spin_lock_irqsave+0x4c/0x60)
>> [<8052788c>] (_raw_spin_lock_irqsave+0x0/0x60) from [<8042c3c4>] (skb_queue_tail+0x20/0x50)
>> r6:bfbb2180 r5:bf1d0190 r4:bf1d0184
>> [<8042c3a4>] (skb_queue_tail+0x0/0x50) from [<8042c4cc>] (sock_queue_err_skb+0xd8/0x188)
>> r6:00000056 r5:bfbb2180 r4:bf1d0000 r3:00000000
>> [<8042c3f4>] (sock_queue_err_skb+0x0/0x188) from [<8042d15c>] (skb_tstamp_tx+0x70/0xa0)
>> r6:bf0dddb0 r5:bf1d0000 r4:bfbb2180 r3:00000004
>> [<8042d0ec>] (skb_tstamp_tx+0x0/0xa0) from [<803561d0>] (fec_enet_tx+0x258/0x268)
>> r6:c089d260 r5:00001c00 r4:bfbd0000
>> [<80355f78>] (fec_enet_tx+0x0/0x268) from [<803562cc>] (fec_enet_interrupt+0xec/0xf8)
>> [<803561e0>] (fec_enet_interrupt+0x0/0xf8) from [<8007d5b0>] (handle_irq_event_percpu+0x54/0x1a0)
>> [<8007d55c>] (handle_irq_event_percpu+0x0/0x1a0) from [<8007d740>] (handle_irq_event+0x44/0x64)
>> [<8007d6fc>] (handle_irq_event+0x0/0x64) from [<80080690>] (handle_fasteoi_irq+0xc4/0x15c)
>> r6:bf0dc000 r5:bf811290 r4:bf811240 r3:00000000
>> [<800805cc>] (handle_fasteoi_irq+0x0/0x15c) from [<8007ceec>] (generic_handle_irq+0x28/0x38)
>> r5:807130c8 r4:00000096
>> [<8007cec4>] (generic_handle_irq+0x0/0x38) from [<8000f16c>] (handle_IRQ+0x54/0xb4)
>> r4:8071d280 r3:00000180
>> [<8000f118>] (handle_IRQ+0x0/0xb4) from [<80008544>] (gic_handle_irq+0x30/0x64)
>> r8:8000e924 r7:f4000100 r6:bf0ddef8 r5:8071c974 r4:f400010c
>> r3:00000000
>> [<80008514>] (gic_handle_irq+0x0/0x64) from [<8000e2e4>] (__irq_svc+0x44/0x5c)
>> Exception stack(0xbf0ddef8 to 0xbf0ddf40)
>>
>> Signed-off-by: Frank Li <Frank.Li@freescale.com>
>> ---
>> Change from v3 to v2
>> * remove return value of fec_enet_tx
>> Change from v1 to v2
>> * ignore TX package count in poll function
>>
>> drivers/net/ethernet/freescale/fec.c | 85 ++++++++++++++++-----------------
>> drivers/net/ethernet/freescale/fec.h | 3 -
>> 2 files changed, 41 insertions(+), 47 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/freescale/fec.c b/drivers/net/ethernet/freescale/fec.c
>> index 0fe68c4..bca5a24 100644
>> --- a/drivers/net/ethernet/freescale/fec.c
>> +++ b/drivers/net/ethernet/freescale/fec.c
>> @@ -246,14 +246,13 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>> struct bufdesc *bdp;
>> void *bufaddr;
>> unsigned short status;
>> - unsigned long flags;
>> + unsigned int index;
>>
>> if (!fep->link) {
>> /* Link is down or autonegotiation is in progress. */
>> return NETDEV_TX_BUSY;
>> }
>>
>> - spin_lock_irqsave(&fep->hw_lock, flags);
>> /* Fill in a Tx ring entry */
>> bdp = fep->cur_tx;
>>
>> @@ -264,7 +263,6 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>> * This should not happen, since ndev->tbusy should be set.
>> */
>> printk("%s: tx queue full!.\n", ndev->name);
>> - spin_unlock_irqrestore(&fep->hw_lock, flags);
>> return NETDEV_TX_BUSY;
>> }
>>
>> @@ -280,13 +278,13 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>> * 4-byte boundaries. Use bounce buffers to copy data
>> * and get it aligned. Ugh.
>> */
>> + if (fep->bufdesc_ex)
>> + index = (struct bufdesc_ex *)bdp -
>> + (struct bufdesc_ex *)fep->tx_bd_base;
>> + else
>> + index = bdp - fep->tx_bd_base;
>> +
>> if (((unsigned long) bufaddr) & FEC_ALIGNMENT) {
>> - unsigned int index;
>> - if (fep->bufdesc_ex)
>> - index = (struct bufdesc_ex *)bdp -
>> - (struct bufdesc_ex *)fep->tx_bd_base;
>> - else
>> - index = bdp - fep->tx_bd_base;
>> memcpy(fep->tx_bounce[index], skb->data, skb->len);
>> bufaddr = fep->tx_bounce[index];
>> }
>> @@ -300,10 +298,7 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>> swap_buffer(bufaddr, skb->len);
>>
>> /* Save skb pointer */
>> - fep->tx_skbuff[fep->skb_cur] = skb;
>> -
>> - ndev->stats.tx_bytes += skb->len;
>> - fep->skb_cur = (fep->skb_cur+1) & TX_RING_MOD_MASK;
>> + fep->tx_skbuff[index] = skb;
>>
>> /* Push the data cache so the CPM does not get stale memory
>> * data.
>> @@ -331,26 +326,22 @@ fec_enet_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>> ebdp->cbd_esc = BD_ENET_TX_INT;
>> }
>> }
>> - /* Trigger transmission start */
>> - writel(0, fep->hwp + FEC_X_DES_ACTIVE);
>> -
>> /* If this was the last BD in the ring, start at the beginning again. */
>> if (status & BD_ENET_TX_WRAP)
>> bdp = fep->tx_bd_base;
>> else
>> bdp = fec_enet_get_nextdesc(bdp, fep->bufdesc_ex);
>>
>> - if (bdp == fep->dirty_tx) {
>> - fep->tx_full = 1;
>> + fep->cur_tx = bdp;
>> +
>> + if (fep->cur_tx == fep->dirty_tx)
>> netif_stop_queue(ndev);
>> - }
>>
>> - fep->cur_tx = bdp;
>> + /* Trigger transmission start */
>> + writel(0, fep->hwp + FEC_X_DES_ACTIVE);
>>
>> skb_tx_timestamp(skb);
>>
>> - spin_unlock_irqrestore(&fep->hw_lock, flags);
>> -
>> return NETDEV_TX_OK;
>> }
>>
>> @@ -406,11 +397,8 @@ fec_restart(struct net_device *ndev, int duplex)
>> writel((unsigned long)fep->bd_dma + sizeof(struct bufdesc)
>> * RX_RING_SIZE, fep->hwp + FEC_X_DES_START);
>>
>> - fep->dirty_tx = fep->cur_tx = fep->tx_bd_base;
>> fep->cur_rx = fep->rx_bd_base;
>>
>> - /* Reset SKB transmit buffers. */
>> - fep->skb_cur = fep->skb_dirty = 0;
>> for (i = 0; i <= TX_RING_MOD_MASK; i++) {
>> if (fep->tx_skbuff[i]) {
>> dev_kfree_skb_any(fep->tx_skbuff[i]);
>> @@ -573,20 +561,35 @@ fec_enet_tx(struct net_device *ndev)
>> struct bufdesc *bdp;
>> unsigned short status;
>> struct sk_buff *skb;
>> + int index = 0;
>>
>> fep = netdev_priv(ndev);
>> - spin_lock(&fep->hw_lock);
>> bdp = fep->dirty_tx;
>>
>> + /* get next bdp of dirty_tx */
>> + if (bdp->cbd_sc & BD_ENET_TX_WRAP)
>> + bdp = fep->tx_bd_base;
>> + else
>> + bdp = fec_enet_get_nextdesc(bdp, fep->bufdesc_ex);
>> +
>> while (((status = bdp->cbd_sc) & BD_ENET_TX_READY) == 0) {
>> - if (bdp == fep->cur_tx && fep->tx_full == 0)
>> +
>> + /* current queue is empty */
>> + if (bdp == fep->cur_tx)
>> break;
>>
>> + if (fep->bufdesc_ex)
>> + index = (struct bufdesc_ex *)bdp -
>> + (struct bufdesc_ex *)fep->tx_bd_base;
>> + else
>> + index = bdp - fep->tx_bd_base;
>> +
>> dma_unmap_single(&fep->pdev->dev, bdp->cbd_bufaddr,
>> FEC_ENET_TX_FRSIZE, DMA_TO_DEVICE);
>> bdp->cbd_bufaddr = 0;
>>
>> - skb = fep->tx_skbuff[fep->skb_dirty];
>> + skb = fep->tx_skbuff[index];
>> +
>> /* Check for errors. */
>> if (status & (BD_ENET_TX_HB | BD_ENET_TX_LC |
>> BD_ENET_TX_RL | BD_ENET_TX_UN |
>> @@ -631,8 +634,9 @@ fec_enet_tx(struct net_device *ndev)
>>
>> /* Free the sk buffer associated with this last transmit */
>> dev_kfree_skb_any(skb);
>> - fep->tx_skbuff[fep->skb_dirty] = NULL;
>> - fep->skb_dirty = (fep->skb_dirty + 1) & TX_RING_MOD_MASK;
>> + fep->tx_skbuff[index] = NULL;
>> +
>> + fep->dirty_tx = bdp;
>>
>> /* Update pointer to next buffer descriptor to be transmitted */
>> if (status & BD_ENET_TX_WRAP)
>> @@ -642,14 +646,12 @@ fec_enet_tx(struct net_device *ndev)
>>
>> /* Since we have freed up a buffer, the ring is no longer full
>> */
>> - if (fep->tx_full) {
>> - fep->tx_full = 0;
>> + if (fep->dirty_tx != fep->cur_tx) {
>> if (netif_queue_stopped(ndev))
>> netif_wake_queue(ndev);
>> }
>> }
>> - fep->dirty_tx = bdp;
>> - spin_unlock(&fep->hw_lock);
>> + return;
>> }
>>
>>
>> @@ -816,7 +818,7 @@ fec_enet_interrupt(int irq, void *dev_id)
>> int_events = readl(fep->hwp + FEC_IEVENT);
>> writel(int_events, fep->hwp + FEC_IEVENT);
>>
>> - if (int_events & FEC_ENET_RXF) {
>> + if (int_events & (FEC_ENET_RXF | FEC_ENET_TXF)) {
>> ret = IRQ_HANDLED;
>>
>> /* Disable the RX interrupt */
>> @@ -827,15 +829,6 @@ fec_enet_interrupt(int irq, void *dev_id)
>> }
>> }
>>
>> - /* Transmit OK, or non-fatal error. Update the buffer
>> - * descriptors. FEC handles all errors, we just discover
>> - * them as part of the transmit process.
>> - */
>> - if (int_events & FEC_ENET_TXF) {
>> - ret = IRQ_HANDLED;
>> - fec_enet_tx(ndev);
>> - }
>> -
>> if (int_events & FEC_ENET_MII) {
>> ret = IRQ_HANDLED;
>> complete(&fep->mdio_done);
>> @@ -851,6 +844,8 @@ static int fec_enet_rx_napi(struct napi_struct *napi, int budget)
>> int pkts = fec_enet_rx(ndev, budget);
>> struct fec_enet_private *fep = netdev_priv(ndev);
>>
>> + fec_enet_tx(ndev);
>> +
>> if (pkts < budget) {
>> napi_complete(napi);
>> writel(FEC_DEFAULT_IMASK, fep->hwp + FEC_IMASK);
>> @@ -1646,6 +1641,7 @@ static int fec_enet_init(struct net_device *ndev)
>>
>> /* ...and the same for transmit */
>> bdp = fep->tx_bd_base;
>> + fep->cur_tx = bdp;
>> for (i = 0; i < TX_RING_SIZE; i++) {
>>
>> /* Initialize the BD for every fragment in the page. */
>> @@ -1657,6 +1653,7 @@ static int fec_enet_init(struct net_device *ndev)
>> /* Set the last buffer to wrap */
>> bdp = fec_enet_get_prevdesc(bdp, fep->bufdesc_ex);
>> bdp->cbd_sc |= BD_SC_WRAP;
>> + fep->dirty_tx = bdp;
>>
>> fec_restart(ndev, 0);
>>
>> diff --git a/drivers/net/ethernet/freescale/fec.h b/drivers/net/ethernet/freescale/fec.h
>> index 01579b8..c0f63be 100644
>> --- a/drivers/net/ethernet/freescale/fec.h
>> +++ b/drivers/net/ethernet/freescale/fec.h
>> @@ -214,8 +214,6 @@ struct fec_enet_private {
>> unsigned char *tx_bounce[TX_RING_SIZE];
>> struct sk_buff *tx_skbuff[TX_RING_SIZE];
>> struct sk_buff *rx_skbuff[RX_RING_SIZE];
>> - ushort skb_cur;
>> - ushort skb_dirty;
>>
>> /* CPM dual port RAM relative addresses */
>> dma_addr_t bd_dma;
>> @@ -227,7 +225,6 @@ struct fec_enet_private {
>> /* The ring entries to be free()ed */
>> struct bufdesc *dirty_tx;
>>
>> - uint tx_full;
>> /* hold while accessing the HW like ringbuffer for tx/rx but not MAC */
>> spinlock_t hw_lock;
>>
>> --
>> 1.7.1
>>
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/1 net-next] net: fec: put tx to napi poll function to fix dead lock
2013-03-20 11:48 ` Shawn Guo
@ 2013-03-22 19:00 ` Fabio Estevam
-1 siblings, 0 replies; 14+ messages in thread
From: Fabio Estevam @ 2013-03-22 19:00 UTC (permalink / raw)
To: Shawn Guo
Cc: Frank Li, B38611, netdev, s.hauer, lznuaa, davem, linux-arm-kernel
On Wed, Mar 20, 2013 at 8:48 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> Frank,
>
> I'm running v3.9-rc3 image with NFS on imx6q, and seeing system fail
> to resume back. The resume seems being stopped by fec driver. If you
> wait long enough, you might see the repeated "eth0: tx queue full!."
> error message.
On mx28, only reverting this patch does not solve the suspend/resume issue.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 1/1 net-next] net: fec: put tx to napi poll function to fix dead lock
@ 2013-03-22 19:00 ` Fabio Estevam
0 siblings, 0 replies; 14+ messages in thread
From: Fabio Estevam @ 2013-03-22 19:00 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Mar 20, 2013 at 8:48 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> Frank,
>
> I'm running v3.9-rc3 image with NFS on imx6q, and seeing system fail
> to resume back. The resume seems being stopped by fec driver. If you
> wait long enough, you might see the repeated "eth0: tx queue full!."
> error message.
On mx28, only reverting this patch does not solve the suspend/resume issue.
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/1 net-next] net: fec: put tx to napi poll function to fix dead lock
2013-03-22 19:00 ` Fabio Estevam
@ 2013-03-24 14:35 ` Shawn Guo
-1 siblings, 0 replies; 14+ messages in thread
From: Shawn Guo @ 2013-03-24 14:35 UTC (permalink / raw)
To: Fabio Estevam
Cc: Frank Li, B38611, netdev, s.hauer, lznuaa, davem, linux-arm-kernel
On Fri, Mar 22, 2013 at 04:00:26PM -0300, Fabio Estevam wrote:
> On Wed, Mar 20, 2013 at 8:48 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> > Frank,
> >
> > I'm running v3.9-rc3 image with NFS on imx6q, and seeing system fail
> > to resume back. The resume seems being stopped by fec driver. If you
> > wait long enough, you might see the repeated "eth0: tx queue full!."
> > error message.
>
> On mx28, only reverting this patch does not solve the suspend/resume issue.
Are you sure? After reverting the patch on next-20130322, I can get
imx28 resume back to console as below.
Shawn
root@freescale ~$ echo mem > /sys/power/state
[ 20.943170] PM: Syncing filesystems ... done.
[ 20.976303] Freezing user space processes ... (elapsed 0.01 seconds) done.
[ 20.998050] Freezing remaining freezable tasks ... (elapsed 0.01 seconds) done.
[ 21.025647] PM: suspend of devices complete after 6.283 msecs
[ 21.032543] PM: late suspend of devices complete after 1.092 msecs
[ 21.040489] PM: noirq suspend of devices complete after 1.603 msecs
[ 21.048045] PM: noirq resume of devices complete after 0.794 msecs
[ 21.055891] PM: early resume of devices complete after 1.088 msecs
[ 21.167790] PM: resume of devices complete after 105.641 msecs
[ 21.174939] Restarting tasks ... done.
root@freescale ~$
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 1/1 net-next] net: fec: put tx to napi poll function to fix dead lock
@ 2013-03-24 14:35 ` Shawn Guo
0 siblings, 0 replies; 14+ messages in thread
From: Shawn Guo @ 2013-03-24 14:35 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Mar 22, 2013 at 04:00:26PM -0300, Fabio Estevam wrote:
> On Wed, Mar 20, 2013 at 8:48 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> > Frank,
> >
> > I'm running v3.9-rc3 image with NFS on imx6q, and seeing system fail
> > to resume back. The resume seems being stopped by fec driver. If you
> > wait long enough, you might see the repeated "eth0: tx queue full!."
> > error message.
>
> On mx28, only reverting this patch does not solve the suspend/resume issue.
Are you sure? After reverting the patch on next-20130322, I can get
imx28 resume back to console as below.
Shawn
root at freescale ~$ echo mem > /sys/power/state
[ 20.943170] PM: Syncing filesystems ... done.
[ 20.976303] Freezing user space processes ... (elapsed 0.01 seconds) done.
[ 20.998050] Freezing remaining freezable tasks ... (elapsed 0.01 seconds) done.
[ 21.025647] PM: suspend of devices complete after 6.283 msecs
[ 21.032543] PM: late suspend of devices complete after 1.092 msecs
[ 21.040489] PM: noirq suspend of devices complete after 1.603 msecs
[ 21.048045] PM: noirq resume of devices complete after 0.794 msecs
[ 21.055891] PM: early resume of devices complete after 1.088 msecs
[ 21.167790] PM: resume of devices complete after 105.641 msecs
[ 21.174939] Restarting tasks ... done.
root at freescale ~$
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v3 1/1 net-next] net: fec: put tx to napi poll function to fix dead lock
2013-03-24 14:35 ` Shawn Guo
@ 2013-04-12 13:38 ` Fabio Estevam
-1 siblings, 0 replies; 14+ messages in thread
From: Fabio Estevam @ 2013-04-12 13:38 UTC (permalink / raw)
To: Shawn Guo
Cc: Frank Li, B38611, netdev, s.hauer, lznuaa, davem, linux-arm-kernel
On Sun, Mar 24, 2013 at 11:35 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> Are you sure? After reverting the patch on next-20130322, I can get
> imx28 resume back to console as below.
Yes, please try 20130412, which has Frank's fix applied and try
suspend/resume sequence several times.
Does it work always for you?
On my mx28evk it usually fails.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH v3 1/1 net-next] net: fec: put tx to napi poll function to fix dead lock
@ 2013-04-12 13:38 ` Fabio Estevam
0 siblings, 0 replies; 14+ messages in thread
From: Fabio Estevam @ 2013-04-12 13:38 UTC (permalink / raw)
To: linux-arm-kernel
On Sun, Mar 24, 2013 at 11:35 AM, Shawn Guo <shawn.guo@linaro.org> wrote:
> Are you sure? After reverting the patch on next-20130322, I can get
> imx28 resume back to console as below.
Yes, please try 20130412, which has Frank's fix applied and try
suspend/resume sequence several times.
Does it work always for you?
On my mx28evk it usually fails.
^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2013-04-12 13:38 UTC | newest]
Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-04 3:34 [PATCH v3 1/1 net-next] net: fec: put tx to napi poll function to fix dead lock Frank Li
2013-03-04 3:34 ` Frank Li
2013-03-04 19:15 ` David Miller
2013-03-04 19:15 ` David Miller
2013-03-20 11:48 ` Shawn Guo
2013-03-20 11:48 ` Shawn Guo
2013-03-20 13:33 ` Frank Li
2013-03-20 13:33 ` Frank Li
2013-03-22 19:00 ` Fabio Estevam
2013-03-22 19:00 ` Fabio Estevam
2013-03-24 14:35 ` Shawn Guo
2013-03-24 14:35 ` Shawn Guo
2013-04-12 13:38 ` Fabio Estevam
2013-04-12 13:38 ` Fabio Estevam
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.