Dmitry Adamushko wrote:


On 31/12/05, *Philippe Gerum* <[EMAIL PROTECTED] <mailto:[EMAIL PROTECTED]>> wrote:

 > ...

     > 2) xnintr_irq_handler() gets a cookie which is == xnshared_irq_t*
    (see
     > xnintr_attach) that may already be invalid at that time or, and
    that's a
     > problem, become invalid during the execution of xnintr_irq_handler().
     > To prevent that, we could add a flag like IRQ_INPROGRESS but
     >
     > either we have to set/remove it on the adeos layer before the
    control is
     > passed to the xnintr_irq_handler() (to be sure that cookie is not
     > xnfree()'d. xnintr_detach() will do busy waiting)
     >

    Synchronizing the pending IRQs in ipipe_virtualize_irq() should be done
    by polling the proper irq_pending count (and _not_ the hi/lo bits which
get cleared before the handler is run).

But irq_pending counter (pending_hits in the recent adeos patch) gets cleared before running the isr handler too.


I was referring to __ipipe_lock_irq() which clears the pending bits but keeps the irq count intact to emulate a PIC masking for a given interrupt.

In ipipe_sync_stage() :
...
            if (--cpudata->irq_pending_hits[irq] == 0) {
                 __clear_bit(rank,
                         &cpudata->irq_pending_lo[level]);
                 ipipe_mark_irq_delivery(ipd,irq,cpuid);
            }
(*)

// run the corresponding isr handler

This code drops the irq_pending_hits to 0 before running the handler.


Doesn't it make the following code wrong ?

    for (_cpuid = 0; _cpuid < nr_cpus; _cpuid++)
            while (ipd->cpudata[_cpuid].irq_pending_hits[irq] > 0)
                    cpu_relax();


as the handler still can be running / or even about to be run when ipd->cpudata[_cpuid].irq_pending_hits[irq] is already 0.

We can remove the (*) code at the end of ipipe_sync_stage() so that irq_pending_hits[irq] may become 0 only after the corresponding isr handler has completed.


I don't like the idea of having the interrupt bookkeeping stuff incompletely updated while calling a handler that might sleep and/or migrate to another CPU.

Another option would be to check for SYNC flag; the syncer sets it before entering the dispatch loop.

At the same time, the irq_pending counter can't be increased anymore since we clear the IPIPE_HANDLE_FLAG before doing the busy waiting in ipipe_unregister_domain() or ipipe_virtualize_irq().
    --

    Philippe.


--
Best regards,
Dmitry Adamushko



--

Philippe.

Reply via email to