Search Linux Wireless

RE: [PATCH 05/15] rtw88: pci: release tx skbs DMAed when stop

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 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



[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Wireless Regulations]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux