Re: [Adeos-main] Re: [Xenomai-core] I-pipe + latency tracing patch

2005-12-31 Thread Philippe Gerum

Jan Kiszka wrote:

Philippe Gerum wrote:


Jan Kiszka wrote:



Philippe Gerum wrote:



I've just rolled out two patches, the first issue of the 1.1 series for
x86, and the accompanying tracer patch contributed by Jan Kiszka and
Luotao Fu. With the latter patch, the I-pipe shall trace the longest
stalled path of the domain with the highest priority. Apply them in that
order:

http://download.gna.org/adeos/patches/v2.6/adeos/i386/adeos-ipipe-2.6.14-i386-1.1-00.patch


http://download.gna.org/adeos/patches/v2.6/adeos/i386/tracer/ipipe-tracer-2.6.14-i386-1.1-00.patch





Two remarks: First, the tracer patch claims more in its config option
than it actually provides - mea culpa. The patch itself does not contain
any instrumentation of ipipe. This has to be fixed. Meanwhile, please
have a look at this posting for instrumentation options:
https://mail.gna.org/public/xenomai-core/2005-12/msg00076.html

Philippe, do you remember the issues I described about my original
ipipe_trace.instr? How can we avoid too short worst-case traces due to
domain unstalling followed by re-stalling inside the same IRQ context?
Do you see further issues with this approach? I think it would be best
if we can provide a clean CONFIG_IPIPE_TRACE_STALLS for the highest (or
later maybe even for an arbitrary) domain together with the tracer.



+static inline void ipipe_trace_stall(struct ipipe_domain *ipd, int code)
+{
+if (__ipipe_pipeline_head_p(ipd)  (ipd != ipipe_root_domain))
+ipipe_trace_begin(code);
+}
+
+static inline void ipipe_trace_unstall(struct ipipe_domain *ipd, int code)
+{
+if (__ipipe_pipeline_head_p(ipd)  (ipd != ipipe_root_domain))
+ipipe_trace_end(code);
+}

The test is wrong in both case. You need to check that ipd is above or
equal to ipipe_current_domain in the pipeline. To determine that quickly
while tracing, you will probably need to insert an integer giving the
position of each domain into the ipipe_domain struct.



So the orderning in __ipipe_pipeline does say nothing about the priority
of the domain? Then this seems to have worked only by chance so far for me.



Of course it does. The thing is that your test must reflect the fact 
that stalling above and up to the current domain actually blocks the 
interrupts for the latter, and unstalling at least from your current 
domain unblocks them. The position value is just a suggestion to quickly 
compare the effective priority of two domains given their descriptor, 
without being stuck with the uncertainty of ipd-priority which might be 
the same for multiple domains, and without having to scan the pipeline 
top-down.



Anyway, fixing this does not seem to address the other issue I
mentioned. I once also traced the evaluation of those two conditions and
found no indication that this triggers the preliminary end-of-stall
reports I'm facing.



And second, the separation between both patches is not clean. There are
tracer related fragments in the 1.1-00 base patch, intentionally? What's
the idea of the separated patches? I mean, doesn't this increase the
maintenance effort?



It's intentional, those (very few) bits always evaluate to false when
the tracer is not in, and become conditional depending on the value of
CONFIG_IPIPE_TRACE when the support available. IOW, they should be seen
as sleeping hooks serving the purpose of allowing a further optional
extension of the I-pipe.



I see. Then these hooks are intended to keep the effort of breaking up
the patches low.



Yes. Actually, the latency tracer is merged into the Adeos CVS tree on 
top of the core system; it's just my patch release script that splits 
them since they are well separated. The remaining hooks do the necessary 
glue between them.





The key issue here is not about ease of maintenance for us, but rather
about ease of use for the people who don't necessarily want to drag
what's fundamentally a debug infrastructure into the codebase of
production systems, even if it's passive and can be compiled out. Adeos
for x86 is about 151k without tracing, and goes beyond 189k with the
tracer, which is nearly a 20% increase. Add to this that since a latency
tracer is now available for vanilla Linux as an independent patch, it's
likely wiser to allow people to keep the I-pipe tracing facility as a
patch option too, so that you won't create conflicts (e.g. mcount).



