[Xenomai] FreeRTOS skin

[Xenomai] FreeRTOS skin

[Matthias Schneider]

Hi all,

please find enclosed a patch with a FreeRTOS skin for xenomai-forge I have 
been  working on for some time. I would like to get some feedback and advice
what still needs to be done to get it accepted in Xenomai. There is a set of 
unit tests included and the possibility to download the original FreeRTOS package 
in order to run most of its (platform independent) test suite. Until now I have 
been working under mercury only. Documentation is available in form of a README 
file in lib/freertos/README, which should also be a good starting point. I have 
not attached "configure" to the patch, so you will need to recreate the configure 
script first before actually running it. You can apply the patch via

git am --keep-cr < 0001-Initial-Freertos-commit.patch
autoreconf
./configure
make

Running the testcases, you might stumble over a
#0  EINVAL in threadobj_unlock_sched(), threadobj.c:59
error, which is due to 
http://www.xenomai.org/pipermail/xenomai/2014-May/030821.html
I will try to continue the discussion there.

I do not consider the patch 100% finished yet and I am still working on following topics:

* more testing  
  * "vStartDynamicPriorityTasks" is not yet passing
  * "vCreateBlockTimeTasks"/"vCreateAltBlockTimeTasks" seem to fail sometimes
  * it has been ages since I ran the vfile system tests.
  * testing with optimization

* test under cobalt (not started yet, still need to get a working setup)
* portBYTE_ALIGNMENT define
* some refactoring (mostly in task.c)

I am publishing this patch already now since I do not want to "finish" it first
before the first review only to find out I have overlooked something that 
should have been done differently. Some points to start a discussion are maybe:

* what kind of documentation is still missing

* correct placement of CANCEL_DEFER and CANCEL_RESTORE

* priority of new threads during prologue (will start conversation based on
http://www.xenomai.org/pipermail/xenomai/2014-February/030117.html
)

I am doing this in my free time, so please be patient when I am not responding 
right away...

Regards,

Matthias
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Initial-Freertos-commit.patch.gz
Type: application/x-gzip
Size: 34769 bytes
Desc: not available
URL: <http://www.xenomai.org/pipermail/xenomai/attachments/20140504/ed4fc483/attachment.bin>

Re: [Xenomai] FreeRTOS skin

[Gilles Chanteperdrix]

On 05/04/2014 06:59 PM, Matthias Schneider wrote:
> Hi all,
> 
> please find enclosed a patch with a FreeRTOS skin for xenomai-forge I have 
> been  working on for some time. I would like to get some feedback and advice
> what still needs to be done to get it accepted in Xenomai. There is a set of 
> unit tests included and the possibility to download the original FreeRTOS package 
> in order to run most of its (platform independent) test suite. Until now I have 
> been working under mercury only. Documentation is available in form of a README 
> file in lib/freertos/README, which should also be a good starting point. I have 
> not attached "configure" to the patch, so you will need to recreate the configure 
> script first before actually running it. You can apply the patch via

Hi,

I am not sure that checking whether a type is compatible with void * 
will not suddenly avoid the assert check for any pointer type in:

