Re: [Xenomai-core] xnshadow_relax in taskexit_event
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
>
___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] xnshadow_relax in taskexit_event
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
___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] xnshadow_relax in taskexit_event
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
>
___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] xnshadow_relax in taskexit_event
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
___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core
Re: [Xenomai-core] xnshadow_relax in taskexit_event
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@gna.org
> https://mail.gna.org/listinfo/xenomai-core
___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core
[Xenomai-core] xnshadow_relax in taskexit_event
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
signature.asc
Description: OpenPGP digital signature
___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core
