Message ID | 20120517103308.GA15830@electric-eye.fr.zoreil.com |
---|---|
State | RFC, archived |
Delegated to: | David Miller |
Headers | show |
2012/05/17 19:33, Francois Romieu wrote: > Shimoda, Yoshihiro <yoshihiro.shimoda.uh@renesas.com> : > [...] >> diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c >> index c64a31c..edc7dfe 100644 >> --- a/drivers/net/ethernet/renesas/sh_eth.c >> +++ b/drivers/net/ethernet/renesas/sh_eth.c > [...] < snip > >> @@ -1678,19 +1710,15 @@ static int sh_eth_start_xmit(struct sk_buff *skb, struct net_device *ndev) >> struct sh_eth_private *mdp = netdev_priv(ndev); >> struct sh_eth_txdesc *txdesc; >> u32 entry; >> - unsigned long flags; >> >> - spin_lock_irqsave(&mdp->lock, flags); >> if ((mdp->cur_tx - mdp->dirty_tx) >= (mdp->num_tx_ring - 4)) { >> if (!sh_eth_txfree(ndev)) { > > There are now two racing sh_eth_txfree and there is no [PATCH v4 7/6]. > > If I may suggest a slightly different approach, I would apply the patch > below before anything NAPI related: Thank you very much for the suggestion. I will modify the NAPI patch to fix the racing using your suggestion. (Because the original driver (before NAPI) was already locked by mdp->lock in sh_eth_interrupt(), I think that the original driver doesn't have the racing issue.) Best regards, Yoshihiro Shimoda -- 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 --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c index d63e09b..6d77462 100644 --- a/drivers/net/ethernet/renesas/sh_eth.c +++ b/drivers/net/ethernet/renesas/sh_eth.c @@ -1495,18 +1495,6 @@ static int sh_eth_start_xmit(struct sk_buff *skb, struct net_device *ndev) u32 entry; unsigned long flags; - spin_lock_irqsave(&mdp->lock, flags); - if ((mdp->cur_tx - mdp->dirty_tx) >= (TX_RING_SIZE - 4)) { - if (!sh_eth_txfree(ndev)) { - if (netif_msg_tx_queued(mdp)) - dev_warn(&ndev->dev, "TxFD exhausted.\n"); - netif_stop_queue(ndev); - spin_unlock_irqrestore(&mdp->lock, flags); - return NETDEV_TX_BUSY; - } - } - spin_unlock_irqrestore(&mdp->lock, flags); - entry = mdp->cur_tx % TX_RING_SIZE; mdp->tx_skbuff[entry] = skb; txdesc = &mdp->tx_ring[entry]; @@ -1531,6 +1519,15 @@ static int sh_eth_start_xmit(struct sk_buff *skb, struct net_device *ndev) if (!(sh_eth_read(ndev, EDTRR) & sh_eth_get_edtrr_trns(mdp))) sh_eth_write(ndev, sh_eth_get_edtrr_trns(mdp), EDTRR); + spin_lock_irqsave(&mdp->lock, flags); + if ((mdp->cur_tx - mdp->dirty_tx) >= (TX_RING_SIZE - 4)) { + if (netif_msg_tx_queued(mdp)) { + dev_warn(&ndev->dev, "TxFD exhausted.\n"); + netif_stop_queue(ndev); + } + } + spin_unlock_irqrestore(&mdp->lock, flags); + return NETDEV_TX_OK; }