Re: [Xenomai-core] Potential problem with rt_eepro100

[Jan Kiszka <jan.kiszka@domain.hid>]

[-- Attachment #1: Type: text/plain, Size: 4784 bytes --]

Am 03.11.2010 21:41, Philippe Gerum wrote:
> On Wed, 2010-11-03 at 20:38 +0100, Anders Blomdell wrote:
>> Jan Kiszka wrote:
>>> Am 03.11.2010 17:46, Anders Blomdell wrote:
>>>> Anders Blomdell wrote:
>>>>> Anders Blomdell wrote:
>>>>>> Jan Kiszka wrote:
>>>>>>> additional barrier. Can you check this?
>>>>>>>
>>>>>>> diff --git a/include/nucleus/sched.h b/include/nucleus/sched.h
>>>>>>> index df56417..66b52ad 100644
>>>>>>> --- a/include/nucleus/sched.h
>>>>>>> +++ b/include/nucleus/sched.h
>>>>>>> @@ -187,6 +187,7 @@ static inline int xnsched_self_resched_p(struct 
>>>>>>> xnsched *sched)
>>>>>>>    if (current_sched != (__sched__))    {                \
>>>>>>>        xnarch_cpu_set(xnsched_cpu(__sched__), 
>>>>>>> current_sched->resched);    \
>>>>>>>        setbits((__sched__)->status, XNRESCHED);                \
>>>>>>> +      xnarch_memory_barrier();                        \
>>>>>>>    }                                    \
>>>>>>>  } while (0)
>>>>>> In progress, if nothing breaks before, I'll report status tomorrow 
>>>>>> morning.
>>>>> It still breaks (in approximately the same way). I'm currently putting a 
>>>>> barrier in the other macro doing a RESCHED, also adding some tracing to 
>>>>> see if a read barrier is needed.
>>>> Nope, no luck there either. Will start interesting tracepoint 
>>>> adding/conversion :-(
>>>
>>> Strange. But it was too easy anyway...
>>>
>>>> Any reason why xn_nucleus_sched_remote should ever report status = 0?
>>>
>>> Really don't know yet. You could trigger on this state and call
>>> ftrace_stop() then. Provided you had the functions tracer enabled, that
>>> should give a nice pictures of what happened before.
>>
>> Isn't there a race betweeen these two (still waiting for compilation to 
>> be finished)?
> 
> We always hold the nklock in both contexts.
> 

But we not not always use atomic ops for manipulating status bits (but
we do in other cases where this is no need - different story). This may
fix the race:

diff --git a/ksrc/nucleus/intr.c b/ksrc/nucleus/intr.c
index d7a772f..af8ebeb 100644
--- a/ksrc/nucleus/intr.c
+++ b/ksrc/nucleus/intr.c
@@ -85,7 +85,7 @@ static void xnintr_irq_handler(unsigned irq, void *cookie);
 
 void xnintr_host_tick(struct xnsched *sched) /* Interrupts off. */
 {
-	__clrbits(sched->status, XNHTICK);
+	clrbits(sched->status, XNHTICK);
 	xnarch_relay_tick();
 }
 
@@ -105,11 +105,13 @@ void xnintr_clock_handler(void)
 	trace_mark(xn_nucleus, irq_enter, "irq %u", XNARCH_TIMER_IRQ);
 	trace_mark(xn_nucleus, tbase_tick, "base %s", nktbase.name);
 
+	xnlock_get(&nklock);
+
 	++sched->inesting;
 	__setbits(sched->status, XNINIRQ);
 
-	xnlock_get(&nklock);
 	xntimer_tick_aperiodic();
+
 	xnlock_put(&nklock);
 
 	xnstat_counter_inc(&nkclock.stat[xnsched_cpu(sched)].hits);
@@ -117,7 +119,7 @@ void xnintr_clock_handler(void)
 		&nkclock.stat[xnsched_cpu(sched)].account, start);
 
 	if (--sched->inesting == 0) {
-		__clrbits(sched->status, XNINIRQ);
+		clrbits(sched->status, XNINIRQ);
 		xnpod_schedule();
 	}
 	/*
@@ -178,7 +180,7 @@ static void xnintr_shirq_handler(unsigned irq, void *cookie)
 	trace_mark(xn_nucleus, irq_enter, "irq %u", irq);
 
 	++sched->inesting;
-	__setbits(sched->status, XNINIRQ);
+	setbits(sched->status, XNINIRQ);
 
 	xnlock_get(&shirq->lock);
 	intr = shirq->handlers;
@@ -220,7 +222,7 @@ static void xnintr_shirq_handler(unsigned irq, void *cookie)
 		xnarch_end_irq(irq);
 
 	if (--sched->inesting == 0) {
-		__clrbits(sched->status, XNINIRQ);
+		clrbits(sched->status, XNINIRQ);
 		xnpod_schedule();
 	}
 
@@ -247,7 +249,7 @@ static void xnintr_edge_shirq_handler(unsigned irq, void *cookie)
 	trace_mark(xn_nucleus, irq_enter, "irq %u", irq);
 
 	++sched->inesting;
-	__setbits(sched->status, XNINIRQ);
+	setbits(sched->status, XNINIRQ);
 
 	xnlock_get(&shirq->lock);
 	intr = shirq->handlers;
@@ -303,7 +305,7 @@ static void xnintr_edge_shirq_handler(unsigned irq, void *cookie)
 		xnarch_end_irq(irq);
 
 	if (--sched->inesting == 0) {
-		__clrbits(sched->status, XNINIRQ);
+		clrbits(sched->status, XNINIRQ);
 		xnpod_schedule();
 	}
 	trace_mark(xn_nucleus, irq_exit, "irq %u", irq);
@@ -446,7 +448,7 @@ static void xnintr_irq_handler(unsigned irq, void *cookie)
 	trace_mark(xn_nucleus, irq_enter, "irq %u", irq);
 
 	++sched->inesting;
-	__setbits(sched->status, XNINIRQ);
+	setbits(sched->status, XNINIRQ);
 
 	xnlock_get(&xnirqs[irq].lock);
 
@@ -493,7 +495,7 @@ static void xnintr_irq_handler(unsigned irq, void *cookie)
 		xnarch_end_irq(irq);
 
 	if (--sched->inesting == 0) {
-		__clrbits(sched->status, XNINIRQ);
+		clrbits(sched->status, XNINIRQ);
 		xnpod_schedule();
 	}
 

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 259 bytes --]