You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2021/06/08 17:48:39 UTC
[GitHub] [incubator-nuttx] patacongo opened a new issue #3875: PID == 0 is not always the IDLE thread.
patacongo opened a new issue #3875:
URL: https://github.com/apache/incubator-nuttx/issues/3875
There are a few places in the code where there is a check if PID == 0 to determine if a thread is an IDLE thread. This is wrong. These checks were all corrected a few years back but we have grown several new bad checks.
Determining if a task is the IDLE thread by checking if pid == 0 is wrong because it is not true in the SMP case where there are multiple IDLE tasks, one for each CPU and the PID if these IDLE threads is not zero in all cases. Each IDLE thask has a unique PID.
In most places, a 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 may apply early in initialization before the the newly created IDLE task has been made ready to run ready-to-run.
IDLE threads could also be distinguished by having priority == 0 as well.
We might need to add a TDB_FLAG_IDLE_TASK flag to handle this case.
Here are a few possible cases and relevant comments that I found. Not all of the checks for pid == 0 may be checks for the IDLE task. Many APIs accept pid == 0 to mean the current PID:
arch/arm/src/arm/arm_initialstate.c: if (tcb->pid == 0)
arch/arm/src/armv6-m/arm_initialstate.c: if (tcb->pid == 0)
arch/arm/src/armv7-a/arm_initialstate.c: if (tcb->pid == 0)
arch/arm/src/armv7-m/arm_initialstate.c: if (tcb->pid == 0)
arch/arm/src/armv7-r/arm_initialstate.c: if (tcb->pid == 0)
arch/arm/src/armv8-m/arm_initialstate.c: if (tcb->pid == 0)
arch/avr/src/avr/up_initialstate.c: if (tcb->pid == 0)
arch/avr/src/avr32/up_initialstate.c: if (tcb->pid == 0)
arch/hc/src/m9s12/m9s12_initialstate.c: if (tcb->pid == 0)
arch/mips/src/mips32/mips_initialstate.c: if (tcb->pid == 0)
arch/misoc/src/lm32/lm32_initialstate.c: if (tcb->pid == 0)
arch/misoc/src/minerva/minerva_initialstate.c: if (tcb->pid == 0)
arch/or1k/src/common/up_initialstate.c: if (tcb->pid == 0)
arch/renesas/src/m16c/m16c_initialstate.c: if (tcb->pid == 0)
arch/renesas/src/rx65n/rx65n_initialstate.c: if (tcb->pid == 0)
arch/renesas/src/sh1/sh1_initialstate.c: if (tcb->pid == 0)
arch/risc-v/src/esp32c3/esp32c3_wifi_adapter.c: if (pid == 0)
arch/risc-v/src/rv32im/riscv_initialstate.c: if (tcb->pid == 0)
arch/risc-v/src/rv64gc/riscv_initialstate.c: if (tcb->pid == 0)
arch/sim/src/sim/up_initialstate.c: if (tcb->pid == 0)
arch/x86/src/i486/up_initialstate.c: if (tcb->pid == 0)
arch/x86_64/src/intel64/up_initialstate.c: if (tcb->pid == 0)
arch/xtensa/src/common/xtensa_initialstate.c: if (tcb->pid == 0)
arch/xtensa/src/esp32/esp32_spiram.c: if (pid == 0 || pid == 1)
arch/xtensa/src/esp32/esp32_wifi_adapter.c: if (pid == 0)
arch/z80/src/z180/z180_initialstate.c: if (tcb->pid == 0)
arch/z80/src/z80/z80_initialstate.c: if (tcb->pid == 0)
include/nuttx/clock.h: * pid - The task ID of the thread of interest. pid == 0 is IDLE thread.
sched/pthread/pthread_cancel.c: /* pid == 0 is the IDLE task (in a single CPU configuration). Callers
sched/sched/sched_cpuload.c: * pid - The task ID of the thread of interest. pid == 0 is the IDLE
sched/task/task_getpid.c: * pid == 0 must be running, and
sched/task/task_getppid.c: * pid == 0 must be running, and
sched/task/task_gettid.c: * pid == 0 must be running, and
--
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #3875: PID == 0 is not always the IDLE thread.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #3875:
URL: https://github.com/apache/incubator-nuttx/issues/3875#issuecomment-857343975
It's intentional to check pid == 0 in some place, e.g. all up_initialstate.c:
> ```
> arch/arm/src/arm/arm_initialstate.c: if (tcb->pid == 0)
> arch/arm/src/armv6-m/arm_initialstate.c: if (tcb->pid == 0)
> arch/arm/src/armv7-a/arm_initialstate.c: if (tcb->pid == 0)
> arch/arm/src/armv7-m/arm_initialstate.c: if (tcb->pid == 0)
> arch/arm/src/armv7-r/arm_initialstate.c: if (tcb->pid == 0)
> arch/arm/src/armv8-m/arm_initialstate.c: if (tcb->pid == 0)
> arch/avr/src/avr/up_initialstate.c: if (tcb->pid == 0)
> arch/avr/src/avr32/up_initialstate.c: if (tcb->pid == 0)
> arch/hc/src/m9s12/m9s12_initialstate.c: if (tcb->pid == 0)
> arch/mips/src/mips32/mips_initialstate.c: if (tcb->pid == 0)
> arch/misoc/src/lm32/lm32_initialstate.c: if (tcb->pid == 0)
> arch/misoc/src/minerva/minerva_initialstate.c: if (tcb->pid == 0)
> arch/or1k/src/common/up_initialstate.c: if (tcb->pid == 0)
> arch/renesas/src/m16c/m16c_initialstate.c: if (tcb->pid == 0)
> arch/renesas/src/rx65n/rx65n_initialstate.c: if (tcb->pid == 0)
> arch/renesas/src/sh1/sh1_initialstate.c: if (tcb->pid == 0)
> arch/risc-v/src/rv32im/riscv_initialstate.c: if (tcb->pid == 0)
> arch/risc-v/src/rv64gc/riscv_initialstate.c: if (tcb->pid == 0)
> arch/sim/src/sim/up_initialstate.c: if (tcb->pid == 0)
> arch/x86/src/i486/up_initialstate.c: if (tcb->pid == 0)
> arch/x86_64/src/intel64/up_initialstate.c: if (tcb->pid == 0)
> arch/xtensa/src/common/xtensa_initialstate.c: if (tcb->pid == 0)
> arch/z80/src/z180/z180_initialstate.c: if (tcb->pid == 0)
> arch/z80/src/z80/z80_initialstate.c: if (tcb->pid == 0)
The special if statement is to setup the idle thread stack pointer in tcb which is just required for the main(CPU0) idle thread since other idle(CPU1-x) threads go through the normal tcb initialization path and then these stack pointer set correctly naturally.
--
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on issue #3875: PID == 0 is not always the IDLE thread.
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #3875:
URL: https://github.com/apache/incubator-nuttx/issues/3875#issuecomment-857733792
> It's intentional to check pid == 0 in some place, e.g. all up_initialstate.c:
> ...
> The special if statement is to setup the idle thread stack pointer in tcb which is just required for the main(CPU0) idle thread since other idle(CPU1-x) threads go through the normal tcb initialization path and then these stack pointer set correctly naturally.
Thanks, that relieves some my concerns. However, I think it is not a very clean, modular solution. I would prefer to see some call out in nx_task() to do that job.
--
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on issue #3875: PID == 0 is not always the IDLE thread.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on issue #3875:
URL: https://github.com/apache/incubator-nuttx/issues/3875#issuecomment-857748820
Do you want add something like up_initial_idle_state? and call it from nx_start.
--
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] patacongo commented on issue #3875: PID == 0 is not always the IDLE thread.
Posted by GitBox <gi...@apache.org>.
patacongo commented on issue #3875:
URL: https://github.com/apache/incubator-nuttx/issues/3875#issuecomment-857803574
>
>
> Do you want add something like up_initial_idle_state? and call it from nx_start.
That seems a little better, but I do not have any strong opinion. Calling a function that has conditional logic that makes it do something other than it was intended for is not good modular design, but otherwise I don't really care very much.
--
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:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 closed issue #3875: PID == 0 is not always the IDLE thread.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 closed issue #3875:
URL: https://github.com/apache/incubator-nuttx/issues/3875
--
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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-nuttx] xiaoxiang781216 commented on issue #3875: PID == 0 is not always the IDLE thread.
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on issue #3875:
URL: https://github.com/apache/incubator-nuttx/issues/3875#issuecomment-857748820
Do you want add something like up_initial_idle_state?
--
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:
users@infra.apache.org