[PATCH v2 1/4] Revert "igb: Disable threaded IRQ for igb_msix_other"

[PATCH v2 1/4] Revert "igb: Disable threaded IRQ for igb_msix_other"

[Wander Lairson Costa]

This reverts commit 338c4d3902feb5be49bfda530a72c7ab860e2c9f.

Sebastian noticed the ISR indirectly acquires spin_locks, which are
sleeping locks under PREEMPT_RT, which leads to kernel splats.

Fixes: 338c4d3902feb ("igb: Disable threaded IRQ for igb_msix_other")
Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Wander Lairson Costa <wander@redhat.com>

---

Changelog:

v2: Add the Fixes tag
Signed-off-by: Wander Lairson Costa <wander@redhat.com>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index b83df5f94b1f5..f1d0881687233 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -907,7 +907,7 @@ static int igb_request_msix(struct igb_adapter *adapter)
 	int i, err = 0, vector = 0, free_vector = 0;
 
 	err = request_irq(adapter->msix_entries[vector].vector,
-			  igb_msix_other, IRQF_NO_THREAD, netdev->name, adapter);
+			  igb_msix_other, 0, netdev->name, adapter);
 	if (err)
 		goto err_out;
 
-- 
2.47.0

Re: [PATCH v2 1/4] Revert "igb: Disable threaded IRQ for igb_msix_other"

[Sebastian Andrzej Siewior]

On 2024-11-06 08:14:26 [-0300], Wander Lairson Costa wrote:
> This reverts commit 338c4d3902feb5be49bfda530a72c7ab860e2c9f.
> 
> Sebastian noticed the ISR indirectly acquires spin_locks, which are
> sleeping locks under PREEMPT_RT, which leads to kernel splats.
> 
> Fixes: 338c4d3902feb ("igb: Disable threaded IRQ for igb_msix_other")
> Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Wander Lairson Costa <wander@redhat.com>

Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

This is the only patch.

Sebastian

Re: [PATCH v2 1/4] Revert "igb: Disable threaded IRQ for igb_msix_other"

[Wander Lairson Costa]

On Fri, Nov 8, 2024 at 4:20 AM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> On 2024-11-06 08:14:26 [-0300], Wander Lairson Costa wrote:
> > This reverts commit 338c4d3902feb5be49bfda530a72c7ab860e2c9f.
> >
> > Sebastian noticed the ISR indirectly acquires spin_locks, which are
> > sleeping locks under PREEMPT_RT, which leads to kernel splats.
> >
> > Fixes: 338c4d3902feb ("igb: Disable threaded IRQ for igb_msix_other")
> > Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> > Signed-off-by: Wander Lairson Costa <wander@redhat.com>
>
> Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
>
> This is the only patch.

Hrm, I had other unrelated .patch files in my directory,
git-send-email might have gotten confused because of that.

>
> Sebastian
>

Re: [PATCH v2 1/4] Revert "igb: Disable threaded IRQ for igb_msix_other"

[Przemek Kitszel]

On 11/6/24 12:14, Wander Lairson Costa wrote:
> This reverts commit 338c4d3902feb5be49bfda530a72c7ab860e2c9f.
> 
> Sebastian noticed the ISR indirectly acquires spin_locks, which are
> sleeping locks under PREEMPT_RT, which leads to kernel splats.

I don't like to slow things down, but it would be great to have a Link:
to the report, and the (minified) splat attached.

> 
> Fixes: 338c4d3902feb ("igb: Disable threaded IRQ for igb_msix_other")
> Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Wander Lairson Costa <wander@redhat.com>
> 
> ---
> 
> Changelog:
> 
> v2: Add the Fixes tag
> Signed-off-by: Wander Lairson Costa <wander@redhat.com>
> ---
>   drivers/net/ethernet/intel/igb/igb_main.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index b83df5f94b1f5..f1d0881687233 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -907,7 +907,7 @@ static int igb_request_msix(struct igb_adapter *adapter)
>   	int i, err = 0, vector = 0, free_vector = 0;
>   
>   	err = request_irq(adapter->msix_entries[vector].vector,
> -			  igb_msix_other, IRQF_NO_THREAD, netdev->name, adapter);
> +			  igb_msix_other, 0, netdev->name, adapter);
>   	if (err)
>   		goto err_out;
>   

Re: [PATCH v2 1/4] Revert "igb: Disable threaded IRQ for igb_msix_other"

[Sebastian Andrzej Siewior]

On 2024-11-08 13:20:28 [+0100], Przemek Kitszel wrote:
> I don't like to slow things down, but it would be great to have a Link:
> to the report, and the (minified) splat attached.

I don't have a splat, I just reviewed the original patch. Please do
delay this.

Sebastian

Re: [PATCH v2 1/4] Revert "igb: Disable threaded IRQ for igb_msix_other"

[Przemek Kitszel]

