From: Kalle Valo <kvalo@codeaurora.org>
To: Brian Norris <briannorris@chromium.org>
Cc: Tony Chuang <yhchuang@realtek.com>,
linux-wireless <linux-wireless@vger.kernel.org>
Subject: Re: [PATCH 05/15] rtw88: pci: release tx skbs DMAed when stop
Date: Fri, 20 Sep 2019 10:26:30 +0300 [thread overview]
Message-ID: <87tv97ctsp.fsf@codeaurora.org> (raw)
In-Reply-To: <CA+ASDXP_7FVEaQrE1V=rX0Vieu8GGWj+it7p4F_8XeRNniEWkw@mail.gmail.com> (Brian Norris's message of "Thu, 19 Sep 2019 17:35:56 -0700")
Brian Norris <briannorris@chromium.org> writes:
> On Tue, Sep 17, 2019 at 7:10 PM Tony Chuang <yhchuang@realtek.com> wrote:
>> > On Mon, Sep 16, 2019 at 12:03 AM <yhchuang@realtek.com> wrote:
>> > >
>> > > From: Yan-Hsuan Chuang <yhchuang@realtek.com>
>> > >
>> > > 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@chromium.org>
> Tested-by: Brian Norris <briannorris@chromium.org>
>
> 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
next prev parent reply other threads:[~2019-09-20 7:26 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-16 7:03 [PATCH 00/15] rtw88: Add support for deep PS mode yhchuang
2019-09-16 7:03 ` [PATCH 01/15] rtw88: remove redundant flag check helper function yhchuang
2019-10-01 9:17 ` Kalle Valo
2019-09-16 7:03 ` [PATCH 02/15] rtw88: configure firmware after HCI started yhchuang
2019-09-21 5:51 ` Kalle Valo
2019-09-16 7:03 ` [PATCH 03/15] rtw88: pci: reset H2C queue indexes in a single write yhchuang
2019-09-16 7:03 ` [PATCH 04/15] rtw88: pci: extract skbs free routine for trx rings yhchuang
2019-09-21 5:47 ` Kalle Valo
2019-09-16 7:03 ` [PATCH 05/15] rtw88: pci: release tx skbs DMAed when stop yhchuang
2019-09-18 0:41 ` Brian Norris
2019-09-18 2:10 ` Tony Chuang
2019-09-20 0:35 ` Brian Norris
2019-09-20 7:26 ` Kalle Valo [this message]
2019-09-20 8:29 ` Tony Chuang
2019-09-20 8:35 ` Kalle Valo
2019-09-20 22:33 ` Brian Norris
2019-09-16 7:03 ` [PATCH 06/15] rtw88: not to enter or leave PS under IRQ yhchuang
2019-09-16 7:03 ` [PATCH 07/15] rtw88: not to control LPS by each vif yhchuang
2019-09-16 7:03 ` [PATCH 08/15] rtw88: remove unused lps state check helper yhchuang
2019-09-16 7:03 ` [PATCH 09/15] rtw88: LPS enter/leave should be protected by lock yhchuang
2019-09-16 7:03 ` [PATCH 10/15] rtw88: leave PS state for dynamic mechanism yhchuang
2019-09-16 7:03 ` [PATCH 11/15] rtw88: add deep power save support yhchuang
2019-09-16 7:03 ` [PATCH 12/15] rtw88: not to enter LPS by coex strategy yhchuang
2019-09-16 7:03 ` [PATCH 13/15] rtw88: select deep PS mode when module is inserted yhchuang
2019-09-16 7:03 ` [PATCH 14/15] rtw88: add deep PS PG mode for 8822c yhchuang
2019-09-16 7:03 ` [PATCH 15/15] rtw88: remove misleading module parameter rtw_fw_support_lps yhchuang
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87tv97ctsp.fsf@codeaurora.org \
--to=kvalo@codeaurora.org \
--cc=briannorris@chromium.org \
--cc=linux-wireless@vger.kernel.org \
--cc=yhchuang@realtek.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.