[Xenomai-core] rt_task_info.status encoding

[Xenomai-core] rt_task_info.status encoding

[Jan Kiszka]

Hi,

the documentation refers to the Native Task Status (T_*) when it comes
to documenting rt_task_info.status. That is not correct. That field
contains far more flags than T_* is describing and, even worse, comes
with two collisions: T_PRIMARY and T_JOINABLE are not reported by
rt_task_inquire, rather T_RELAX (!T_PRIMARY, arrrg...) and T_HELD.

I see two ways out of this:

 a) Redirect the documentation to the nucleus thread state flags.

 b) Redefine the numerical values of T_PRIMARY and T_JOINABLE (the spare
    bits are unused with the native skin), add missing but possibly
    interesting flags as T_-constants and ensure that T_PRIMARY and
    T_JOINABLE are correctly injected on rt_task_inquire from user
    space.

Somehow I tend to prefer a)...

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2
Corporate Competence Center Embedded Linux

Re: [Xenomai-core] rt_task_info.status encoding

[Philippe Gerum]

Jan Kiszka wrote:
> Hi,
> 
> the documentation refers to the Native Task Status (T_*) when it comes
> to documenting rt_task_info.status. That is not correct. That field
> contains far more flags than T_* is describing and, even worse, comes
> with two collisions: T_PRIMARY and T_JOINABLE are not reported by
> rt_task_inquire, rather T_RELAX (!T_PRIMARY, arrrg...) and T_HELD.
>

T_PRIMARY is NOT meant to be reported by rt_task_inquire(), and actually, its
value was picked to collide, to reflect the fact that it was a one-way
specifier. You can't use T_RELAX because what is needed is a bit to force a
transition to primary mode using rt_task_set_mode(), which is the actual source
of all uglinesses. Aside of this, the nucleus naturally wants a "relaxed state"
bit, and would not get any help from a "primary mode" bit for threads.

We could have used a T_RELAX bit to clear in rt_task_set_mode() instead of
T_PRIMARY to set, but unfortunately, such a negative logic would have been
somewhat confusing to users, since what is provided is the secondary -> primary
mode switch.

Sending back the current mode in rt_task_inquire() would lead to two additional
issues:
1) if for some reason, we would like to switch the caller to secondary mode at
some point to be able to provide a more complete status, the primary/secondary
status returned would make no sense at all. The fact that we don't do it now
does not preclude the need to do it in future releases.
2) rt_task_set_mode(..., T_PRIMARY) is already vastly misused in a number of
applications, sometimes uselessly, most of the time in a way that event kills
performances. Giving an interface to get back the current mode would close the
loop, triggering a whole set of new terminally silly usage of that hack.
Applications should NEVER use that feature, it was initially designed for
internal code (i.e. RTDM if my memory serves me well). Actually, the more I
think of it, the more I convinced that I'm going to slaughter this crap in 2.5,
providing an internal syscall from the XENOMAI_SYS class instead for use only in
proper contexts.

T_JOINABLE might be reported, though, that is a different story.

> I see two ways out of this:
> 
>  a) Redirect the documentation to the nucleus thread state flags.
> 

Which means that the documentation of the skin depends on the implementation of
the core. Bad idea.

>  b) Redefine the numerical values of T_PRIMARY and T_JOINABLE (the spare
>     bits are unused with the native skin), add missing but possibly
>     interesting flags as T_-constants and ensure that T_PRIMARY and
>     T_JOINABLE are correctly injected on rt_task_inquire from user
>     space.

Maybe for T_JOINABLE. Gilles?

> 
> Somehow I tend to prefer a)...
> 
> Jan
> 


-- 
Philippe.

Re: [Xenomai-core] rt_task_info.status encoding

[Gilles Chanteperdrix]

