Messages in this thread | | | Date | Wed, 16 Nov 2022 11:27:58 +0100 | From | Francois Romieu <> | Subject | Re: [PATCH net v2 3/3] net: nixge: fix tx queue handling |
| |
Zhang Changzhong <zhangchangzhong@huawei.com> : > On 2022/11/16 7:04, Francois Romieu wrote: > > Zhang Changzhong <zhangchangzhong@huawei.com> : [...] > >> diff --git a/drivers/net/ethernet/ni/nixge.c b/drivers/net/ethernet/ni/nixge.c > >> index 91b7ebc..3776a03 100644 > >> --- a/drivers/net/ethernet/ni/nixge.c > >> +++ b/drivers/net/ethernet/ni/nixge.c > > [...] > >> static netdev_tx_t nixge_start_xmit(struct sk_buff *skb, > >> @@ -518,10 +523,15 @@ static netdev_tx_t nixge_start_xmit(struct sk_buff *skb, > >> cur_p = &priv->tx_bd_v[priv->tx_bd_tail]; > >> tx_skb = &priv->tx_skb[priv->tx_bd_tail]; > >> > >> - if (nixge_check_tx_bd_space(priv, num_frag + 1)) { > >> - if (!netif_queue_stopped(ndev)) > >> - netif_stop_queue(ndev); > >> - return NETDEV_TX_OK; > >> + if (unlikely(nixge_check_tx_bd_space(priv, num_frag + 1))) { > >> + /* Should not happen as last start_xmit call should have > >> + * checked for sufficient space and queue should only be > >> + * woken when sufficient space is available. > >> + */ > > > > Almost. IRQ triggering after nixge_start_xmit::netif_stop_queue and > > before nixge_start_xmit::smp_mb may wrongly wake queue. > > > > I don't know what you mean by "wronly wake queue". The queue is woken > only when there is sufficient for next packet.
Between nixge_start_xmit::netif_stop_queue and nixge_start_xmit::smp_mb, "next" packet is current packet in hard_start_xmit. However said current packet may not be accounted for in the IRQ context transmit completion handler.
[nixge_start_xmit]
++priv->tx_bd_tail; priv->tx_bd_tail %= TX_BD_NUM;
/* Stop queue if next transmit may not have space */ if (nixge_check_tx_bd_space(priv, MAX_SKB_FRAGS + 1)) { netif_stop_queue(ndev);
Which value does [nixge_start_xmit_done] read as priv->tx_bd_tail at this point ? The value set a few lines above or some older value ?
/* Matches barrier in nixge_start_xmit_done */ smp_mb();
/* Space might have just been freed - check again */ if (!nixge_check_tx_bd_space(priv, MAX_SKB_FRAGS + 1)) netif_wake_queue(ndev); }
-- Ueimor
| |