public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net-next v2] declance: Remove IRQF_ONESHOT
@ 2026-01-27 13:53 Sebastian Andrzej Siewior
  2026-01-27 15:46 ` Maciej W. Rozycki
  2026-01-29  3:50 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 12+ messages in thread
From: Sebastian Andrzej Siewior @ 2026-01-27 13:53 UTC (permalink / raw)
  To: netdev, linux-mips
  Cc: Maciej W. Rozycki, Jakub Kicinski, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni

Passing IRQF_ONESHOT ensures that the interrupt source is masked until
the secondary (threaded) handler is done. If only a primary handler is
used then the flag makes no sense because the interrupt can not fire
(again) while its handler is running.
The flag also disallows force-threading of the primary handler and the
irq-core will warn about this as of commit aef30c8d569c0 ("genirq: Warn
about using IRQF_ONESHOT without a threaded handler").

The IRQF_ONESHOT flag was added in commit 0fabe1021f8bc ("MIPS:
DECstation I/O ASIC DMA interrupt classes"). It moved
clear_ioasic_dma_irq() from the driver into the irq-chip.
For EOI interrupts the clear_ioasic_dma_irq() callback is now invoked as
->irq_eoi() which is invoked after the IRQ was handled while the
interrupt is masked due to IRQF_ONESHOT. Without IRQF_ONESHOT it would
be invoked while interrupt is unmasked (but interrupts are disabled).

If it is *required* to invoke EOI-ack while the interrupt is masked (and
not a misunderstanding) due to irq-chip cascading/ hierarchical reasons
then using handle_fasteoi_mask_irq() as flow-handler would be the right
way to do so.

Remove IRQF_ONESHOT to irqflags.

Cc: Andrew Lunn <andrew+netdev@lunn.ch>
Cc: David S. Miller <davem@davemloft.net>
Cc: Eric Dumazet <edumazet@google.com>
Cc: Jakub Kicinski <kuba@kernel.org>
Cc: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---

The mentioned genirq commit is in -next.

 drivers/net/ethernet/amd/declance.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/amd/declance.c b/drivers/net/ethernet/amd/declance.c
index 8d05a0c5f2d5d..e6d56fcdc1ddd 100644
--- a/drivers/net/ethernet/amd/declance.c
+++ b/drivers/net/ethernet/amd/declance.c
@@ -813,7 +813,7 @@ static int lance_open(struct net_device *dev)
 	if (lp->dma_irq >= 0) {
 		unsigned long flags;
 
-		if (request_irq(lp->dma_irq, lance_dma_merr_int, IRQF_ONESHOT,
+		if (request_irq(lp->dma_irq, lance_dma_merr_int, 0,
 				"lance error", dev)) {
 			free_irq(dev->irq, dev);
 			printk("%s: Can't get DMA IRQ %d\n", dev->name,
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2] declance: Remove IRQF_ONESHOT
  2026-01-27 13:53 [PATCH net-next v2] declance: Remove IRQF_ONESHOT Sebastian Andrzej Siewior
@ 2026-01-27 15:46 ` Maciej W. Rozycki
  2026-01-27 16:54   ` Sebastian Andrzej Siewior
  2026-01-29  3:50 ` patchwork-bot+netdevbpf
  1 sibling, 1 reply; 12+ messages in thread
From: Maciej W. Rozycki @ 2026-01-27 15:46 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: netdev, linux-mips, Jakub Kicinski, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni

On Tue, 27 Jan 2026, Sebastian Andrzej Siewior wrote:

