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

[Christiano F. Haesbaert]

On 22 May 2013 03:39, Sepherosa Ziehau <sepherosa at gmail.com> wrote:
> 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.
>

I see, I hadn't considered it.

>> _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.
>

That answers, thanks.