Actually, both traces should not collide as long as only one is active
at the same time.



Unfortunately, we can't bet on this for the vanilla kernel part, who 
knows what's going to happen to this support in the future?



Anyway, I already assumed that this more or less psychological aspect of
patch size makes a difference. I don't have a problem with this separation!

Jan



--

Philippe.

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


[Xenomai-core] [PATCH, RFC] nucleus:shared irq and possible ipipe problem

2005-12-31 Thread Dmitry Adamushko

Hi everybody,

I have got a few synch-related problems while adding the code for supporting the rt shared irqs to
the nucleus layer. But at first let's take a look at some adeos code that, well, possibly has the
same problem.

let's say the primary domain is interested in irq = TEST_IRQ.

CPU 0:

- TEST_IRQ occurs.

- ipipe_handle_irq() The local interrupts are off on entry.

 testbit(IPIPE_HANDLE_FLAG,
ipd-irqs[irq].control) shows whether a given domain is interested
in handling the irq.

 Then cpu-local data is mainly used, e.g. cpudata-irq_hits[irq]++ and proper changes of pending_lo/hi
 
 ...
 
CPU 1:

- ... - rthal_irq_release() - ipipe_virtualize_irq(TEST_IRQ, ... handler = NULL, ...)

- Here, __ipipe_pipe_lock + interrupts off.

 o ipipe_virtualize_irq() drops the IPIPE_HANDLE_FLAG and sets up ipd-irqs[irq].handler to NULL.
 
 First observations, at the same time the TEST_IRQ still can be marked as pending
 (i.e. some_cpudata-irq_pending_lo/hi and irq_hits)!

CPU 0:

- actually, ipipe_handle_irq() now (if not yet done before) may be calling __ipipe_set_irq_bit(ipd,cpuid,irq)
 but noone is interested in this irq == TEST_IRQ already. But no matter, 
 the fact is that cpudata-irq_* of a given cpu denotes a pending irq, let's go further.

- later on, ipipe_sync_stage() is called for a given domain and cpu.

 It handles all irqs which are marked for the domain and cpu. So it's based only on the check of
 cpudata-irq_(pending_(hi,lo), hits) fields.
 Let's recall that TEST_IRQ is marked here as well...
 
 In some way (ipipe_call_root_*irq_handler() or
directly ipd-irqs[irq].handler()) the isr handler is called
 and boom! it's NULL.
 

Have I missed something that prevents this?

-

In a sense, the synch. problem I have mentioned at the beginning of this mail reminds this scenario.

The draft patch is enclosed just to elustrate an idea.

There are 2 problems:

1) we probably don't want to hold any lock while processing the
handlers list (xnintr_intr_handler(): for all shirq-handlers).

Here we may use the approach used by linux in manage.c::free_irq() vs.
handle.c::__do_IRQ() that calls handle_IRQ_event() without the
desc-lock being locked.
The magic is in free_irq() : it removes the action item from the list
but then falls into busy waiting until the IRQ_INPROGRESS flag is
removed. And only then it deletes the action item.
At that time, the action item is already not on the list but still
points to the valid tail of the list so the iteration may be proceeded
even if the current item == deleted one.
Blah, just I guess the presupposition here is that the operation of deletion (free_irq():: *pp = action-next) is atomic.

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)

or we may set/remove the flag in the xnintr_irq_handler() but have to ignore the passed cookie and 
get it as cookie = rthal_irq_cookie(ipd,irq). Mmm, not very gracefully I'd say.

Ok, it's enough for the New Year's Eve.

Happy New Year to everybody! I wish you all the best for the New Year :o)

-- Best regards,Dmitry Adamushko





nucleus_intr.c.patch
Description: Binary data


nucleus_intr.h.patch
Description: Binary data
___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core


[Xenomai-core] Re: [PATCH, RFC] nucleus:shared irq and possible ipipe problem

2005-12-31 Thread Philippe Gerum

Dmitry Adamushko wrote:


Hi everybody,

I have got a few synch-related problems while adding the code for 
supporting the rt shared irqs to
the nucleus layer. But at first let's take a look at some adeos code 
that, well, possibly has the

same problem.

let's say the primary domain is interested in irq = TEST_IRQ.

