> 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: I thought you're talking about IEEE80211_CONF_PS instead of IEEE80211_CONF_IDLE. > > 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. It's OK for me, didn't realize that this is a serious problem, so I missed it. Also if possible you should queue patch 2, that reordering will cause two H2C skbs not be released because HCI hasn't started, everytime enter/leave IDLE state (rtw_power_[on|off]). Should I resend and add a v5.4 prefix or something? Yan-Hsuan
