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

[Woojung.Huh]

Hi Chris,

> > > @@ -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 for detail explanation.
IMHO, it is from Ai review than reproducible and/or failure during tests
I would suggest making as separate net-next patch than net fix[1] if others are ok.

[1] https://lore.kernel.org/all/20261020180757.0d08b6cb@xxxxxxxxxx/

Thanks
Woojung