diff mbox

[1/5] ucc_geth: Reduce IRQ off in xmit path

Message ID OF540E33B1.6013BC69-ONC1257A7F.002C410C-C1257A7F.002C94CB@transmode.se
State RFC, archived
Delegated to: David Miller
Headers show

Commit Message

Joakim Tjernlund Sept. 20, 2012, 8:06 a.m. UTC 
Francois Romieu <romieu@fr.zoreil.com> wrote on 2012/09/20 00:34:16:
>
> 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.

Right, good catch!

Surprisingly the driver never showed any malfunction even though I hit
it pretty hard.

I will send a V2 of this patch where I move the assignment inside the IRQ
off part:

 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
diff mbox

Patch

--- a/drivers/net/ethernet/freescale/ucc_geth.c
+++ b/drivers/net/ethernet/freescale/ucc_geth.c
@@ -3200,8 +3200,6 @@  static int ucc_geth_start_xmit(struct sk_buff *skb, struct net_device *dev)
        /* Start from the next BD that should be filled */
        bd = ugeth->txBd[txQ];
        bd_status = in_be32((u32 __iomem *)bd);
-       /* Save the skb pointer so we can free it later */
-       ugeth->tx_skbuff[txQ][ugeth->skb_curtx[txQ]] = skb;

        /* Update the current skb pointer (wrapping if this was the last) */
        ugeth->skb_curtx[txQ] =
@@ -3209,6 +3207,8 @@  static int ucc_geth_start_xmit(struct sk_buff *skb, struct net_device *dev)
             1) & TX_RING_MOD_MASK(ugeth->ug_info->bdRingLenTx[txQ]);

        spin_lock_irqsave(&ugeth->lock, flags);
+       /* Save the skb pointer so we can free it later */
+       ugeth->tx_skbuff[txQ][ugeth->skb_curtx[txQ]] = skb;
        /* set up the buffer descriptor */
        out_be32(&((struct qe_bd __iomem *)bd)->buf,
                      dma_map_single(ugeth->dev, skb->data,