CPU 0:

-  TEST_IRQ occurs.

-  ipipe_handle_irq()The local interrupts are off on entry.

testbit(IPIPE_HANDLE_FLAG, ipd-irqs[irq].control) shows whether a 
given domain is interested in handling the irq.


Then cpu-local data is mainly used, e.g. cpudata-irq_hits[irq]++ 
and proper changes of pending_lo/hi
   
...
   
CPU 1:


-  ... - rthal_irq_release() -  ipipe_virtualize_irq(TEST_IRQ, ... 
handler = NULL, ...)


-  Here, __ipipe_pipe_lock + interrupts off.

o  ipipe_virtualize_irq() drops the IPIPE_HANDLE_FLAG and sets up 
ipd-irqs[irq].handler to NULL.
   
First observations, at the same time the TEST_IRQ still can be 
marked as pending

(i.e. some_cpudata-irq_pending_lo/hi and irq_hits)!

CPU 0:

-  actually, ipipe_handle_irq() now (if not yet done before) may be 
calling __ipipe_set_irq_bit(ipd,cpuid,irq)

but noone is interested in this irq == TEST_IRQ already. But no matter,
the fact is that cpudata-irq_* of a given cpu denotes a pending 
irq, let's go further.


-  later on, ipipe_sync_stage() is called for a given domain and cpu.

It handles all irqs which are marked for the domain and cpu. So it's 
based only on the check of

cpudata-irq_(pending_(hi,lo), hits) fields.
Let's recall that TEST_IRQ is marked here as well...
   
In some way (ipipe_call_root_*irq_handler() or directly 
ipd-irqs[irq].handler()) the isr handler is called

and boom! it's NULL.
   


Have I missed something that prevents this?



Nope, good spot, that could indeed happen in SMP configs. The code is 
expected to shut the given interrupt source _before_ calling 
rthal_irq_release(). But, the root of the problem is that 
rthal_irq_release() doesn't make sure that all _pending_ IRQs from the 
given kind are synchronized before processing. We need the equivalent of 
Linux's synchronize_irq() here, and I would tend to implement this in 
the Adeos layer directly, in ipipe_virtualize_irq() for the NULL handler 
case, since it's a matter of general consistency.



-

In a sense, the synch. problem I have mentioned at the beginning of this 
mail reminds this scenario.


The draft patch is enclosed just to elustrate an idea.

There are 2 problems:

1) we probably don't want to hold any lock while processing the handlers 
list (xnintr_intr_handler(): for all shirq-handlers).


Here we may use the approach used by linux in manage.c::free_irq() vs. 
handle.c::__do_IRQ() that calls handle_IRQ_event() without the 
desc-lock being locked.
The magic is in free_irq() : it removes the action item from the list 
but then falls into busy waiting until the IRQ_INPROGRESS flag is 
removed. And only then it deletes the action item.
At that time, the action item is already not on the list but still 
points to the valid tail of the list so the iteration may be proceeded 
even if the current item == deleted one.
Blah, just I guess the presupposition here is that the operation of 
deletion (free_irq():: *pp = action-next) is atomic.


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). The prerequisite is to call 
ipipe_virtualize_irq() for an unstalled domain, or at least forcibly 
unstalling it there. I would see something along these lines, which is 
already used to drain the pending IRQs before unlinking a domain from 
the pipeline:


spin_unlock_irqrestore_hw(__ipipe_pipelock, flags);

ipipe_unstall_pipeline_from(ipd);

clear_bit(IPIPE_HANDLE_FLAG, ipd-irqs[irq].control);
clear_bit(IPIPE_STICKY_FLAG, ipd-irqs[irq].control);
set_bit(IPIPE_PASS_FLAG, ipd-irqs[irq].control);
/* or tweak the modemask directly */

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

spin_lock_irqsave_hw(__ipipe_pipelock, flags);

or we may set/remove the flag in the xnintr_irq_handler() but have to 
ignore the passed cookie and
get it as cookie =  rthal_irq_cookie(ipd,irq). Mmm, not very gracefully 
I'd say.


Eeek...



Ok, it's enough for the New Year's Eve.

Happy New Year to everybody! I wish you all the best for the 

