Philippe Gerum wrote:

> On Wed, 2006-08-30 at 15:29 +0200, Jan Kiszka wrote:
>   
>> Dmitry Adamushko wrote:
>>     
>>> On 29/08/06, Jan Kiszka <[EMAIL PROTECTED]> wrote:
>>>       
>>>> Dmitry Adamushko wrote:
>>>>
>>>> I think the additional costs of maintaining an error counter are almost
>>>> negligible. The test is in the unlikely path, and the first condition
>>>> already keeps us away from touching the counter.
>>>>         
>>> But it's updated (unhandled = 0) any time the ISR(s) report something
>>> different from XN_ISR_NONE.  Hence, it's at the beginning of the xnintr_t
>>> structure, hopefully, at the same cache line with other highly-used members
>>> (i.e. isr, cookie and hits).
>>>       
>> Mmh, considering this and also the existing code I wonder if we could
>> optimise this a bit. I'm only looking at xnintr_irq_handler now (sharing
>> is slow anyway): currently the intr object is touched both before
>> (naturally...) and after the call to the ISR handler. Maybe we can push
>> all accesses before the handler for the fast path. E.g.:
>>
>> int unhandled = intr->unhandled;
>>
>> intr->unhandled = 0;
>> ++intr->hits;
>> s = intr->isr(...);
>>
>> if (s == XN_ISR_NONE) {
>>      intr->unhandled = ++unhandled;
>>      if (unhandled == XNINTR_MAX_UNHANDLED)
>>              ALARM!
>> }
>>
>>     
>
> Without speculating whether this could actually reduce cache misses or
> not when the branch is taken, the main issue I see here is that we would
> optimize the error case, at the expense of an additional memory fetching

No, it's the common path. Otherwise, I would have stopped already. I don't
expect further memory access because the head of intr should be cached
already.

> in the common case, and perhaps one CPU register available less for
> processing the normal path in the worst scenario, which would not be
> particularly efficient on x86.
>
>   
At least on x86 it looks good: all local variables are in registers
anyway, unhandled now too. Is someone able to test this? Maybe also
on a non-x86 arch?

Jan


--- ksrc/nucleus/intr.c (revision 1527)
+++ ksrc/nucleus/intr.c (working copy)
@@ -374,6 +374,7 @@ void xnintr_clock_handler(void)
        xnintr_irq_handler(nkclock.irq, &nkclock);
 }
 
+#define XNINTR_MAX_UNHANDLED   1000
 /*
  * Low-level interrupt handler dispatching the ISRs -- Called with
  * interrupts off.
@@ -383,15 +384,28 @@ static void xnintr_irq_handler(unsigned 
 {
        xnsched_t *sched = xnpod_current_sched();
        xnintr_t *intr = (xnintr_t *)cookie;
+       unsigned int unhandled = intr->unhandled;
        int s;
 
        xnarch_memory_barrier();
 
        xnltt_log_event(xeno_ev_ienter, irq);
 
+       intr->unhandled = 0;
+       ++intr->hits;
+
        ++sched->inesting;
        s = intr->isr(intr);
-       ++intr->hits;
+
+       if (unlikely(s == XN_ISR_NONE)) {
+               intr->unhandled = ++unhandled;
+               if (unlikely(unhandled == XNINTR_MAX_UNHANDLED)) {
+                       xnlogerr("xnintr_check_status: %d of unhandled "
+                                " consequent interrupts. Disabling the IRQ "
+                                "line #%d\n", XNINTR_MAX_UNHANDLED, irq);
+                       s |= XN_ISR_NOENABLE;
+               }
+       }
 
        if (s & XN_ISR_PROPAGATE)
                xnarch_chain_irq(irq);



Attachment: signature.asc
Description: OpenPGP digital signature

_______________________________________________
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core

Reply via email to