* [Xenomai] FreeRTOS skin #2
@ 2014-05-22 19:46 Matthias Schneider
2014-06-02 18:46 ` Matthias Schneider
0 siblings, 1 reply; 12+ messages in thread
From: Matthias Schneider @ 2014-05-22 19:46 UTC (permalink / raw
To: xenomai@xenomai.org
Hi all,
after having rework extensively the FreeRTOS skin patch I feel ready
for the second round of review comments. I hope no one will mind me
starting a new thread for discussing it. I have tried to address all
of the mentioned issues so far, plus many more.
Among them:
* reworked alignment macros
* #define portLONG long
* remapped error codes
* reworked scheduler start and stop completely as suggested
* reworked Scheduler.queued_task, .state and .num_tasks
* added new (internal) scheduler state taskSCHEDULER_SUSPENDING
* many other improvements and simplifications
* only take tasklist_lock for short time
Like suggested, I have gone for the simpler implementation of the task
scheduler functionality and I added warnings and checks in case unsupported
use cases are triggered (e.g. holding the scheduler lock on mp systems, etc.)
This still needs to be reflected in the docu.
I have not yet addressed the issues of
* vPortEnterCritical()/vPortExitCritical()
since they are used extensively in the demo code (which is completely
hardware independent), but this can still be easily adapted.
* starting thread priority inheritance (need to start a new
discussion thread)
* there is still a workaround for the cobalt priorities in the patch...
Regards,
Matthias
p.s. Thank you a lot for your responsiveness addressing issues reported
by me, including all the refactoring that is making my life easier!
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Initial-Freertos-commit.-2.patch.gz
Type: application/x-gzip
Size: 34572 bytes
Desc: not available
URL: <http://www.xenomai.org/pipermail/xenomai/attachments/20140522/0136d1b5/attachment.bin>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Xenomai] FreeRTOS skin #2
2014-05-22 19:46 [Xenomai] FreeRTOS skin #2 Matthias Schneider
@ 2014-06-02 18:46 ` Matthias Schneider
2014-06-20 7:55 ` Philippe Gerum
0 siblings, 1 reply; 12+ messages in thread
From: Matthias Schneider @ 2014-06-02 18:46 UTC (permalink / raw
To: Matthias Schneider, xenomai@xenomai.org
----- Original Message -----
> From: Matthias Schneider <ma30002000@yahoo.de>
> To: "xenomai@xenomai.org" <xenomai@xenomai.org>
> Cc:
> Sent: Thursday, May 22, 2014 9:46 PM
> Subject: [Xenomai] FreeRTOS skin #2
>
> Hi all,
>
> after having rework extensively the FreeRTOS skin patch I feel ready
> for the second round of review comments. I hope no one will mind me
> starting a new thread for discussing it. I have tried to address all
> of the mentioned issues so far, plus many more.
>
> Among them:
>
> * reworked alignment macros
> * #define portLONG long
> * remapped error codes
> * reworked scheduler start and stop completely as suggested
> * reworked Scheduler.queued_task, .state and .num_tasks
> * added new (internal) scheduler state taskSCHEDULER_SUSPENDING
> * many other improvements and simplifications
> * only take tasklist_lock for short time
>
> Like suggested, I have gone for the simpler implementation of the task
> scheduler functionality and I added warnings and checks in case unsupported
> use cases are triggered (e.g. holding the scheduler lock on mp systems, etc.)
> This still needs to be reflected in the docu.
>
> I have not yet addressed the issues of
>
> * vPortEnterCritical()/vPortExitCritical()
> since they are used extensively in the demo code (which is completely
> hardware independent), but this can still be easily adapted.
>
> * starting thread priority inheritance (need to start a new
> discussion thread)
>
> * there is still a workaround for the cobalt priorities in the patch...
>
> Regards,
> Matthias
>
> p.s. Thank you a lot for your responsiveness addressing issues reported
> by me, including all the refactoring that is making my life easier!
Please find enclosed a new version of the FreeRTOS skin. It adapts to the
latest changes in forge/next and also removes the previously included
workarounds - thanks to the changes done based on the first review round
they were no longer necessary.
Regards,
Matthias
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Initial-Freertos-commit-no.-3.patch.gz
Type: application/x-gzip
Size: 34384 bytes
Desc: not available
URL: <http://www.xenomai.org/pipermail/xenomai/attachments/20140602/7a477986/attachment.bin>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Xenomai] FreeRTOS skin #2
2014-06-02 18:46 ` Matthias Schneider
@ 2014-06-20 7:55 ` Philippe Gerum
2014-06-23 10:31 ` Matthias Schneider
0 siblings, 1 reply; 12+ messages in thread
From: Philippe Gerum @ 2014-06-20 7:55 UTC (permalink / raw
To: Matthias Schneider, xenomai@xenomai.org
On 06/02/2014 08:46 PM, Matthias Schneider wrote:
> Please find enclosed a new version of the FreeRTOS skin. It adapts to the
> latest changes in forge/next and also removes the previously included
> workarounds - thanks to the changes done based on the first review round
> they were no longer necessary.
>
Thanks. Here we go:
* vTaskStartScheduler()
Dropping the task_list lock then yielding is superfluous, the caller
will wait on the condvar for the scheduler state to switch to SUSPENDING
anyway, implicitly releasing this lock before going to sleep.
* vTaskDelay()
Cobalt wraps sched_yield(), so you likely want to call
__RT(sched_yield()) to pick the proper implementation depending on the
underlying real-time core.
* __xTaskGenericCreate()
- It looks like the mutex attribute initialized locally is a left over
from a previous implementation.
- Drawing unique identifier labels should be done using a name
generator, e.g.
static DEFINE_NAME_GENERATOR(task_namegen, struct freertos_task, name);
...
generate_name(task->name, name, &task_namegen);
The open-coded method also used by other emulators is racy.
* vPortEnter/ExitCritical()
- wouldn't these calls better emulated by threadobj_lock_sched()
assuming they stand for a global serialization mechanism?
* Task registry support
The code looks outdated, using fsobstacks is recommended. The registry
helpers in lib/alchemy/alarm.c illustrate a basic usage.
* vTaskSuspendAll()
Running check_processors() there may switch the caller to linux mode
at each invocation, due to calling sysconf/sched_getaffinity indirectly.
I would rather assume that the CPU affinity is not allowed to change
once started, and determine whether a task is bound to more than a
single CPU in the trampoline code. Then vTaskSuspendAll() may do a
simple flag-based test for checking the consistency, still allowing
different freertos tasks to be pinned on distinct CPUs though. At any
rate, this would be an acceptable restriction, since freertos does not
seem to be SMP-capable in its reference implementation anyway.
Next time, I'll start reviewing other parts of the emulator.
HTH,
--
Philippe.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Xenomai] FreeRTOS skin #2
2014-06-20 7:55 ` Philippe Gerum
@ 2014-06-23 10:31 ` Matthias Schneider
2014-06-23 20:27 ` Matthias Schneider
0 siblings, 1 reply; 12+ messages in thread
From: Matthias Schneider @ 2014-06-23 10:31 UTC (permalink / raw
To: Philippe Gerum, xenomai@xenomai.org
----- Original Message -----
> From: Philippe Gerum <rpm@xenomai.org>
> To: Matthias Schneider <ma30002000@yahoo.de>; "xenomai@xenomai.org" <xenomai@xenomai.org>
> Cc:
> Sent: Friday, June 20, 2014 9:55 AM
> Subject: Re: [Xenomai] FreeRTOS skin #2
>
> On 06/02/2014 08:46 PM, Matthias Schneider wrote:
>
>
>> Please find enclosed a new version of the FreeRTOS skin. It adapts to the
>> latest changes in forge/next and also removes the previously included
>> workarounds - thanks to the changes done based on the first review round
>> they were no longer necessary.
>>
>
> Thanks. Here we go:
>
> * vTaskStartScheduler()
>
> Dropping the task_list lock then yielding is superfluous, the caller
> will wait on the condvar for the scheduler state to switch to SUSPENDING
> anyway, implicitly releasing this lock before going to sleep.
Done.
>
> * vTaskDelay()
>
> Cobalt wraps sched_yield(), so you likely want to call
> __RT(sched_yield()) to pick the proper implementation depending on the
> underlying real-time core.
Done.
> * __xTaskGenericCreate()
>
> - It looks like the mutex attribute initialized locally is a left over
> from a previous implementation.
>
> - Drawing unique identifier labels should be done using a name
> generator, e.g.
>
> static DEFINE_NAME_GENERATOR(task_namegen, struct freertos_task, name);
> ...
> generate_name(task->name, name, &task_namegen);
> The open-coded method also used by other emulators is racy.
Done.
Note that some freertos specific code remains to make duplicated task names
unique.
> * vPortEnter/ExitCritical()
>
> - wouldn't these calls better emulated by threadobj_lock_sched()
> assuming they stand for a global serialization mechanism?
Unfortunately, this seems cause some test cases in the demo to fail.
Since these tests are rather time consuming to investigate, I would like
to leave this item open until all other issues have been addressed. We
had also talked about other options concerning this subject as well
(removing the function or leaving it empty), so I still need some
time to think about it.
> * Task registry support
>
> The code looks outdated, using fsobstacks is recommended. The registry
> helpers in lib/alchemy/alarm.c illustrate a basic usage.
Done.
>
> * vTaskSuspendAll()
>
> Running check_processors() there may switch the caller to linux mode
> at each invocation, due to calling sysconf/sched_getaffinity indirectly.
> I would rather assume that the CPU affinity is not allowed to change
> once started, and determine whether a task is bound to more than a
> single CPU in the trampoline code. Then vTaskSuspendAll() may do a
> simple flag-based test for checking the consistency, still allowing
> different freertos tasks to be pinned on distinct CPUs though. At any
> rate, this would be an acceptable restriction, since freertos does not
> seem to be SMP-capable in its reference implementation anyway.
Ok, reading the affinity information once and caching it makes sense to
avoid the switches to secondary mode. However, vTaskSuspendAll() / vTaskResumeAll() only works as expected when all tasks are pinned to a
single cpu, storing this information on a by-task basis will not be sufficient.
Now if there is one place to retrieve this information, which would it be?
I can think of freertos_init() / freertos_scheduler_init() or
vTaskStartScheduler(). However, freertos_init() would probably not be
able to allow setting process cpu affinity programatically at the
beginning of main() if I am not mistaken.
> Next time, I'll start reviewing other parts of the emulator.
Great, thanks in advance for all the input..
> HTH,
>
> --
> Philippe.
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Xenomai] FreeRTOS skin #2
2014-06-23 10:31 ` Matthias Schneider
@ 2014-06-23 20:27 ` Matthias Schneider
2014-07-01 16:48 ` Matthias Schneider
0 siblings, 1 reply; 12+ messages in thread
From: Matthias Schneider @ 2014-06-23 20:27 UTC (permalink / raw
To: Philippe Gerum, xenomai@xenomai.org
----- Original Message -----
> From: Matthias Schneider <ma30002000@yahoo.de>
> To: Philippe Gerum <rpm@xenomai.org>; "xenomai@xenomai.org" <xenomai@xenomai.org>
> Cc:
> Sent: Monday, June 23, 2014 12:31 PM
> Subject: Re: [Xenomai] FreeRTOS skin #2
>
> ----- Original Message -----
>> From: Philippe Gerum <rpm@xenomai.org>
>> To: Matthias Schneider <ma30002000@yahoo.de>;
> "xenomai@xenomai.org" <xenomai@xenomai.org>
>> Cc:
>> Sent: Friday, June 20, 2014 9:55 AM
>> Subject: Re: [Xenomai] FreeRTOS skin #2
>>
>> On 06/02/2014 08:46 PM, Matthias Schneider wrote:
>>
>>
>>> Please find enclosed a new version of the FreeRTOS skin. It adapts to
> the
>>> latest changes in forge/next and also removes the previously included
>>> workarounds - thanks to the changes done based on the first review
> round
>>> they were no longer necessary.
>>>
>>
>> Thanks. Here we go:
>>
>> * vTaskStartScheduler()
>>
>> Dropping the task_list lock then yielding is superfluous, the caller
>> will wait on the condvar for the scheduler state to switch to SUSPENDING
>> anyway, implicitly releasing this lock before going to sleep.
>
> Done.
>
>>
>> * vTaskDelay()
>>
>> Cobalt wraps sched_yield(), so you likely want to call
>> __RT(sched_yield()) to pick the proper implementation depending on the
>> underlying real-time core.
> Done.
>
>> * __xTaskGenericCreate()
>>
>> - It looks like the mutex attribute initialized locally is a left over
>> from a previous implementation.
>>
>> - Drawing unique identifier labels should be done using a name
>> generator, e.g.
>>
>> static DEFINE_NAME_GENERATOR(task_namegen, struct freertos_task, name);
>> ...
>> generate_name(task->name, name, &task_namegen);
>> The open-coded method also used by other emulators is racy.
> Done.
> Note that some freertos specific code remains to make duplicated task names
> unique.
>
>> * vPortEnter/ExitCritical()
>>
>> - wouldn't these calls better emulated by threadobj_lock_sched()
>> assuming they stand for a global serialization mechanism?
> Unfortunately, this seems cause some test cases in the demo to fail.
> Since these tests are rather time consuming to investigate, I would like
> to leave this item open until all other issues have been addressed. We
> had also talked about other options concerning this subject as well
> (removing the function or leaving it empty), so I still need some
> time to think about it.
>> * Task registry support
>>
>> The code looks outdated, using fsobstacks is recommended. The registry
>> helpers in lib/alchemy/alarm.c illustrate a basic usage.
> Done.
>
>>
>> * vTaskSuspendAll()
>>
>> Running check_processors() there may switch the caller to linux mode
>> at each invocation, due to calling sysconf/sched_getaffinity indirectly.
>> I would rather assume that the CPU affinity is not allowed to change
>> once started, and determine whether a task is bound to more than a
>> single CPU in the trampoline code. Then vTaskSuspendAll() may do a
>> simple flag-based test for checking the consistency, still allowing
>> different freertos tasks to be pinned on distinct CPUs though. At any
>> rate, this would be an acceptable restriction, since freertos does not
>> seem to be SMP-capable in its reference implementation anyway.
> Ok, reading the affinity information once and caching it makes sense to
> avoid the switches to secondary mode. However, vTaskSuspendAll() /
> vTaskResumeAll() only works as expected when all tasks are pinned to a
> single cpu, storing this information on a by-task basis will not be sufficient.
> Now if there is one place to retrieve this information, which would it be?
> I can think of freertos_init() / freertos_scheduler_init() or
> vTaskStartScheduler(). However, freertos_init() would probably not be
> able to allow setting process cpu affinity programatically at the
> beginning of main() if I am not mistaken.
>> Next time, I'll start reviewing other parts of the emulator.
> Great, thanks in advance for all the input..
>
>> HTH,
>>
>> --
>> Philippe.
>>
Please find enclosed a revised patch.
Matthias
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Initial-Freertos-commit-no.-4.patch.gz
Type: application/x-gzip
Size: 34354 bytes
Desc: not available
URL: <http://www.xenomai.org/pipermail/xenomai/attachments/20140623/2a044048/attachment.bin>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Xenomai] FreeRTOS skin #2
2014-06-23 20:27 ` Matthias Schneider
@ 2014-07-01 16:48 ` Matthias Schneider
2014-07-09 7:42 ` Philippe Gerum
2014-07-09 7:46 ` Philippe Gerum
0 siblings, 2 replies; 12+ messages in thread
From: Matthias Schneider @ 2014-07-01 16:48 UTC (permalink / raw
To: Matthias Schneider, Philippe Gerum, xenomai@xenomai.org
----- Original Message -----
> From: Matthias Schneider <ma30002000@yahoo.de>
> To: Philippe Gerum <rpm@xenomai.org>; "xenomai@xenomai.org" <xenomai@xenomai.org>
> Cc:
> Sent: Monday, June 23, 2014 10:27 PM
> Subject: Re: [Xenomai] FreeRTOS skin #2
>
> ----- Original Message -----
>
>> From: Matthias Schneider <ma30002000@yahoo.de>
>> To: Philippe Gerum <rpm@xenomai.org>; "xenomai@xenomai.org"
> <xenomai@xenomai.org>
>> Cc:
>> Sent: Monday, June 23, 2014 12:31 PM
>> Subject: Re: [Xenomai] FreeRTOS skin #2
>>
>> ----- Original Message -----
>>> From: Philippe Gerum <rpm@xenomai.org>
>>> To: Matthias Schneider <ma30002000@yahoo.de>;
>> "xenomai@xenomai.org" <xenomai@xenomai.org>
>>> Cc:
>>> Sent: Friday, June 20, 2014 9:55 AM
>>> Subject: Re: [Xenomai] FreeRTOS skin #2
>>>
>>> On 06/02/2014 08:46 PM, Matthias Schneider wrote:
>>>
>>>
>>>> Please find enclosed a new version of the FreeRTOS skin. It adapts
> to
>> the
>>>> latest changes in forge/next and also removes the previously
> included
>>>> workarounds - thanks to the changes done based on the first review
>
>> round
>>>> they were no longer necessary.
>>>>
>>>
>>> Thanks. Here we go:
>>>
>>> * vTaskStartScheduler()
>>>
>>> Dropping the task_list lock then yielding is superfluous, the caller
>>> will wait on the condvar for the scheduler state to switch to
> SUSPENDING
>>> anyway, implicitly releasing this lock before going to sleep.
>>
>> Done.
>>
>>>
>>> * vTaskDelay()
>>>
>>> Cobalt wraps sched_yield(), so you likely want to call
>>> __RT(sched_yield()) to pick the proper implementation depending on the
>
>>> underlying real-time core.
>> Done.
>>
>>> * __xTaskGenericCreate()
>>>
>>> - It looks like the mutex attribute initialized locally is a left over
>
>>> from a previous implementation.
>>>
>>> - Drawing unique identifier labels should be done using a name
>>> generator, e.g.
>>>
>>> static DEFINE_NAME_GENERATOR(task_namegen, struct freertos_task,
> name);
>>> ...
>>> generate_name(task->name, name, &task_namegen);
>>> The open-coded method also used by other emulators is racy.
>> Done.
>> Note that some freertos specific code remains to make duplicated task names
>
>> unique.
>>
>>> * vPortEnter/ExitCritical()
>>>
>>> - wouldn't these calls better emulated by threadobj_lock_sched()
>>> assuming they stand for a global serialization mechanism?
>> Unfortunately, this seems cause some test cases in the demo to fail.
>> Since these tests are rather time consuming to investigate, I would like
>> to leave this item open until all other issues have been addressed. We
>> had also talked about other options concerning this subject as well
>> (removing the function or leaving it empty), so I still need some
>> time to think about it.
>>> * Task registry support
>>>
>>> The code looks outdated, using fsobstacks is recommended. The registry
>
>>> helpers in lib/alchemy/alarm.c illustrate a basic usage.
>> Done.
>>
>>>
>>> * vTaskSuspendAll()
>>>
>>> Running check_processors() there may switch the caller to linux mode
>>> at each invocation, due to calling sysconf/sched_getaffinity
> indirectly.
>>> I would rather assume that the CPU affinity is not allowed to change
>>> once started, and determine whether a task is bound to more than a
>>> single CPU in the trampoline code. Then vTaskSuspendAll() may do a
>>> simple flag-based test for checking the consistency, still allowing
>>> different freertos tasks to be pinned on distinct CPUs though. At any
>>> rate, this would be an acceptable restriction, since freertos does not
>
>>> seem to be SMP-capable in its reference implementation anyway.
>> Ok, reading the affinity information once and caching it makes sense to
>> avoid the switches to secondary mode. However, vTaskSuspendAll() /
>> vTaskResumeAll() only works as expected when all tasks are pinned to a
>> single cpu, storing this information on a by-task basis will not be
> sufficient.
>> Now if there is one place to retrieve this information, which would it be?
>> I can think of freertos_init() / freertos_scheduler_init() or
>> vTaskStartScheduler(). However, freertos_init() would probably not be
>> able to allow setting process cpu affinity programatically at the
>> beginning of main() if I am not mistaken.
>>> Next time, I'll start reviewing other parts of the emulator.
>> Great, thanks in advance for all the input..
>>
>>> HTH,
>>>
>>> --
>>> Philippe.
>>>
>
>
> Please find enclosed a revised patch.
>
> Matthias
Please find enclosed another calling the check_processors() only when starting
the scheduler and remembering its output. It is also adapted to the latest changes.
Any comments will be appreciated.
Matthias
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Initial-Freertos-commit-no.-5.patch.gz
Type: application/gzip
Size: 34306 bytes
Desc: not available
URL: <http://www.xenomai.org/pipermail/xenomai/attachments/20140701/e7483dac/attachment.bin>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Xenomai] FreeRTOS skin #2
2014-07-01 16:48 ` Matthias Schneider
@ 2014-07-09 7:42 ` Philippe Gerum
2014-07-09 8:24 ` Philippe Gerum
2014-07-14 19:18 ` Matthias Schneider
2014-07-09 7:46 ` Philippe Gerum
1 sibling, 2 replies; 12+ messages in thread
From: Philippe Gerum @ 2014-07-09 7:42 UTC (permalink / raw
To: Matthias Schneider, xenomai@xenomai.org
On 07/01/2014 06:48 PM, Matthias Schneider wrote:
> Please find enclosed another calling the check_processors() only when starting
> the scheduler and remembering its output. It is also adapted to the latest changes.
> Any comments will be appreciated.
>
- clusters allowing key dups are available using the *_dup() call form
- vTaskEndScheduler() can be simplified, and should not panic if it
cannot lock a vanishing thread pulled from the task list.
commit 9239281e4badb56a2449723e0204c597ed3743b1
Author: Philippe Gerum <rpm@xenomai.org>
Date: Wed Jul 9 09:26:55 2014 +0200
freertos: use cluster with duplicate key support
diff --git a/lib/freertos/task.c b/lib/freertos/task.c
index f7d99a5..d201711 100644
--- a/lib/freertos/task.c
+++ b/lib/freertos/task.c
@@ -407,7 +407,6 @@ static signed portBASE_TYPE
__xTaskGenericCreate(struct freertos_task *task,
struct corethread_attributes cta;
struct threadobj_init_data idata;
int ret, cprio;
- size_t i = 0, id_pos;
cprio = convert_freertos_task_priority(uxPriority);
@@ -427,24 +426,7 @@ static signed portBASE_TYPE
__xTaskGenericCreate(struct freertos_task *task,
generate_name(task->name, (const char *)name, &task_namegen);
- /* since cluster entries must have unique names, we try to append
- names that seem to exist already with an id. */
- for (i = 0; i < TASK_DUPLICATED_NAME_MAX_ID; ++i) {
- ret = cluster_addobj(&freertos_task_table, task->name, &task->cobj);
- if (ret != -EEXIST)
- break;
- if (i == 0) {
- id_pos = strlen(task->name);
- id_pos = (id_pos > TASK_DUPLICATED_NAME_MAX_ID_MAX_POS)
- ? TASK_DUPLICATED_NAME_MAX_ID_MAX_POS
- : id_pos;
- }
- sprintf(task->name + id_pos, "%1d", i);
- }
- if (ret == -EEXIST) {
- warning("duplicate task name: %s (used > %d times)",
- task->name, TASK_DUPLICATED_NAME_MAX_ID);
- }
+ ret = cluster_addobj_dup(&freertos_task_table, task->name, &task->cobj);
if (__bt(ret)) {
threadobj_uninit(&task->thobj);
return pdFALSE;
commit c867515edfd63ce7abc0b76095e0613ceb8f7f22
Author: Philippe Gerum <rpm@xenomai.org>
Date: Wed Jul 9 09:21:03 2014 +0200
freertos: fix vTaskEndScheduler()
diff --git a/lib/freertos/task.c b/lib/freertos/task.c
index 0af799a..f7d99a5 100644
--- a/lib/freertos/task.c
+++ b/lib/freertos/task.c
@@ -445,8 +445,7 @@ static signed portBASE_TYPE
__xTaskGenericCreate(struct freertos_task *task,
warning("duplicate task name: %s (used > %d times)",
task->name, TASK_DUPLICATED_NAME_MAX_ID);
}
- if (ret) {
- __bt(ret);
+ if (__bt(ret)) {
threadobj_uninit(&task->thobj);
return pdFALSE;
}
@@ -781,35 +780,33 @@ void vTaskStartScheduler(void) {
CANCEL_RESTORE(svc);
}
-void vTaskEndScheduler(void) {
+void vTaskEndScheduler(void)
+{
struct service svc;
- struct freertos_task *task, *tmp, *current;
+ struct freertos_task *task, *current;
CANCEL_DEFER(svc);
- threadobj_lock_sched();
+ current = freertos_task_current();
__RT(pthread_mutex_lock(&freertos_scheduler.tasklist_lock));
freertos_scheduler.state = taskSCHEDULER_SUSPENDING;
- __RT(pthread_mutex_unlock(&freertos_scheduler.tasklist_lock));
- current = freertos_task_current();
- if (!pvlist_empty(&freertos_task_list)) {
- pvlist_for_each_entry_safe(task, tmp, &freertos_task_list, next) {
- if (task == current)
- continue;
-
- if (get_freertos_task(task))
- threadobj_cancel(&task->thobj);
- else
- panic("could not lock task %s", task->name);
- }
+ while (!pvlist_empty(&freertos_task_list)) {
+ task = pvlist_pop_entry(&freertos_task_list,
+ struct freertos_task, next);
+ if (task == current)
+ continue;
+ __RT(pthread_mutex_unlock(&freertos_scheduler.tasklist_lock));
+ if (get_freertos_task(task))
+ threadobj_cancel(&task->thobj);
+ __RT(pthread_mutex_lock(&freertos_scheduler.tasklist_lock));
}
- threadobj_unlock_sched();
- CANCEL_RESTORE(svc);
-
+ __RT(pthread_mutex_unlock(&freertos_scheduler.tasklist_lock));
threadobj_lock(¤t->thobj);
threadobj_cancel(¤t->thobj);
+
+ CANCEL_RESTORE(svc);
}
portBASE_TYPE xTaskGetSchedulerState(void)
--
Philippe.
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Xenomai] FreeRTOS skin #2
2014-07-01 16:48 ` Matthias Schneider
2014-07-09 7:42 ` Philippe Gerum
@ 2014-07-09 7:46 ` Philippe Gerum
1 sibling, 0 replies; 12+ messages in thread
From: Philippe Gerum @ 2014-07-09 7:46 UTC (permalink / raw
To: Matthias Schneider, xenomai@xenomai.org
On 07/01/2014 06:48 PM, Matthias Schneider wrote:
> Please find enclosed another calling the check_processors() only when starting
> the scheduler and remembering its output. It is also adapted to the latest changes.
> Any comments will be appreciated.
>
I don't think checking the registry contents in the testsuite is worth
it: the output format from the registry may change over time, leaving
tests parsing this output unfixed and outdated (e.g. we would not
receive any compilation error in case of mismatch). Besides, the tester
may want to disable the registry support at build time.
--
Philippe.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Xenomai] FreeRTOS skin #2
2014-07-09 7:42 ` Philippe Gerum
@ 2014-07-09 8:24 ` Philippe Gerum
2014-07-14 19:12 ` Matthias Schneider
2014-07-14 19:18 ` Matthias Schneider
1 sibling, 1 reply; 12+ messages in thread
From: Philippe Gerum @ 2014-07-09 8:24 UTC (permalink / raw
To: Matthias Schneider, xenomai@xenomai.org
On 07/09/2014 09:42 AM, Philippe Gerum wrote:
> On 07/01/2014 06:48 PM, Matthias Schneider wrote:
>> Please find enclosed another calling the check_processors() only when
>> starting
>> the scheduler and remembering its output. It is also adapted to the
>> latest changes.
>> Any comments will be appreciated.
>>
>
> - clusters allowing key dups are available using the *_dup() call form
I just noticed that you would need uniquement labels for pushing your tasks to the registry anyway, so the following patch on top of the previous one would likely do:
diff --git a/lib/freertos/task.c b/lib/freertos/task.c
index d201711..d005163 100644
--- a/lib/freertos/task.c
+++ b/lib/freertos/task.c
@@ -426,10 +426,17 @@ static signed portBASE_TYPE __xTaskGenericCreate(struct freertos_task *task,
generate_name(task->name, (const char *)name, &task_namegen);
- ret = cluster_addobj_dup(&freertos_task_table, task->name, &task->cobj);
- if (__bt(ret)) {
- threadobj_uninit(&task->thobj);
- return pdFALSE;
+ for (;;) {
+ ret = cluster_addobj(&freertos_task_table, task->name, &task->cobj);
+ if (ret == -EEXIST) {
+ generate_name(task->name, NULL, &task_namegen);
+ continue;
+ }
+ if (__bt(ret)) {
+ threadobj_uninit(&task->thobj);
+ return pdFALSE;
+ }
+ break;
}
registry_init_file_obstack(&task->fsobj, ®istry_ops);
--
Philippe.
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [Xenomai] FreeRTOS skin #2
2014-07-09 8:24 ` Philippe Gerum
@ 2014-07-14 19:12 ` Matthias Schneider
0 siblings, 0 replies; 12+ messages in thread
From: Matthias Schneider @ 2014-07-14 19:12 UTC (permalink / raw
To: Philippe Gerum, xenomai@xenomai.org
----- Original Message -----
> From: Philippe Gerum <rpm@xenomai.org>
> To: Matthias Schneider <ma30002000@yahoo.de>; "xenomai@xenomai.org" <xenomai@xenomai.org>
> Cc:
> Sent: Wednesday, July 9, 2014 10:24 AM
> Subject: Re: [Xenomai] FreeRTOS skin #2
>
> On 07/09/2014 09:42 AM, Philippe Gerum wrote:
>> On 07/01/2014 06:48 PM, Matthias Schneider wrote:
>>> Please find enclosed another calling the check_processors() only when
>>> starting
>>> the scheduler and remembering its output. It is also adapted to the
>>> latest changes.
>>> Any comments will be appreciated.
>>>
>>
>> - clusters allowing key dups are available using the *_dup() call form
>
> I just noticed that you would need uniquement labels for pushing your tasks to
> the registry anyway, so the following patch on top of the previous one would
> likely do:
>
> diff --git a/lib/freertos/task.c b/lib/freertos/task.c
> index d201711..d005163 100644
> --- a/lib/freertos/task.c
> +++ b/lib/freertos/task.c
> @@ -426,10 +426,17 @@ static signed portBASE_TYPE __xTaskGenericCreate(struct
> freertos_task *task,
>
> generate_name(task->name, (const char *)name, &task_namegen);
>
> - ret = cluster_addobj_dup(&freertos_task_table, task->name,
> &task->cobj);
> - if (__bt(ret)) {
> - threadobj_uninit(&task->thobj);
> - return pdFALSE;
> + for (;;) {
> + ret = cluster_addobj(&freertos_task_table, task->name,
> &task->cobj);
> + if (ret == -EEXIST) {
> + generate_name(task->name, NULL, &task_namegen);
> + continue;
> + }
> + if (__bt(ret)) {
> + threadobj_uninit(&task->thobj);
> + return pdFALSE;
> + }
> + break;
> }
>
> registry_init_file_obstack(&task->fsobj, ®istry_ops);
>
>
> --
> Philippe.
>
This changes the effective names of the duplicated tasks. However,
I do not see a big deal about it, so I have added this change to my patch.
Matthias
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Xenomai] FreeRTOS skin #2
2014-07-09 7:42 ` Philippe Gerum
2014-07-09 8:24 ` Philippe Gerum
@ 2014-07-14 19:18 ` Matthias Schneider
2014-07-14 20:15 ` Philippe Gerum
1 sibling, 1 reply; 12+ messages in thread
From: Matthias Schneider @ 2014-07-14 19:18 UTC (permalink / raw
To: Philippe Gerum, xenomai@xenomai.org
----- Original Message -----
> From: Philippe Gerum <rpm@xenomai.org>
> To: Matthias Schneider <ma30002000@yahoo.de>; "xenomai@xenomai.org" <xenomai@xenomai.org>
> Cc:
> Sent: Wednesday, July 9, 2014 9:42 AM
> Subject: Re: [Xenomai] FreeRTOS skin #2
>
> On 07/01/2014 06:48 PM, Matthias Schneider wrote:
>> Please find enclosed another calling the check_processors() only when
> starting
>> the scheduler and remembering its output. It is also adapted to the latest
> changes.
>> Any comments will be appreciated.
>>
>
> - clusters allowing key dups are available using the *_dup() call form
> - vTaskEndScheduler() can be simplified, and should not panic if it
> cannot lock a vanishing thread pulled from the task list.
>
> --
> Philippe.
>
The vTaskEndScheduler() patch unfortunately now leads to a frozen
vTaskStartScheduler() function when shutting down in some tests.
This is probably due to freertos_scheduler.tasklist_end_cond not being
triggered when the list gets empty in vTaskEndScheduler() ? also,
freertos_scheduler.num_tasks seems not to be decremented when removing
elemets from the list via pvlist_pop_entry().
Even when calling pthread_cond_signal() at the end of vTaskEndScheduler(),
I still get the freeze. Any idea?
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [Xenomai] FreeRTOS skin #2
2014-07-14 19:18 ` Matthias Schneider
@ 2014-07-14 20:15 ` Philippe Gerum
0 siblings, 0 replies; 12+ messages in thread
From: Philippe Gerum @ 2014-07-14 20:15 UTC (permalink / raw
To: Matthias Schneider, xenomai@xenomai.org
On 07/14/2014 09:18 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: Wednesday, July 9, 2014 9:42 AM
>> Subject: Re: [Xenomai] FreeRTOS skin #2
>>
>> On 07/01/2014 06:48 PM, Matthias Schneider wrote:
>>> Please find enclosed another calling the check_processors() only when
>> starting
>>> the scheduler and remembering its output. It is also adapted to the latest
>> changes.
>>> Any comments will be appreciated.
>>>
>>
>> - clusters allowing key dups are available using the *_dup() call form
>> - vTaskEndScheduler() can be simplified, and should not panic if it
>> cannot lock a vanishing thread pulled from the task list.
>>
>> --
>
>> Philippe.
>>
>
> The vTaskEndScheduler() patch unfortunately now leads to a frozen
> vTaskStartScheduler() function when shutting down in some tests.
>
> This is probably due to freertos_scheduler.tasklist_end_cond not being
> triggered when the list gets empty in vTaskEndScheduler() ? also,
That condvar is shared between the task finalizer and
vTaskStartScheduler() instead. If no task terminates after
vTaskEndScheduler() is called, then we will be missing the trigger
indeed. The logic needs some fixing. I also just noticed that the
start_cond variable seems redundant with the purpose of
threadobj_wait_start().
> freertos_scheduler.num_tasks seems not to be decremented when removing
> elemets from the list via pvlist_pop_entry().
>
> Even when calling pthread_cond_signal() at the end of vTaskEndScheduler(),
> I still get the freeze. Any idea?
>
>
--
Philippe.
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2014-07-14 20:15 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-22 19:46 [Xenomai] FreeRTOS skin #2 Matthias Schneider
2014-06-02 18:46 ` Matthias Schneider
2014-06-20 7:55 ` Philippe Gerum
2014-06-23 10:31 ` Matthias Schneider
2014-06-23 20:27 ` Matthias Schneider
2014-07-01 16:48 ` Matthias Schneider
2014-07-09 7:42 ` Philippe Gerum
2014-07-09 8:24 ` Philippe Gerum
2014-07-14 19:12 ` Matthias Schneider
2014-07-14 19:18 ` Matthias Schneider
2014-07-14 20:15 ` Philippe Gerum
2014-07-09 7:46 ` Philippe Gerum
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.