[Xenomai-core] [PATCH, RFC] nucleus:shared irq and possible ipipe problem

2005-12-31 Thread Dmitry Adamushko

Hi everybody,

I have got a few synch-related problems while adding the code for supporting the rt shared irqs to
the nucleus layer. But at first let's take a look at some adeos code that, well, possibly has the
same problem.

let's say the primary domain is interested in irq = TEST_IRQ.

CPU 0:

- TEST_IRQ occurs.

- ipipe_handle_irq() The local interrupts are off on entry.

 testbit(IPIPE_HANDLE_FLAG,
ipd-irqs[irq].control) shows whether a given domain is interested
in handling the irq.

 Then cpu-local data is mainly used, e.g. cpudata-irq_hits[irq]++ and proper changes of pending_lo/hi
 
 ...
 
CPU 1:

- ... - rthal_irq_release() - ipipe_virtualize_irq(TEST_IRQ, ... handler = NULL, ...)

- Here, __ipipe_pipe_lock + interrupts off.

 o ipipe_virtualize_irq() drops the IPIPE_HANDLE_FLAG and sets up ipd-irqs[irq].handler to NULL.
 
 First observations, at the same time the TEST_IRQ still can be marked as pending
 (i.e. some_cpudata-irq_pending_lo/hi and irq_hits)!

CPU 0:

- actually, ipipe_handle_irq() now (if not yet done before) may be calling __ipipe_set_irq_bit(ipd,cpuid,irq)
 but noone is interested in this irq == TEST_IRQ already. But no matter, 
 the fact is that cpudata-irq_* of a given cpu denotes a pending irq, let's go further.

- later on, ipipe_sync_stage() is called for a given domain and cpu.

 It handles all irqs which are marked for the domain and cpu. So it's based only on the check of
 cpudata-irq_(pending_(hi,lo), hits) fields.
 Let's recall that TEST_IRQ is marked here as well...
 
 In some way (ipipe_call_root_*irq_handler() or
directly ipd-irqs[irq].handler()) the isr handler is called
 and boom! it's NULL.
 

Have I missed something that prevents this?

-

In a sense, the synch. problem I have mentioned at the beginning of this mail reminds this scenario.

The draft patch is enclosed just to elustrate an idea.

There are 2 problems:

1) we probably don't want to hold any lock while processing the
handlers list (xnintr_intr_handler(): for all shirq-handlers).

Here we may use the approach used by linux in manage.c::free_irq() vs.
handle.c::__do_IRQ() that calls handle_IRQ_event() without the
desc-lock being locked.
The magic is in free_irq() : it removes the action item from the list
but then falls into busy waiting until the IRQ_INPROGRESS flag is
removed. And only then it deletes the action item.
At that time, the action item is already not on the list but still
points to the valid tail of the list so the iteration may be proceeded
even if the current item == deleted one.
Blah, just I guess the presupposition here is that the operation of deletion (free_irq():: *pp = action-next) is atomic.

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)

or we may set/remove the flag in the xnintr_irq_handler() but have to ignore the passed cookie and 
get it as cookie = rthal_irq_cookie(ipd,irq). Mmm, not very gracefully I'd say.

Ok, it's enough for the New Year's Eve.

Happy New Year to everybody! I wish you all the best for the New Year :o)

-- Best regards,Dmitry Adamushko





nucleus_intr.c.patch
Description: Binary data


nucleus_intr.h.patch
Description: Binary data


[Xenomai-core] Re: [PATCH, RFC] nucleus:shared irq and possible ipipe problem

2005-12-31 Thread Philippe Gerum

Dmitry Adamushko wrote:


Hi everybody,

I have got a few synch-related problems while adding the code for 
supporting the rt shared irqs to
the nucleus layer. But at first let's take a look at some adeos code 
that, well, possibly has the

same problem.

let's say the primary domain is interested in irq = TEST_IRQ.

CPU 0:

-  TEST_IRQ occurs.

-  ipipe_handle_irq()The local interrupts are off on entry.

testbit(IPIPE_HANDLE_FLAG, ipd-irqs[irq].control) shows whether a 
given domain is interested in handling the irq.


