Hi Bjoern,

This patch should not be needed. In Linux the "handler" is equivalent to the fast IRQ in FreeBSD, but the LinuxKPI lowers it to be the software interrupt. Likewise spinlocks are mutexes under FreeBSD.

You can add the "thread_handler" function pointer, but there is no need for a separate thread for that!

Just implement it like this:

diff --git a/sys/compat/linuxkpi/common/src/linux_compat.c b/sys/compat/linuxkpi/common/src/linux_compat.c
index a8f090ed..f76ba385 100644
--- a/sys/compat/linuxkpi/common/src/linux_compat.c
+++ b/sys/compat/linuxkpi/common/src/linux_compat.c
@@ -2465,7 +2465,8 @@ linux_irq_handler(void *ent)
                return;
 
        irqe = ent;
-       irqe->handler(irqe->irq, irqe->arg);
+       if (irqe->handler(irqe->irq, irqe->arg) == IRQ_WAKE_THREAD)
+               irqe->thread_handler(irqe->irq, irqe->arg);
 }
 
 #if defined(__i386__) || defined(__amd64__)

"fast interrupt" or also called "interrupt filter"

In D30549#686444, @hselasky wrote:

Hi Bjoern,

This patch should not be needed. In Linux the "handler" is equivalent to the fast IRQ in FreeBSD, but the LinuxKPI lowers it to be the software interrupt.

Sorry, I am not sure I follow.

We do the normal bus_alloc_resource_any(..,SYS_RES_IRQ,..); bus_setup_intr(..,ithread,.); there on the BSD device_t.
Where would this be any different from any other FreeBSD device driver?

Whether Linux needs a separate thread there or not is not the question, the question is does our FreeBSD compat code need it?

Driver code does acquire the spinlock (so mtx in our case) in the thread_handler code; and if we do initialize them in linux_spin_lock_init() with MTX_DEF | MTX_NOWITNESS it still means we can sleep, right? So running this code in the ithread is not a good idea ("You can do whatever you want in an ithread routine, except sleep." quoting BUS_SETUP_INTR(9)).

I probably stared at this too long to not see the obvious part I am missing?

In D30549#686692, @bz wrote:

In D30549#686444, @hselasky wrote:

Hi Bjoern,

This patch should not be needed. In Linux the "handler" is equivalent to the fast IRQ in FreeBSD, but the LinuxKPI lowers it to be the software interrupt.

Sorry, I am not sure I follow.

We do the normal bus_alloc_resource_any(..,SYS_RES_IRQ,..); bus_setup_intr(..,ithread,.); there on the BSD device_t.
Where would this be any different from any other FreeBSD device driver?

Whether Linux needs a separate thread there or not is not the question, the question is does our FreeBSD compat code need it?

Driver code does acquire the spinlock (so mtx in our case) in the thread_handler code; and if we do initialize them in linux_spin_lock_init() with MTX_DEF | MTX_NOWITNESS it still means we can sleep, right? So running this code in the ithread is not a good idea ("You can do whatever you want in an ithread routine, except sleep." quoting BUS_SETUP_INTR(9)).

I probably stared at this too long to not see the obvious part I am missing?

Ok that's an as silly statement;... let me re-phrase: can we be sure Linux interrupt handlers/threads never call any code which sleeps? Is that by definition of their "rules"?

Hi Bjoern,

Ok that's an as silly statement;... let me re-phrase: can we be sure Linux interrupt handlers/threads never call any code which sleeps? Is that by definition of their "rules"?

Yes, it can sleep, so simply wrap the callback in the WAKE case with:

THREAD_SLEEPING_OK();
callback goes here;
THREAD_NO_SLEEPING();

And don't create another thread for this stuff!

There is an issue you don't see, and that is that the IRQ itself is masked until the FreeBSD IRQ handler returns. When you create a simple task, the IRQ gets unmasked too early. Linux has special code to do the exact same thing for the task, but this is not possible in FreeBSD.

Software IRQ threads in FreeBSD are simply like other threads. Good practice is however that they don't sleep, but you can make an exception for the WAKE case in the LinuxKPI!

Was that clear, or do you still have questions?

--HPS

Thanks for the explanations!
I'll update it with your suggestions + the additional NULL check for the thread_handler.

FYI: You can optimise the NULL check by installing a dummy handler when NULL is configured there.

Yes, you got it right.