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

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
_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 ?
I understand this is a tradeoff.

Thanks.