Philippe Gerum wrote:
> Jan Kiszka wrote:
>> Hi,
>>
>> the documentation refers to the Native Task Status (T_*) when it comes
>> to documenting rt_task_info.status. That is not correct. That field
>> contains far more flags than T_* is describing and, even worse, comes
>> with two collisions: T_PRIMARY and T_JOINABLE are not reported by
>> rt_task_inquire, rather T_RELAX (!T_PRIMARY, arrrg...) and T_HELD.
>>
> 
> T_PRIMARY is NOT meant to be reported by rt_task_inquire(), and actually, its
> value was picked to collide, to reflect the fact that it was a one-way
> specifier. You can't use T_RELAX because what is needed is a bit to force a
> transition to primary mode using rt_task_set_mode(), which is the actual source
> of all uglinesses. Aside of this, the nucleus naturally wants a "relaxed state"
> bit, and would not get any help from a "primary mode" bit for threads.
> 
> We could have used a T_RELAX bit to clear in rt_task_set_mode() instead of
> T_PRIMARY to set, but unfortunately, such a negative logic would have been
> somewhat confusing to users, since what is provided is the secondary -> primary
> mode switch.
> 
> Sending back the current mode in rt_task_inquire() would lead to two additional
> issues:
> 1) if for some reason, we would like to switch the caller to secondary mode at
> some point to be able to provide a more complete status, the primary/secondary
> status returned would make no sense at all. The fact that we don't do it now
> does not preclude the need to do it in future releases.
> 2) rt_task_set_mode(..., T_PRIMARY) is already vastly misused in a number of
> applications, sometimes uselessly, most of the time in a way that event kills
> performances. Giving an interface to get back the current mode would close the
> loop, triggering a whole set of new terminally silly usage of that hack.
> Applications should NEVER use that feature, it was initially designed for
> internal code (i.e. RTDM if my memory serves me well). Actually, the more I
> think of it, the more I convinced that I'm going to slaughter this crap in 2.5,
> providing an internal syscall from the XENOMAI_SYS class instead for use only in
> proper contexts.
> 
> T_JOINABLE might be reported, though, that is a different story.
> 
>> I see two ways out of this:
>>
>>  a) Redirect the documentation to the nucleus thread state flags.
>>
> 
> Which means that the documentation of the skin depends on the implementation of
> the core. Bad idea.

We also have a difficulty: when pdf documentations are generated, the
nucleus and native skin documentation end up in different documents. So,
we can not redirect the native skin documentation to nucleus.

> 
>>  b) Redefine the numerical values of T_PRIMARY and T_JOINABLE (the spare
>>     bits are unused with the native skin), add missing but possibly
>>     interesting flags as T_-constants and ensure that T_PRIMARY and
>>     T_JOINABLE are correctly injected on rt_task_inquire from user
>>     space.
> 
> Maybe for T_JOINABLE. Gilles?

Ok for me.

As a side note, I happen to use pthread_set_mode_np to trigger manual
mode transitions: I do this before calling the "socket" service to
trigger rtnet socket creation in secondary mode, where it can call
safely the kernel allocation methods. So, I am not really in favor of
removing this service.

However, if this usage disappears, and if we fix the issue with mode
switches and mutexes, I am Ok with removing the voluntary mode switches,
as I agree that they can be misused.

-- 
					    Gilles.

Re: [Xenomai-core] rt_task_info.status encoding

[Jan Kiszka]

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

Philippe Gerum wrote:
> Jan Kiszka wrote:
>> Hi,
>>
>> the documentation refers to the Native Task Status (T_*) when it comes
>> to documenting rt_task_info.status. That is not correct. That field
>> contains far more flags than T_* is describing and, even worse, comes
>> with two collisions: T_PRIMARY and T_JOINABLE are not reported by
>> rt_task_inquire, rather T_RELAX (!T_PRIMARY, arrrg...) and T_HELD.
>>
> 
> T_PRIMARY is NOT meant to be reported by rt_task_inquire(), and actually, its
> value was picked to collide, to reflect the fact that it was a one-way

So you preferred to break rt_task_inquire instead of letting it return a
consistent value? Not really?

> specifier. You can't use T_RELAX because what is needed is a bit to force a
> transition to primary mode using rt_task_set_mode(), which is the actual source
> of all uglinesses. Aside of this, the nucleus naturally wants a "relaxed state"
> bit, and would not get any help from a "primary mode" bit for threads.

I'm not arguing for removing T_PRIMARY, I was just struggling with the
confusing values rt_task_inquire reported to me.

> 
> We could have used a T_RELAX bit to clear in rt_task_set_mode() instead of
> T_PRIMARY to set, but unfortunately, such a negative logic would have been
> somewhat confusing to users, since what is provided is the secondary -> primary
> mode switch.
> 
> Sending back the current mode in rt_task_inquire() would lead to two additional
> issues:
> 1) if for some reason, we would like to switch the caller to secondary mode at
> some point to be able to provide a more complete status, the primary/secondary
> status returned would make no sense at all. The fact that we don't do it now
> does not preclude the need to do it in future releases.

Sorry, but this is very far fetched.

