| Message ID | 20241106111427.7272-1-wander@redhat.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 50d325bb05cef24a2105e40e7cace5e2b237236d |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2,1/4] Revert "igb: Disable threaded IRQ for igb_msix_other" | expand |
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
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 >
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; >
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
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
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.
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
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
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".
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).
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!
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;
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(-)