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 2020/10/23 10:24:01 UTC

[GitHub] [incubator-nuttx] xiaoxiang781216 opened a new pull request #2077: Fix the stack color failure when the thread stack setup through up_use_stack

xiaoxiang781216 opened a new pull request #2077:
URL: https://github.com/apache/incubator-nuttx/pull/2077


   ## Summary
   Since tcb doesn't setup pid before sched call up_use_stack, the code fail to detect the idle thread. One possible fix is reorder the code in sched to initialize pid first, but it seem to break the task load from elf:
   https://github.com/apache/incubator-nuttx/pull/2000
   so let's setup the idle thread stack info directly, and remove the idle thread detecton code from up_use_stack.
   
   ## Impact
   
   ## 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #2077: Fix the stack color failure when the thread stack setup through up_use_stack

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #2077:
URL: https://github.com/apache/incubator-nuttx/pull/2077#issuecomment-715318598


   > @xiaoxiang781216
   > Could you describe the testing?
   
   Done.


----------------------------------------------------------------
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] masayuki2009 merged pull request #2077: Fix the stack color failure when the thread stack setup through up_use_stack

Posted by GitBox <gi...@apache.org>.
masayuki2009 merged pull request #2077:
URL: https://github.com/apache/incubator-nuttx/pull/2077


   


----------------------------------------------------------------
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] masayuki2009 commented on pull request #2077: Fix the stack color failure when the thread stack setup through up_use_stack

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #2077:
URL: https://github.com/apache/incubator-nuttx/pull/2077#issuecomment-744793170


   > @masayuki2009 could you try again? I just run the patch with sabre-6quad:smp plus QEMU, ostest and smp can run sucessfully.
   
   @xiaoxiang781216 
   All tests have passed.


----------------------------------------------------------------
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] masayuki2009 commented on pull request #2077: Fix the stack color failure when the thread stack setup through up_use_stack

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #2077:
URL: https://github.com/apache/incubator-nuttx/pull/2077#issuecomment-744759674


   > @masayuki2009 could you try again? I just run the patch with sabre-6quad:smp plus QEMU, ostest and smp can run sucessfully.
   
   @xiaoxiang781216 
   OK. I'll check this PR later.


----------------------------------------------------------------
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] masayuki2009 commented on a change in pull request #2077: Fix the stack color failure when the thread stack setup through up_use_stack

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #2077:
URL: https://github.com/apache/incubator-nuttx/pull/2077#discussion_r542902265



##########
File path: arch/arm/src/arm/arm_initialstate.c
##########
@@ -75,8 +75,10 @@ void up_initial_state(struct tcb_s *tcb)
 
   if (tcb->pid == 0)
     {
-      up_use_stack(tcb, (void *)(g_idle_topstack -
-        CONFIG_IDLETHREAD_STACKSIZE), CONFIG_IDLETHREAD_STACKSIZE);
+      tcb->stack_alloc_ptr = (void *)(g_idle_topstack -
+                                      CONFIG_IDLETHREAD_STACKSIZE);
+      tcb->adj_stack_ptr   = (void *)g_idle_topstack;
+      tcb->adj_stack_size  = CONFIG_IDLETHREAD_STACKSIZE;
     }

Review comment:
       @xiaoxiang781216 
   
   I think we should add such special logic for pid=0 case to up_use_stack().
   It would reduce maintenance costs.
   
   What do you think?
   




----------------------------------------------------------------
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 a change in pull request #2077: Fix the stack color failure when the thread stack setup through up_use_stack

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2077:
URL: https://github.com/apache/incubator-nuttx/pull/2077#discussion_r543011793



##########
File path: arch/arm/src/arm/arm_initialstate.c
##########
@@ -75,8 +75,10 @@ void up_initial_state(struct tcb_s *tcb)
 
   if (tcb->pid == 0)
     {
-      up_use_stack(tcb, (void *)(g_idle_topstack -
-        CONFIG_IDLETHREAD_STACKSIZE), CONFIG_IDLETHREAD_STACKSIZE);
+      tcb->stack_alloc_ptr = (void *)(g_idle_topstack -
+                                      CONFIG_IDLETHREAD_STACKSIZE);
+      tcb->adj_stack_ptr   = (void *)g_idle_topstack;
+      tcb->adj_stack_size  = CONFIG_IDLETHREAD_STACKSIZE;
     }

