netdev - Re: [PATCH iwl-net 0/4] igb: fix igb_msix_other() handling for PREEMPT

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] [day] [month] [year] [list]

Message-ID: <CAAq0SUmVn57F5hc=iJkS1-8WPrguOcEYrirZ7hFgiFxhcTCowQ@mail.gmail.com>
Date: Thu, 26 Dec 2024 10:24:43 -0300
From: Wander Lairson Costa <wander@...hat.com>
To: Tony Nguyen <anthony.l.nguyen@...el.com>, 
	Przemek Kitszel <przemyslaw.kitszel@...el.com>, Andrew Lunn <andrew+netdev@...n.ch>, 
	"David S. Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, 
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, 
	Sebastian Andrzej Siewior <bigeasy@...utronix.de>, Clark Williams <clrkwllms@...nel.org>, 
	Steven Rostedt <rostedt@...dmis.org>, Jeff Garzik <jgarzik@...hat.com>, 
	Auke Kok <auke-jan.h.kok@...el.com>, 
	"moderated list:INTEL ETHERNET DRIVERS" <intel-wired-lan@...ts.osuosl.org>, 
	"open list:NETWORKING DRIVERS" <netdev@...r.kernel.org>, open list <linux-kernel@...r.kernel.org>, 
	"open list:Real-time Linux (PREEMPT_RT):Keyword:PREEMPT_RT" <linux-rt-devel@...ts.linux.dev>
Subject: Re: [PATCH iwl-net 0/4] igb: fix igb_msix_other() handling for PREEMPT_RT

On Wed, Dec 4, 2024 at 8:43 AM Wander Lairson Costa <wander@...hat.com> wrote:
>
> This is the second attempt at fixing the behavior of igb_msix_other()
> for PREEMPT_RT. The previous attempt [1] was reverted [2] following
> concerns raised by Sebastian [3].
>
> The initial approach proposed converting vfs_lock to a raw_spinlock,
> a minor change intended to make it safe. However, it became evident
> that igb_rcv_msg_from_vf() invokes kcalloc with GFP_ATOMIC,
> which is unsafe in interrupt context on PREEMPT_RT systems.
>
> To address this, the solution involves splitting igb_msg_task()
> into two parts:
>
>     * One part invoked from the IRQ context.
>     * Another part called from the threaded interrupt handler.
>
> To accommodate this, vfs_lock has been restructured into a double
> lock: a spinlock_t and a raw_spinlock_t. In the revised design:
>
>     * igb_disable_sriov() locks both spinlocks.
>     * Each part of igb_msg_task() locks the appropriate spinlock for
>     its execution context.
>
> It is worth noting that the double lock mechanism is only active under
> PREEMPT_RT. For non-PREEMPT_RT builds, the additional raw_spinlock_t
> field is ommited.
>
> If the extra raw_spinlock_t field can be tolerated under
> !PREEMPT_RT (even though it remains unused), we can eliminate the
> need for #ifdefs and simplify the code structure.
>
> I will be on vacation from December 7th to Christmas and will address
> review comments upon my return.
>
> If possible, I kindly request the Intel team to perform smoke tests
> on both stock and realtime kernels to catch any potential issues with
> this patch series.
>
> Cheers,
> Wander
>
> [1] https://lore.kernel.org/all/20240920185918.616302-2-wander@redhat.com/
> [2] https://lore.kernel.org/all/20241104124050.22290-1-wander@redhat.com/
> [3] https://lore.kernel.org/all/20241104110708.gFyxRFlC@linutronix.de/
>
>
> Wander Lairson Costa (4):
>   igb: narrow scope of vfs_lock in SR-IOV cleanup
>   igb: introduce raw vfs_lock to igb_adapter
>   igb: split igb_msg_task()
>   igb: fix igb_msix_other() handling for PREEMPT_RT
>
>  drivers/net/ethernet/intel/igb/igb.h      |   4 +
>  drivers/net/ethernet/intel/igb/igb_main.c | 160 +++++++++++++++++++---
>  2 files changed, 148 insertions(+), 16 deletions(-)
>
> --
> 2.47.0
>

I had requested Red Hat Network QA to run regression tests on this,
and they recently reported that no issues were found.