@@ -328,6 +328,7 @@ static inline int pshared_check(void *heap, void *addr)
        ({                                                              \
                type handle;                                            \
                assert(__builtin_types_compatible_p(typeof(type), unsigned long) || \
+               __builtin_types_compatible_p(typeof(type), void*) || \
                       __builtin_types_compatible_p(typeof(type), uintptr_t)); \
                assert(ptr == NULL || __memchk(__main_heap, ptr));      \
                handle = (type)ptr;                                     \
@@ -337,6 +338,7 @@ static inline int pshared_check(void *heap, void *addr)
        ({                                                              \
                type *ptr;                                              \
                assert(__builtin_types_compatible_p(typeof(handle), unsigned long) || \
+               __builtin_types_compatible_p(typeof(handle), void*) || \
                       __builtin_types_compatible_p(typeof(handle), uintptr_t)); \
                ptr = (type *)handle;                                   \
                ptr;                                                    \


That is a bit complicated:
+#if portBYTE_ALIGNMENT == 8
+       #define portBYTE_ALIGNMENT_MASK (0x0007)
+#endif
+
+#if portBYTE_ALIGNMENT == 4
+       #define portBYTE_ALIGNMENT_MASK (0x0003)
+#endif
+
+#if portBYTE_ALIGNMENT == 2
+       #define portBYTE_ALIGNMENT_MASK (0x0001)
+#endif
+
+#if portBYTE_ALIGNMENT == 1
+       #define portBYTE_ALIGNMENT_MASK (0x0000)
+#endif
+
+#ifndef portBYTE_ALIGNMENT_MASK
+       #error "Invalid portBYTE_ALIGNMENT definition"
+#endif

It seems simpler to me to define portBYTE_ALIGNNMENT_MASK to 
portByteALIGNMENT - 1 and trigger an error if portBYTE_ALIGNMENT 
is more than 8 or not a power of 2.

On 64 bits machines, long and int are different, so, you probably want 
#define portLONG long
To avoid unpleasant surprises in:

+#define portCHAR               char
+#define portFLOAT              float
+#define portDOUBLE             double
+#define portLONG               int

I am not sure what is portBYTE_ALIGNEMENT, but I guess sizeof(long) would 
make more sense.
+#define portBYTE_ALIGNMENT     8 //FIXME: double check...

I believe there is an error in these macros definition:
This one looks strange:
+#define portNOP() __asm__ __volatile__ ("nop");

If you do not care whether portNOP is a barrier you can drop the "volatile"
otherwise you should add "memory" as clobber.


It may not be a good idea to define errors to overlap with errno errors in:
+/* Error definitions. */
+#define errCOULD_NOT_ALLOCATE_REQUIRED_MEMORY  (-1)
+#define errNO_TASK_TO_RUN                      (-2)
+#define errQUEUE_BLOCKED                       (-4)
+#define errQUEUE_YIELD                         (-5)

I would either define these errors as errno errors or give them values which
do not correspond to any errno constant.


+#define xSemaphoreAltGive(xSemaphore) xSemaphoreGive(xMutex)
+
+#define xSemaphoreGiveFromISR(xSemaphore, pxHigherPriorityTaskWoken) \
+       xSemaphoreGive(xMutex)
+

About the testsuite: does it really make sense to include both the unpatched 
testsuite and xenomai-specific patches? I mean if you want to include the 
tests in xenomai sources, then include them patched, or do not include them, 
and include the patch.

-- 
                                                                Gilles.

Re: [Xenomai] FreeRTOS skin

[Matthias Schneider]

----- Original Message -----

> From: Gilles Chanteperdrix <gilles.chanteperdrix@xenomai.org>
> To: Matthias Schneider <ma30002000@yahoo.de>; "xenomai@xenomai.org" <xenomai@xenomai.org>
> Cc: 
> Sent: Sunday, May 4, 2014 8:42 PM
> Subject: Re: [Xenomai] FreeRTOS skin
> 
> On 05/04/2014 06:59 PM, Matthias Schneider wrote:
>> Hi all,
>> 
>> please find enclosed a patch with a FreeRTOS skin for xenomai-forge I have 
>> been  working on for some time. I would like to get some feedback and advice
>> what still needs to be done to get it accepted in Xenomai. There is a set of 
>> unit tests included and the possibility to download the original FreeRTOS package 
>> in order to run most of its (platform independent) test suite. Until now I have 
>> been working under mercury only. Documentation is available in form of a README 
>> file in lib/freertos/README, which should also be a good starting point. I have 
>> not attached "configure" to the patch, so you will need to recreate the configure 
>> script first before actually running it. You can apply the patch via
> 
> 
> Hi,
> 
> I am not sure that checking whether a type is compatible with void * 
> will not suddenly avoid the assert check for any pointer type in:
> 
> @@ -328,6 +328,7 @@ static inline int pshared_check(void *heap, void *addr)
>         ({                                                              
> \
>                 type handle;                                            \
>                 assert(__builtin_types_compatible_p(typeof(type), unsigned long)  || \
> +              __builtin_types_compatible_p(typeof(type), void*) || \
>                       __builtin_types_compatible_p(typeof(type), uintptr_t)); 
> \
>                 assert(ptr == NULL || __memchk(__main_heap, ptr));      \
>                 handle = (type)ptr;                                    \
> @@ -337,6 +338,7 @@ static inline int pshared_check(void *heap, void *addr)
>         ({                                                              
> \
>                 type *ptr;                                              \
>                 assert(__builtin_types_compatible_p(typeof(handle), unsigned  long) || \
> +              __builtin_types_compatible_p(typeof(handle), void*) || \
>                       __builtin_types_compatible_p(typeof(handle), uintptr_t)); 
> \
>                 ptr = (type *)handle;                                  \
>                 ptr;                                                    \
> 
I do agree with the argument; actually originally I had typedef'ed 
xQueueHandle, xTaskHandle and xSemaphorehandle as uintptr_t.
However, when compiling original FreeRTOS applications like the
"demo", I ran into many warnings (warning: comparison between 
pointer and integer) when comparing the handles to "NULL" 
like 

if (queueHandle != NULL) {}

Now I see three possibilities:
* stick with the void* and allow it in the check (admittedly 
  greatly reducing its use)
* change the application code
* suppress warnings when compiling application code (not really 
  my favorite)

Originally the handles are tyepdef'ed to void* as well.
Any ideas?


> That is a bit complicated:
> +#if portBYTE_ALIGNMENT == 8
> +      #define portBYTE_ALIGNMENT_MASK (0x0007)
> +#endif
> +
> +#if portBYTE_ALIGNMENT == 4
> +      #define portBYTE_ALIGNMENT_MASK (0x0003)
> +#endif
> +
> +#if portBYTE_ALIGNMENT == 2
> +      #define portBYTE_ALIGNMENT_MASK (0x0001)
> +#endif
> +
> +#if portBYTE_ALIGNMENT == 1
> +      #define portBYTE_ALIGNMENT_MASK (0x0000)
> +#endif
> +
> +#ifndef portBYTE_ALIGNMENT_MASK
> +      #error "Invalid portBYTE_ALIGNMENT definition"
> +#endif
> 
> It seems simpler to me to define portBYTE_ALIGNNMENT_MASK to 
> portByteALIGNMENT - 1 and trigger an error if portBYTE_ALIGNMENT 
> is more than 8 or not a power of 2.

I could change it to:

#if portBYTE_ALIGNMENT > 8 || portBYTE_ALIGNMENT < 1 || \
    ((portBYTE_ALIGNMENT & ~(portBYTE_ALIGNMENT-1))!=portBYTE_ALIGNMENT)
#error Check portBYTE_ALIGNMENT definition
#endif

#define portBYTE_ALIGNMENT_MASK (portBYTE_ALIGNMENT - 1)

> On 64 bits machines, long and int are different, so, you probably want 
> #define portLONG long
> To avoid unpleasant surprises in:
> 
> +#define portCHAR              char
> +#define portFLOAT              float
> +#define portDOUBLE            double
> +#define portLONG              int

Changed to 
#define portLONG              long

> I am not sure what is portBYTE_ALIGNEMENT, but I guess sizeof(long) would 
> make more sense.
> +#define portBYTE_ALIGNMENT    8 //FIXME: double check...

Seems to be used internally byt the original FreeRTOS implementation,
but is in a public header, so keep it for completeness' sake.
Changing to sizeof(long) as suggested.

> I believe there is an error in these macros definition:
> This one looks strange:
> +#define portNOP() __asm__ __volatile__ ("nop");
> 
> If you do not care whether portNOP is a barrier you can drop the  "volatile"
> otherwise you should add "memory" as clobber.

The original FreeRTOS implementation is "just" volatile (actually it is 
defined exactly the same way as in my header file), so it is a little 
difficult to guess the original authors' motives. Since I do not have any
sample code that makes use of this macro, I am not sure of the implications 
on existing source code when changing it, independent from removing the volatile
or adding the clobbering.

> It may not be a good idea to define errors to overlap with errno errors in:
> +/* Error definitions. */
> +#define errCOULD_NOT_ALLOCATE_REQUIRED_MEMORY  (-1)
> +#define errNO_TASK_TO_RUN                      (-2)
> +#define errQUEUE_BLOCKED                      (-4)
> +#define errQUEUE_YIELD                        (-5)
> 
> I would either define these errors as errno errors or give them values which
> do not correspond to any errno constant.

I could change the first error code to

#define errCOULD_NOT_ALLOCATE_REQUIRED_MEMORY   (-ENOMEM)

and remove the other three codes - they are only needed for the 

"CoRoutine"-feature, which is not supported by my xenomai 
implementation and were included only for completeness sake.

I have also changed TaskGenericaCreate to actually return 
errCOULD_NOT_ALLOCATE_REQUIRED_MEMORY.

> +#define xSemaphoreAltGive(xSemaphore) xSemaphoreGive(xMutex)
> +
> +#define xSemaphoreGiveFromISR(xSemaphore, pxHigherPriorityTaskWoken) \
> +      xSemaphoreGive(xMutex)
> +
> 
> About the testsuite: does it really make sense to include both the unpatched 
> testsuite and xenomai-specific patches? I mean if you want to include the 
> tests in xenomai sources, then include them patched, or do not include them, 
> and include the patch.

I cannot 100% follow here. There is one xenomai-specific test suite included,
which is written for xenomai specifically and the original FreeRTOS "demo". 
The FreeRTOS "demo" is downloadable with and will be patched by the patches 
that are checked in. This demo code might be checked in with patches applied 
alternatively. Is it that what you were suggesting? Or did I simply not make 
clear the difference between the "testsuite" and the "demo"?

Matthias

Re: [Xenomai] FreeRTOS skin

[Philippe Gerum]

On 05/04/2014 11:22 PM, Matthias Schneider wrote:
> ----- Original Message -----
> 
>> From: Gilles Chanteperdrix <gilles.chanteperdrix@xenomai.org>
>> To: Matthias Schneider <ma30002000@yahoo.de>; "xenomai@xenomai.org" <xenomai@xenomai.org>
>> Cc:
>> Sent: Sunday, May 4, 2014 8:42 PM
>> Subject: Re: [Xenomai] FreeRTOS skin
>>   
>> On 05/04/2014 06:59 PM, Matthias Schneider wrote:
>>> Hi all,
>>>   
>>> please find enclosed a patch with a FreeRTOS skin for xenomai-forge I have
>>> been  working on for some time. I would like to get some feedback and advice
>>> what still needs to be done to get it accepted in Xenomai. There is a set of
>>> unit tests included and the possibility to download the original FreeRTOS package
>>> in order to run most of its (platform independent) test suite. Until now I have
>>> been working under mercury only. Documentation is available in form of a README
>>> file in lib/freertos/README, which should also be a good starting point. I have
>>> not attached "configure" to the patch, so you will need to recreate the configure
>>> script first before actually running it. You can apply the patch via
>>   
>>   
>> Hi,
>>   
>> I am not sure that checking whether a type is compatible with void *
>> will not suddenly avoid the assert check for any pointer type in:
>>   
>> @@ -328,6 +328,7 @@ static inline int pshared_check(void *heap, void *addr)
>>          ({
>> \
>>                  type handle;                                            \
>>                  assert(__builtin_types_compatible_p(typeof(type), unsigned long)  || \
>> +              __builtin_types_compatible_p(typeof(type), void*) || \
>>                        __builtin_types_compatible_p(typeof(type), uintptr_t));
>> \
>>                  assert(ptr == NULL || __memchk(__main_heap, ptr));      \
>>                  handle = (type)ptr;                                    \
>> @@ -337,6 +338,7 @@ static inline int pshared_check(void *heap, void *addr)
>>          ({
>> \
>>                  type *ptr;                                              \
>>                  assert(__builtin_types_compatible_p(typeof(handle), unsigned  long) || \
>> +              __builtin_types_compatible_p(typeof(handle), void*) || \
>>                        __builtin_types_compatible_p(typeof(handle), uintptr_t));
>> \
>>                  ptr = (type *)handle;                                  \
>>                  ptr;                                                    \
>>   
> I do agree with the argument; actually originally I had typedef'ed
> xQueueHandle, xTaskHandle and xSemaphorehandle as uintptr_t.
> However, when compiling original FreeRTOS applications like the
> "demo", I ran into many warnings (warning: comparison between
> pointer and integer) when comparing the handles to "NULL"
> like
> 
> if (queueHandle != NULL) {}
> 
> Now I see three possibilities:
> * stick with the void* and allow it in the check (admittedly
>    greatly reducing its use)
> * change the application code
> * suppress warnings when compiling application code (not really
>    my favorite)
> 
> Originally the handles are tyepdef'ed to void* as well.
> Any ideas?
> 

The point of this assertion is to make sure that your skin-defined object handle is wide enough to convey a native pointer directly, for the process-private configuration (with shared multi-processing enabled, this type would convey an offset instead).

There is no point in using a runtime assertion for C, we should be doing this instead:

diff --git a/include/copperplate/heapobj.h b/include/copperplate/heapobj.h
index 38deb89..6b5617c 100644
--- a/include/copperplate/heapobj.h
+++ b/include/copperplate/heapobj.h
@@ -324,21 +324,30 @@ static inline int pshared_check(void *heap, void *addr)
 	return 0;
 }
 
+#ifdef __cplusplus
+#define __check_ref_width(__dst, __src)			\
+	({						\
+		assert(sizeof(__dst) >= sizeof(__src));	\
+		(typeof(__dst))__src;			\
+	})
+#else
+#define __check_ref_width(__dst, __src)					\
+	__builtin_choose_expr(						\
+		sizeof(__dst) >= sizeof(__src), (typeof(__dst))__src,	\
+		((void)0))
+#endif
+
 #define mainheap_ref(ptr, type)						\
 	({								\
 		type handle;						\
-		assert(__builtin_types_compatible_p(typeof(type), unsigned long) || \
-		       __builtin_types_compatible_p(typeof(type), uintptr_t)); \
+		handle = __check_ref_width(handle, ptr);		\
 		assert(ptr == NULL || __memchk(__main_heap, ptr));	\
-		handle = (type)ptr;					\
 		handle;							\
 	})
 #define mainheap_deref(handle, type)					\
 	({								\
 		type *ptr;						\
-		assert(__builtin_types_compatible_p(typeof(handle), unsigned long) || \
-		       __builtin_types_compatible_p(typeof(handle), uintptr_t)); \
-		ptr = (type *)handle;					\
+		ptr = __check_ref_width(ptr, handle);			\
 		ptr;							\
 	})


-- 
Philippe.

Re: [Xenomai] FreeRTOS skin

[Philippe Gerum]

On 05/04/2014 06:59 PM, Matthias Schneider wrote:
> Hi all,
>
> please find enclosed a patch with a FreeRTOS skin for xenomai-forge I have
> been  working on for some time. I would like to get some feedback and advice
> what still needs to be done to get it accepted in Xenomai. There is a set of
> unit tests included and the possibility to download the original FreeRTOS package
> in order to run most of its (platform independent) test suite. Until now I have
> been working under mercury only. Documentation is available in form of a README
> file in lib/freertos/README, which should also be a good starting point.

Ok, thanks for this. Let's address issues gradually, starting with the 
task module.

I'd suggest to revisit the following mechanisms to get our feet wet: 
vTaskStartScheduler()/vTaskEndScheduler() and 
vTaskSuspendAll()/vTaskResumeAll().

- synchronizing on scheduler start/stop events can be made simpler, by 
waiting on a barrier after threadobj_wait_start() has returned in the 
task trampoline, meanwhile starting all tasks copperplate-wise at 
creation time via threadobj_start(). Such additional barrier should be a 
condition variable protected by your current scheduler.lock mutex, 
signaled by vTaskStartScheduler(). Tasks would have to check for the 
scheduler state before breaking the wait loop.

- assuming that threadobj_cancel() already waits for the canceled thread 
to finalize, you should only need to change the scheduler object state 
in vTaskEndScheduler() prior to cancelling all tasks. You should not 
need the extra syncobj.

- vTaskSuspendAll()/vTaskResumeAll() maintain useless status flags. 
Copperplate provides __THREAD_S_SUSPENDED in the core status of threads 
for the very same purpose.

The other set of issues is all about locking is managed. A couple of hints:

- vPortEnterCritical()/vPortExitCritical() can't be emulated since we 
can't mask IRQs in userland (and we certainly don't want to even try 
doing so). This would be a RTDM driver's business in the Xenomai model. 
Looking at the FreeRTOS doc, the closest approximation would be 
threadobj_sched_lock() in userland, but since hardware drivers should 
move to kernel space over RTDM, I'm unsure emulating these calls has any 
benefit in our context.

- threadobj_sched_lock() does not prevent the caller from scheduling
out, it only stops involuntary preemption when running non-blocking 
code. Typically, locking a mutex is potentially blocking code, so the 
guarantee disappears in that case. IIUC, the current code assumes more 
than this. Moreover, issuing a sleeping call voluntarily when holding 
this lock is logically wrong, even if Xenomai won't jump over the window 
doing so, unlike many RTOS vTaskStartScheduler()).

- the read/write_lock_nocancel() form is reserved for protecting 
sections which are known not to traverse cancellation points, as defined 
by POSIX 
(http://pubs.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_09.html). 
This is about self-documentation of the code, and optimizing execution 
by omitting costly management of cleanup blocks when it is deemed safe 
to do so.

- CANCEL_DEFER() / CANCEL_RESTORE() should protect sections against 
asynchronous cancellation, we typically do this before invoking the 
copperplate layer, so that we may assume that pthread_cancel() won't be 
a problem when running inner code, provided we have been careful about 
handling read/write lock cleanups. These macros work by disabling 
asynchronous cancellation for the current thread when 
--enable-async-cancel was given at build time. Otherwise, this part is a 
nop.

- Scheduler.queued_task, .state and .num_tasks all seem logically 
related. So .num_tasks should be updated holding Scheduler.lock, instead 
of using an atomic type. We'd need to issue smp_rmb() prior to reading 
that counter locklessly.

HTH,

-- 
Philippe.

Re: [Xenomai] FreeRTOS skin

[Philippe Gerum]

On 05/05/2014 12:33 PM, Philippe Gerum wrote:
> On 05/04/2014 06:59 PM, Matthias Schneider wrote:
>> Hi all,
>>
>> please find enclosed a patch with a FreeRTOS skin for xenomai-forge I
>> have
>> been  working on for some time. I would like to get some feedback and
>> advice
>> what still needs to be done to get it accepted in Xenomai. There is a
>> set of
>> unit tests included and the possibility to download the original
>> FreeRTOS package
>> in order to run most of its (platform independent) test suite. Until
>> now I have
>> been working under mercury only. Documentation is available in form of
>> a README
>> file in lib/freertos/README, which should also be a good starting point.
>
> Ok, thanks for this. Let's address issues gradually, starting with the
> task module.
>

In addition to the above, there are a few coding style items:

- Please always take the fastest possible path when exiting on error,
   so that we don't have to cripple the normal processing following the
   failure point with error-specific checks. e.g. what is done with the
   "task" variable in vTaskStartScheduler(), within the thread
   cancellation loop, ends up being convoluted and confusing, compared
   to unwinding and leaving the routine immediately after the error is
   detected.

- (void)function_call() is useless visual pollution and tells nothing
   about the only thing that matters to the reader: were you right in
   omitting the check? Instead, tagging the called routine with the
   ((warn_unused_result)) attribute when problems may likely arise from
   not checking the return value, seems a better way to detect
   potential bugs at build time, which would cause the (void) cast to
   be ignored when checking the call sites anyway.

- Prefer C-style comments over C++-style ones.

-- 
Philippe.

Re: [Xenomai] FreeRTOS skin

[Gilles Chanteperdrix]

On 05/05/2014 12:33 PM, Philippe Gerum wrote:
> - vTaskSuspendAll()/vTaskResumeAll() maintain useless status flags.
> Copperplate provides __THREAD_S_SUSPENDED in the core status of threads
> for the very same purpose.

I believe in FreeRTOS vTaskSuspendAll/vTaskResumeAll is more like a task 
lock mechanism than a service suspending/resuming all tasks for real. Of 
course this only provides mutual exclusion on UP.


-- 
					    Gilles.

Re: [Xenomai] FreeRTOS skin

[Philippe Gerum]

On 05/05/2014 04:03 PM, Gilles Chanteperdrix wrote:
> On 05/05/2014 12:33 PM, Philippe Gerum wrote:
>> - vTaskSuspendAll()/vTaskResumeAll() maintain useless status flags.
>> Copperplate provides __THREAD_S_SUSPENDED in the core status of threads
>> for the very same purpose.
>
> I believe in FreeRTOS vTaskSuspendAll/vTaskResumeAll is more like a task
> lock mechanism than a service suspending/resuming all tasks for real. Of
> course this only provides mutual exclusion on UP.
>
>

Definitely, this is threadobj_sched_lock() in all its glory. A bit of a 
misnomer. So the current implementation could be simplified a lot it seems.

-- 
Philippe.

Re: [Xenomai] FreeRTOS skin

[Matthias Schneider]

----- Original Message -----
> From: Philippe Gerum <rpm@xenomai.org>
> To: Matthias Schneider <ma30002000@yahoo.de>; "xenomai@xenomai.org" <xenomai@xenomai.org>
> Cc: 
> Sent: Monday, May 5, 2014 3:37 PM
> Subject: Re: [Xenomai] FreeRTOS skin
> 
> On 05/05/2014 12:33 PM, Philippe Gerum wrote:
>>  On 05/04/2014 06:59 PM, Matthias Schneider wrote:
>>>  Hi all,
>>> 
>>>  please find enclosed a patch with a FreeRTOS skin for xenomai-forge I
>>>  have
>>>  been  working on for some time. I would like to get some feedback and
>>>  advice
>>>  what still needs to be done to get it accepted in Xenomai. There is a
>>>  set of
>>>  unit tests included and the possibility to download the original
>>>  FreeRTOS package
>>>  in order to run most of its (platform independent) test suite. Until
>>>  now I have
>>>  been working under mercury only. Documentation is available in form of
>>>  a README
>>>  file in lib/freertos/README, which should also be a good starting 
> point.
>> 
>>  Ok, thanks for this. Let's address issues gradually, starting with the
>>  task module.
>> 
> 
> In addition to the above, there are a few coding style items:
> 
> - Please always take the fastest possible path when exiting on error,
>    so that we don't have to cripple the normal processing following the
>    failure point with error-specific checks. e.g. what is done with the
>    "task" variable in vTaskStartScheduler(), within the thread
>    cancellation loop, ends up being convoluted and confusing, compared
>    to unwinding and leaving the routine immediately after the error is
>    detected.


I will review the code and simplify occurrences of the mentioned pattern.
However I do not see this pattern in this particular code fragment, since it basically does the following:

* take the first entry of the task list
* check if the task object can be locked
* unlock the scheduler lock
* if it was possible to lock the task, cancel it
* else wait 1ms and try to lock it again

No error condition leads to leaving this loop. I do agree however, that the
code is convoluted, mainly due to the erroneous-looking nested locks, one
for protecting freertos_task_list and one locking the threadobj. The list should be locked while accessing it, while holding it when cancelling the threadobj will lead to deadlocks (task_finalizer takes freertos_scheduler.lock).


> - (void)function_call() is useless visual pollution and tells nothing
>    about the only thing that matters to the reader: were you right in
>    omitting the check? Instead, tagging the called routine with the
>    ((warn_unused_result)) attribute when problems may likely arise from
>    not checking the return value, seems a better way to detect
>    potential bugs at build time, which would cause the (void) cast to
>    be ignored when checking the call sites anyway.
Removed the casts.

> - Prefer C-style comments over C++-style ones.
Changed the comments.

> 
> 
> -- 
> Philippe.
>

Re: [Xenomai] FreeRTOS skin

[Philippe Gerum]

On 05/06/2014 09:09 AM, Matthias Schneider wrote:
> ----- Original Message -----
>> From: Philippe Gerum <rpm@xenomai.org>
>> To: Matthias Schneider <ma30002000@yahoo.de>; "xenomai@xenomai.org" <xenomai@xenomai.org>
>> Cc:
>> Sent: Monday, May 5, 2014 3:37 PM
>> Subject: Re: [Xenomai] FreeRTOS skin
>>
>> On 05/05/2014 12:33 PM, Philippe Gerum wrote:
>>>    On 05/04/2014 06:59 PM, Matthias Schneider wrote:
>>>>    Hi all,
>>>>
>>>>    please find enclosed a patch with a FreeRTOS skin for xenomai-forge I
>>>>    have
>>>>    been  working on for some time. I would like to get some feedback and
>>>>    advice
>>>>    what still needs to be done to get it accepted in Xenomai. There is a
>>>>    set of
>>>>    unit tests included and the possibility to download the original
>>>>    FreeRTOS package
>>>>    in order to run most of its (platform independent) test suite. Until
>>>>    now I have
>>>>    been working under mercury only. Documentation is available in form of
>>>>    a README
>>>>    file in lib/freertos/README, which should also be a good starting
>> point.
>>>
>>>    Ok, thanks for this. Let's address issues gradually, starting with the
>>>    task module.
>>>
>>
>> In addition to the above, there are a few coding style items:
>>
>> - Please always take the fastest possible path when exiting on error,
>>     so that we don't have to cripple the normal processing following the
>>     failure point with error-specific checks. e.g. what is done with the
>>     "task" variable in vTaskStartScheduler(), within the thread
>>     cancellation loop, ends up being convoluted and confusing, compared
>>     to unwinding and leaving the routine immediately after the error is
>>     detected.
>
>
> I will review the code and simplify occurrences of the mentioned pattern.
> However I do not see this pattern in this particular code fragment, since it basically does the following:
>
> * take the first entry of the task list

The first check for list emptiness looks racy, as it is not covered by 
the scheduler.lock.

> * check if the task object can be locked
> * unlock the scheduler lock
> * if it was possible to lock the task, cancel it
> * else wait 1ms and try to lock it again

If you found it in your task queue but can't lock it, then call Houston 
because we have a problem: the task finalizer where this task is dropped 
from the list runs prior to deleting the lock. However, testing for the 
magic word on success is indeed required in the current implementation, 
since the thread lock is dropped shortly prior to canceling the thread 
in copperplate.

You could use pvlist_for_each_entry_safe() for iterating over the task 
queue.

>
> No error condition leads to leaving this loop. I do agree however, that the
> code is convoluted, mainly due to the erroneous-looking nested locks, one
> for protecting freertos_task_list and one locking the threadobj. The list should be locked while accessing it, while holding it when cancelling the threadobj will lead to deadlocks (task_finalizer takes freertos_scheduler.lock).
>

Setting task to NULL upon locking error, only for the purpose of 
avoiding the magic check in the normal code flow is the issue I mentioned:

		if (task && threadobj_lock(&task->thobj) == -EINVAL)
			task = NULL;

		if (task && threadobj_get_magic(&task->thobj) != task_magic) {
			threadobj_unlock(&task->thobj);
			task = NULL;
		}

>
>> - (void)function_call() is useless visual pollution and tells nothing
>>     about the only thing that matters to the reader: were you right in
>>     omitting the check? Instead, tagging the called routine with the
>>     ((warn_unused_result)) attribute when problems may likely arise from
>>     not checking the return value, seems a better way to detect
>>     potential bugs at build time, which would cause the (void) cast to
>>     be ignored when checking the call sites anyway.
> Removed the casts.
>

Thanks, I'll add some warn_unused_result tags to some routines from the 
copperplate layer. syncobj_wait* services come to mind, since these may 
return -EIDRM or -EINTR, which are situations which have to influence 
the code flow when received.

-- 
Philippe.

Re: [Xenomai] FreeRTOS skin

[Philippe Gerum]

On 05/06/2014 09:46 AM, Philippe Gerum wrote:
> On 05/06/2014 09:09 AM, Matthias Schneider wrote:
>> ----- Original Message -----
>>> From: Philippe Gerum <rpm@xenomai.org>
>>> To: Matthias Schneider <ma30002000@yahoo.de>; "xenomai@xenomai.org"
>>> <xenomai@xenomai.org>
>>> Cc:
>>> Sent: Monday, May 5, 2014 3:37 PM
>>> Subject: Re: [Xenomai] FreeRTOS skin
>>>
>>> On 05/05/2014 12:33 PM, Philippe Gerum wrote:
>>>>    On 05/04/2014 06:59 PM, Matthias Schneider wrote:
>>>>>    Hi all,
>>>>>
>>>>>    please find enclosed a patch with a FreeRTOS skin for
>>>>> xenomai-forge I
>>>>>    have
>>>>>    been  working on for some time. I would like to get some
>>>>> feedback and
>>>>>    advice
>>>>>    what still needs to be done to get it accepted in Xenomai. There
>>>>> is a
>>>>>    set of
>>>>>    unit tests included and the possibility to download the original
>>>>>    FreeRTOS package
>>>>>    in order to run most of its (platform independent) test suite.
>>>>> Until
>>>>>    now I have
>>>>>    been working under mercury only. Documentation is available in
>>>>> form of
>>>>>    a README
>>>>>    file in lib/freertos/README, which should also be a good starting
>>> point.
>>>>
>>>>    Ok, thanks for this. Let's address issues gradually, starting
>>>> with the
>>>>    task module.
>>>>
>>>
>>> In addition to the above, there are a few coding style items:
>>>
>>> - Please always take the fastest possible path when exiting on error,
>>>     so that we don't have to cripple the normal processing following the
>>>     failure point with error-specific checks. e.g. what is done with the
>>>     "task" variable in vTaskStartScheduler(), within the thread
>>>     cancellation loop, ends up being convoluted and confusing, compared
>>>     to unwinding and leaving the routine immediately after the error is
>>>     detected.
>>
>>
>> I will review the code and simplify occurrences of the mentioned pattern.
>> However I do not see this pattern in this particular code fragment,
>> since it basically does the following:
>>
>> * take the first entry of the task list
>
> The first check for list emptiness looks racy, as it is not covered by
> the scheduler.lock.
>
>> * check if the task object can be locked
>> * unlock the scheduler lock
>> * if it was possible to lock the task, cancel it
>> * else wait 1ms and try to lock it again
>
> If you found it in your task queue but can't lock it, then call Houston
> because we have a problem: the task finalizer where this task is dropped
> from the list runs prior to deleting the lock.

No, I'm wrong. There is still a race which could allow that. So we need 
both checks.

-- 
Philippe.

Re: [Xenomai] FreeRTOS skin

[Matthias Schneider]

> From: Philippe Gerum <rpm@xenomai.org>
> On 05/06/2014 09:09 AM, Matthias Schneider wrote:
>>> On 05/05/2014 12:33 PM, Philippe Gerum wrote:
>>>>     On 05/04/2014 06:59 PM, Matthias Schneider wrote:
>>>>>     Hi all,
>>>>> 
>>>>>     please find enclosed a patch with a FreeRTOS skin for xenomai-forge I
>>>>>     have  been  working on for some time. 
[...] 
>>>> 
>>>>     Ok, thanks for this. Let's address issues gradually,  starting with the
>>>>     task module.
>>>> 
>>> 
>>> In addition to the above, there are a few coding style items:
>>> 
>>> - Please always take the fastest possible path when exiting on error,
>>>     so that we don't have to cripple the normal processing following the
>>>     failure point with error-specific checks. e.g. what is done with  the
>>>     "task" variable in vTaskStartScheduler(), within the thread
>>>     cancellation loop, ends up being convoluted and confusing, compared
>>>     to unwinding and leaving the routine immediately after the error is
>>>     detected.
>> 
>> 
>> I will review the code and simplify occurrences of the mentioned pattern.
>> However I do not see this pattern in this particular code fragment, since 
> it basically does the following:
>> 
>> * take the first entry of the task list
> 
> The first check for list emptiness looks racy, as it is not covered by 
> the scheduler.lock.

True. See below.


>> * check if the task object can be locked
>> * unlock the scheduler lock
>> * if it was possible to lock the task, cancel it
>> * else wait 1ms and try to lock it again
> 
> If you found it in your task queue but can't lock it, then call Houston 
> because we have a problem: the task finalizer where this task is dropped 
> from the list runs prior to deleting the lock. However, testing for the 
> magic word on success is indeed required in the current implementation, 
> since the thread lock is dropped shortly prior to canceling the thread 
> in copperplate.
> 
> You could use pvlist_for_each_entry_safe() for iterating over the task 
> queue.
> 
>> 
>> No error condition leads to leaving this loop. I do agree however, that the
>> code is convoluted, mainly due to the erroneous-looking nested locks, one
>> for protecting freertos_task_list and one locking the threadobj. The list 
> should be locked while accessing it, while holding it when cancelling the 
> threadobj will lead to deadlocks (task_finalizer takes freertos_scheduler.lock).
>> 
> 
> Setting task to NULL upon locking error, only for the purpose of 
> avoiding the magic check in the normal code flow is the issue I mentioned:
> 
>         if (task && threadobj_lock(&task->thobj) == -EINVAL)
>             task = NULL;
> 
>         if (task && threadobj_get_magic(&task->thobj) != 
> task_magic) {
>             threadobj_unlock(&task->thobj);
>             task = NULL;
>         }
> 
[...] 

Ok let's simplify the code (and add the missing lock):

write_lock_nocancel(&freertos_scheduler.lock);
freertos_scheduler.state = taskSCHEDULER_NOT_STARTED;

/* cancel all tasks in list */
while (!pvlist_empty(&freertos_task_list)) {
    task = pvlist_first_entry(&freertos_task_list, struct freertos_task, next);
    if (threadobj_lock(&task->thobj) != -EINVAL) {
            if (threadobj_get_magic(&task->thobj) == task_magic) {
                    /* need to leave freertos_scheduler.lock since it is
                        taken in task_finalizer while we block in
                        threadobj_cancel */
                    write_unlock(&freertos_scheduler.lock);
                    threadobj_cancel(&task->thobj);
                    write_lock_nocancel(&freertos_scheduler.lock);
                    continue;
            }
            threadobj_unlock(&task->thobj);
    }
    /* Need to release the lock for a while since a task in deletion
        may need it to unregister itself from freertos_task_list */
    write_unlock(&freertos_scheduler.lock);
    struct timespec rqt;
    clockobj_ticks_to_timeout(&freertos_clock,
                              0.001 * xGetTicksPerSecond(),
                              &rqt);
    threadobj_sleep(&rqt);
    write_lock_nocancel(&freertos_scheduler.lock);
}
write_unlock(&freertos_scheduler.lock);

I had used pvlist_for_each_entry_safe() originally, but due to the possibility 
having to access a single task multiple times, I ended up with two nested loops.

> -- 
> Philippe.
>

Re: [Xenomai] FreeRTOS skin

[Matthias Schneider]

----- Original Message -----
> From: Philippe Gerum <rpm@xenomai.org>
> To: Matthias Schneider <ma30002000@yahoo.de>; "xenomai@xenomai.org" <xenomai@xenomai.org>
> Cc: 
> Sent: Monday, May 5, 2014 12:33 PM
> Subject: Re: [Xenomai] FreeRTOS skin
> 
> On 05/04/2014 06:59 PM, Matthias Schneider wrote:
> 
>> Hi all,
>> 
>> please find enclosed a patch with a FreeRTOS skin for xenomai-forge I have
>> been  working on for some time. I would like to get some feedback and advice
>> what still needs to be done to get it accepted in Xenomai. There is a set of
>> unit tests included and the possibility to download the original FreeRTOS package
>> in order to run most of its (platform independent) test suite. Until now I have
>> been working under mercury only. Documentation is available in form of a README
>> file in lib/freertos/README, which should also be a good starting point.
> 
> Ok, thanks for this. Let's address issues gradually, starting with the task module.

I will try to address some of the mentioned issues now, the rest later:

> I'd suggest to revisit the following mechanisms to get our feet wet: 
> vTaskStartScheduler()/vTaskEndScheduler() and 
> vTaskSuspendAll()/vTaskResumeAll().
> 
> - synchronizing on scheduler start/stop events can be made simpler, by 
> waiting on a barrier after threadobj_wait_start() has returned in the 
> task trampoline, meanwhile starting all tasks copperplate-wise at 
> creation time via threadobj_start(). Such additional barrier should be a 
> condition variable protected by your current scheduler.lock mutex, 
> signaled by vTaskStartScheduler(). Tasks would have to check for the 
> scheduler state before breaking the wait loop.
> 
> - assuming that threadobj_cancel() already waits for the canceled thread 
> to finalize, you should only need to change the scheduler object state 
> in vTaskEndScheduler() prior to cancelling all tasks. You should not 
> need the extra syncobj.

Are you referring to freertos_scheduler.sem? It is used to blocke the 
main thread while the "scheduer" is running. Even when moving the task 
cancellation loop to inside the vTaskEndScheduler(), vTaskStartScheduler()
would still need to block until vTaskEndScheduler() is a

> - vTaskSuspendAll()/vTaskResumeAll() maintain useless status flags. 
> Copperplate provides __THREAD_S_SUSPENDED in the core status of threads 
> for the very same purpose.

This is not entirely correct. Consider the following case (part of the demo
actually):

vTaskSuspendAll();
vTaskSuspend(th1);
vTaskResumeAll();

This is supposed to leave th1 suspended after vTaskResumeAll(). This 
information needs to be stored somewhere. After vTaskSuspendAll(), all
except the current thread have __THREAD_S_SUSPENDED set, so another way to
store this informaion is needed. I have also experimented modelling these 
functions using threadobj_sched_lock() & co., however, even on UP machines
the following code would not behave as supposed:

vTaskSuspendAll();
vTaskDelay(xGetTicksPerSecond() * 0.01);
vTaskResumeAll();

Obviously, during vTaskDelay() other tasks of lower than "sched" prio may
be executed.

> The other set of issues is all about locking is managed. A couple of hints:
> 
> - vPortEnterCritical()/vPortExitCritical() can't be emulated since we 
> can't mask IRQs in userland (and we certainly don't want to even try 
> doing so). This would be a RTDM driver's business in the Xenomai model. 
> Looking at the FreeRTOS doc, the closest approximation would be 
> threadobj_sched_lock() in userland, but since hardware drivers should 
> move to kernel space over RTDM, I'm unsure emulating these calls has any 
> benefit in our context.

Correct. I though I had seen use of it in the plateform-independent demo
previously, but apparently I remember incorrectly. Defining them as empty
functions thus.

> 
> - threadobj_sched_lock() does not prevent the caller from scheduling
> out, it only stops involuntary preemption when running non-blocking 
> code. Typically, locking a mutex is potentially blocking code, so the 
> guarantee disappears in that case. IIUC, the current code assumes more 
> than this. Moreover, issuing a sleeping call voluntarily when holding 
> this lock is logically wrong, even if Xenomai won't jump over the window 
> doing so, unlike many RTOS vTaskStartScheduler()).
> 
> - the read/write_lock_nocancel() form is reserved for protecting 
> sections which are known not to traverse cancellation points, as defined 
> by POSIX 
> (http://pubs.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_09.html). 
> This is about self-documentation of the code, and optimizing execution 
> by omitting costly management of cleanup blocks when it is deemed safe 
> to do so.
> 
> - CANCEL_DEFER() / CANCEL_RESTORE() should protect sections against 
> asynchronous cancellation, we typically do this before invoking the 
> copperplate layer, so that we may assume that pthread_cancel() won't be 
> a problem when running inner code, provided we have been careful about 
> handling read/write lock cleanups. These macros work by disabling 
> asynchronous cancellation for the current thread when 
> --enable-async-cancel was given at build time. Otherwise, this part is a 
> nop.
> 
> - Scheduler.queued_task, .state and .num_tasks all seem logically 
> related. So .num_tasks should be updated holding Scheduler.lock, instead 
> of using an atomic type. We'd need to issue smp_rmb() prior to reading 
> that counter locklessly.
> 
> HTH,
> 
> -- 
> Philippe.
>

Re: [Xenomai] FreeRTOS skin

[Philippe Gerum]

On 05/06/2014 07:19 PM, Matthias Schneider wrote:
>
> Are you referring to freertos_scheduler.sem? It is used to blocke the
> main thread while the "scheduer" is running. Even when moving the task
> cancellation loop to inside the vTaskEndScheduler(), vTaskStartScheduler()
> would still need to block until vTaskEndScheduler() is a
>

I mean you don't need a syncobj. This is overly complex for the purpose, 
you can use a sem_t. syncobj implements extended semantics we commonly 
find in RTOS, which are not available with regular POSIX ipc. But in 
this case, we don't need them: we just need to wait for a pulse, once.

>> - vTaskSuspendAll()/vTaskResumeAll() maintain useless status flags.
>> Copperplate provides __THREAD_S_SUSPENDED in the core status of threads
>> for the very same purpose.
>
> This is not entirely correct. Consider the following case (part of the demo
> actually):
>
> vTaskSuspendAll();
> vTaskSuspend(th1);
> vTaskResumeAll();
>
> This is supposed to leave th1 suspended after vTaskResumeAll(). This
> information needs to be stored somewhere. After vTaskSuspendAll(), all
> except the current thread have __THREAD_S_SUSPENDED set, so another way to
> store this informaion is needed. I have also experimented modelling these
> functions using threadobj_sched_lock() & co., however, even on UP machines
> the following code would not behave as supposed:
>
> vTaskSuspendAll();
> vTaskDelay(xGetTicksPerSecond() * 0.01);
> vTaskResumeAll();
>
> Obviously, during vTaskDelay() other tasks of lower than "sched" prio may
> be executed.

vTaskSuspendAll() very much looks like plain scheduler lock, preventing 
the caller from scheduling out on preemption. If so, then the whole 
issue disappears, there would not be any cumulative blocking states.

>
>> The other set of issues is all about locking is managed. A couple of hints:
>>
>> - vPortEnterCritical()/vPortExitCritical() can't be emulated since we
>> can't mask IRQs in userland (and we certainly don't want to even try
>> doing so). This would be a RTDM driver's business in the Xenomai model.
>> Looking at the FreeRTOS doc, the closest approximation would be
>> threadobj_sched_lock() in userland, but since hardware drivers should
>> move to kernel space over RTDM, I'm unsure emulating these calls has any
>> benefit in our context.
>
> Correct. I though I had seen use of it in the plateform-independent demo
> previously, but apparently I remember incorrectly. Defining them as empty
> functions thus.
>

I would rather not define them, to make sure the user knows about the 
issue when the compiler complains about it. Silently ignoring a hard 
critical section is unlikely to help.

-- 
Philippe.

Re: [Xenomai] FreeRTOS skin

[Matthias Schneider]

----- Original Message -----

> From: Philippe Gerum <rpm@xenomai.org>
> To: Matthias Schneider <ma30002000@yahoo.de>; "xenomai@xenomai.org" <xenomai@xenomai.org>
> Cc: 
> Sent: Tuesday, May 6, 2014 7:47 PM
> Subject: Re: [Xenomai] FreeRTOS skin
> 
> On 05/06/2014 07:19 PM, Matthias Schneider wrote:
>> 
>>  Are you referring to freertos_scheduler.sem? It is used to blocke the
>>  main thread while the "scheduer" is running. Even when moving the 
> task
>>  cancellation loop to inside the vTaskEndScheduler(), vTaskStartScheduler()
>>  would still need to block until vTaskEndScheduler() is a
>> 
> 
> I mean you don't need a syncobj. This is overly complex for the purpose, 
> you can use a sem_t. syncobj implements extended semantics we commonly 
> find in RTOS, which are not available with regular POSIX ipc. But in 
> this case, we don't need them: we just need to wait for a pulse, once.

Ok, got it. Now the last question concerning this, should it be a
_RT(sem_init) or _STD(sem_init)? Does it matter in this case?

>>>  - vTaskSuspendAll()/vTaskResumeAll() maintain useless status flags.
>>>  Copperplate provides __THREAD_S_SUSPENDED in the core status of threads
>>>  for the very same purpose.
>> 
>>  This is not entirely correct. Consider the following case (part of the demo
>>  actually):
>> 
>>  vTaskSuspendAll();
>>  vTaskSuspend(th1);
>>  vTaskResumeAll();
>> 
>>  This is supposed to leave th1 suspended after vTaskResumeAll(). This
>>  information needs to be stored somewhere. After vTaskSuspendAll(), all
>>  except the current thread have __THREAD_S_SUSPENDED set, so another way to
>>  store this informaion is needed. I have also experimented modelling these
>>  functions using threadobj_sched_lock() & co., however, even on UP machines
>>  the following code would not behave as supposed:
>> 
>>  vTaskSuspendAll();
>>  vTaskDelay(xGetTicksPerSecond() * 0.01);
>>  vTaskResumeAll();
>> 
>>  Obviously, during vTaskDelay() other tasks of lower than "sched"  prio may
>>  be executed.
> 
> vTaskSuspendAll() very much looks like plain scheduler lock, preventing 
> the caller from scheduling out on preemption. If so, then the whole 
> issue disappears, there would not be any cumulative blocking states.


So the recommendation would be to go for the threadobj_sched_lock()
approach? But how can I address the mentioned issue of being scheduled
out when waiting? It would make a lot of things a lot easier, so much
for sure...

>>>  The other set of issues is all about locking is managed. A couple of hints:
>>> 
>>>  - vPortEnterCritical()/vPortExitCritical() can't be emulated since we
>>>  can't mask IRQs in userland (and we certainly don't want to even try
>>>  doing so). This would be a RTDM driver's business in the Xenomai model.
>>>  Looking at the FreeRTOS doc, the closest approximation would be
>>>  threadobj_sched_lock() in userland, but since hardware drivers should
>>>  move to kernel space over RTDM, I'm unsure emulating these calls has any
>>>  benefit in our context.
>> 
>>  Correct. I though I had seen use of it in the plateform-independent demo
>>  previously, but apparently I remember incorrectly. Defining them as empty
>>  functions thus.
>> 
> 
> I would rather not define them, to make sure the user knows about the 
> issue when the compiler complains about it. Silently ignoring a hard 
> critical section is unlikely to help.

The platform-independent demo uses them a lot, although it does not 
deal with interrupts at all.. If I remove them, I will need to patch 
the demo, probably to the point where checking in the whole patched 
demo will make more sense than adding more and more patch files.
Or maybe it should be something like threadobj_sched_lock() instead?

> 
> -- 
> Philippe.
>

Re: [Xenomai] FreeRTOS skin

[Matthias Schneider]

> From: Matthias Schneider <ma30002000@yahoo.de>

>>  From: Gilles Chanteperdrix <gilles.chanteperdrix@xenomai.org>
>>  On 05/04/2014 06:59 PM, Matthias Schneider wrote:
>>>  Hi all,
>>>  
>>>  please find enclosed a patch with a FreeRTOS skin for xenomai-forge I have 
>>>  been  working on for some time. I would like to get some feedback and advice
>>>  what still needs to be done to get it accepted in Xenomai. 

[...]


I have removed all already addressed items (most of them) raised by Gilles. I 
will publish an updated patch soon. However, the following points are still
open:

>>  I believe there is an error in these macros definition:
>>  This one looks strange:
>>  +#define portNOP() __asm__ __volatile__ ("nop");
>>  
>>  If you do not care whether portNOP is a barrier you can drop the "volatile"
>>  otherwise you should add "memory" as clobber.
> 
> The original FreeRTOS implementation is "just" volatile (actually it is 
> defined exactly the same way as in my header file), so it is a little 
> difficult to guess the original authors' motives. Since I do not have any
> sample code that makes use of this macro, I am not sure of the implications 
> on existing source code when changing it, independent from removing the volatile
> or adding the clobbering.

Not sure how to go on here, I propose to leave it as it is.

>>  +#define xSemaphoreAltGive(xSemaphore) xSemaphoreGive(xMutex)
>>  +
>>  +#define xSemaphoreGiveFromISR(xSemaphore, pxHigherPriorityTaskWoken) \
>>  +      xSemaphoreGive(xMutex)
>>  +
>>  
>>  About the testsuite: does it really make sense to include both the unpatched 
>>  testsuite and xenomai-specific patches? I mean if you want to include the 
>>  tests in xenomai sources, then include them patched, or do not include them, 
>>  and include the patch.
>
> I cannot 100% follow here. There is one xenomai-specific test suite included,
> which is written for xenomai specifically and the original FreeRTOS "demo". 
> The FreeRTOS "demo" is downloadable with and will be patched by the 
> patches 
> that are checked in. This demo code might be checked in with patches applied 
> alternatively. Is it that what you were suggesting? Or did I simply not make 
> clear the difference between the "testsuite" and the "demo"?


Anything else I can do to clarify?

Matthias

Re: [Xenomai] FreeRTOS skin

[Matthias Schneider]

----- Original Message -----

> From: Matthias Schneider <ma30002000@yahoo.de>
> To: Philippe Gerum <rpm@xenomai.org>; "xenomai@xenomai.org" <xenomai@xenomai.org>
> Cc: 
> Sent: Tuesday, May 6, 2014 6:50 PM
> Subject: Re: [Xenomai] FreeRTOS skin
> 
>>  From: Philippe Gerum <rpm@xenomai.org>
>>  On 05/06/2014 09:09 AM, Matthias Schneider wrote:
>>>>  On 05/05/2014 12:33 PM, Philippe Gerum wrote:

>>>>  In addition to the above, there are a few coding style items:
>>>>  - Please always take the fastest possible path when exiting on error,
>>>>      so that we don't have to cripple the normal processing following the
>>>>      failure point with error-specific checks. e.g. what is done with the
>>>>      "task" variable in vTaskStartScheduler(), within the thread
>>>>      cancellation loop, ends up being convoluted and confusing, compared
>>>>      to unwinding and leaving the routine immediately after the error is
>>>>      detected.
>>>  
>>>  


> [...] 
> 
> Ok let's simplify the code (and add the missing lock):
> 
> write_lock_nocancel(&freertos_scheduler.lock);
> freertos_scheduler.state = taskSCHEDULER_NOT_STARTED;
> 
> /* cancel all tasks in list */
> while (!pvlist_empty(&freertos_task_list)) {
>     task = pvlist_first_entry(&freertos_task_list, struct freertos_task, 
> next);
>     if (threadobj_lock(&task->thobj) != -EINVAL) {
>             if (threadobj_get_magic(&task->thobj) == task_magic) {
>                     /* need to leave freertos_scheduler.lock since it is
>                         taken in task_finalizer while we block in
>                         threadobj_cancel */
>                     write_unlock(&freertos_scheduler.lock);
>                     threadobj_cancel(&task->thobj);
>                     write_lock_nocancel(&freertos_scheduler.lock);
>                     continue;
>             }
>             threadobj_unlock(&task->thobj);
>     }
>     /* Need to release the lock for a while since a task in deletion
>         may need it to unregister itself from freertos_task_list */
>     write_unlock(&freertos_scheduler.lock);
>     struct timespec rqt;
>     clockobj_ticks_to_timeout(&freertos_clock,
>                               0.001 * xGetTicksPerSecond(),
>                               &rqt);
>     threadobj_sleep(&rqt);
>     write_lock_nocancel(&freertos_scheduler.lock);
> }
> write_unlock(&freertos_scheduler.lock);
> 
> I had used pvlist_for_each_entry_safe() originally, but due to the possibility 
> having to access a single task multiple times, I ended up with two nested loops.


I will include this implementation in my next patch. This patch also includes
changes of comments to c-style and removal of (void)-casting ignored return values.
Also, I have adapted the patch to the latest (warn_unused_result) attributes.

Matthias

Re: [Xenomai] FreeRTOS skin

[Matthias Schneider]

----- Original Message -----

> From: Philippe Gerum <rpm@xenomai.org>
> To: Matthias Schneider <ma30002000@yahoo.de>; "xenomai@xenomai.org" <xenomai@xenomai.org>
> Cc: 
> Sent: Monday, May 5, 2014 12:33 PM
> Subject: Re: [Xenomai] FreeRTOS skin
> 
> On 05/04/2014 06:59 PM, Matthias Schneider wrote:
> 
>>  Hi all,
>> 
>>  please find enclosed a patch with a FreeRTOS skin for xenomai-forge I have
>>  been  working on for some time. I would like to get some feedback and advice
>>  what still needs to be done to get it accepted in Xenomai. There is a set of
>>  unit tests included and the possibility to download the original FreeRTOS package
>>  in order to run most of its (platform independent) test suite. Until now I have
>>  been working under mercury only. Documentation is available in form of a README
>>  file in lib/freertos/README, which should also be a good starting point.
> 
> Ok, thanks for this. Let's address issues gradually, starting with the 
> task module.
> 
[...]
> 
> - the read/write_lock_nocancel() form is reserved for protecting 
> sections which are known not to traverse cancellation points, as defined 
> by POSIX 
> (http://pubs.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_09.html). 
> This is about self-documentation of the code, and optimizing execution 
> by omitting costly management of cleanup blocks when it is deemed safe 
> to do so.
> 
> - CANCEL_DEFER() / CANCEL_RESTORE() should protect sections against 
> asynchronous cancellation, we typically do this before invoking the 
> copperplate layer, so that we may assume that pthread_cancel() won't be 
> a problem when running inner code, provided we have been careful about 
> handling read/write lock cleanups. These macros work by disabling 
> asynchronous cancellation for the current thread when 
> --enable-async-cancel was given at build time. Otherwise, this part is a 
> nop.

Obviously the *nocancel locks in my previous patch (task.c) were incorrect. 
However I am not sure which way to replace them - I see several possibilities:

a) 
push_cleanup_lock(&freertos_scheduler.lock); 
write_lock(&freertos_scheduler.lock); 
//will correctly clean up when thread is cancelled

b)
write_lock_safe(&freertos_scheduler.lock); 
// will disable cancellation until lock is released

c)
do no care about locks in cancelled threads and just use 
write_lock(&freertos_scheduler.lock)

Since freertos_scheduler.lock is essential for the correct functioning of the 
FreeRTOS skin, c) is probably out.

Any hint?

Thanks,
Matthias

Re: [Xenomai] FreeRTOS skin

[Philippe Gerum]

On 05/18/2014 08:30 PM, Matthias Schneider wrote:
> ----- Original Message -----
>
>> From: Philippe Gerum <rpm@xenomai.org>
>> To: Matthias Schneider <ma30002000@yahoo.de>; "xenomai@xenomai.org" <xenomai@xenomai.org>
>> Cc:
>> Sent: Monday, May 5, 2014 12:33 PM
>> Subject: Re: [Xenomai] FreeRTOS skin
>>
>> On 05/04/2014 06:59 PM, Matthias Schneider wrote:
>>
>>>   Hi all,
>>>
>>>   please find enclosed a patch with a FreeRTOS skin for xenomai-forge I have
>>>   been  working on for some time. I would like to get some feedback and advice
>>>   what still needs to be done to get it accepted in Xenomai. There is a set of
>>>   unit tests included and the possibility to download the original FreeRTOS package
>>>   in order to run most of its (platform independent) test suite. Until now I have
>>>   been working under mercury only. Documentation is available in form of a README
>>>   file in lib/freertos/README, which should also be a good starting point.
>>
>> Ok, thanks for this. Let's address issues gradually, starting with the
>> task module.
>>
> [...]
>>
>> - the read/write_lock_nocancel() form is reserved for protecting
>> sections which are known not to traverse cancellation points, as defined
>> by POSIX
>> (http://pubs.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_09.html).
>> This is about self-documentation of the code, and optimizing execution
>> by omitting costly management of cleanup blocks when it is deemed safe
>> to do so.
>>
>> - CANCEL_DEFER() / CANCEL_RESTORE() should protect sections against
>> asynchronous cancellation, we typically do this before invoking the
>> copperplate layer, so that we may assume that pthread_cancel() won't be
>> a problem when running inner code, provided we have been careful about
>> handling read/write lock cleanups. These macros work by disabling
>> asynchronous cancellation for the current thread when
>> --enable-async-cancel was given at build time. Otherwise, this part is a
>> nop.
>
> Obviously the *nocancel locks in my previous patch (task.c) were incorrect.
> However I am not sure which way to replace them - I see several possibilities:
>
> a)
> push_cleanup_lock(&freertos_scheduler.lock);
> write_lock(&freertos_scheduler.lock);
> //will correctly clean up when thread is cancelled
>
> b)
> write_lock_safe(&freertos_scheduler.lock);
> // will disable cancellation until lock is released
>
> c)
> do no care about locks in cancelled threads and just use
> write_lock(&freertos_scheduler.lock)
>
> Since freertos_scheduler.lock is essential for the correct functioning of the
> FreeRTOS skin, c) is probably out.
>
> Any hint?
>

I would restrict this lock to protecting manipulations of the task list, 
nothing more. It seems to be covering too much code right now. Assuming 
this, you would only need the _nocancel form to protect basic list 
operations which do not cross any cancellation point.

Having vTaskStartScheduler() wait for task termination should be as 
simple as a condvar-based protocol, e.g.

__RT(mutex_lock(&tasklist_lock));

for (;;) {
	if (pvlist_empty(&task_list))
		break;
	__RT(pthread_cond_wait(&tasklist_cond, &tasklist_lock));
}

__RT(mutex_unlock(&tasklist_lock));

You could then synchronize with the task finalizer, e.g.

task_finalizer:

__RT(mutex_lock(&tasklist_lock));

pvlist_remove(&task->next);

if (freertos_scheduler.state != taskSCHEDULER_RUNNING && 
pvlist_empty(&task_list))
	__RT(pthread_cond_signal(&tasklist_cond));

__RT(mutex_lock(&tasklist_lock));

I would also introduce a new scheduler state, e.g. 
taskSCHEDULER_STOPPING, which could be used to prevent a concurrent call 
to vTaskStartScheduler() to respawn the system until the ongoing 
shutdown has eventually completed.

PS: The __RT() modifier means to pick the dual kernel version of this 
call over Cobalt (i.e. the real-time one implemented by Xenomai), or the 
regular glibc one over Mercury. __STD() means to always pick the 
glibc-based call, regardless of the real-time core underneath.

If you mention no modifier, either of __RT/__STD may be assumed 
depending on whether symbol wrapping is enabled at link time. To prevent 
any ambiguity, a skin should always specify the appropriate modifier for 
calls the Cobalt core overrides from the glibc, since applications may 
or may not require symbol wrapping.

-- 
Philippe.

Re: [Xenomai] FreeRTOS skin

[Matthias Schneider]

----- Original Message -----
> From: Philippe Gerum <rpm@xenomai.org>
> To: Matthias Schneider <ma30002000@yahoo.de>; "xenomai@xenomai.org" <xenomai@xenomai.org>
> Cc: 
> Sent: Monday, May 19, 2014 9:46 AM
> Subject: Re: [Xenomai] FreeRTOS skin
> 
> On 05/18/2014 08:30 PM, Matthias Schneider wrote:
>>  ----- Original Message -----
>> 
>>>  From: Philippe Gerum <rpm@xenomai.org>
>>>  To: Matthias Schneider <ma30002000@yahoo.de>; 
> "xenomai@xenomai.org" <xenomai@xenomai.org>
>>>  Cc:
>>>  Sent: Monday, May 5, 2014 12:33 PM
>>>  Subject: Re: [Xenomai] FreeRTOS skin
>>> 
>>>  On 05/04/2014 06:59 PM, Matthias Schneider wrote:
>>> 
>>>>    Hi all,
>>>> 
>>>>    please find enclosed a patch with a FreeRTOS skin for 
> xenomai-forge I have
>>>>    been  working on for some time. I would like to get some feedback 
> and advice
>>>>    what still needs to be done to get it accepted in Xenomai. There 
> is a set of
>>>>    unit tests included and the possibility to download the original 
> FreeRTOS package
>>>>    in order to run most of its (platform independent) test suite. 
> Until now I have
>>>>    been working under mercury only. Documentation is available in 
> form of a README
>>>>    file in lib/freertos/README, which should also be a good starting 
> point.
>>> 
>>>  Ok, thanks for this. Let's address issues gradually, starting with 
> the
>>>  task module.
>>> 
>>  [...]
>>> 
>>>  - the read/write_lock_nocancel() form is reserved for protecting
>>>  sections which are known not to traverse cancellation points, as 
> defined
>>>  by POSIX
>>> 
> (http://pubs.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_09.html).
>>>  This is about self-documentation of the code, and optimizing execution
>>>  by omitting costly management of cleanup blocks when it is deemed safe
>>>  to do so.
>>> 
>>>  - CANCEL_DEFER() / CANCEL_RESTORE() should protect sections against
>>>  asynchronous cancellation, we typically do this before invoking the
>>>  copperplate layer, so that we may assume that pthread_cancel() 
> won't be
>>>  a problem when running inner code, provided we have been careful about
>>>  handling read/write lock cleanups. These macros work by disabling
>>>  asynchronous cancellation for the current thread when
>>>  --enable-async-cancel was given at build time. Otherwise, this part is 
> a
>>>  nop.
>> 
>>  Obviously the *nocancel locks in my previous patch (task.c) were incorrect.
>>  However I am not sure which way to replace them - I see several 
> possibilities:
>> 
>>  a)
>>  push_cleanup_lock(&freertos_scheduler.lock);
>>  write_lock(&freertos_scheduler.lock);
>>  //will correctly clean up when thread is cancelled
>> 
>>  b)
>>  write_lock_safe(&freertos_scheduler.lock);
>>  // will disable cancellation until lock is released
>> 
>>  c)
>>  do no care about locks in cancelled threads and just use
>>  write_lock(&freertos_scheduler.lock)
>> 
>>  Since freertos_scheduler.lock is essential for the correct functioning of 
> the
>>  FreeRTOS skin, c) is probably out.
>> 
>>  Any hint?
>> 
> 
> I would restrict this lock to protecting manipulations of the task list, 
> nothing more. It seems to be covering too much code right now. Assuming 
> this, you would only need the _nocancel form to protect basic list 
> operations which do not cross any cancellation point.
> 
> Having vTaskStartScheduler() wait for task termination should be as 
> simple as a condvar-based protocol, e.g.
> 
> __RT(mutex_lock(&tasklist_lock));
> 
> for (;;) {
>     if (pvlist_empty(&task_list))
>         break;
>     __RT(pthread_cond_wait(&tasklist_cond, &tasklist_lock));
> }
> 
> __RT(mutex_unlock(&tasklist_lock));
> 
> You could then synchronize with the task finalizer, e.g.
> 
> task_finalizer:
> 
> __RT(mutex_lock(&tasklist_lock));
> 
> pvlist_remove(&task->next);
> 
> if (freertos_scheduler.state != taskSCHEDULER_RUNNING && 
> pvlist_empty(&task_list))
>     __RT(pthread_cond_signal(&tasklist_cond));
> 
> __RT(mutex_lock(&tasklist_lock));
> 
> I would also introduce a new scheduler state, e.g. 
> taskSCHEDULER_STOPPING, which could be used to prevent a concurrent call 
> to vTaskStartScheduler() to respawn the system until the ongoing 
> shutdown has eventually completed.
> 
> PS: The __RT() modifier means to pick the dual kernel version of this 
> call over Cobalt (i.e. the real-time one implemented by Xenomai), or the 
> regular glibc one over Mercury. __STD() means to always pick the 
> glibc-based call, regardless of the real-time core underneath.
> 
> If you mention no modifier, either of __RT/__STD may be assumed 
> depending on whether symbol wrapping is enabled at link time. To prevent 
> any ambiguity, a skin should always specify the appropriate modifier for 
> calls the Cobalt core overrides from the glibc, since applications may 
> or may not require symbol wrapping.
> 

