* [Adeos-main] Unchecked ipipe_test_pipeline_from
@ 2009-11-09 18:07 Jan Kiszka
2009-11-09 18:12 ` Gilles Chanteperdrix
0 siblings, 1 reply; 8+ messages in thread
From: Jan Kiszka @ 2009-11-09 18:07 UTC (permalink / raw)
To: Philippe Gerum; +Cc: adeos-main
Hi Philippe,
just noticed: The __ipipe_check_percpu_access of __ipipe_get_cpu_var,
added in 2.6.29, makes ipipe_test_pipeline_from unusable for debugging
purposes. It now triggers a false positive warning if the caller did not
disabled interrupts or stalled its pipeline. One such user under Xenomai
is rthal_local_irq_disabled, and that is used to check RTDM driver
handlers /wrt leaking IRQ masks.
Introduce a debug-free ipipe_check_pipeline_from for
rthal_local_irq_disabled?
Jan
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Adeos-main] Unchecked ipipe_test_pipeline_from
2009-11-09 18:07 [Adeos-main] Unchecked ipipe_test_pipeline_from Jan Kiszka
@ 2009-11-09 18:12 ` Gilles Chanteperdrix
2009-11-09 18:25 ` Jan Kiszka
0 siblings, 1 reply; 8+ messages in thread
From: Gilles Chanteperdrix @ 2009-11-09 18:12 UTC (permalink / raw)
To: Jan Kiszka; +Cc: adeos-main, Philippe Gerum
Jan Kiszka wrote:
> Hi Philippe,
>
> just noticed: The __ipipe_check_percpu_access of __ipipe_get_cpu_var,
> added in 2.6.29, makes ipipe_test_pipeline_from unusable for debugging
> purposes. It now triggers a false positive warning if the caller did not
> disabled interrupts or stalled its pipeline. One such user under Xenomai
> is rthal_local_irq_disabled, and that is used to check RTDM driver
> handlers /wrt leaking IRQ masks.
It does not look like a false positive. If the task issuing the call to
rthal_local_irq_disabled function was migrated at the wrong time, it
could check the stall flag on the wrong cpu. So, it looks like
rthal_local_irq_disabled should be fixed to turn off irqs during the check.
--
Gilles
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Adeos-main] Unchecked ipipe_test_pipeline_from
2009-11-09 18:12 ` Gilles Chanteperdrix
@ 2009-11-09 18:25 ` Jan Kiszka
2009-11-09 18:40 ` Gilles Chanteperdrix
0 siblings, 1 reply; 8+ messages in thread
From: Jan Kiszka @ 2009-11-09 18:25 UTC (permalink / raw)
To: Gilles Chanteperdrix; +Cc: adeos-main, Philippe Gerum
Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Hi Philippe,
>>
>> just noticed: The __ipipe_check_percpu_access of __ipipe_get_cpu_var,
>> added in 2.6.29, makes ipipe_test_pipeline_from unusable for debugging
>> purposes. It now triggers a false positive warning if the caller did not
>> disabled interrupts or stalled its pipeline. One such user under Xenomai
>> is rthal_local_irq_disabled, and that is used to check RTDM driver
>> handlers /wrt leaking IRQ masks.
>
> It does not look like a false positive. If the task issuing the call to
> rthal_local_irq_disabled function was migrated at the wrong time, it
> could check the stall flag on the wrong cpu.
Unless you want to test the migration logic itself, a plain task in
whatever domain should never see a CPU-depend rthal_local_irq_disabled -
migration should never alter the context in this respect.
> So, it looks like
> rthal_local_irq_disabled should be fixed to turn off irqs during the check.
>
Would work, but would also be more heavy-weighted then needed.
Jan
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Adeos-main] Unchecked ipipe_test_pipeline_from
2009-11-09 18:25 ` Jan Kiszka
@ 2009-11-09 18:40 ` Gilles Chanteperdrix
2009-11-09 18:50 ` Jan Kiszka
0 siblings, 1 reply; 8+ messages in thread
From: Gilles Chanteperdrix @ 2009-11-09 18:40 UTC (permalink / raw)
To: Jan Kiszka; +Cc: adeos-main, Philippe Gerum
Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> Hi Philippe,
>>>
>>> just noticed: The __ipipe_check_percpu_access of __ipipe_get_cpu_var,
>>> added in 2.6.29, makes ipipe_test_pipeline_from unusable for debugging
>>> purposes. It now triggers a false positive warning if the caller did not
>>> disabled interrupts or stalled its pipeline. One such user under Xenomai
>>> is rthal_local_irq_disabled, and that is used to check RTDM driver
>>> handlers /wrt leaking IRQ masks.
>> It does not look like a false positive. If the task issuing the call to
>> rthal_local_irq_disabled function was migrated at the wrong time, it
>> could check the stall flag on the wrong cpu.
>
> Unless you want to test the migration logic itself, a plain task in
> whatever domain should never see a CPU-depend rthal_local_irq_disabled -
> migration should never alter the context in this respect.
>
>> So, it looks like
>> rthal_local_irq_disabled should be fixed to turn off irqs during the check.
>>
>
> Would work, but would also be more heavy-weighted then needed.
If it is debug stuff, does the heavy-weight matter that much?
--
Gilles
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Adeos-main] Unchecked ipipe_test_pipeline_from
2009-11-09 18:40 ` Gilles Chanteperdrix
@ 2009-11-09 18:50 ` Jan Kiszka
2009-11-09 23:01 ` Philippe Gerum
0 siblings, 1 reply; 8+ messages in thread
From: Jan Kiszka @ 2009-11-09 18:50 UTC (permalink / raw)
To: Gilles Chanteperdrix; +Cc: adeos-main, Philippe Gerum
Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Jan Kiszka wrote:
>>>> Hi Philippe,
>>>>
>>>> just noticed: The __ipipe_check_percpu_access of __ipipe_get_cpu_var,
>>>> added in 2.6.29, makes ipipe_test_pipeline_from unusable for debugging
>>>> purposes. It now triggers a false positive warning if the caller did not
>>>> disabled interrupts or stalled its pipeline. One such user under Xenomai
>>>> is rthal_local_irq_disabled, and that is used to check RTDM driver
>>>> handlers /wrt leaking IRQ masks.
>>> It does not look like a false positive. If the task issuing the call to
>>> rthal_local_irq_disabled function was migrated at the wrong time, it
>>> could check the stall flag on the wrong cpu.
>> Unless you want to test the migration logic itself, a plain task in
>> whatever domain should never see a CPU-depend rthal_local_irq_disabled -
>> migration should never alter the context in this respect.
>>
>>> So, it looks like
>>> rthal_local_irq_disabled should be fixed to turn off irqs during the check.
>>>
>> Would work, but would also be more heavy-weighted then needed.
>
> If it is debug stuff, does the heavy-weight matter that much?
>
Actually, I'm no longer sure that a preemption check for
ipipe_stall_pipeline_from makes sense at all. Am I missing some case?
Jan
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Adeos-main] Unchecked ipipe_test_pipeline_from
2009-11-09 18:50 ` Jan Kiszka
@ 2009-11-09 23:01 ` Philippe Gerum
2009-11-09 23:10 ` Jan Kiszka
0 siblings, 1 reply; 8+ messages in thread
From: Philippe Gerum @ 2009-11-09 23:01 UTC (permalink / raw)
To: Jan Kiszka; +Cc: adeos-main
On Mon, 2009-11-09 at 19:50 +0100, Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
> > Jan Kiszka wrote:
> >> Gilles Chanteperdrix wrote:
> >>> Jan Kiszka wrote:
> >>>> Hi Philippe,
> >>>>
> >>>> just noticed: The __ipipe_check_percpu_access of __ipipe_get_cpu_var,
> >>>> added in 2.6.29, makes ipipe_test_pipeline_from unusable for debugging
> >>>> purposes. It now triggers a false positive warning if the caller did not
> >>>> disabled interrupts or stalled its pipeline. One such user under Xenomai
> >>>> is rthal_local_irq_disabled, and that is used to check RTDM driver
> >>>> handlers /wrt leaking IRQ masks.
> >>> It does not look like a false positive. If the task issuing the call to
> >>> rthal_local_irq_disabled function was migrated at the wrong time, it
> >>> could check the stall flag on the wrong cpu.
> >> Unless you want to test the migration logic itself, a plain task in
> >> whatever domain should never see a CPU-depend rthal_local_irq_disabled -
> >> migration should never alter the context in this respect.
> >>
A plain task over the root domain which is subject to a migration may
end up testing the stall bit on the wrong (dest) CPU, which may execute
a different domain than the one running on the source CPU. As soon as
the domain is not a context invariant, you do have a serious issue
coming up. So this does matter a lot, as the bug fixed some time ago in
ipipe_check_context() showed.
> >>> So, it looks like
> >>> rthal_local_irq_disabled should be fixed to turn off irqs during the check.
> >>>
> >> Would work, but would also be more heavy-weighted then needed.
> >
> > If it is debug stuff, does the heavy-weight matter that much?
> >
>
> Actually, I'm no longer sure that a preemption check for
> ipipe_stall_pipeline_from makes sense at all. Am I missing some case?
>
Same as above. Albeit there may not be forced CPU migration over the
Xenomai domain, this is perfectly possible on the root one.
> Jan
>
--
Philippe.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Adeos-main] Unchecked ipipe_test_pipeline_from
2009-11-09 23:01 ` Philippe Gerum
@ 2009-11-09 23:10 ` Jan Kiszka
2009-11-10 10:27 ` Philippe Gerum
0 siblings, 1 reply; 8+ messages in thread
From: Jan Kiszka @ 2009-11-09 23:10 UTC (permalink / raw)
To: Philippe Gerum; +Cc: adeos-main
[-- Attachment #1: Type: text/plain, Size: 2145 bytes --]
Philippe Gerum wrote:
> On Mon, 2009-11-09 at 19:50 +0100, Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Jan Kiszka wrote:
>>>> Gilles Chanteperdrix wrote:
>>>>> Jan Kiszka wrote:
>>>>>> Hi Philippe,
>>>>>>
>>>>>> just noticed: The __ipipe_check_percpu_access of __ipipe_get_cpu_var,
>>>>>> added in 2.6.29, makes ipipe_test_pipeline_from unusable for debugging
>>>>>> purposes. It now triggers a false positive warning if the caller did not
>>>>>> disabled interrupts or stalled its pipeline. One such user under Xenomai
>>>>>> is rthal_local_irq_disabled, and that is used to check RTDM driver
>>>>>> handlers /wrt leaking IRQ masks.
>>>>> It does not look like a false positive. If the task issuing the call to
>>>>> rthal_local_irq_disabled function was migrated at the wrong time, it
>>>>> could check the stall flag on the wrong cpu.
>>>> Unless you want to test the migration logic itself, a plain task in
>>>> whatever domain should never see a CPU-depend rthal_local_irq_disabled -
>>>> migration should never alter the context in this respect.
>>>>
>
> A plain task over the root domain which is subject to a migration may
> end up testing the stall bit on the wrong (dest) CPU, which may execute
> a different domain than the one running on the source CPU. As soon as
Yeah, makes sense now (with source and dest swapped, though).
> the domain is not a context invariant, you do have a serious issue
> coming up. So this does matter a lot, as the bug fixed some time ago in
> ipipe_check_context() showed.
>
>>>>> So, it looks like
>>>>> rthal_local_irq_disabled should be fixed to turn off irqs during the check.
>>>>>
>>>> Would work, but would also be more heavy-weighted then needed.
>>> If it is debug stuff, does the heavy-weight matter that much?
>>>
>> Actually, I'm no longer sure that a preemption check for
>> ipipe_stall_pipeline_from makes sense at all. Am I missing some case?
>>
>
> Same as above. Albeit there may not be forced CPU migration over the
> Xenomai domain, this is perfectly possible on the root one.
OK, will fix the checks in RTDM then.
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Adeos-main] Unchecked ipipe_test_pipeline_from
2009-11-09 23:10 ` Jan Kiszka
@ 2009-11-10 10:27 ` Philippe Gerum
0 siblings, 0 replies; 8+ messages in thread
From: Philippe Gerum @ 2009-11-10 10:27 UTC (permalink / raw)
To: Jan Kiszka; +Cc: adeos-main
On Tue, 2009-11-10 at 00:10 +0100, Jan Kiszka wrote:
> Philippe Gerum wrote:
> > On Mon, 2009-11-09 at 19:50 +0100, Jan Kiszka wrote:
> >> Gilles Chanteperdrix wrote:
> >>> Jan Kiszka wrote:
> >>>> Gilles Chanteperdrix wrote:
> >>>>> Jan Kiszka wrote:
> >>>>>> Hi Philippe,
> >>>>>>
> >>>>>> just noticed: The __ipipe_check_percpu_access of __ipipe_get_cpu_var,
> >>>>>> added in 2.6.29, makes ipipe_test_pipeline_from unusable for debugging
> >>>>>> purposes. It now triggers a false positive warning if the caller did not
> >>>>>> disabled interrupts or stalled its pipeline. One such user under Xenomai
> >>>>>> is rthal_local_irq_disabled, and that is used to check RTDM driver
> >>>>>> handlers /wrt leaking IRQ masks.
> >>>>> It does not look like a false positive. If the task issuing the call to
> >>>>> rthal_local_irq_disabled function was migrated at the wrong time, it
> >>>>> could check the stall flag on the wrong cpu.
> >>>> Unless you want to test the migration logic itself, a plain task in
> >>>> whatever domain should never see a CPU-depend rthal_local_irq_disabled -
> >>>> migration should never alter the context in this respect.
> >>>>
> >
> > A plain task over the root domain which is subject to a migration may
> > end up testing the stall bit on the wrong (dest) CPU, which may execute
> > a different domain than the one running on the source CPU. As soon as
>
> Yeah, makes sense now (with source and dest swapped, though).
Yes, I got it backward for the src/dst mention. The issue is indeed that
the address of the status word on src gets computed before a migration
is triggered to dst, so the action ends up being wrongly applied to src.
>
> > the domain is not a context invariant, you do have a serious issue
> > coming up. So this does matter a lot, as the bug fixed some time ago in
> > ipipe_check_context() showed.
> >
> >>>>> So, it looks like
> >>>>> rthal_local_irq_disabled should be fixed to turn off irqs during the check.
> >>>>>
> >>>> Would work, but would also be more heavy-weighted then needed.
> >>> If it is debug stuff, does the heavy-weight matter that much?
> >>>
> >> Actually, I'm no longer sure that a preemption check for
> >> ipipe_stall_pipeline_from makes sense at all. Am I missing some case?
> >>
> >
> > Same as above. Albeit there may not be forced CPU migration over the
> > Xenomai domain, this is perfectly possible on the root one.
>
> OK, will fix the checks in RTDM then.
>
> Jan
>
> _______________________________________________
> Adeos-main mailing list
> Adeos-main@domain.hid
> https://mail.gna.org/listinfo/adeos-main
--
Philippe.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2009-11-10 10:27 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-11-09 18:07 [Adeos-main] Unchecked ipipe_test_pipeline_from Jan Kiszka
2009-11-09 18:12 ` Gilles Chanteperdrix
2009-11-09 18:25 ` Jan Kiszka
2009-11-09 18:40 ` Gilles Chanteperdrix
2009-11-09 18:50 ` Jan Kiszka
2009-11-09 23:01 ` Philippe Gerum
2009-11-09 23:10 ` Jan Kiszka
2009-11-10 10:27 ` Philippe Gerum
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.