> 2) rt_task_set_mode(..., T_PRIMARY) is already vastly misused in a number of
> applications, sometimes uselessly, most of the time in a way that event kills
> performances. Giving an interface to get back the current mode would close the
> loop, triggering a whole set of new terminally silly usage of that hack.
> Applications should NEVER use that feature, it was initially designed for
> internal code (i.e. RTDM if my memory serves me well). Actually, the more I
> think of it, the more I convinced that I'm going to slaughter this crap in 2.5,
> providing an internal syscall from the XENOMAI_SYS class instead for use only in
> proper contexts.

This is a different issue.

See, I needed rt_task_inquire for precisely the purpose it is (mostly)
designed for: tracing / debugging. And for that purpose, a valid
T_PRIMARY bit is of very high importance. I even implemented the same
inquire service for POSIX in the meantime so that I was able to
implement all functional tests I needed for the fast mutexes.

If you really argue for removing a T_PRIMARY-equivalent from
RT_TASK_INFO, you may also argue for removing the SIGXCPU helper - the
user should better not know in which context some of his threads
currently runs.

> 
> T_JOINABLE might be reported, though, that is a different story.
> 
>> I see two ways out of this:
>>
>>  a) Redirect the documentation to the nucleus thread state flags.
>>
> 
> Which means that the documentation of the skin depends on the implementation of
> the core. Bad idea.

OK, for the sake of API cleanness, I'm fine with wrapped state flags.
Will prepare the required patch to add missing (but also sufficiently
abstract) states and mask the rest from rt_task_inquire.

Jan


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

Re: [Xenomai-core] rt_task_info.status encoding

[Jan Kiszka]

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

Gilles Chanteperdrix wrote:
> Philippe Gerum wrote:
>> Jan Kiszka wrote:
>>> Hi,
>>>
>>> the documentation refers to the Native Task Status (T_*) when it comes
>>> to documenting rt_task_info.status. That is not correct. That field
>>> contains far more flags than T_* is describing and, even worse, comes
>>> with two collisions: T_PRIMARY and T_JOINABLE are not reported by
>>> rt_task_inquire, rather T_RELAX (!T_PRIMARY, arrrg...) and T_HELD.
>>>
>> T_PRIMARY is NOT meant to be reported by rt_task_inquire(), and actually, its
>> value was picked to collide, to reflect the fact that it was a one-way
>> specifier. You can't use T_RELAX because what is needed is a bit to force a
>> transition to primary mode using rt_task_set_mode(), which is the actual source
>> of all uglinesses. Aside of this, the nucleus naturally wants a "relaxed state"
>> bit, and would not get any help from a "primary mode" bit for threads.
>>
>> We could have used a T_RELAX bit to clear in rt_task_set_mode() instead of
>> T_PRIMARY to set, but unfortunately, such a negative logic would have been
>> somewhat confusing to users, since what is provided is the secondary -> primary
>> mode switch.
>>
>> Sending back the current mode in rt_task_inquire() would lead to two additional
>> issues:
>> 1) if for some reason, we would like to switch the caller to secondary mode at
>> some point to be able to provide a more complete status, the primary/secondary
>> status returned would make no sense at all. The fact that we don't do it now
>> does not preclude the need to do it in future releases.
>> 2) rt_task_set_mode(..., T_PRIMARY) is already vastly misused in a number of
>> applications, sometimes uselessly, most of the time in a way that event kills
>> performances. Giving an interface to get back the current mode would close the
>> loop, triggering a whole set of new terminally silly usage of that hack.
>> Applications should NEVER use that feature, it was initially designed for
>> internal code (i.e. RTDM if my memory serves me well). Actually, the more I
>> think of it, the more I convinced that I'm going to slaughter this crap in 2.5,
>> providing an internal syscall from the XENOMAI_SYS class instead for use only in
>> proper contexts.
>>
>> T_JOINABLE might be reported, though, that is a different story.
>>
>>> I see two ways out of this:
>>>
>>>  a) Redirect the documentation to the nucleus thread state flags.
>>>
>> Which means that the documentation of the skin depends on the implementation of
>> the core. Bad idea.
> 
> We also have a difficulty: when pdf documentations are generated, the
> nucleus and native skin documentation end up in different documents. So,
> we can not redirect the native skin documentation to nucleus.

Then even more fixing is required here:

#define T_BLOCKED  XNPEND     /**< See #XNPEND    */
...

Will take the chance.

Jan


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

Re: [Xenomai-core] rt_task_info.status encoding

[Philippe Gerum]

Gilles Chanteperdrix wrote:
> Philippe Gerum wrote:
>> Jan Kiszka wrote:
>>> Hi,
>>>
>>> the documentation refers to the Native Task Status (T_*) when it comes
>>> to documenting rt_task_info.status. That is not correct. That field
>>> contains far more flags than T_* is describing and, even worse, comes
>>> with two collisions: T_PRIMARY and T_JOINABLE are not reported by
>>> rt_task_inquire, rather T_RELAX (!T_PRIMARY, arrrg...) and T_HELD.
>>>
>> T_PRIMARY is NOT meant to be reported by rt_task_inquire(), and actually, its
>> value was picked to collide, to reflect the fact that it was a one-way
>> specifier. You can't use T_RELAX because what is needed is a bit to force a
>> transition to primary mode using rt_task_set_mode(), which is the actual source
>> of all uglinesses. Aside of this, the nucleus naturally wants a "relaxed state"
>> bit, and would not get any help from a "primary mode" bit for threads.
>>
>> We could have used a T_RELAX bit to clear in rt_task_set_mode() instead of
>> T_PRIMARY to set, but unfortunately, such a negative logic would have been
>> somewhat confusing to users, since what is provided is the secondary -> primary
>> mode switch.
>>
>> Sending back the current mode in rt_task_inquire() would lead to two additional
>> issues:
>> 1) if for some reason, we would like to switch the caller to secondary mode at
>> some point to be able to provide a more complete status, the primary/secondary
>> status returned would make no sense at all. The fact that we don't do it now
>> does not preclude the need to do it in future releases.
>> 2) rt_task_set_mode(..., T_PRIMARY) is already vastly misused in a number of
>> applications, sometimes uselessly, most of the time in a way that event kills
>> performances. Giving an interface to get back the current mode would close the
>> loop, triggering a whole set of new terminally silly usage of that hack.
>> Applications should NEVER use that feature, it was initially designed for
>> internal code (i.e. RTDM if my memory serves me well). Actually, the more I
>> think of it, the more I convinced that I'm going to slaughter this crap in 2.5,
>> providing an internal syscall from the XENOMAI_SYS class instead for use only in
>> proper contexts.
>>
>> T_JOINABLE might be reported, though, that is a different story.
>>
>>> I see two ways out of this:
>>>
>>>  a) Redirect the documentation to the nucleus thread state flags.
>>>
>> Which means that the documentation of the skin depends on the implementation of
>> the core. Bad idea.
> 
> We also have a difficulty: when pdf documentations are generated, the
> nucleus and native skin documentation end up in different documents. So,
> we can not redirect the native skin documentation to nucleus.
> 
>>>  b) Redefine the numerical values of T_PRIMARY and T_JOINABLE (the spare
>>>     bits are unused with the native skin), add missing but possibly
>>>     interesting flags as T_-constants and ensure that T_PRIMARY and
>>>     T_JOINABLE are correctly injected on rt_task_inquire from user
>>>     space.
>> Maybe for T_JOINABLE. Gilles?
> 
> Ok for me.
> 
> As a side note, I happen to use pthread_set_mode_np to trigger manual
> mode transitions: I do this before calling the "socket" service to
> trigger rtnet socket creation in secondary mode, where it can call
> safely the kernel allocation methods. So, I am not really in favor of
> removing this service.
>

The idea is not to remove the service, but rather to make perfectly clear that
such feature is not for normal usage. I did agree a long time ago that it could
be needed by low-level skin code when Jan asked for it, but I'm against its
widespread use.

> However, if this usage disappears, and if we fix the issue with mode
> switches and mutexes, I am Ok with removing the voluntary mode switches,
> as I agree that they can be misused.
> 

It should be kept, but moved out from the mainline API, only accessible via a
direct XENOMAI_SYS() call, not using a high level interface such as
rt_task_set_mode(). The message should be: this is an internal service for skin
developers, if you need it for regular application code, then something looks
broken in your code. Userland code that implements extended debugging facilities
or any kind of tricky instrumentation feature would still be allowed to call the
direct interface, but at least, this would not be done carelessly anymore.
Hopefully.

-- 
Philippe.

Re: [Xenomai-core] rt_task_info.status encoding

[Philippe Gerum]

Jan Kiszka wrote:
> Philippe Gerum wrote:
>> Jan Kiszka wrote:
>>> Hi,
>>>
>>> the documentation refers to the Native Task Status (T_*) when it comes
>>> to documenting rt_task_info.status. That is not correct. That field
>>> contains far more flags than T_* is describing and, even worse, comes
>>> with two collisions: T_PRIMARY and T_JOINABLE are not reported by
>>> rt_task_inquire, rather T_RELAX (!T_PRIMARY, arrrg...) and T_HELD.
>>>
>> T_PRIMARY is NOT meant to be reported by rt_task_inquire(), and actually, its
>> value was picked to collide, to reflect the fact that it was a one-way
> 
> So you preferred to break rt_task_inquire instead of letting it return a
> consistent value? Not really?

As explained earlier, the thing you have to understand is that rt_task_inquire()
is not broken, it just does not work your way, which is a different issue.

> 
>> specifier. You can't use T_RELAX because what is needed is a bit to force a
>> transition to primary mode using rt_task_set_mode(), which is the actual source
>> of all uglinesses. Aside of this, the nucleus naturally wants a "relaxed state"
>> bit, and would not get any help from a "primary mode" bit for threads.
> 
> I'm not arguing for removing T_PRIMARY, I was just struggling with the
> confusing values rt_task_inquire reported to me.
> 

This is because you want T_PRIMARY to be part of the rt_task_inquire() return
values. It must not, really. When apps start playing with the current mode,
things start falling apart. I don't want to make T_PRIMARY a first-class citizen
of the interface; considering it as a legitimate value of an inquiry service
would do that. So that is a NAK.

>> We could have used a T_RELAX bit to clear in rt_task_set_mode() instead of
>> T_PRIMARY to set, but unfortunately, such a negative logic would have been
>> somewhat confusing to users, since what is provided is the secondary -> primary
>> mode switch.
>>
>> Sending back the current mode in rt_task_inquire() would lead to two additional
>> issues:
>> 1) if for some reason, we would like to switch the caller to secondary mode at
>> some point to be able to provide a more complete status, the primary/secondary
>> status returned would make no sense at all. The fact that we don't do it now
>> does not preclude the need to do it in future releases.
> 
> Sorry, but this is very far fetched.
> 

Sorry, you are wrong. rt_task_inquire() is currently marked as __xn_exec_any,
which means that it is processed in the current thread mode, but always from the
Xenomai stage in the pipeline. If at some point, we extend rt_task_inquire() to
return information from the Linux realm (e.g. protected by a RCU construct), we
would have to move that call to __xn_exec_lostage. Conversely, if at some point
the rt_task_inquire() service is changed so that getting information from the
nucleus involves blocking, then we would have to make that call __xn_exec_primary.

Far fetched, even impossible that we would need that? Not that much actually. A
number of people have native Xenomai kernel modules interacting with native
Xenomai apps in userland; those modules maintain a significant portion of the
application logic, and some did reimplement some kind of rt_task_inquire() for
debugging and logging purposes. Instead of that, we could allow the in-kernel
part to be asked for additional information via a callback from the
rt_task_inquire() implementation, using the RT_TASK_INFO struct as a common
return header, that could be completed with more data.
In that case, rt_task_inquire() would have to be flexible when it comes to the
thread mode, currently it is not, because you may only fetch information that is
immediately available from the Xenomai stage, without blocking.

Therefore, in both cases, T_PRIMARY would become meaningless as a return value.

>> 2) rt_task_set_mode(..., T_PRIMARY) is already vastly misused in a number of
>> applications, sometimes uselessly, most of the time in a way that event kills
>> performances. Giving an interface to get back the current mode would close the
>> loop, triggering a whole set of new terminally silly usage of that hack.
>> Applications should NEVER use that feature, it was initially designed for
>> internal code (i.e. RTDM if my memory serves me well). Actually, the more I
>> think of it, the more I convinced that I'm going to slaughter this crap in 2.5,
>> providing an internal syscall from the XENOMAI_SYS class instead for use only in
>> proper contexts.
> 
> This is a different issue.
> 
> See, I needed rt_task_inquire for precisely the purpose it is (mostly)
> designed for: tracing / debugging. And for that purpose, a valid
> T_PRIMARY bit is of very high importance. I even implemented the same
> inquire service for POSIX in the meantime so that I was able to
> implement all functional tests I needed for the fast mutexes.
>

See, the point is that T_PRIMARY is often misused because it belongs to the
mainline interface whilst it should only be used in very limited situations. I
did acknowledge a long time ago that skin developers may need it badly though,
this is why I provided this service to you in the first place. The point is: the
interface to that service should make perfectly clear that it is internal stuff
that should not be part of the toolbox for writing applications.
I.e. direct XENOMAI_SYS() call.

> If you really argue for removing a T_PRIMARY-equivalent from
> RT_TASK_INFO, you may also argue for removing the SIGXCPU helper - the
> user should better not know in which context some of his threads
> currently runs.
> 

You are missing the point: SIGXCPU is about helping users to find a bug when
they messed up mode management involuntarily. Moving T_PRIMARY out of the
mainline interface is about preventing users to mess up mode management voluntarily.

Said differently, is there any situation where an application has no other
choice than requiring a transition from secondary to primary explicitly? As far
as I understand Xenomai properly, the answer is no, because the underlying skin
interface should deal with that issue when needed (i.e. direct XENOMAI_SYS call
again). In last resort, if the application guy badly needs it, it must be for
debugging/logging purposes the same way you did need that as well for RTDM,
therefore he may still use the XENOMAI_SYS interface directly. If he does so,
then he must know what he does, since the call he uses is part of the low-level
Xenomai layer. If that breaks or makes something terribly inefficient, then no
complaints.

>> T_JOINABLE might be reported, though, that is a different story.
>>
>>> I see two ways out of this:
>>>
>>>  a) Redirect the documentation to the nucleus thread state flags.
>>>
>> Which means that the documentation of the skin depends on the implementation of
>> the core. Bad idea.
> 
> OK, for the sake of API cleanness, I'm fine with wrapped state flags.
> Will prepare the required patch to add missing (but also sufficiently
> abstract) states and mask the rest from rt_task_inquire.
> 
> Jan
> 


-- 
Philippe.

Re: [Xenomai-core] rt_task_info.status encoding

[Jan Kiszka]

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

Philippe Gerum wrote:
> Jan Kiszka wrote:
>> Philippe Gerum wrote:
>>> Jan Kiszka wrote:
>>>> Hi,
>>>>
>>>> the documentation refers to the Native Task Status (T_*) when it comes
>>>> to documenting rt_task_info.status. That is not correct. That field
>>>> contains far more flags than T_* is describing and, even worse, comes
>>>> with two collisions: T_PRIMARY and T_JOINABLE are not reported by
>>>> rt_task_inquire, rather T_RELAX (!T_PRIMARY, arrrg...) and T_HELD.
>>>>
>>> T_PRIMARY is NOT meant to be reported by rt_task_inquire(), and actually, its
>>> value was picked to collide, to reflect the fact that it was a one-way
>> So you preferred to break rt_task_inquire instead of letting it return a
>> consistent value? Not really?
> 
> As explained earlier, the thing you have to understand is that rt_task_inquire()
> is not broken, it just does not work your way, which is a different issue.

At the bare minimum, its documentation is broken. It points to state
values which rt_task_inquire does not return.

> 
>>> specifier. You can't use T_RELAX because what is needed is a bit to force a
>>> transition to primary mode using rt_task_set_mode(), which is the actual source
>>> of all uglinesses. Aside of this, the nucleus naturally wants a "relaxed state"
>>> bit, and would not get any help from a "primary mode" bit for threads.
>> I'm not arguing for removing T_PRIMARY, I was just struggling with the
>> confusing values rt_task_inquire reported to me.
>>
> 
> This is because you want T_PRIMARY to be part of the rt_task_inquire() return
> values. It must not, really. When apps start playing with the current mode,
> things start falling apart. I don't want to make T_PRIMARY a first-class citizen
> of the interface; considering it as a legitimate value of an inquiry service
> would do that. So that is a NAK.

I start to believe we are arguing with different (miss-)use case in
mind. Mine is definitely not about "helping" the user to switch the
thread mode even more actively. It is about validating application
states, it is about thread state reflection without any other actions
than reporting errors. T_PRIMARY is part of the picture for the dual
kernel Xenomai version, and it will remain such as long as there are two
kernels. Even better, it is a very helpful application debugging tool
when it comes to runtime validation of their real-time behavior. Again,
it is NOT about promoting more use of rt_task_set_mode!

> 
>>> We could have used a T_RELAX bit to clear in rt_task_set_mode() instead of
>>> T_PRIMARY to set, but unfortunately, such a negative logic would have been
>>> somewhat confusing to users, since what is provided is the secondary -> primary
>>> mode switch.
>>>
>>> Sending back the current mode in rt_task_inquire() would lead to two additional
>>> issues:
>>> 1) if for some reason, we would like to switch the caller to secondary mode at
>>> some point to be able to provide a more complete status, the primary/secondary
>>> status returned would make no sense at all. The fact that we don't do it now
>>> does not preclude the need to do it in future releases.
>> Sorry, but this is very far fetched.
>>
> 
> Sorry, you are wrong. rt_task_inquire() is currently marked as __xn_exec_any,
> which means that it is processed in the current thread mode, but always from the
> Xenomai stage in the pipeline. If at some point, we extend rt_task_inquire() to
> return information from the Linux realm (e.g. protected by a RCU construct), we
> would have to move that call to __xn_exec_lostage. Conversely, if at some point
> the rt_task_inquire() service is changed so that getting information from the
> nucleus involves blocking, then we would have to make that call __xn_exec_primary.
> 
> Far fetched, even impossible that we would need that? Not that much actually. A
> number of people have native Xenomai kernel modules interacting with native
> Xenomai apps in userland; those modules maintain a significant portion of the
> application logic, and some did reimplement some kind of rt_task_inquire() for
> debugging and logging purposes. Instead of that, we could allow the in-kernel
> part to be asked for additional information via a callback from the
> rt_task_inquire() implementation, using the RT_TASK_INFO struct as a common
> return header, that could be completed with more data.
> In that case, rt_task_inquire() would have to be flexible when it comes to the
> thread mode, currently it is not, because you may only fetch information that is
> immediately available from the Xenomai stage, without blocking.
> 
> Therefore, in both cases, T_PRIMARY would become meaningless as a return value.

We will nevertheless be able to take a snapshot (for debugging
purposes!) of the thread state before any potential mode switch, see my
patch for mirroring the mode into user space for fast xnsynch services.

Moreover, making a formerly primary-safe service secondary (just like we
discussed for rt_task_set_priority) is NEVER a good idea. It will easily
break existing apps. If you start extending rt_task_inquire like that, I
would quickly jump in and request to extend it in a way that existing
users would NOT trigger such mode switches, or provide a separate
interface. You would also not expect from rt_task_inquire that is
suddenly changes the priority of the caller, would you?

> 
>>> 2) rt_task_set_mode(..., T_PRIMARY) is already vastly misused in a number of
>>> applications, sometimes uselessly, most of the time in a way that event kills
>>> performances. Giving an interface to get back the current mode would close the
>>> loop, triggering a whole set of new terminally silly usage of that hack.
>>> Applications should NEVER use that feature, it was initially designed for
>>> internal code (i.e. RTDM if my memory serves me well). Actually, the more I
>>> think of it, the more I convinced that I'm going to slaughter this crap in 2.5,
>>> providing an internal syscall from the XENOMAI_SYS class instead for use only in
>>> proper contexts.
>> This is a different issue.
>>
>> See, I needed rt_task_inquire for precisely the purpose it is (mostly)
>> designed for: tracing / debugging. And for that purpose, a valid
>> T_PRIMARY bit is of very high importance. I even implemented the same
>> inquire service for POSIX in the meantime so that I was able to
>> implement all functional tests I needed for the fast mutexes.
>>
> 
> See, the point is that T_PRIMARY is often misused because it belongs to the
> mainline interface whilst it should only be used in very limited situations. I
> did acknowledge a long time ago that skin developers may need it badly though,
> this is why I provided this service to you in the first place. The point is: the
> interface to that service should make perfectly clear that it is internal stuff
> that should not be part of the toolbox for writing applications.
> I.e. direct XENOMAI_SYS() call.
> 
>> If you really argue for removing a T_PRIMARY-equivalent from
>> RT_TASK_INFO, you may also argue for removing the SIGXCPU helper - the
>> user should better not know in which context some of his threads
>> currently runs.
>>
> 
> You are missing the point: SIGXCPU is about helping users to find a bug when
> they messed up mode management involuntarily. Moving T_PRIMARY out of the
> mainline interface is about preventing users to mess up mode management voluntarily.

Nope, it is just as well about helping to find involuntary mode switches
at application module boundaries. Granted, reflection on the current
thread mode /can/ be misused, just like many tools can be used to do
weird things, but then let us rather discuss about how to overcome
rt_task_set_mode(T_PRIMARY) and not how to make developers' life harder
while they try to find inconsistent application states (there are cases
where SIGXCPU cannot be used).

Jan


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

Re: [Xenomai-core] rt_task_info.status encoding

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> I start to believe we are arguing with different (miss-)use case in
> mind. Mine is definitely not about "helping" the user to switch the
> thread mode even more actively. It is about validating application
> states, it is about thread state reflection without any other actions
> than reporting errors. T_PRIMARY is part of the picture for the dual
> kernel Xenomai version, and it will remain such as long as there are two
> kernels. Even better, it is a very helpful application debugging tool
> when it comes to runtime validation of their real-time behavior. Again,
> it is NOT about promoting more use of rt_task_set_mode!