Considering that the simple approach was the one I had originally employed
until I implemented vTaskResumeAll and vTaskSuspendAll
I suppose it all comes down how to implement these two functions.

I am nearly convinced to implement them via threadobj_lock_sched() and 
threadobj_unlock_sched(), which should make most of the proposed 
re-simplifications possible, but I have still two points:

* On smp machines, it will not behave as expected since the mentioned
functions will prevent the current thread from being scheduled out,
but not from other tasks being executed on other CPUs if I am correct

* functions like vTaskDelay / threadobj_sleep() will not perform as expected
since they may cause the curren task being schduled out.

Is there anything I can do about these?

Also, functions like xTaskIsTaskSuspended() and uxTaskPriorityGet need to be
adapted to return simulated values, but this should be easy.

Re: [Xenomai] FreeRTOS skin

[Philippe Gerum]

On 05/20/2014 09:06 AM, Matthias Schneider wrote:
>
>
>
>
>
> ----- Original Message -----
>> From: Philippe Gerum <rpm@xenomai.org>
>> To: Matthias Schneider <ma30002000@yahoo.de>; "xenomai@xenomai.org" <xenomai@xenomai.org>
>> Cc:
>> Sent: Monday, May 19, 2014 9:46 AM
>> Subject: Re: [Xenomai] FreeRTOS skin
>>
>> On 05/18/2014 08:30 PM, Matthias Schneider wrote:
>>>   ----- Original Message -----
>>>
>>>>   From: Philippe Gerum <rpm@xenomai.org>
>>>>   To: Matthias Schneider <ma30002000@yahoo.de>;
>> "xenomai@xenomai.org" <xenomai@xenomai.org>
>>>>   Cc:
>>>>   Sent: Monday, May 5, 2014 12:33 PM
>>>>   Subject: Re: [Xenomai] FreeRTOS skin
>>>>
>>>>   On 05/04/2014 06:59 PM, Matthias Schneider wrote:
>>>>
>>>>>     Hi all,
>>>>>
>>>>>     please find enclosed a patch with a FreeRTOS skin for
>> xenomai-forge I have
>>>>>     been  working on for some time. I would like to get some feedback
>> and advice
>>>>>     what still needs to be done to get it accepted in Xenomai. There
>> is a set of
>>>>>     unit tests included and the possibility to download the original
>> FreeRTOS package
>>>>>     in order to run most of its (platform independent) test suite.
>> Until now I have
>>>>>     been working under mercury only. Documentation is available in
>> form of a README
>>>>>     file in lib/freertos/README, which should also be a good starting
>> point.
>>>>
>>>>   Ok, thanks for this. Let's address issues gradually, starting with
>> the
>>>>   task module.
>>>>
>>>   [...]
>>>>
>>>>   - the read/write_lock_nocancel() form is reserved for protecting
>>>>   sections which are known not to traverse cancellation points, as
>> defined
>>>>   by POSIX
>>>>
>> (http://pubs.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_09.html).
>>>>   This is about self-documentation of the code, and optimizing execution
>>>>   by omitting costly management of cleanup blocks when it is deemed safe
>>>>   to do so.
>>>>
>>>>   - CANCEL_DEFER() / CANCEL_RESTORE() should protect sections against
>>>>   asynchronous cancellation, we typically do this before invoking the
>>>>   copperplate layer, so that we may assume that pthread_cancel()
>> won't be
>>>>   a problem when running inner code, provided we have been careful about
>>>>   handling read/write lock cleanups. These macros work by disabling
>>>>   asynchronous cancellation for the current thread when
>>>>   --enable-async-cancel was given at build time. Otherwise, this part is
>> a
>>>>   nop.
>>>
>>>   Obviously the *nocancel locks in my previous patch (task.c) were incorrect.
>>>   However I am not sure which way to replace them - I see several
>> possibilities:
>>>
>>>   a)
>>>   push_cleanup_lock(&freertos_scheduler.lock);
>>>   write_lock(&freertos_scheduler.lock);
>>>   //will correctly clean up when thread is cancelled
>>>
>>>   b)
>>>   write_lock_safe(&freertos_scheduler.lock);
>>>   // will disable cancellation until lock is released
>>>
>>>   c)
>>>   do no care about locks in cancelled threads and just use
>>>   write_lock(&freertos_scheduler.lock)
>>>
>>>   Since freertos_scheduler.lock is essential for the correct functioning of
>> the
>>>   FreeRTOS skin, c) is probably out.
>>>
>>>   Any hint?
>>>
>>
>> I would restrict this lock to protecting manipulations of the task list,
>> nothing more. It seems to be covering too much code right now. Assuming
>> this, you would only need the _nocancel form to protect basic list
>> operations which do not cross any cancellation point.
>>
>> Having vTaskStartScheduler() wait for task termination should be as
>> simple as a condvar-based protocol, e.g.
>>
>> __RT(mutex_lock(&tasklist_lock));
>>
>> for (;;) {
>>      if (pvlist_empty(&task_list))
>>          break;
>>      __RT(pthread_cond_wait(&tasklist_cond, &tasklist_lock));
>> }
>>
>> __RT(mutex_unlock(&tasklist_lock));
>>
>> You could then synchronize with the task finalizer, e.g.
>>
>> task_finalizer:
>>
>> __RT(mutex_lock(&tasklist_lock));
>>
>> pvlist_remove(&task->next);
>>
>> if (freertos_scheduler.state != taskSCHEDULER_RUNNING &&
>> pvlist_empty(&task_list))
>>      __RT(pthread_cond_signal(&tasklist_cond));
>>
>> __RT(mutex_lock(&tasklist_lock));
>>
>> I would also introduce a new scheduler state, e.g.
>> taskSCHEDULER_STOPPING, which could be used to prevent a concurrent call
>> to vTaskStartScheduler() to respawn the system until the ongoing
>> shutdown has eventually completed.
>>
>> PS: The __RT() modifier means to pick the dual kernel version of this
>> call over Cobalt (i.e. the real-time one implemented by Xenomai), or the
>> regular glibc one over Mercury. __STD() means to always pick the
>> glibc-based call, regardless of the real-time core underneath.
>>
>> If you mention no modifier, either of __RT/__STD may be assumed
>> depending on whether symbol wrapping is enabled at link time. To prevent
>> any ambiguity, a skin should always specify the appropriate modifier for
>> calls the Cobalt core overrides from the glibc, since applications may
>> or may not require symbol wrapping.
>>
>
> Considering that the simple approach was the one I had originally employed
> until I implemented vTaskResumeAll and vTaskSuspendAll
> I suppose it all comes down how to implement these two functions.
>
> I am nearly convinced to implement them via threadobj_lock_sched() and
> threadobj_unlock_sched(), which should make most of the proposed
> re-simplifications possible, but I have still two points:
>
> * On smp machines, it will not behave as expected since the mentioned
> functions will prevent the current thread from being scheduled out,
> but not from other tasks being executed on other CPUs if I am correct
>

