| Message ID | 1347987385-19071-1-git-send-email-Joakim.Tjernlund@transmode.se |
|---|---|
| State | Superseded, archived |
| Delegated to: | David Miller |
| Headers | show |
Joakim Tjernlund <Joakim.Tjernlund@transmode.se> : > Currently ucc_geth_start_xmit wraps IRQ off for the > whole body just to be safe. > Reduce the IRQ off period to a minimum. The driver does not do much work in its irq handler. You may as well convert it to the usual tg3-ish locking style (i.e. almost no locking).
Francois Romieu <romieu@fr.zoreil.com> wrote on 2012/09/19 00:39:38: > > Joakim Tjernlund <Joakim.Tjernlund@transmode.se> : > > Currently ucc_geth_start_xmit wraps IRQ off for the > > whole body just to be safe. > > Reduce the IRQ off period to a minimum. > > The driver does not do much work in its irq handler. You may as well > convert it to the usual tg3-ish locking style (i.e. almost no locking). You mean broadcom/tg3.c? It is a bit much to look at ATM for me and there almost no locking with my patch also. Could possibly be improved further but I am happy for now. Jocke -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Joakim Tjernlund <Joakim.Tjernlund@transmode.se> : > Currently ucc_geth_start_xmit wraps IRQ off for the > whole body just to be safe. > Reduce the IRQ off period to a minimum. It opens a window in ucc_geth_start_xmit where the skb slot in ugeth->tx_skbuff[txQ] is set and T_RA has not been written into the descriptor status. Consider a racing poll : the !skb test in ucc_geth_tx may not work as expected.
diff --git a/drivers/net/ethernet/freescale/ucc_geth.c b/drivers/net/ethernet/freescale/ucc_geth.c index 9ac14f8..e609c93 100644 --- a/drivers/net/ethernet/freescale/ucc_geth.c +++ b/drivers/net/ethernet/freescale/ucc_geth.c @@ -3181,8 +3181,6 @@ static int ucc_geth_start_xmit(struct sk_buff *skb, struct net_device *dev) ugeth_vdbg("%s: IN", __func__); - spin_lock_irqsave(&ugeth->lock, flags); - dev->stats.tx_bytes += skb->len; /* Start from the next BD that should be filled */ @@ -3196,6 +3194,7 @@ static int ucc_geth_start_xmit(struct sk_buff *skb, struct net_device *dev) (ugeth->skb_curtx[txQ] + 1) & TX_RING_MOD_MASK(ugeth->ug_info->bdRingLenTx[txQ]); + spin_lock_irqsave(&ugeth->lock, flags); /* set up the buffer descriptor */ out_be32(&((struct qe_bd __iomem *)bd)->buf, dma_map_single(ugeth->dev, skb->data, @@ -3207,6 +3206,8 @@ static int ucc_geth_start_xmit(struct sk_buff *skb, struct net_device *dev) /* set bd status and length */ out_be32((u32 __iomem *)bd, bd_status); + spin_unlock_irqrestore(&ugeth->lock, flags); + /* Move to next BD in the ring */ if (!(bd_status & T_W)) @@ -3238,8 +3239,6 @@ static int ucc_geth_start_xmit(struct sk_buff *skb, struct net_device *dev) uccf = ugeth->uccf; out_be16(uccf->p_utodr, UCC_FAST_TOD); #endif - spin_unlock_irqrestore(&ugeth->lock, flags); - return NETDEV_TX_OK; }
Currently ucc_geth_start_xmit wraps IRQ off for the whole body just to be safe. Reduce the IRQ off period to a minimum. Signed-off-by: Joakim Tjernlund <Joakim.Tjernlund@transmode.se> --- drivers/net/ethernet/freescale/ucc_geth.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-)