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/07/01 11:18:34 UTC
[GitHub] [incubator-nuttx] GUIDINGLI opened a new pull request #4016: Add heap & stack check to idle thread
GUIDINGLI opened a new pull request #4016:
URL: https://github.com/apache/incubator-nuttx/pull/4016
## Summary
Add heap & stack check to idle thread
## Impact
idle
## Testing
--
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] davids5 commented on a change in pull request #4016: Add heap & stack check to idle thread
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #4016:
URL: https://github.com/apache/incubator-nuttx/pull/4016#discussion_r663073718
##########
File path: sched/init/nx_start.c
##########
@@ -789,17 +789,35 @@ void nx_start(void)
sinfo("CPU0: Beginning Idle Loop\n");
for (; ; )
{
- /* Check heap & stack in idle thread */
+#if defined(CONFIG_STACK_COLORATION) && CONFIG_STACK_USAGE_SAFE_PERCENT > 0
- kmm_checkcorruption();
+ /* Check stack in idle thread */
-#if defined(CONFIG_STACK_COLORATION) && defined(CONFIG_DEBUG_MM)
- for (i = 0; i < CONFIG_MAX_TASKS && g_pidhash[i].tcb; i++)
+ for (i = 0; i < CONFIG_MAX_TASKS; i++)
{
- assert(up_check_tcbstack_remain(g_pidhash[i].tcb) > 0);
+ FAR struct tcb_s *tcb;
+ irqstate_t flags;
+
+ flags = enter_critical_section();
Review comment:
Condition it be condition on DEBUG?
##########
File path: sched/init/nx_start.c
##########
@@ -789,17 +789,35 @@ void nx_start(void)
sinfo("CPU0: Beginning Idle Loop\n");
for (; ; )
{
- /* Check heap & stack in idle thread */
+#if defined(CONFIG_STACK_COLORATION) && CONFIG_STACK_USAGE_SAFE_PERCENT > 0
- kmm_checkcorruption();
+ /* Check stack in idle thread */
-#if defined(CONFIG_STACK_COLORATION) && defined(CONFIG_DEBUG_MM)
- for (i = 0; i < CONFIG_MAX_TASKS && g_pidhash[i].tcb; i++)
+ for (i = 0; i < CONFIG_MAX_TASKS; i++)
{
- assert(up_check_tcbstack_remain(g_pidhash[i].tcb) > 0);
+ FAR struct tcb_s *tcb;
+ irqstate_t flags;
+
+ flags = enter_critical_section();
Review comment:
Condition it be conditioned on DEBUG?
--
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] GUIDINGLI commented on a change in pull request #4016: Add heap & stack check to idle thread
Posted by GitBox <gi...@apache.org>.
GUIDINGLI commented on a change in pull request #4016:
URL: https://github.com/apache/incubator-nuttx/pull/4016#discussion_r662717400
##########
File path: sched/init/nx_start.c
##########
@@ -789,17 +789,35 @@ void nx_start(void)
sinfo("CPU0: Beginning Idle Loop\n");
for (; ; )
{
- /* Check heap & stack in idle thread */
+#if defined(CONFIG_STACK_COLORATION) && CONFIG_STACK_USAGE_SAFE_PERCENT > 0
- kmm_checkcorruption();
+ /* Check stack in idle thread */
-#if defined(CONFIG_STACK_COLORATION) && defined(CONFIG_DEBUG_MM)
- for (i = 0; i < CONFIG_MAX_TASKS && g_pidhash[i].tcb; i++)
+ for (i = 0; i < CONFIG_MAX_TASKS; i++)
{
- assert(up_check_tcbstack_remain(g_pidhash[i].tcb) > 0);
+ FAR struct tcb_s *tcb;
+ irqstate_t flags;
+
+ flags = enter_critical_section();
Review comment:
Yes, you are right, so I add debug config CONFIG_STACK_USAGE_SAFE_PERCENT, default NOT open.
In the production code, this feature should be closed, but it is useful for project developing & debugging.
would be better as separate task that can report the margin
---- Where ? each task report its own task usage ?
Or create a debugging thread to only check this, then the priority is ? I this there is no difference with IDLE thread.
Once there is stack overflow, then report or PANIC is no difference. IN the production code we will code PANIC debug.
--
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] davids5 commented on a change in pull request #4016: Add heap & stack check to idle thread
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #4016:
URL: https://github.com/apache/incubator-nuttx/pull/4016#discussion_r662540025
##########
File path: sched/init/nx_start.c
##########
@@ -789,17 +789,35 @@ void nx_start(void)
sinfo("CPU0: Beginning Idle Loop\n");
for (; ; )
{
- /* Check heap & stack in idle thread */
+#if defined(CONFIG_STACK_COLORATION) && CONFIG_STACK_USAGE_SAFE_PERCENT > 0
- kmm_checkcorruption();
+ /* Check stack in idle thread */
-#if defined(CONFIG_STACK_COLORATION) && defined(CONFIG_DEBUG_MM)
- for (i = 0; i < CONFIG_MAX_TASKS && g_pidhash[i].tcb; i++)
+ for (i = 0; i < CONFIG_MAX_TASKS; i++)
{
- assert(up_check_tcbstack_remain(g_pidhash[i].tcb) > 0);
+ FAR struct tcb_s *tcb;
+ irqstate_t flags;
+
+ flags = enter_critical_section();
Review comment:
This seems like 1) it would not be power management friendly. 2) it is too draconian. This would be bad in production code.
Margin is margin and if the interrupt stack is used it is a real margin. If there is margin we do not want a hardfault. The would be better as separate task that can report the margin, not fault.
--
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 a change in pull request #4016: Add heap & stack check to idle thread
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4016:
URL: https://github.com/apache/incubator-nuttx/pull/4016#discussion_r662777515
##########
File path: sched/init/nx_start.c
##########
@@ -789,17 +789,35 @@ void nx_start(void)
sinfo("CPU0: Beginning Idle Loop\n");
for (; ; )
{
- /* Check heap & stack in idle thread */
+#if defined(CONFIG_STACK_COLORATION) && CONFIG_STACK_USAGE_SAFE_PERCENT > 0
- kmm_checkcorruption();
+ /* Check stack in idle thread */
-#if defined(CONFIG_STACK_COLORATION) && defined(CONFIG_DEBUG_MM)
- for (i = 0; i < CONFIG_MAX_TASKS && g_pidhash[i].tcb; i++)
+ for (i = 0; i < CONFIG_MAX_TASKS; i++)
{
- assert(up_check_tcbstack_remain(g_pidhash[i].tcb) > 0);
+ FAR struct tcb_s *tcb;
+ irqstate_t flags;
+
+ flags = enter_critical_section();
Review comment:
@davids5 this code path is only for deveopment, and it's guarded by CONFIG_STACK_USAGE_SAFE_PERCENT, which is in the OFF state by default. For the separate task solution, it already exist here:
https://github.com/apache/incubator-nuttx-apps/tree/master/system/stackmonitor
Since the separate task solution may report the overflow too later, the check in IDLE thread is very useful in the development phase.
--
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 a change in pull request #4016: Add heap & stack check to idle thread
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4016:
URL: https://github.com/apache/incubator-nuttx/pull/4016#discussion_r663083983
##########
File path: sched/init/nx_start.c
##########
@@ -789,17 +789,35 @@ void nx_start(void)
sinfo("CPU0: Beginning Idle Loop\n");
for (; ; )
{
- /* Check heap & stack in idle thread */
+#if defined(CONFIG_STACK_COLORATION) && CONFIG_STACK_USAGE_SAFE_PERCENT > 0
- kmm_checkcorruption();
+ /* Check stack in idle thread */
-#if defined(CONFIG_STACK_COLORATION) && defined(CONFIG_DEBUG_MM)
- for (i = 0; i < CONFIG_MAX_TASKS && g_pidhash[i].tcb; i++)
+ for (i = 0; i < CONFIG_MAX_TASKS; i++)
{
- assert(up_check_tcbstack_remain(g_pidhash[i].tcb) > 0);
+ FAR struct tcb_s *tcb;
+ irqstate_t flags;
+
+ flags = enter_critical_section();
Review comment:
There are two check:
1. heap check which is guarded by CONFIG_MM_DEBUG
2. stack check which isn't guarded by DEBUG, but by a new option STACK_USAGE_SAFE_PERCENT, it's redundant to let user select both DEBUG and STACK_USAGE_SAFE_PERCENT to turn on this feature. user can disable the check by set STACK_USAGE_SAFE_PERCENT=0(the default value)
--
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 merged pull request #4016: Add heap & stack check to idle thread
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #4016:
URL: https://github.com/apache/incubator-nuttx/pull/4016
--
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] GUIDINGLI commented on a change in pull request #4016: Add heap & stack check to idle thread
Posted by GitBox <gi...@apache.org>.
GUIDINGLI commented on a change in pull request #4016:
URL: https://github.com/apache/incubator-nuttx/pull/4016#discussion_r663385262
##########
File path: Kconfig
##########
@@ -1758,6 +1758,16 @@ config STACK_COLORATION
Only supported by a few architectures.
+config STACK_USAGE_SAFE_PERCENT
+ int "Stack usage safe precent"
+ default 0
+ range 0 100
+ depends on STACK_COLORATION
+ ---help---
+ Stack usage precent = up_check_tcbstack() * 100 / tcb->adj_stack_size,
+ this should lower then STACK_USAGE_SAFE_PERCENT.
+ Idle thread will timely check stack usage when this marco value > 0.
Review comment:
done, thanks
--
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 a change in pull request #4016: Add heap & stack check to idle thread
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4016:
URL: https://github.com/apache/incubator-nuttx/pull/4016#discussion_r663083983
##########
File path: sched/init/nx_start.c
##########
@@ -789,17 +789,35 @@ void nx_start(void)
sinfo("CPU0: Beginning Idle Loop\n");
for (; ; )
{
- /* Check heap & stack in idle thread */
+#if defined(CONFIG_STACK_COLORATION) && CONFIG_STACK_USAGE_SAFE_PERCENT > 0
- kmm_checkcorruption();
+ /* Check stack in idle thread */
-#if defined(CONFIG_STACK_COLORATION) && defined(CONFIG_DEBUG_MM)
- for (i = 0; i < CONFIG_MAX_TASKS && g_pidhash[i].tcb; i++)
+ for (i = 0; i < CONFIG_MAX_TASKS; i++)
{
- assert(up_check_tcbstack_remain(g_pidhash[i].tcb) > 0);
+ FAR struct tcb_s *tcb;
+ irqstate_t flags;
+
+ flags = enter_critical_section();
Review comment:
No, by new option STACK_USAGE_SAFE_PERCENT, it's redundant to let user select both DEBUG and STACK_USAGE_SAFE_PERCENT to turn on this feature. user can disable the check by set STACK_USAGE_SAFE_PERCENT=0(the default value)
--
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 a change in pull request #4016: Add heap & stack check to idle thread
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #4016:
URL: https://github.com/apache/incubator-nuttx/pull/4016#discussion_r663046610
##########
File path: sched/init/nx_start.c
##########
@@ -789,17 +789,35 @@ void nx_start(void)
sinfo("CPU0: Beginning Idle Loop\n");
for (; ; )
{
- /* Check heap & stack in idle thread */
+#if defined(CONFIG_STACK_COLORATION) && CONFIG_STACK_USAGE_SAFE_PERCENT > 0
- kmm_checkcorruption();
+ /* Check stack in idle thread */
-#if defined(CONFIG_STACK_COLORATION) && defined(CONFIG_DEBUG_MM)
- for (i = 0; i < CONFIG_MAX_TASKS && g_pidhash[i].tcb; i++)
+ for (i = 0; i < CONFIG_MAX_TASKS; i++)
{
- assert(up_check_tcbstack_remain(g_pidhash[i].tcb) > 0);
+ FAR struct tcb_s *tcb;
+ irqstate_t flags;
+
+ flags = enter_critical_section();
Review comment:
@davids5 what you opinion?
--
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] davids5 commented on a change in pull request #4016: Add heap & stack check to idle thread
Posted by GitBox <gi...@apache.org>.
davids5 commented on a change in pull request #4016:
URL: https://github.com/apache/incubator-nuttx/pull/4016#discussion_r663088043
##########
File path: Kconfig
##########
@@ -1758,6 +1758,16 @@ config STACK_COLORATION
Only supported by a few architectures.
+config STACK_USAGE_SAFE_PERCENT
+ int "Stack usage safe precent"
+ default 0
+ range 0 100
+ depends on STACK_COLORATION
+ ---help---
+ Stack usage precent = up_check_tcbstack() * 100 / tcb->adj_stack_size,
+ this should lower then STACK_USAGE_SAFE_PERCENT.
+ Idle thread will timely check stack usage when this marco value > 0.
Review comment:
```suggestion
Idle thread will timely check stack usage when this marco value > 0.
N.B. This feature should not be used in production code.
```
--
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