Re: [PATCH net-next v1 3/5] net: lan743x: quiesce TX before freeing the rings

[Chris Babroski]

Hi Woojung,

> > @@ -3336,6 +3337,9 @@ static int lan743x_netdev_close(struct net_device
> > *netdev)
> > 
> >         lan743x_phylink_disconnect(adapter);
> > 
> > +       synchronize_net();
> > +       netif_tx_disable(netdev);
> 
> synchronize_net() is called inside of lan743x_tx_close() and lan743x_rx_close()
> via netif_napi_del(). Do you see current order of calling is an issue?
> 
> > +
> >         for (index = 0; index < adapter->used_tx_channels; index++)
> >                 lan743x_tx_close(&adapter->tx[index]);

This was a change introduced after AI review flagged a potential race
condition:
1. On CPU A, the TX NAPI poll now checks netif_device_present() and gets
   true.
2. On CPU B, netif_device_detach() runs, then lan743x_netdev_close()
   calls netif_tx_disable() and the queue is stopped.
3. CPU A calls netif_tx_wake_queue() and the queue is running again.
4. The qdisc is still attached in the suspend and reset paths, so the
   stack transmits into the ring while lan743x_tx_close() frees it.

So the synchronize_net() called via lan743x_tx_close() can't prevent
NAPI poll from waking the queue again.

The fix is to wait for NAPI poll to complete with synchronize_net()
first to prevent unexpected queue wake before netif_tx_disable() stops
the queue and waits for transmits to complete.

This is unlikely to happen as part of the TX hang scenario because the
reset in that case comes after TX has stopped. The pre-existing
conditions that could see this would be suspend, reboot, or kexec on a
multi-core system under TX load.

Given that this is not directly related to TX hang recovery I could
drop it from this patch series if you prefer.

Thanks,
Chris