Re: [PATCH net-next v2] declance: Remove IRQF_ONESHOT

Maciej W. Rozycki
2026-01-27T15:46:12Z

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