netdev - Re: long delays in rtl8723 drivers in irq disabled sections

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 14 Nov 2019 22:45:17 +0100
From:   Lucas Stach <dev@...xeye.de>
To:     Larry Finger <Larry.Finger@...inger.net>,
        Pkshih <pkshih@...ltek.com>, wlanfae <wlanfae@...ltek.com>
Cc:     "linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: long delays in rtl8723 drivers in irq disabled sections

Hi Larry,

Am Donnerstag, den 14.11.2019, 15:25 -0600 schrieb Larry Finger:
[...]
> > > I don't know if this function needs to guard against something running
> > > in the IRQ handler, so depending on the answer to that the solution
> > > might be as simple as not disabling IRQs when taking the spinlock.
> > > 
> > > kworker/-276     4d...    0us : _raw_spin_lock_irqsave
> > > kworker/-276     4d...    0us : rtl8723_phy_rf_serial_read <-rtl8723de_phy_set_rf_reg
> > > kworker/-276     4d...    1us : rtl8723_phy_query_bb_reg <-rtl8723_phy_rf_serial_read
> > > kworker/-276     4d...    3us : rtl8723_phy_set_bb_reg <-rtl8723_phy_rf_serial_read
> > > kworker/-276     4d...    4us : __const_udelay <-rtl8723_phy_rf_serial_read
> > > kworker/-276     4d...    4us!: delay_mwaitx <-rtl8723_phy_rf_serial_read
> > > kworker/-276     4d... 1004us : rtl8723_phy_set_bb_reg <-rtl8723_phy_rf_serial_read
> > > [...]
> > > 
> > 
> > I check TX/RX interrupt handlers, and I don't find one calls RF read function
> > by now. I suspect that old code controls RF to do PS in interrupt context, so
> > _irqsave version is used to ensure read RF isn't interrupted or deadlock.
> > So, I change spin_lock to non-irqsave version, and do some tests on 8723BE
> > that works well.
> > 
> > What do you think about two fixes mentioned above? If they're ok, I can send
> > two patches to resolve this long delays.
> 
> Lucas,
> 
> If the above patch fixes the problem with the 8723de, I will modify the GitHub 
> driver. Although 8723de will be added to rtw88, I will keep the driver in 
> rtlwifi_new.

I'm currently running the rtlwifi_new based modules, modified with the
reduced waits as suggested by PK, as well as removing the IRQ disable
from the spinlocks in both rtl8723de_phy_query_rf_reg() and
rtl8723de_phy_set_rf_reg().

I can confirm that with those changes rtl8723de no longer shows up my
latency traces.

Regards,
Lucas