Comment doesn't seem to match code on kern_intr.c

[Sepherosa Ziehau]

On Tue, May 21, 2013 at 8:30 PM, Christiano F. Haesbaert
<haesbaert at haesbaert.org> wrote:
> Hi,
>
> Not sure if this is the correct mailing list, correct me if not please.
>
> I've been reading the interrupt handling code in dragonfly, and I
> stubled across this comment on kern_intr.c:758
>
> /*
>  * Interrupt threads run this as their main loop.
>  *
>  * The handler begins execution outside a critical section and no MP lock.
>  *
>  * The i_running state starts at 0.  When an interrupt occurs, the hardware
>  * interrupt is disabled and sched_ithd_hard() The HW interrupt remains
>  * disabled until all routines have run.  We then call ithread_done() to
>  * reenable the HW interrupt and deschedule us until the next interrupt.
>  *
>  * We are responsible for atomically checking i_running and ithread_done()
>  * is responsible for atomically checking for platform-specific delayed
>  * interrupts.  i_running for our irq is only set in the context of our cpu,
>  * so a critical section is a sufficient interlock.
>  */
>
> http://gitweb.dragonflybsd.org/dragonfly.git/blame/HEAD:/sys/kern/kern_intr.c
>
> But reading the code makes me believe it in fact runs the handler

It means the ithread_handler() itself, instead of the interrupt
handlers registered by drivers.  But some of the comment in that
comment block is really out-dated; I have just committed a fix.

> _within_ a critical section, as can be seen between lines 800-845. On
> lines 877 and 878 it even does a exit/enter dance which seems to be to
> force a preemption point and maybe run a higher priority thread.
>
> Am I reading something wrong ?
> If not, that would imply that all interrupt handlers are in fact
> nonpreemptible, at least not while running the code itself.
>
> If I may abuse and indulge a second question, it seems that if you
> have 2 interrupts arriving on the same ioapic and on the same pin,
> they end up on the same ithread. So if I got this right, the interrupt
> stub does one "wakeup" and the ithread handles both devices.
>
> Could you share the decision behind this ? Why not have 2 ithreads and
> the interrupt stub wake up them both ?

PCI legacy (line based) interrupts are shared, and they are level
triggered.  When the shared interrupt comes, you can't tell for sure
which hardware actually generates the interrupt, so if you have two
ithreads here, you will have to schedule both of them, thus you will
have extra scheduling cost and it probably gives you no benefit.

Best Regards,
sephe

-- 
Tomorrow Will Never Die