> The IRQF_ONESHOT flag was added in commit 0fabe1021f8bc ("MIPS:
> DECstation I/O ASIC DMA interrupt classes"). It moved
> clear_ioasic_dma_irq() from the driver into the irq-chip.
> For EOI interrupts the clear_ioasic_dma_irq() callback is now invoked as
> ->irq_eoi() which is invoked after the IRQ was handled while the
> interrupt is masked due to IRQF_ONESHOT. Without IRQF_ONESHOT it would
> be invoked while interrupt is unmasked (but interrupts are disabled).

 Umm, are they?  It's been a while, but have I missed anything?  As I 
recall ->irq_mask() is called exactly so that interrupts can be re-enabled 
at the CPU level right way so as not to block other sources which may have 
a low latency requirement while a hardirq handler is running.

> If it is *required* to invoke EOI-ack while the interrupt is masked (and
> not a misunderstanding) due to irq-chip cascading/ hierarchical reasons
> then using handle_fasteoi_mask_irq() as flow-handler would be the right
> way to do so.

 Since this is a level-triggered interrupt unmasking it before the EOI 
will make it retrigger right away and loop forever.  And as the 
description of commit 0fabe1021f8b ("MIPS: DECstation I/O ASIC DMA 
interrupt classes") says the interrupt must not be acked before EOI, as 
the problematic transfer would restart while the IRQ handler is still 
running:

16 R/W0C LANCE DMA memory read error
         This bit is set to 1 and DMA disabled, when the LANCE DMA
         encounters a memory read error. The LANCE then times out and
         interrupts the processor, which handles the problem. The LPR can
         read the address of the error. The bit may be cleared by writing a 0;
         writing a 1 has no effect.

so a combined ACK-EOI is the only correct way to handle it.

 Yes, the handler is sort of a placeholder, but the structure of handling 
ought to be correct regardless.

 One issue here is this is a recovery handler for an error scenario that 
is not supposed to happen with a correctly operating system.  I've never 
seen it actually fire, which is also why there's no actual recovery action 
implemented.  Perhaps such an error could be induced for verification 
purposes, I don't know.  All in all this code may have to rely solely on 
hw specs.

 I need more data to conclude whether this is the right change to make I'm 
afraid.  Thank you for looking into it though.

  Maciej

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2] declance: Remove IRQF_ONESHOT
  2026-01-27 15:46 ` Maciej W. Rozycki
@ 2026-01-27 16:54   ` Sebastian Andrzej Siewior
  2026-01-27 18:35     ` Maciej W. Rozycki
  0 siblings, 1 reply; 12+ messages in thread
From: Sebastian Andrzej Siewior @ 2026-01-27 16:54 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: netdev, linux-mips, Jakub Kicinski, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni

On 2026-01-27 15:46:10 [+0000], Maciej W. Rozycki wrote:
> On Tue, 27 Jan 2026, Sebastian Andrzej Siewior wrote:
> 
> > The IRQF_ONESHOT flag was added in commit 0fabe1021f8bc ("MIPS:
> > DECstation I/O ASIC DMA interrupt classes"). It moved
> > clear_ioasic_dma_irq() from the driver into the irq-chip.
> > For EOI interrupts the clear_ioasic_dma_irq() callback is now invoked as
> > ->irq_eoi() which is invoked after the IRQ was handled while the
> > interrupt is masked due to IRQF_ONESHOT. Without IRQF_ONESHOT it would
> > be invoked while interrupt is unmasked (but interrupts are disabled).
> 
>  Umm, are they?  It's been a while, but have I missed anything?  As I 
> recall ->irq_mask() is called exactly so that interrupts can be re-enabled 
> at the CPU level right way so as not to block other sources which may have 
> a low latency requirement while a hardirq handler is running.

A hardirq is always serviced with disabled interrupts from CPU point of
view. There were exceptions in the IDE department as far as I remember
where it was possible to enable interrupts from CPUs point of view while
the interrupt was serviced. That was 2.2/2.4 time frame and edge
interrupts made it possible.

It is not required to mask the interrupt while the handler is invoked
unless it is required to properly ACK the interrupt from device's and
IRQ-chip point of view.
The sole purpose of IRQF_ONESHOT was to disable the interrupt source
while the threaded interrupt is running. This driver has none.

> > If it is *required* to invoke EOI-ack while the interrupt is masked (and
> > not a misunderstanding) due to irq-chip cascading/ hierarchical reasons
> > then using handle_fasteoi_mask_irq() as flow-handler would be the right
> > way to do so.
> 
>  Since this is a level-triggered interrupt unmasking it before the EOI 
> will make it retrigger right away and loop forever.  And as the 
> description of commit 0fabe1021f8b ("MIPS: DECstation I/O ASIC DMA 
> interrupt classes") says the interrupt must not be acked before EOI, as 
> the problematic transfer would restart while the IRQ handler is still 
> running:

If it is a level interrupt and MASK/ACK in the irq-chip then the device
will issue the interrupt again if the source of the interrupt (i.e. the
error) has not been solved. Since the handler does only a printk() it
should trigger again.

> 16 R/W0C LANCE DMA memory read error
>          This bit is set to 1 and DMA disabled, when the LANCE DMA
>          encounters a memory read error. The LANCE then times out and
>          interrupts the processor, which handles the problem. The LPR can
>          read the address of the error. The bit may be cleared by writing a 0;
>          writing a 1 has no effect.
> 
> so a combined ACK-EOI is the only correct way to handle it.

Make sense. So first the driver needs to set this bit and then IRQ-chip
would need to see the EOI after the handler run. You would still need to
cancel/ tear down the transfer before that.

Again, with the interrupt handler setup as-is lance_dma_merr_int() will
run with disabled interrupts.

>  Yes, the handler is sort of a placeholder, but the structure of handling 
> ought to be correct regardless.
> 
>  One issue here is this is a recovery handler for an error scenario that 
> is not supposed to happen with a correctly operating system.  I've never 
> seen it actually fire, which is also why there's no actual recovery action 
> implemented.  Perhaps such an error could be induced for verification 
> purposes, I don't know.  All in all this code may have to rely solely on 
> hw specs.
> 
>  I need more data to conclude whether this is the right change to make I'm 
> afraid.  Thank you for looking into it though.

Fair enough. Would it work for you if we scratch this from net-next and
you route this or something else via the mips tree?

>   Maciej

Sebastian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2] declance: Remove IRQF_ONESHOT
  2026-01-27 16:54   ` Sebastian Andrzej Siewior
@ 2026-01-27 18:35     ` Maciej W. Rozycki
  2026-03-29 20:27       ` Maciej W. Rozycki
  0 siblings, 1 reply; 12+ messages in thread
From: Maciej W. Rozycki @ 2026-01-27 18:35 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: netdev, linux-mips, Jakub Kicinski, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni

On Tue, 27 Jan 2026, Sebastian Andrzej Siewior wrote:

> >  Umm, are they?  It's been a while, but have I missed anything?  As I 
> > recall ->irq_mask() is called exactly so that interrupts can be re-enabled 
> > at the CPU level right way so as not to block other sources which may have 
> > a low latency requirement while a hardirq handler is running.
> 
> A hardirq is always serviced with disabled interrupts from CPU point of
> view. There were exceptions in the IDE department as far as I remember
> where it was possible to enable interrupts from CPUs point of view while
> the interrupt was serviced. That was 2.2/2.4 time frame and edge
> interrupts made it possible.

 Fair enough.

> It is not required to mask the interrupt while the handler is invoked
> unless it is required to properly ACK the interrupt from device's and
> IRQ-chip point of view.
> The sole purpose of IRQF_ONESHOT was to disable the interrupt source
> while the threaded interrupt is running. This driver has none.

 Ack.

> >  Since this is a level-triggered interrupt unmasking it before the EOI 
> > will make it retrigger right away and loop forever.  And as the 
> > description of commit 0fabe1021f8b ("MIPS: DECstation I/O ASIC DMA 
> > interrupt classes") says the interrupt must not be acked before EOI, as 
> > the problematic transfer would restart while the IRQ handler is still 
> > running:
> 
> If it is a level interrupt and MASK/ACK in the irq-chip then the device
> will issue the interrupt again if the source of the interrupt (i.e. the
> error) has not been solved. Since the handler does only a printk() it
> should trigger again.

 It won't retrigger, as a memory read error event is transient, e.g. a bus 
timeout or an uncorrected ECC error, and the resulting interrupt is only 
recorded internally in the IOASIC.  IOW, the interrupt is edge-triggered 
and the interrupt status bit is *the* origin of the outgoing interrupt to 
the CPU.  Once cleared the IRQ is gone until the next error happens.  But 
it cannot be cleared early as with ordinary edge-triggered interrupts, 
because of the dual function of the interrupt status bit, as described in 
the comment referring IRQF_ONESHOT in arch/mips/dec/ioasic-irq.c.

 FWIW it seems at the very least the handler ought to report the offending 
address and rate-limit the message.  Since this is a networking driver the 
resulting data corruption is probably OK as that can happen to transferred 
data anywhere en route and it's up to the high-level receiver to cope with 
that.  Given that it is outgoing data there's little that can be done too.  
Maybe the whole data transfer can be aborted, but I'm not familiar enough 
with LANCE hardware to know off-hand how to do that.  As I say, this is 
exceedingly rare an event, so I guess taking the path of least resistance 
make sense.

> > 16 R/W0C LANCE DMA memory read error
> >          This bit is set to 1 and DMA disabled, when the LANCE DMA
> >          encounters a memory read error. The LANCE then times out and
> >          interrupts the processor, which handles the problem. The LPR can
> >          read the address of the error. The bit may be cleared by writing a 0;
> >          writing a 1 has no effect.
> > 
> > so a combined ACK-EOI is the only correct way to handle it.
> 
> Make sense. So first the driver needs to set this bit and then IRQ-chip
> would need to see the EOI after the handler run. You would still need to
> cancel/ tear down the transfer before that.

 This is R/W0C, that is read/write-zero-to-clear.  Only hardware can set 
the bit to 1 and only software can set the bit to 0; any attempt made by 
software to flip the bit from 0 to 1 is ignored.  This guarantees write 
atomicity: you can ack any DMA interrupt(s) by just a single 32-bit write 
to the register with no need to read it first.

 Upon a DMA error the hardware writes 1, which asserts the IRQ and stops 
DMA.  Only having handled the situation the handler is supposed to write 
0, which deasserts the IRQ and resumes DMA.

 FAOD the IOASIC has IRQ and (third-party) DMA controller features both on 
chip, providing for the double function of the DMA interrupt status bits; 
they are all handled internally.  Non-DMA interrupts work the usual way: 
when the originating external device has deasserted its IRQ line, the 
corresponding interrupt status bit, which is R/O rather than R/W0C, gets 
cleared accordingly, deasserting the IOASIC's IRQ output for the CPU.

> >  Yes, the handler is sort of a placeholder, but the structure of handling 
> > ought to be correct regardless.
> > 
> >  One issue here is this is a recovery handler for an error scenario that 
> > is not supposed to happen with a correctly operating system.  I've never 
> > seen it actually fire, which is also why there's no actual recovery action 
> > implemented.  Perhaps such an error could be induced for verification 
> > purposes, I don't know.  All in all this code may have to rely solely on 
> > hw specs.
> > 
> >  I need more data to conclude whether this is the right change to make I'm 
> > afraid.  Thank you for looking into it though.
> 
> Fair enough. Would it work for you if we scratch this from net-next and
> you route this or something else via the mips tree?

 No need to, I think I understand the situation now.  Surely the comment 
referring IRQF_ONESHOT in arch/mips/dec/ioasic-irq.c needs to be removed, 
but otherwise this is:

Acked-by: Maciej W. Rozycki <macro@orcam.me.uk>

Thank you for clarifying this to me, and doing the clean-up in the first 
place!

  Maciej

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2] declance: Remove IRQF_ONESHOT
  2026-01-27 13:53 [PATCH net-next v2] declance: Remove IRQF_ONESHOT Sebastian Andrzej Siewior
  2026-01-27 15:46 ` Maciej W. Rozycki
@ 2026-01-29  3:50 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 12+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-01-29  3:50 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: netdev, linux-mips, macro, kuba, andrew+netdev, davem, edumazet,
	pabeni

Hello:

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

On Tue, 27 Jan 2026 14:53:34 +0100 you wrote:
> Passing IRQF_ONESHOT ensures that the interrupt source is masked until
> the secondary (threaded) handler is done. If only a primary handler is
> used then the flag makes no sense because the interrupt can not fire
> (again) while its handler is running.
> The flag also disallows force-threading of the primary handler and the
> irq-core will warn about this as of commit aef30c8d569c0 ("genirq: Warn
> about using IRQF_ONESHOT without a threaded handler").
> 
> [...]

Here is the summary with links:
  - [net-next,v2] declance: Remove IRQF_ONESHOT
    https://git.kernel.org/netdev/net-next/c/701b40f8bde1

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



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2] declance: Remove IRQF_ONESHOT
  2026-01-27 18:35     ` Maciej W. Rozycki
@ 2026-03-29 20:27       ` Maciej W. Rozycki
  2026-05-04 22:35         ` Maciej W. Rozycki
  0 siblings, 1 reply; 12+ messages in thread
From: Maciej W. Rozycki @ 2026-03-29 20:27 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: netdev, linux-mips, Jakub Kicinski, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni

On Tue, 27 Jan 2026, Maciej W. Rozycki wrote:

> > >  I need more data to conclude whether this is the right change to make I'm 
> > > afraid.  Thank you for looking into it though.
> > 
> > Fair enough. Would it work for you if we scratch this from net-next and
> > you route this or something else via the mips tree?
> 
>  No need to, I think I understand the situation now.  Surely the comment 
> referring IRQF_ONESHOT in arch/mips/dec/ioasic-irq.c needs to be removed, 
> but otherwise this is:
> 
> Acked-by: Maciej W. Rozycki <macro@orcam.me.uk>
> 
> Thank you for clarifying this to me, and doing the clean-up in the first 
> place!

 I've now got back to it and while preparing the justification for the 
removal of the IRQF_ONESHOT recommendation and having looked through 
Documentation/core-api/real-time/differences.rst I became stumped and 
need a further clarification after all.

 I read in the document that:

"However, on a PREEMPT_RT system, interrupts are forced-threaded and no 
longer run in hard IRQ context."

and:

"All interrupts are forced-threaded in a PREEMPT_RT system. The exceptions 
are interrupts that are requested with the IRQF_NO_THREAD, IRQF_PERCPU, or 
IRQF_ONESHOT flags."

-- do I infer correctly that on a PREEMPT_RT system in the absence of any 
flags passed to request_irq() the handler requested such as one concerned 
here (i.e. lance_dma_merr_int()) will run with interrupts locally enabled 
on the CPU?

 If so, then either we need to go back and make sure the originating IRQ 
line is masked throughout the execution of the handler (and no standard 
irq-flow method provides it), or any IOASIC DMA error interrupt handlers, 
including this one, have to use the IRQF_NO_THREAD flag instead or the CPU 
will hang looping on the interrupt being retriggered at enable time, as in 
the absence of masking the interrupt output of the interrupt controller 
remains active until the final EOI action.  Have I missed anything?

 Mind that this is somewhat theoretical, given that declance.c is only for 
systems using the MIPS CPU and arch/mips does not enable ARCH_SUPPORTS_RT, 
however should it do sometime, I'd rather all the hell didn't break loose.  
And there's previous art already as I can see IRQF_NO_THREAD used through 
arch/mips, following commit 5a4a4ad851dd ("MIPS: Mark cascade and low 
level interrupts IRQF_NO_THREAD").

  Maciej

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2] declance: Remove IRQF_ONESHOT
  2026-03-29 20:27       ` Maciej W. Rozycki
@ 2026-05-04 22:35         ` Maciej W. Rozycki
  2026-05-05  7:29           ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 12+ messages in thread
From: Maciej W. Rozycki @ 2026-05-04 22:35 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: netdev, linux-mips, Jakub Kicinski, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni

On Sun, 29 Mar 2026, Maciej W. Rozycki wrote:

>  I've now got back to it and while preparing the justification for the 
> removal of the IRQF_ONESHOT recommendation and having looked through 
> Documentation/core-api/real-time/differences.rst I became stumped and 
> need a further clarification after all.
> 
>  I read in the document that:
> 
> "However, on a PREEMPT_RT system, interrupts are forced-threaded and no 
> longer run in hard IRQ context."
> 
> and:
> 
> "All interrupts are forced-threaded in a PREEMPT_RT system. The exceptions 
> are interrupts that are requested with the IRQF_NO_THREAD, IRQF_PERCPU, or 
> IRQF_ONESHOT flags."
> 
> -- do I infer correctly that on a PREEMPT_RT system in the absence of any 
> flags passed to request_irq() the handler requested such as one concerned 
> here (i.e. lance_dma_merr_int()) will run with interrupts locally enabled 
> on the CPU?

 No reply, but I've gone through irq_setup_forced_threading() now and my 
inference was indeed correct, and the handler does need to be installed 
with IRQF_NO_THREAD.  I'll send corrective patches shortly.

  Maciej

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2] declance: Remove IRQF_ONESHOT
  2026-05-04 22:35         ` Maciej W. Rozycki
@ 2026-05-05  7:29           ` Sebastian Andrzej Siewior
  2026-05-05 12:02             ` Maciej W. Rozycki
  0 siblings, 1 reply; 12+ messages in thread
From: Sebastian Andrzej Siewior @ 2026-05-05  7:29 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: netdev, linux-mips, Jakub Kicinski, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni

On 2026-05-04 23:35:42 [+0100], Maciej W. Rozycki wrote:
> On Sun, 29 Mar 2026, Maciej W. Rozycki wrote:
> 
> >  I've now got back to it and while preparing the justification for the 
> > removal of the IRQF_ONESHOT recommendation and having looked through 
> > Documentation/core-api/real-time/differences.rst I became stumped and 
> > need a further clarification after all.
> > 
> >  I read in the document that:
> > 
> > "However, on a PREEMPT_RT system, interrupts are forced-threaded and no 
> > longer run in hard IRQ context."
> > 
> > and:
> > 
> > "All interrupts are forced-threaded in a PREEMPT_RT system. The exceptions 
> > are interrupts that are requested with the IRQF_NO_THREAD, IRQF_PERCPU, or 
> > IRQF_ONESHOT flags."
> > 
> > -- do I infer correctly that on a PREEMPT_RT system in the absence of any 
> > flags passed to request_irq() the handler requested such as one concerned 
> > here (i.e. lance_dma_merr_int()) will run with interrupts locally enabled 
> > on the CPU?
> 
>  No reply, but I've gone through irq_setup_forced_threading() now and my 
> inference was indeed correct, and the handler does need to be installed 
> with IRQF_NO_THREAD.  I'll send corrective patches shortly.

Sorry. I missed that previous email.
IRQF_NO_THREAD will not force-thread the interrupt handler so it will
run with interrupts disabled.

With force-threading enabled, the interrupt handler is masked in the
IRQ-chip until after the threaded-handler run. See the cond_unmask_irq()
in handle_level_irq() or the mask_irq() & cond_unmask_eoi_irq() in
handle_fasteoi_ack_irq(). That means the hw-IRQ is done, the thread is
running with interrupts enabled but the hw-IRQ will not trigger again.
The cited commit 5a4a4ad851dd8 ("MIPS: Mark cascade and low level
interrupts IRQF_NO_THREAD") is different as it acts on cascading
interrupts which is not what we have here.

If you request a threaded interrupt you must either provide two handler
and the primary must mask the interrupt so it does not fire again or you
pass a flag such as IRQF_ONESHOT and which point the IRQ subsystem will
mask the IRQ within the irqchip so it does not fire again.

>   Maciej

Sebastian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2] declance: Remove IRQF_ONESHOT
  2026-05-05  7:29           ` Sebastian Andrzej Siewior
