netdev - Re: [PATCH net-next] r8169: Use a raw_spinlock

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]

Message-ID: <d48727e7-451a-39f4-5878-0ba957c92114@gmail.com>
Date: Mon, 22 May 2023 22:00:17 +0200
From: Heiner Kallweit <hkallweit1@...il.com>
To: Sebastian Andrzej Siewior <bigeasy@...utronix.de>, netdev@...r.kernel.org
Cc: "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Mike Galbraith <efault@....de>, Paolo Abeni <pabeni@...hat.com>,
 Thomas Gleixner <tglx@...utronix.de>, nic_swsd@...ltek.com
Subject: Re: [PATCH net-next] r8169: Use a raw_spinlock_t for the register
 locks.

On 22.05.2023 15:41, Sebastian Andrzej Siewior wrote:
> The driver's interrupt service routine is requested with the
> IRQF_NO_THREAD if MSI is available. This means that the routine is
> invoked in hardirq context even on PREEMPT_RT. The routine itself is
> relatively short and schedules a worker, performs register access and
> schedules NAPI. On PREEMPT_RT, scheduling NAPI from hardirq results in
> waking ksoftirqd for further processing so using NAPI threads with this
> driver is highly recommended since it NULL routes the threaded-IRQ
> efforts.
> 
> Adding rtl_hw_aspm_clkreq_enable() to the ISR is problematic on
> PREEMPT_RT because the function uses spinlock_t locks which become
> sleeping locks on PREEMPT_RT. The locks are only used to protect
> register access and don't nest into other functions or locks. They are
> also not used for unbounded period of time. Therefore it looks okay to
> convert them to raw_spinlock_t.
> 
> Convert the three locks which are used from the interrupt service
> routine to raw_spinlock_t.
> 
> Fixes: e1ed3e4d91112 ("r8169: disable ASPM during NAPI poll")
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@...utronix.de>
> ---
> 
> This is the only ethernet driver doing IRQF_NO_THREAD. It was recently
> enabled and based on thread it was argued to offer better performance
> with threaded interrupts and NAPI threads. So I'm trying this instead of
> reverting the whole thing since it does not seem necessary. Maybe the
> NAPI-threads could some special treats, I take a look.
> 
>  drivers/net/ethernet/realtek/r8169_main.c | 44 +++++++++++------------
>  1 file changed, 22 insertions(+), 22 deletions(-)
> 

Reviewed-by: Heiner Kallweit <hkallweit1@...il.com>