Then cpu-local data is mainly used, e.g. cpudata-irq_hits[irq]++ 
and proper changes of pending_lo/hi
   
...
   
CPU 1:


-  ... - rthal_irq_release() -  ipipe_virtualize_irq(TEST_IRQ, ... 
handler = NULL, ...)


-  Here, __ipipe_pipe_lock + interrupts off.

o  ipipe_virtualize_irq() drops the IPIPE_HANDLE_FLAG and sets up 
ipd-irqs[irq].handler to NULL.
   
First observations, at the same time the TEST_IRQ still can be 
marked as pending

(i.e. some_cpudata-irq_pending_lo/hi and irq_hits)!

CPU 0:

-  actually, ipipe_handle_irq() now (if not yet done before) may be 
calling __ipipe_set_irq_bit(ipd,cpuid,irq)

but noone is interested in this irq == TEST_IRQ already. But no matter,
the fact is that cpudata-irq_* of a given cpu denotes a pending 
irq, let's go further.


-  later on, ipipe_sync_stage() is called for a given domain and cpu.

It handles all irqs which are marked for the domain and cpu. So it's 
based only on the check of

cpudata-irq_(pending_(hi,lo), hits) fields.
Let's recall that TEST_IRQ is marked here as well...
   
In some way (ipipe_call_root_*irq_handler() or directly 
ipd-irqs[irq].handler()) the isr handler is called

and boom! it's NULL.
   


Have I missed something that prevents this?



Nope, good spot, that could indeed happen in SMP configs. The code is 
expected to shut the given interrupt source _before_ calling 
rthal_irq_release(). But, the root of the problem is that 
rthal_irq_release() doesn't make sure that all _pending_ IRQs from the 
given kind are synchronized before processing. We need the equivalent of 
Linux's synchronize_irq() here, and I would tend to implement this in 
the Adeos layer directly, in ipipe_virtualize_irq() for the NULL handler 
case, since it's a matter of general consistency.



-

In a sense, the synch. problem I have mentioned at the beginning of this 
mail reminds this scenario.


The draft patch is enclosed just to elustrate an idea.

There are 2 problems:

1) we probably don't want to hold any lock while processing the handlers 
list (xnintr_intr_handler(): for all shirq-handlers).


Here we may use the approach used by linux in manage.c::free_irq() vs. 
handle.c::__do_IRQ() that calls handle_IRQ_event() without the 
desc-lock being locked.
The magic is in free_irq() : it removes the action item from the list 
but then falls into busy waiting until the IRQ_INPROGRESS flag is 
removed. And only then it deletes the action item.
At that time, the action item is already not on the list but still 
points to the valid tail of the list so the iteration may be proceeded 
even if the current item == deleted one.
Blah, just I guess the presupposition here is that the operation of 
deletion (free_irq():: *pp = action-next) is atomic.


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). The prerequisite is to call 
ipipe_virtualize_irq() for an unstalled domain, or at least forcibly 
unstalling it there. I would see something along these lines, which is 
already used to drain the pending IRQs before unlinking a domain from 
the pipeline:


spin_unlock_irqrestore_hw(__ipipe_pipelock, flags);

ipipe_unstall_pipeline_from(ipd);

clear_bit(IPIPE_HANDLE_FLAG, ipd-irqs[irq].control);
clear_bit(IPIPE_STICKY_FLAG, ipd-irqs[irq].control);
set_bit(IPIPE_PASS_FLAG, ipd-irqs[irq].control);
/* or tweak the modemask directly */

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

spin_lock_irqsave_hw(__ipipe_pipelock, flags);

or we may set/remove the flag in the xnintr_irq_handler() but have to 
ignore the passed cookie and
get it as cookie =  rthal_irq_cookie(ipd,irq). Mmm, not very gracefully 
I'd say.


Eeek...



Ok, it's enough for the New Year's Eve.

Happy New Year to everybody! I wish you all the best for the 

Re: [Adeos-main] Re: [Xenomai-core] I-pipe + latency tracing patch

2005-12-31 Thread Philippe Gerum

Jan Kiszka wrote:

Philippe Gerum wrote:


Jan Kiszka wrote:



Philippe Gerum wrote:



I've just rolled out two patches, the first issue of the 1.1 series for
x86, and the accompanying tracer patch contributed by Jan Kiszka and
Luotao Fu. With the latter patch, the I-pipe shall trace the longest
stalled path of the domain with the highest priority. Apply them in that
order:

http://download.gna.org/adeos/patches/v2.6/adeos/i386/adeos-ipipe-2.6.14-i386-1.1-00.patch


http://download.gna.org/adeos/patches/v2.6/adeos/i386/tracer/ipipe-tracer-2.6.14-i386-1.1-00.patch





Two remarks: First, the tracer patch claims more in its config option
than it actually provides - mea culpa. The patch itself does not contain
any instrumentation of ipipe. This has to be fixed. Meanwhile, please
have a look at this posting for instrumentation options:
https://mail.gna.org/public/xenomai-core/2005-12/msg00076.html

Philippe, do you remember the issues I described about my original
ipipe_trace.instr? How can we avoid too short worst-case traces due to
domain unstalling followed by re-stalling inside the same IRQ context?
Do you see further issues with this approach? I think it would be best
if we can provide a clean CONFIG_IPIPE_TRACE_STALLS for the highest (or
later maybe even for an arbitrary) domain together with the tracer.



+static inline void ipipe_trace_stall(struct ipipe_domain *ipd, int code)
+{
+if (__ipipe_pipeline_head_p(ipd)  (ipd != ipipe_root_domain))
+ipipe_trace_begin(code);
+}
+
+static inline void ipipe_trace_unstall(struct ipipe_domain *ipd, int code)
+{
+if (__ipipe_pipeline_head_p(ipd)  (ipd != ipipe_root_domain))
+ipipe_trace_end(code);
+}

The test is wrong in both case. You need to check that ipd is above or
equal to ipipe_current_domain in the pipeline. To determine that quickly
while tracing, you will probably need to insert an integer giving the
position of each domain into the ipipe_domain struct.



So the orderning in __ipipe_pipeline does say nothing about the priority
of the domain? Then this seems to have worked only by chance so far for me.



Of course it does. The thing is that your test must reflect the fact 
that stalling above and up to the current domain actually blocks the 
interrupts for the latter, and unstalling at least from your current 
domain unblocks them. The position value is just a suggestion to quickly 
compare the effective priority of two domains given their descriptor, 
without being stuck with the uncertainty of ipd-priority which might be 
the same for multiple domains, and without having to scan the pipeline 
top-down.



Anyway, fixing this does not seem to address the other issue I
mentioned. I once also traced the evaluation of those two conditions and
found no indication that this triggers the preliminary end-of-stall
reports I'm facing.



And second, the separation between both patches is not clean. There are
tracer related fragments in the 1.1-00 base patch, intentionally? What's
the idea of the separated patches? I mean, doesn't this increase the
maintenance effort?



It's intentional, those (very few) bits always evaluate to false when
the tracer is not in, and become conditional depending on the value of
CONFIG_IPIPE_TRACE when the support available. IOW, they should be seen
as sleeping hooks serving the purpose of allowing a further optional
extension of the I-pipe.



I see. Then these hooks are intended to keep the effort of breaking up
the patches low.



Yes. Actually, the latency tracer is merged into the Adeos CVS tree on 
top of the core system; it's just my patch release script that splits 
them since they are well separated. The remaining hooks do the necessary 
glue between them.





The key issue here is not about ease of maintenance for us, but rather
about ease of use for the people who don't necessarily want to drag
what's fundamentally a debug infrastructure into the codebase of
production systems, even if it's passive and can be compiled out. Adeos
for x86 is about 151k without tracing, and goes beyond 189k with the
tracer, which is nearly a 20% increase. Add to this that since a latency
tracer is now available for vanilla Linux as an independent patch, it's
likely wiser to allow people to keep the I-pipe tracing facility as a
patch option too, so that you won't create conflicts (e.g. mcount).



Actually, both traces should not collide as long as only one is active
at the same time.



Unfortunately, we can't bet on this for the vanilla kernel part, who 
knows what's going to happen to this support in the future?



Anyway, I already assumed that this more or less psychological aspect of
patch size makes a difference. I don't have a problem with this separation!

Jan



--

Philippe.