@ 2026-05-05 12:02             ` Maciej W. Rozycki
  2026-05-05 12:32               ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 12+ messages in thread
From: Maciej W. Rozycki @ 2026-05-05 12:02 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: netdev, linux-mips, Jakub Kicinski, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni

On Tue, 5 May 2026, Sebastian Andrzej Siewior wrote:

> >  No reply, but I've gone through irq_setup_forced_threading() now and my 
> > inference was indeed correct, and the handler does need to be installed 
> > with IRQF_NO_THREAD.  I'll send corrective patches shortly.
> 
> Sorry. I missed that previous email.
> IRQF_NO_THREAD will not force-thread the interrupt handler so it will
> run with interrupts disabled.

 That's one clear option I've identified and given that the handler 
amounts to a printk() call it may well be the path of least resistance 
solution.

> With force-threading enabled, the interrupt handler is masked in the
> IRQ-chip until after the threaded-handler run. See the cond_unmask_irq()
> in handle_level_irq() or the mask_irq() & cond_unmask_eoi_irq() in
> handle_fasteoi_ack_irq(). That means the hw-IRQ is done, the thread is
> running with interrupts enabled but the hw-IRQ will not trigger again.
> The cited commit 5a4a4ad851dd8 ("MIPS: Mark cascade and low level
> interrupts IRQF_NO_THREAD") is different as it acts on cascading
> interrupts which is not what we have here.

 Not with the current handle_fasteoi_irq() handler.

 And actually not with handle_fasteoi_ack_irq() either, which doesn't call 
mask_irq() unless IRQF_ONESHOT has been requested (but ->irq_ack() could 
be repurposed to do masking), however handle_fasteoi_mask_irq() seems a 
matching candidate.  To use that handler the platform would have to select 
IRQ_DOMAIN_HIERARCHY and IRQ_FASTEOI_HIERARCHY_HANDLERS, although none of 
the stuff beyond just handle_fasteoi_mask_irq() appears relevant, so it 
seems like a waste of memory.  Note that the handlers are much more recent 
than the driver and back in the time IRQF_ONESHOT seemed a reasonable 
approach.

 The handle_level_irq() handler is irrelevant, because we do need to issue 
the EOI for deassertion.

> If you request a threaded interrupt you must either provide two handler
> and the primary must mask the interrupt so it does not fire again or you
> pass a flag such as IRQF_ONESHOT and which point the IRQ subsystem will
> mask the IRQ within the irqchip so it does not fire again.

 Well, yes, but you've just removed the flag from this driver, so either 
the flag has to reinstated or the driver adjusted differently for the 
threaded case to be handled correctly.

  Maciej

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2] declance: Remove IRQF_ONESHOT
  2026-05-05 12:02             ` Maciej W. Rozycki
