From mboxrd@z Thu Jan 1 00:00:00 1970 Message-ID: Date: Sat, 7 Jan 2006 21:05:06 +0200 From: Dmitry Adamushko In-Reply-To: <43B69CEC.1040606@domain.hid> MIME-Version: 1.0 Content-Type: multipart/alternative; boundary="----=_Part_3408_23838151.1136660706742" References: <43B69CEC.1040606@domain.hid> Subject: [Xenomai-core] Re: [PATCH, RFC] nucleus:shared irq and possible ipipe problem List-Id: "Xenomai life and development \(bug reports, patches, discussions\)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Philippe Gerum Cc: xenomai@xenomai.org ------=_Part_3408_23838151.1136660706742 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Content-Disposition: inline On 31/12/05, Philippe Gerum wrote: > ... > 2) xnintr_irq_handler() gets a cookie which is =3D=3D xnshared_irq_t* (se= e > > 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 i= s > > 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. In ipipe_sync_stage() : ... if (--cpudata->irq_pending_hits[irq] =3D=3D 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 =3D 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. 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 ------=_Part_3408_23838151.1136660706742 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Content-Disposition: inline

On 31/12/05, Philippe Gerum <rpm@xenomai.org= mai.org> wrote:

> ...

> 2)= xnintr_irq_handler() gets a cookie which is =3D=3D 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<= br>> xnfree()'d. xnintr_detach() will do busy waiting)
>

Sy= nchronizing 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 clear= ed before running the isr handler too.

In ipipe_sync_stage() :
...
            if (--cpudata->= irq_pending_hits[irq] =3D=3D 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= =3D 0; _cpuid < nr_cpus; _cpuid++)
     &nb= sp;  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.

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().
 

--

P= hilippe.

--
Best regards,
Dmitry Adamushko

------=_Part_3408_23838151.1136660706742--