* [Xenomai-core] xnshadow_relax in taskexit_event
@ 2009-03-08 10:28 Jan Kiszka
2009-03-08 11:03 ` Philippe Gerum
0 siblings, 1 reply; 6+ messages in thread
From: Jan Kiszka @ 2009-03-08 10:28 UTC (permalink / raw)
To: xenomai-core
[-- Attachment #1: Type: text/plain, Size: 664 bytes --]
Hi,
this looks suspicious to me:
static inline void do_taskexit_event(struct task_struct *p)
{
...
if (xnpod_shadow_p())
xnshadow_relax(0);
A) The only call context of this hook is do_exit() - and that's Linux
kernel code which should always run in secondary mode, no?
B) Even if we were called once in a while from primary mode here, too,
the check xnpod_shadow_p() would only tell us that we are a shadow
thread, not in which mode we currently run.
I've tested a removal of this hunk and found no regressions so far. Will
post a patch unless someone can explain why we actually need this (and
why A) and B) are non-issues).
Jan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Xenomai-core] xnshadow_relax in taskexit_event
2009-03-08 10:28 [Xenomai-core] xnshadow_relax in taskexit_event Jan Kiszka
@ 2009-03-08 11:03 ` Philippe Gerum
2009-03-09 9:19 ` Jan Kiszka
0 siblings, 1 reply; 6+ messages in thread
From: Philippe Gerum @ 2009-03-08 11:03 UTC (permalink / raw)
To: Jan Kiszka; +Cc: xenomai-core
Jan Kiszka wrote:
> Hi,
>
> this looks suspicious to me:
>
> static inline void do_taskexit_event(struct task_struct *p)
> {
> ...
> if (xnpod_shadow_p())
> xnshadow_relax(0);
>
> A) The only call context of this hook is do_exit() - and that's Linux
> kernel code which should always run in secondary mode, no?
>
It is a left-over from the days when do_exit() could be called from primary
context directly on behalf on the shadow unmapping code; now we go through an
APC, so that should be ok.
> B) Even if we were called once in a while from primary mode here, too,
> the check xnpod_shadow_p() would only tell us that we are a shadow
> thread, not in which mode we currently run.
>
Nak, this test is fine, even if probably useless now. xnpod_shadow_p()
implicitely checks that your current Xenomai thread is not in secondary mode,
because you cannot find the XNSHADOW bit set in the root thread tcb, which must
be active in secondary mode.
> I've tested a removal of this hunk and found no regressions so far. Will
> post a patch unless someone can explain why we actually need this (and
> why A) and B) are non-issues).
Let's replace this code with a BUG_ON(xnpod_shadow_p()).
>
> Jan
>
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> Xenomai-core mailing list
> Xenomai-core@domain.hid
> https://mail.gna.org/listinfo/xenomai-core
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Xenomai-core] xnshadow_relax in taskexit_event
2009-03-08 11:03 ` Philippe Gerum
@ 2009-03-09 9:19 ` Jan Kiszka
2009-03-09 9:32 ` Philippe Gerum
0 siblings, 1 reply; 6+ messages in thread
From: Jan Kiszka @ 2009-03-09 9:19 UTC (permalink / raw)
To: rpm; +Cc: xenomai-core
Philippe Gerum wrote:
> Jan Kiszka wrote:
>> Hi,
>>
>> this looks suspicious to me:
>>
>> static inline void do_taskexit_event(struct task_struct *p)
>> {
>> ...
>> if (xnpod_shadow_p())
>> xnshadow_relax(0);
>>
>> A) The only call context of this hook is do_exit() - and that's Linux
>> kernel code which should always run in secondary mode, no?
>>
>
> It is a left-over from the days when do_exit() could be called from primary
> context directly on behalf on the shadow unmapping code; now we go through an
> APC, so that should be ok.
Mmm, then I wonder if we shouldn't be able to drop the explicit
xnpod_schedule() from do_taskexit_event() as well: we always enter this
code over the ROOT thread, and xnpod_delete_thread() will only kill a
suspended thread. So there should be no need to reschedule afterwards.
Will post an updated patch.
Jan
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Xenomai-core] xnshadow_relax in taskexit_event
2009-03-09 9:19 ` Jan Kiszka
@ 2009-03-09 9:32 ` Philippe Gerum
2009-03-09 9:39 ` Jan Kiszka
0 siblings, 1 reply; 6+ messages in thread
From: Philippe Gerum @ 2009-03-09 9:32 UTC (permalink / raw)
To: Jan Kiszka; +Cc: xenomai-core
Jan Kiszka wrote:
> Philippe Gerum wrote:
>> Jan Kiszka wrote:
>>> Hi,
>>>
>>> this looks suspicious to me:
>>>
>>> static inline void do_taskexit_event(struct task_struct *p)
>>> {
>>> ...
>>> if (xnpod_shadow_p())
>>> xnshadow_relax(0);
>>>
>>> A) The only call context of this hook is do_exit() - and that's Linux
>>> kernel code which should always run in secondary mode, no?
>>>
>> It is a left-over from the days when do_exit() could be called from primary
>> context directly on behalf on the shadow unmapping code; now we go through an
>> APC, so that should be ok.
>
> Mmm, then I wonder if we shouldn't be able to drop the explicit
> xnpod_schedule() from do_taskexit_event() as well: we always enter this
> code over the ROOT thread, and xnpod_delete_thread() will only kill a
> suspended thread. So there should be no need to reschedule afterwards.
> Will post an updated patch.
>
Nak, the deletion hook may alter the scheduling state.
> Jan
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Xenomai-core] xnshadow_relax in taskexit_event
2009-03-09 9:32 ` Philippe Gerum
@ 2009-03-09 9:39 ` Jan Kiszka
2009-03-09 10:16 ` Philippe Gerum
0 siblings, 1 reply; 6+ messages in thread
From: Jan Kiszka @ 2009-03-09 9:39 UTC (permalink / raw)
To: rpm; +Cc: xenomai-core
Philippe Gerum wrote:
> Jan Kiszka wrote:
>> Philippe Gerum wrote:
>>> Jan Kiszka wrote:
>>>> Hi,
>>>>
>>>> this looks suspicious to me:
>>>>
>>>> static inline void do_taskexit_event(struct task_struct *p)
>>>> {
>>>> ...
>>>> if (xnpod_shadow_p())
>>>> xnshadow_relax(0);
>>>>
>>>> A) The only call context of this hook is do_exit() - and that's Linux
>>>> kernel code which should always run in secondary mode, no?
>>>>
>>> It is a left-over from the days when do_exit() could be called from
>>> primary context directly on behalf on the shadow unmapping code; now
>>> we go through an APC, so that should be ok.
>>
>> Mmm, then I wonder if we shouldn't be able to drop the explicit
>> xnpod_schedule() from do_taskexit_event() as well: we always enter this
>> code over the ROOT thread, and xnpod_delete_thread() will only kill a
>> suspended thread. So there should be no need to reschedule afterwards.
>> Will post an updated patch.
>>
>
> Nak, the deletion hook may alter the scheduling state.
OK. Is such a hook expected to call xnsched_set_resched in that case,
ie. can we drop it?
Jan
--
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [Xenomai-core] xnshadow_relax in taskexit_event
2009-03-09 9:39 ` Jan Kiszka
@ 2009-03-09 10:16 ` Philippe Gerum
0 siblings, 0 replies; 6+ messages in thread
From: Philippe Gerum @ 2009-03-09 10:16 UTC (permalink / raw)
To: Jan Kiszka; +Cc: xenomai-core
Jan Kiszka wrote:
> Philippe Gerum wrote:
>> Jan Kiszka wrote:
>>> Philippe Gerum wrote:
>>>> Jan Kiszka wrote:
>>>>> Hi,
>>>>>
>>>>> this looks suspicious to me:
>>>>>
>>>>> static inline void do_taskexit_event(struct task_struct *p)
>>>>> {
>>>>> ...
>>>>> if (xnpod_shadow_p())
>>>>> xnshadow_relax(0);
>>>>>
>>>>> A) The only call context of this hook is do_exit() - and that's Linux
>>>>> kernel code which should always run in secondary mode, no?
>>>>>
>>>> It is a left-over from the days when do_exit() could be called from
>>>> primary context directly on behalf on the shadow unmapping code; now
>>>> we go through an APC, so that should be ok.
>>> Mmm, then I wonder if we shouldn't be able to drop the explicit
>>> xnpod_schedule() from do_taskexit_event() as well: we always enter this
>>> code over the ROOT thread, and xnpod_delete_thread() will only kill a
>>> suspended thread. So there should be no need to reschedule afterwards.
>>> Will post an updated patch.
>>>
>> Nak, the deletion hook may alter the scheduling state.
>
> OK. Is such a hook expected to call xnsched_set_resched in that case,
> ie. can we drop it?
>
Yes, 2.5 only.
> Jan
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2009-03-09 10:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-03-08 10:28 [Xenomai-core] xnshadow_relax in taskexit_event Jan Kiszka
2009-03-08 11:03 ` Philippe Gerum
2009-03-09 9:19 ` Jan Kiszka
2009-03-09 9:32 ` Philippe Gerum
2009-03-09 9:39 ` Jan Kiszka
2009-03-09 10:16 ` 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.