Review comment:
       We agreed that idle thread colorization should be done in assembly code from https://github.com/apache/incubator-nuttx/pull/1369, so up_use_stack isn't used to color the idle thread stack anymore.
   BTW, up_use_stack and idle thread tcb is seldom referenced in the code base and the check is introduced in https://github.com/apache/incubator-nuttx/pull/1369 too.




----------------------------------------------------------------
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 a change in pull request #2077: Fix the stack color failure when the thread stack setup through up_use_stack

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on a change in pull request #2077:
URL: https://github.com/apache/incubator-nuttx/pull/2077#discussion_r543011793



##########
File path: arch/arm/src/arm/arm_initialstate.c
##########
@@ -75,8 +75,10 @@ void up_initial_state(struct tcb_s *tcb)
 
   if (tcb->pid == 0)
     {
-      up_use_stack(tcb, (void *)(g_idle_topstack -
-        CONFIG_IDLETHREAD_STACKSIZE), CONFIG_IDLETHREAD_STACKSIZE);
+      tcb->stack_alloc_ptr = (void *)(g_idle_topstack -
+                                      CONFIG_IDLETHREAD_STACKSIZE);
+      tcb->adj_stack_ptr   = (void *)g_idle_topstack;
+      tcb->adj_stack_size  = CONFIG_IDLETHREAD_STACKSIZE;
     }

Review comment:
       Becase we agreed that idle thread colorization should be done in assembly code from https://github.com/apache/incubator-nuttx/pull/1369, up_use_stack isn't used to color the idle thread stack anymore.
   BTW, up_use_stack and idle thread tcb is seldom referenced in the code base and the check is introduced in https://github.com/apache/incubator-nuttx/pull/1369 too. So it make sense to remove the check.




----------------------------------------------------------------
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] masayuki2009 commented on pull request #2077: Fix the stack color failure when the thread stack setup through up_use_stack

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #2077:
URL: https://github.com/apache/incubator-nuttx/pull/2077#issuecomment-715656458


   Hmm, this PR still has the issues which I reported on #2000
   
   For examples,
   - sabre-6quad:smp with QEMU and maix-bit:smp with QEMU stop when smp app started. 
   - stm32f4discovery:elf and maix-bit:elf with QEMU stop while 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #2077: Fix the stack color failure when the thread stack setup through up_use_stack

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #2077:
URL: https://github.com/apache/incubator-nuttx/pull/2077#issuecomment-744445714


   @masayuki2009 could you try again? I just run the patch with sabre-6quad:smp plus QEMU, ostest and smp can run sucessfully.


----------------------------------------------------------------
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] masayuki2009 commented on a change in pull request #2077: Fix the stack color failure when the thread stack setup through up_use_stack

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on a change in pull request #2077:
URL: https://github.com/apache/incubator-nuttx/pull/2077#discussion_r543796587



##########
File path: arch/arm/src/arm/arm_initialstate.c
##########
@@ -75,8 +75,10 @@ void up_initial_state(struct tcb_s *tcb)
 
   if (tcb->pid == 0)
     {
-      up_use_stack(tcb, (void *)(g_idle_topstack -
-        CONFIG_IDLETHREAD_STACKSIZE), CONFIG_IDLETHREAD_STACKSIZE);
+      tcb->stack_alloc_ptr = (void *)(g_idle_topstack -
+                                      CONFIG_IDLETHREAD_STACKSIZE);
+      tcb->adj_stack_ptr   = (void *)g_idle_topstack;
+      tcb->adj_stack_size  = CONFIG_IDLETHREAD_STACKSIZE;
     }

Review comment:
       OK.
   




----------------------------------------------------------------
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 pull request #2077: Fix the stack color failure when the thread stack setup through up_use_stack

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #2077:
URL: https://github.com/apache/incubator-nuttx/pull/2077#issuecomment-715783667


   I will take a look, 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #2077: Fix the stack color failure when the thread stack setup through up_use_stack

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #2077:
URL: https://github.com/apache/incubator-nuttx/pull/2077#issuecomment-715309591


   @xiaoxiang781216 
   Could you describe the 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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #2077: Fix the stack color failure when the thread stack setup through up_use_stack

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #2077:
URL: https://github.com/apache/incubator-nuttx/pull/2077#issuecomment-715323148


   @xiaoxiang781216 
   Did you test this PR with the existing board and configurations?
   


----------------------------------------------------------------
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 pull request #2077: Fix the stack color failure when the thread stack setup through up_use_stack

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #2077:
URL: https://github.com/apache/incubator-nuttx/pull/2077#issuecomment-715353047


   Try on sim


----------------------------------------------------------------
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