@ 2026-05-05 12:32               ` Sebastian Andrzej Siewior
  2026-05-05 14:00                 ` Maciej W. Rozycki
  0 siblings, 1 reply; 12+ messages in thread
From: Sebastian Andrzej Siewior @ 2026-05-05 12:32 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: netdev, linux-mips, Jakub Kicinski, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni

On 2026-05-05 13:02:16 [+0100], Maciej W. Rozycki wrote:
> > With force-threading enabled, the interrupt handler is masked in the
> > IRQ-chip until after the threaded-handler run. See the cond_unmask_irq()
> > in handle_level_irq() or the mask_irq() & cond_unmask_eoi_irq() in
> > handle_fasteoi_ack_irq(). That means the hw-IRQ is done, the thread is
> > running with interrupts enabled but the hw-IRQ will not trigger again.
> > The cited commit 5a4a4ad851dd8 ("MIPS: Mark cascade and low level
> > interrupts IRQF_NO_THREAD") is different as it acts on cascading
> > interrupts which is not what we have here.
> 
>  Not with the current handle_fasteoi_irq() handler.

Why? There is a mask_irq() in the ONESHOT case.

>  And actually not with handle_fasteoi_ack_irq() either, which doesn't call 
> mask_irq() unless IRQF_ONESHOT has been requested (but ->irq_ack() could 
> be repurposed to do masking), however handle_fasteoi_mask_irq() seems a 
> matching candidate.  To use that handler the platform would have to select 
> IRQ_DOMAIN_HIERARCHY and IRQ_FASTEOI_HIERARCHY_HANDLERS, although none of 
> the stuff beyond just handle_fasteoi_mask_irq() appears relevant, so it 
> seems like a waste of memory.  Note that the handlers are much more recent 
> than the driver and back in the time IRQF_ONESHOT seemed a reasonable 
> approach.

If there is a chain and you have multiple controllers then the different
low-lever handler might be the way to go.
But IRQF_ONESHOT servers a different purpose. If you use request_irq()
then there is no threaded-handler and as such IRQF_ONESHOT does nothing
except avoiding force-threading the primary handler. But that is a
side-effet. If you want to avoid to force-threading the irq-handler then
IRQF_NO_THREAD would be the right flag.

>  The handle_level_irq() handler is irrelevant, because we do need to issue 
> the EOI for deassertion.
> 
> > If you request a threaded interrupt you must either provide two handler
> > and the primary must mask the interrupt so it does not fire again or you
> > pass a flag such as IRQF_ONESHOT and which point the IRQ subsystem will
> > mask the IRQ within the irqchip so it does not fire again.
> 
>  Well, yes, but you've just removed the flag from this driver, so either 
> the flag has to reinstated or the driver adjusted differently for the 
> threaded case to be handled correctly.

But there is a plain request_irq() so the handler is invoked directly in
hardirq context with disabled interrupts. In forced-threaded context,
such as on PREEMPT_RT, the IRQF_ONESHOT is added.

>   Maciej

Sebastian

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2] declance: Remove IRQF_ONESHOT
  2026-05-05 12:32               ` Sebastian Andrzej Siewior
@ 2026-05-05 14:00                 ` Maciej W. Rozycki
  2026-05-05 15:24                   ` Sebastian Andrzej Siewior
  0 siblings, 1 reply; 12+ messages in thread
From: Maciej W. Rozycki @ 2026-05-05 14:00 UTC (permalink / raw)
  To: Sebastian Andrzej Siewior
  Cc: netdev, linux-mips, Jakub Kicinski, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni

On Tue, 5 May 2026, Sebastian Andrzej Siewior wrote:

> > > With force-threading enabled, the interrupt handler is masked in the
> > > IRQ-chip until after the threaded-handler run. See the cond_unmask_irq()
> > > in handle_level_irq() or the mask_irq() & cond_unmask_eoi_irq() in
> > > handle_fasteoi_ack_irq(). That means the hw-IRQ is done, the thread is
> > > running with interrupts enabled but the hw-IRQ will not trigger again.
> > > The cited commit 5a4a4ad851dd8 ("MIPS: Mark cascade and low level
> > > interrupts IRQF_NO_THREAD") is different as it acts on cascading
> > > interrupts which is not what we have here.
> > 
> >  Not with the current handle_fasteoi_irq() handler.
> 
> Why? There is a mask_irq() in the ONESHOT case.

 Yes, in that case there is.

> > > If you request a threaded interrupt you must either provide two handler
> > > and the primary must mask the interrupt so it does not fire again or you
> > > pass a flag such as IRQF_ONESHOT and which point the IRQ subsystem will
> > > mask the IRQ within the irqchip so it does not fire again.
> > 
> >  Well, yes, but you've just removed the flag from this driver, so either 
> > the flag has to reinstated or the driver adjusted differently for the 
> > threaded case to be handled correctly.
> 
> But there is a plain request_irq() so the handler is invoked directly in
> hardirq context with disabled interrupts. In forced-threaded context,
> such as on PREEMPT_RT, the IRQF_ONESHOT is added.

 Ah, I missed that IRQF_ONESHOT bit in irq_setup_forced_threading().  So 
it seems like we're good.  I'll proceed with the original change then to 
just amend IOASIC DMA IRQ documentation.  Thank you for patience again.

  Maciej

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH net-next v2] declance: Remove IRQF_ONESHOT
  2026-05-05 14:00                 ` Maciej W. Rozycki
