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/10/17 00:38:20 UTC

[GitHub] [incubator-nuttx] Ouss4 opened a new pull request #4669: arch/xtensa/xtensa_cpupause.c: Allow a spin before taking the g_cpu_wait spinlock.

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


   ## Summary
   
   If we repeatedly call `up_cpu_pause` and `up_cpu_resume`, there would be
   cases where the next call to `up_cpu_pause` happens while the other CPU is
   still responding to the previous resume request.  In this case the
   `DEBUGASSERT` will trigger.  We should allow the first CPU to wait until the
   other CPU has finished responding to the resume request.
   
   ## Impact
   Xtensa multicore chips.
   
   ## Testing
   `esp32-devkitc:wapi_smp` with other patches to come that makes it work again.
   


-- 
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 #4669: arch/xtensa/xtensa_cpupause.c: Allow a spin before taking the g_cpu_wait spinlock.

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


   


-- 
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 #4669: arch/xtensa/xtensa_cpupause.c: Allow a spin before taking the g_cpu_wait spinlock.

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



##########
File path: arch/xtensa/src/common/xtensa_cpupause.c
##########
@@ -231,10 +231,12 @@ int up_cpu_pause(int cpu)
    * handler from returning until up_cpu_resume() is called; g_cpu_paused
    * is a handshake that will prevent this function from returning until
    * the CPU is actually paused.
+   * Note that we might spin before getting g_cpu_wait, this just means that
+   * the other CPU still hasn't finished responding to the previous resume
+   * request.
    */
 
-  DEBUGASSERT(!spin_islocked(&g_cpu_wait[cpu]) &&

Review comment:
       The similar assert happen in other arch, should we relax other arch too?

##########
File path: arch/xtensa/src/common/xtensa_cpupause.c
##########
@@ -231,10 +231,12 @@ int up_cpu_pause(int cpu)
    * handler from returning until up_cpu_resume() is called; g_cpu_paused
    * is a handshake that will prevent this function from returning until
    * the CPU is actually paused.
+   * Note that we might spin before getting g_cpu_wait, this just means that
+   * the other CPU still hasn't finished responding to the previous resume
+   * request.
    */
 
-  DEBUGASSERT(!spin_islocked(&g_cpu_wait[cpu]) &&

Review comment:
       Ok, let's merge this PR first. Please consider provide the patch to fix other arch too, 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] Ouss4 commented on a change in pull request #4669: arch/xtensa/xtensa_cpupause.c: Allow a spin before taking the g_cpu_wait spinlock.

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



##########
File path: arch/xtensa/src/common/xtensa_cpupause.c
##########
@@ -231,10 +231,12 @@ int up_cpu_pause(int cpu)
    * handler from returning until up_cpu_resume() is called; g_cpu_paused
    * is a handshake that will prevent this function from returning until
    * the CPU is actually paused.
+   * Note that we might spin before getting g_cpu_wait, this just means that
+   * the other CPU still hasn't finished responding to the previous resume
+   * request.
    */
 
-  DEBUGASSERT(!spin_islocked(&g_cpu_wait[cpu]) &&

Review comment:
       I think other archs will also hit the same issue if `up_cpu_pause` and `up_cpu_resume` are called repeatedly. 

##########
File path: arch/xtensa/src/common/xtensa_cpupause.c
##########
@@ -231,10 +231,12 @@ int up_cpu_pause(int cpu)
    * handler from returning until up_cpu_resume() is called; g_cpu_paused
    * is a handshake that will prevent this function from returning until
    * the CPU is actually paused.
+   * Note that we might spin before getting g_cpu_wait, this just means that
+   * the other CPU still hasn't finished responding to the previous resume
+   * request.
    */
 
-  DEBUGASSERT(!spin_islocked(&g_cpu_wait[cpu]) &&

Review comment:
       I think other archs will also hit the same issue if `up_cpu_pause` and `up_cpu_resume` are called repeatedly.  So I also think we should relax that assertion in the other archs as well.
   




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