SIGXCPU may be used validate current thread mode, and it has the
advantage that it can not be misused.

-- 
					    Gilles.

Re: [Xenomai-core] rt_task_info.status encoding

[Jan Kiszka]

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> I start to believe we are arguing with different (miss-)use case in
>> mind. Mine is definitely not about "helping" the user to switch the
>> thread mode even more actively. It is about validating application
>> states, it is about thread state reflection without any other actions
>> than reporting errors. T_PRIMARY is part of the picture for the dual
>> kernel Xenomai version, and it will remain such as long as there are two
>> kernels. Even better, it is a very helpful application debugging tool
>> when it comes to runtime validation of their real-time behavior. Again,
>> it is NOT about promoting more use of rt_task_set_mode!
> 
> SIGXCPU may be used validate current thread mode, and it has the
> advantage that it can not be misused.

You are not always able to install or switch signal handlers when
crossing application module boundaries. And you can't use it to validate
the opposite (RT thread calls into lengthy, not RT-context suited
library function).

Jan

Re: [Xenomai-core] rt_task_info.status encoding

[Gilles Chanteperdrix]

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
>> Jan Kiszka wrote:
>>> I start to believe we are arguing with different (miss-)use case in
>>> mind. Mine is definitely not about "helping" the user to switch the
>>> thread mode even more actively. It is about validating application
>>> states, it is about thread state reflection without any other actions
>>> than reporting errors. T_PRIMARY is part of the picture for the dual
>>> kernel Xenomai version, and it will remain such as long as there are two
>>> kernels. Even better, it is a very helpful application debugging tool
>>> when it comes to runtime validation of their real-time behavior. Again,
>>> it is NOT about promoting more use of rt_task_set_mode!
>> SIGXCPU may be used validate current thread mode, and it has the
>> advantage that it can not be misused.
> 
> You are not always able to install or switch signal handlers when
> crossing application module boundaries. And you can't use it to validate
> the opposite (RT thread calls into lengthy, not RT-context suited
> library function).

For me, one problem of SIGXCPU is when one non RT thread acquires a
mutex shared with a RT thread, and then calls some functions which cause
it to switch to secondary mode. But we could conceivably modify the
nucleus to detect this kind of situation.

Another problem is that malloc, free, new and delete do not necessarily
cause a switch to secondary mode. But this also could conceivably be
solved by wrapping/overriding these calls and call the SIGXCPU handler
in the wrapper.

Other than that, I do not see what you mean.

-- 
					    Gilles.

Re: [Xenomai-core] rt_task_info.status encoding

[Jan Kiszka]

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

Gilles Chanteperdrix wrote:
> Jan Kiszka wrote:
>> Gilles Chanteperdrix wrote:
>>> Jan Kiszka wrote:
>>>> I start to believe we are arguing with different (miss-)use case in
>>>> mind. Mine is definitely not about "helping" the user to switch the
>>>> thread mode even more actively. It is about validating application
>>>> states, it is about thread state reflection without any other actions
>>>> than reporting errors. T_PRIMARY is part of the picture for the dual
>>>> kernel Xenomai version, and it will remain such as long as there are two
>>>> kernels. Even better, it is a very helpful application debugging tool
>>>> when it comes to runtime validation of their real-time behavior. Again,
>>>> it is NOT about promoting more use of rt_task_set_mode!
>>> SIGXCPU may be used validate current thread mode, and it has the
>>> advantage that it can not be misused.
>> You are not always able to install or switch signal handlers when
>> crossing application module boundaries. And you can't use it to validate
>> the opposite (RT thread calls into lengthy, not RT-context suited
>> library function).
> 
> For me, one problem of SIGXCPU is when one non RT thread acquires a
> mutex shared with a RT thread, and then calls some functions which cause
> it to switch to secondary mode. But we could conceivably modify the
> nucleus to detect this kind of situation.
> 
> Another problem is that malloc, free, new and delete do not necessarily
> cause a switch to secondary mode. But this also could conceivably be
> solved by wrapping/overriding these calls and call the SIGXCPU handler
> in the wrapper.

Yes, and add getimeofday to this list. Even worse: When it triggers, the
backtrace stops in the vsyscall page, not allowing to identify the caller.

> 
> Other than that, I do not see what you mean.

SIGXCPU becomes useless if you are unable to install a signal handler,
e.g. from within an invoked library. rt_task_inquire is then an
alternative mechanism to add debug assertions.

Jan


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