@ 2026-05-05 15:24                   ` Sebastian Andrzej Siewior
  0 siblings, 0 replies; 12+ messages in thread
From: Sebastian Andrzej Siewior @ 2026-05-05 15:24 UTC (permalink / raw)
  To: Maciej W. Rozycki
  Cc: netdev, linux-mips, Jakub Kicinski, Andrew Lunn, David S. Miller,
	Eric Dumazet, Paolo Abeni

On 2026-05-05 15:00:56 [+0100], Maciej W. Rozycki wrote:
> 
>  Ah, I missed that IRQF_ONESHOT bit in irq_setup_forced_threading().  So 
> it seems like we're good.  I'll proceed with the original change then to 
> just amend IOASIC DMA IRQ documentation.  Thank you for patience again.

No worries, you are welcome.
I'm not if sure if you may need to change the primary handler if the
interrupt flow is EOI and cascading based on what you wrote. If you have
access to the HW then you it should be easy to test given the
`threadirqs' argument should expose problems.

>   Maciej

Sebastian

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2026-05-05 15:24 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-27 13:53 [PATCH net-next v2] declance: Remove IRQF_ONESHOT Sebastian Andrzej Siewior
2026-01-27 15:46 ` Maciej W. Rozycki
2026-01-27 16:54   ` Sebastian Andrzej Siewior
2026-01-27 18:35     ` Maciej W. Rozycki
2026-03-29 20:27       ` Maciej W. Rozycki
2026-05-04 22:35         ` Maciej W. Rozycki
2026-05-05  7:29           ` Sebastian Andrzej Siewior
2026-05-05 12:02             ` Maciej W. Rozycki
2026-05-05 12:32               ` Sebastian Andrzej Siewior
2026-05-05 14:00                 ` Maciej W. Rozycki
2026-05-05 15:24                   ` Sebastian Andrzej Siewior
2026-01-29  3:50 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox