GUIDINGLI commented on code in PR #7018:
URL: https://github.com/apache/incubator-nuttx/pull/7018#discussion_r964556230


##########
sched/wqueue/kwork_thread.c:
##########
@@ -169,19 +168,20 @@ static int work_thread(int argc, FAR char *argv[])
           /* Mark the work as no longer being queued */
 
           work->worker = NULL;
+          leave_critical_section(flags);
 
           /* Do the work.  Re-enable interrupts while the work is being
            * performed... we don't have any idea how long this will take!
            */
 
-          leave_critical_section(flags);
           CALL_WORKER(worker, arg);
-          flags = enter_critical_section();
+        }
+      else
+        {
+          leave_critical_section(flags);

Review Comment:
   Now the order:
   while (1)
   {
     enter_critical_section();
     nxsem_wait_uninterruptible();
     ...
     // access the work_s with critical_section
     ...
     leave_critical_section();
   }
   
   The restore will make sure we are back to critical_section state, but it is 
different to understand for a new reader.
   
   After modify:
   
   while (1)
   {
     nxsem_wait_uninterruptible();
     enter_critical_section();
     ...
     // access the work_s with critical_section
     ...
     leave_critical_section();
   }
   
   This is easy to understand.
   



-- 
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-unsubscr...@nuttx.apache.org

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

Reply via email to