On 11/8/24 13:28, Sebastian Andrzej Siewior wrote:
> On 2024-11-08 13:20:28 [+0100], Przemek Kitszel wrote:
>> I don't like to slow things down, but it would be great to have a Link:
>> to the report, and the (minified) splat attached.
> 
> I don't have a splat, I just reviewed the original patch. Please do
> delay this.
> 
> Sebastian

Rafal,
you have provided your Tested-by tag for the original (not reverted)
patch, could you please re-test it on RT kernel?

Przemek

Re: [PATCH v2 1/4] Revert "igb: Disable threaded IRQ for igb_msix_other"

[Jacob Keller]

On 11/8/2024 4:28 AM, Sebastian Andrzej Siewior wrote:
> On 2024-11-08 13:20:28 [+0100], Przemek Kitszel wrote:
>> I don't like to slow things down, but it would be great to have a Link:
>> to the report, and the (minified) splat attached.
> 
> I don't have a splat, I just reviewed the original patch. Please do
> delay this.
> 
> Sebastian

It will definitely splat on RT kernels at some point, if there is a
spinlock.

Re: [PATCH v2 1/4] Revert "igb: Disable threaded IRQ for igb_msix_other"

[Sebastian Andrzej Siewior]

On 2024-11-08 15:00:48 [-0800], Jacob Keller wrote:
> 
> 
> On 11/8/2024 4:28 AM, Sebastian Andrzej Siewior wrote:
> > On 2024-11-08 13:20:28 [+0100], Przemek Kitszel wrote:
> >> I don't like to slow things down, but it would be great to have a Link:
> >> to the report, and the (minified) splat attached.
> > 
> > I don't have a splat, I just reviewed the original patch. Please do
> > delay this.

this clearly lacks a `not'

> > Sebastian
> 
> It will definitely splat on RT kernels at some point, if there is a
> spinlock.

exactly my point.

Sebastian

Re: [PATCH v2 1/4] Revert "igb: Disable threaded IRQ for igb_msix_other"

[Przemek Kitszel]

On 11/11/24 13:53, Sebastian Andrzej Siewior wrote:
> On 2024-11-08 15:00:48 [-0800], Jacob Keller wrote:
>>
>>
>> On 11/8/2024 4:28 AM, Sebastian Andrzej Siewior wrote:
>>> On 2024-11-08 13:20:28 [+0100], Przemek Kitszel wrote:
>>>> I don't like to slow things down, but it would be great to have a Link:
>>>> to the report, and the (minified) splat attached.
>>>
>>> I don't have a splat, I just reviewed the original patch. Please do
>>> delay this.
> 
> this clearly lacks a `not'

sorry for the slowdown then,
Acked-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>

> 
>>> Sebastian
>>
>> It will definitely splat on RT kernels at some point, if there is a
>> spinlock.
> 
> exactly my point.
still would be great to add some basic RT tests in our VAL,
but it's unrelated to this patch

Re: [PATCH v2 1/4] Revert "igb: Disable threaded IRQ for igb_msix_other"

[Jakub Kicinski]

On Tue, 12 Nov 2024 15:52:34 +0100 Przemek Kitszel wrote:
> Acked-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>

Do you want us to take this directly?
To be clear - we only treat an ack from the maintainer who usually
sends us patches as implicit "please take this directly".

Re: [PATCH v2 1/4] Revert "igb: Disable threaded IRQ for igb_msix_other"

[Przemek Kitszel]

On 11/12/24 16:04, Jakub Kicinski wrote:
> On Tue, 12 Nov 2024 15:52:34 +0100 Przemek Kitszel wrote:
>> Acked-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
> 
> Do you want us to take this directly?
> To be clear - we only treat an ack from the maintainer who usually
> sends us patches as implicit "please take this directly".

Please take this one directly, Tony is OOO today, same for Jake
that could've provided also his RB otherwise.

I usually don't Ack intel-only series to avoid such ambiguities.

for this particular patch I wanted to say "fine for me, on behalf
of Intel", especially that this time I didn't provided meaningful
feedback on the code (put in that way to be nice for myself);
we have a track record of discussions preventing application to
the iwl*/dev-queue (as for this patch).

Re: [PATCH v2 1/4] Revert "igb: Disable threaded IRQ for igb_msix_other"

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed,  6 Nov 2024 08:14:26 -0300 you wrote:
> This reverts commit 338c4d3902feb5be49bfda530a72c7ab860e2c9f.
> 
> Sebastian noticed the ISR indirectly acquires spin_locks, which are
> sleeping locks under PREEMPT_RT, which leads to kernel splats.
> 
> Fixes: 338c4d3902feb ("igb: Disable threaded IRQ for igb_msix_other")
> Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Signed-off-by: Wander Lairson Costa <wander@redhat.com>
> 
> [...]

Here is the summary with links:
  - [v2,1/4] Revert "igb: Disable threaded IRQ for igb_msix_other"
    https://git.kernel.org/netdev/net/c/50d325bb05ce

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html