Yes. But does the FreeRTOS API spec state that such calls should be 
SMP-capable? Is there any FreeRTOS incarnation leveraging multi-core 
systems?

> * functions like vTaskDelay / threadobj_sleep() will not perform as expected
> since they may cause the curren task being schduled out.
>
> Is there anything I can do about these?
>
> Also, functions like xTaskIsTaskSuspended() and uxTaskPriorityGet need to be
> adapted to return simulated values, but this should be easy.
>

A more general note about sched locking in a Xenomai environment (both 
Cobalt and Mercury):

threadobj_lock/unlock_sched() will not even prevent the locking thread 
to be scheduled out, it will prevent involuntary preemption, which is 
quite different. If that thread has to block internally for gaining 
access to any resource, it will schedule out, including on UP systems 
(think of a copy-on-write fault or blocking on a native kernel mutex 
with Mercury, or relaxing a thread with Cobalt, both threads holding the 
scheduler lock when this happens).

On most legacy RTOS with no MMU or sleeping locks, this won't happen, 
therefore blocking voluntarily while holding a sched lock will actually 
freeze the system entirely, as this is not considered valid action from 
the application. Xenomai allows blocking voluntarily while holding such 
lock, which is automatically dropped during the sleep time, because 
there are situations imposed on us by the host GPOS which may require this.

Generally speaking, sched locking/unlocking is a fundamentally wrong 
locking method: it's just about using a single giant traffic light for 
controlling access to resources.

This (anti-)feature is provided only to emulate closely enough those 
legacy RTOS APIs for supporting applications, but using it for 
synchronizing emulators internally is definitely not the way to go.

In short, sched locking is not simple, it's just coarse-grained and I 
would go on to say "unreliable by design" over Mercury, since there is 
no way to prevent a thread from being blocked on some kernel resource by 
the host GPOS running the emulator. The only option would be to force 
all threads to suspend in a signal handler (and make sure they did 
before proceeding), which seems to me seriously overkill, given that 
such locking construct should be discouraged.

This said, I agree that vTaskResumeAll() and vTaskSuspendAll() should 
map over threadobj_unlock_sched() and threadobj_lock_sched() 
respectively, hoping for the best possible emulation of the scheduler lock.

-- 
Philippe.