patacongo commented on a change in pull request #3871:
URL: https://github.com/apache/incubator-nuttx/pull/3871#discussion_r647645024
##########
File path: arch/arm/src/armv7-m/arm_initialstate.c
##########
@@ -65,8 +66,9 @@ void up_initial_state(struct tcb_s *tcb)
{
tcb->stack_alloc_ptr = (void *)(g_idle_topstack -
CONFIG_IDLETHREAD_STACKSIZE);
- tcb->stack_base_ptr = tcb->stack_alloc_ptr;
- tcb->adj_stack_size = CONFIG_IDLETHREAD_STACKSIZE;
+ tcb->stack_base_ptr = tcb->stack_alloc_ptr;
+ tcb->adj_stack_size = CONFIG_IDLETHREAD_STACKSIZE -
+ sizeof(struct task_info_s);
Review comment:
1. Shouldn't the task info be initialized to zero?
memset(tcb->stack_alloc_ptr, 0, sizeof(sizeof(struct task_info_s));
2. This does not work in SMP mode. There are multiple IDLE tasks and they
do not all have pid == 0
Determining if a task is the IDLE thread by checking if pid == 0 is
discouraged in all cases for this reason. In most places, the foolproof check
is that tcb->flink == NULL which is true in all cases AFTER initialization when
the TCB is placed in the ready-to-run list (single CPU mode) or in the assigned
tasks list(s) (SMP mode). That IDLE task is always at the end of these lists
and, hence, always has flink == NULL. That rule, however, probably not apply
in this case, however. It is probably too early in initialization and the
newly created task is not yet ready-to-run.
We might need to add a TDB_FLAG_IDLE_TASK flag to handle this case. See
Issue #3875
Same comments on all similar changes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
