Brian Norris <briannorris@xxxxxxxxxxxx> writes: > On Tue, Sep 17, 2019 at 7:10 PM Tony Chuang <yhchuang@xxxxxxxxxxx> wrote: >> > On Mon, Sep 16, 2019 at 12:03 AM <yhchuang@xxxxxxxxxxx> wrote: >> > > >> > > From: Yan-Hsuan Chuang <yhchuang@xxxxxxxxxxx> >> > > >> > > Interrupt is disabled to stop PCI, which means the skbs >> > > queued for each TX ring will not be released via DMA >> > > interrupt. >> > >> > In what cases do you hit this? I think you do this when entering PS >> > mode, no? But then, see below. >> >> I'll hit this when ieee80211_ops::stop, or rtw_power_off. >> Both are to turn off the device, so there's no more DMA activities. >> If we don't release the SKBs that are not released by DMA interrupt >> when powering off, these could be leaked. > > Ah, I was a bit confused. So it does get called from "PS" routines: > rtw_enter_ips() -> rtw_core_stop() > but that "IPS" mode means "Inactive" Power Save, and it's only used > when transitioning into idle states (IEEE80211_CONF_IDLE). > > Incidentally, I think this also may explain many of the leaks I've > been seeing elsewhere, when I leave a device sitting and scanning for > a very long time -- each scan attempt is making a single transition > out-and-back to IPS mode, which meant it may be leaking any > outstanding TX DMA. And testing confirms this: if I just bring up the > interface, run a scan, then bring it down, I see many fewer unmaps > than maps. Doing this enough times, I run out of contiguous DMA memory > and the device stops working. This fixes that problem for me. So: > > Reviewed-by: Brian Norris <briannorris@xxxxxxxxxxxx> > Tested-by: Brian Norris <briannorris@xxxxxxxxxxxx> > > I wonder if, given the problems I've seen (the driver can become > totally ineroperable), this patch and the previous patch (its only > real dependency) should be fast-tracked to the current release. I agree, this sounds like a serious problem. So I'm planning to queue patches 4 and 5 to v5.4, if it's ok for Tony. -- Kalle Valo
