Fix-Point commented on code in PR #16231:
URL: https://github.com/apache/nuttx/pull/16231#discussion_r2074646362


##########
sched/wqueue/kwork_thread.c:
##########
@@ -148,33 +193,54 @@ static int work_thread(int argc, FAR char *argv[])
   kworker = (FAR struct kworker_s *)
             ((uintptr_t)strtoul(argv[2], NULL, 16));
 
-  flags = spin_lock_irqsave(&wqueue->lock);
-  sched_lock();
-
-  /* Loop forever */
+  /* Loop until wqueue->exit != 0.
+   * Since the only way to set wqueue->exit is to call work_queue_free(),
+   * there is no need for entering the critical section.
+   */
 
   while (!wqueue->exit)
     {
-      /* And check each entry in the work queue.  Since we have disabled
+      /* And check first entry in the work queue. Since we have disabled
        * interrupts we know:  (1) we will not be suspended unless we do
        * so ourselves, and (2) there will be no changes to the work queue
        */
 
-      /* Remove the ready-to-execute work from the list */
+      flags = spin_lock_irqsave(&wqueue->lock);
+      sched_lock();
+
+      /* If the wqueue timer is expired and non-active, it indicates that
+       * there might be expired work in the pending queue.
+       */
 
-      while (!list_is_empty(&wqueue->q))
+      if (!WDOG_ISACTIVE(&wqueue->timer))
         {
-          work = list_first_entry(&wqueue->q, struct work_s, u.s.node);
+          unsigned int wake_count = work_dispatch(wqueue);
 
-          list_delete(&work->u.s.node);
+          spin_unlock_irqrestore(&wqueue->lock, flags);
+          sched_unlock();
+
+          /* Note that the thread execution this function is also
+           * a worker thread, which has already been woken up by the timer.
+           * So only `count - 1` semaphore will be posted.
+           */
 
-          if (work->worker == NULL)
+          while (wake_count-- > 1)
             {
-              continue;
+              nxsem_post(&wqueue->sem);

Review Comment:
   If we moved this `nxsem_post` to the `work_dispatch`, we have to set a timer 
that immediately fires, which is less-efficient. Here we are doing what the 
timer callback should do to wake up at least 1 worker thread.



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