On Fri, Feb 16, 2018 at 09:34:39AM +0000, Jose Abreu wrote: > Hi Niklas, > > Thank you for looking into this! > > On 13-02-2018 13:33, Niklas Cassel wrote: > > Hello Jose, > > > > > > I remember that you had a problem > > with a use after free in stmmac_tx_clean(). > > I still don't think that it is related to > > commit 05cf0d1bf4, however, when comparing > > the stmmac driver to the amd-xgbe driver > > I realized that: > > > > xgbe_tx_poll() has both a smp_rmb() after fetching > > cur_tx, and also a dma_rmb() after reading the own > > bit, before reading any other descriptor fields. > > > > stmmac_tx_clean() has neither a smp_rmb() or a > > dma_rmb().
This still applies as far as I can tell. > > > > > > Also > > xgbe_dev_xmit() has a dma_wmb() _before_ setting > > the own bit, and a smp_wmb() after setting the own > > bit. > > > > stmmac simply has a dma_wmb() _after_ setting the > > own bit. For dwmac4, stmmac actually has a another dma_wmb() hidden in dwmac4_rd_prepare_tx_desc(). It might be more obvious, since there already is a dma_wmb() in stmmac_xmit()/stmmac_tso_xmit(), if this was also placed there. We already have a set_owner() abstraction. xgbe also has a dma_wmb() in dwmac4_release_tx_desc(), which we appear to be missing. Pehaps we want it right after the priv->hw->desc->release_tx_desc(p, priv->mode) call, so we don't hide it in the different implementations. > > If you can still reproduce your problem quite easily, > > perhaps you could try to make stmmac look more like > > xgbe in these regards, and see if that solves your > > use after free crash in stmmac_tx_clean(). > > At the time it would be easily reproduced if I flooded the > interface. I will check xgbe barriers and check if it should also > be used in stmmac. I've fixed the tx timeout for multiqueues when running iperf3 (mss was not set per tx queue, as reported in another thread). Will send out a patch today. Regards, Niklas