masayuki2009 commented on a change in pull request #2298:
URL: https://github.com/apache/incubator-nuttx/pull/2298#discussion_r523497562



##########
File path: sched/sched/sched_waitid.c
##########
@@ -143,6 +143,14 @@ int nx_waitid(int idtype, id_t id, FAR siginfo_t *info, 
int options)
   sigemptyset(&set);
   nxsig_addset(&set, SIGCHLD);
 
+  /* NOTE: sched_lock() is not enough for SMP
+   * because the child task is running on another CPU
+   */
+
+#ifdef CONFIG_SMP
+  irqstate_t flags = enter_critical_section();

Review comment:
       @Ouss4 
   I tried the following change in sched_waitid.c and sched_waitpid.c
   The ostest passed for the all SMP configurations.
   
   However, Wi-Fi streaming plus stress testing with spresense:wifi_smp 
(NCPUS=4, DEBUG_ASSERTIONS=y)
   failed with DEBUGASSERTION after 3.5 hours. (FYI, without the following 
changes, the test continued over 9 hours)
   
   I think replacing sched_lock() and sched_unlock() with a critical section 
should be a future task.
   What do you think?
   
   ```
   #ifdef CONFIG_SMP                                                            
                                                                                
                       
     /* NOTE: sched_lock() is not enough for SMP                                
                                                                                
                       
      * because the child task is running on another CPU                        
                                                                                
                       
      */                                                                        
                                                                                
                       
                                                                                
                                                                                
                       
     irqstate_t flags = enter_critical_section();                               
                                                                                
                       
   #else                                                                        
                                                                                
                       
     /* Disable pre-emption so that nothing changes while the loop executes */  
                                                                                
                       
                                                                                
                                                                                
                       
     sched_lock();                                                              
                                                                                
                       
   #endif 
   
   ...
   
   #ifdef CONFIG_SMP                                                            
                                                                                
                       
     leave_critical_section(flags);                                             
                                                                                
                       
   #else                                                                        
                                                                                
                       
     sched_unlock();                                                            
                                                                                
                       
   #endif   
   ```




----------------------------------------------------------------
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:
us...@infra.apache.org


Reply via email to