Revert "sched/wqueue: replace the notification mech from signal to semaphore"

This reverts commit a3a0f578bc.

Documentation/reference/os/wqueue.rst

@@ -71,6 +71,10 @@ to match the highest priority client.
 **Common Configuration Options**. These options apply to all work
 queues:
 
+-  ``CONFIG_SIG_SIGWORK`` The signal number that will be used to
+   wake-up the worker thread. This same signal is used with the
+   Default: 17
+
 Low Priority Kernel Work Queue
 ------------------------------
 

include/nuttx/wqueue.h

@@ -56,6 +56,8 @@
  *   priority worker thread.  Default: 224
  * CONFIG_SCHED_HPWORKSTACKSIZE - The stack size allocated for the worker
  *   thread.  Default: 2048.
+ * CONFIG_SIG_SIGWORK - The signal number that will be used to wake-up
+ *   the worker thread.  Default: 17
  *
  * CONFIG_SCHED_LPWORK. If CONFIG_SCHED_LPWORK is selected then a lower-
  *   priority work queue will be created.  This lower priority work queue

include/signal.h

@@ -224,6 +224,16 @@
 #  endif
 #endif
 
+/* SIGWORK is used to wake up various internal, NuttX worker thread */
+
+#if defined(CONFIG_SCHED_WORKQUEUE) || defined(CONFIG_PAGING)
+#  ifndef CONFIG_SIG_SIGWORK
+#    define SIGWORK     17  /* Used to wake up the work queue */
+#  else
+#    define SIGWORK     CONFIG_SIG_SIGWORK
+#  endif
+#endif
+
 /* sigprocmask() "how" definitions. Only one of the following can be specified: */
 
 #define SIG_BLOCK       1  /* Block the given signals */

sched/Kconfig

@@ -1556,6 +1556,15 @@ config SIG_SIGCONDTIMEDOUT
 		This non-standard signal number is used the implementation of
 		pthread_cond_timedwait(). Default 16.
 
+config SIG_SIGWORK
+	int "SIGWORK"
+	default 17
+	depends on SCHED_WORKQUEUE || LIB_USRWORK
+	---help---
+		SIGWORK is a non-standard signal used to wake up the internal NuttX
+		worker thread.  This setting specifies the signal number that will be
+		used for SIGWORK.  Default: 17
+
 endmenu # Signal Numbers
 endmenu # Signal Configuration
 

sched/wqueue/Make.defs

@@ -23,11 +23,21 @@
 ifeq ($(CONFIG_SCHED_WORKQUEUE),y)
 
 CSRCS += kwork_queue.c kwork_process.c kwork_cancel.c kwork_signal.c
-CSRCS += kwork_thread.c
 
+# Add high priority work queue files
+
+ifeq ($(CONFIG_SCHED_HPWORK),y)
+CSRCS += kwork_hpthread.c
+endif
+
+# Add low priority work queue files
+
+ifeq ($(CONFIG_SCHED_LPWORK),y)
+CSRCS += kwork_lpthread.c
 ifeq ($(CONFIG_PRIORITY_INHERITANCE),y)
 CSRCS += kwork_inherit.c
 endif # CONFIG_PRIORITY_INHERITANCE
+endif # CONFIG_SCHED_LPWORK
 
 # Add work queue notifier support
 

sched/wqueue/kwork_hpthread.c

@@ -1,5 +1,5 @@
 /****************************************************************************
- * sched/wqueue/kwork_thread.c
+ * sched/wqueue/kwork_hpthread.c
  *
  * Licensed to the Apache Software Foundation (ASF) under one or more
  * contributor license agreements.  See the NOTICE file distributed with
@@ -26,42 +26,34 @@
 
 #include <unistd.h>
 #include <sched.h>
-#include <stdio.h>
 #include <string.h>
-#include <stdlib.h>
 #include <errno.h>
 #include <queue.h>
 #include <debug.h>
 
 #include <nuttx/wqueue.h>
 #include <nuttx/kthread.h>
+#include <nuttx/kmalloc.h>
+#include <nuttx/clock.h>
 
 #include "wqueue/wqueue.h"
 
-#if defined(CONFIG_SCHED_WORKQUEUE)
+#ifdef CONFIG_SCHED_HPWORK
 
 /****************************************************************************
  * Public Data
  ****************************************************************************/
 
-#if defined(CONFIG_SCHED_HPWORK)
 /* The state of the kernel mode, high priority work queue(s). */
 
 struct hp_wqueue_s g_hpwork;
-#endif /* CONFIG_SCHED_HPWORK */
-
-#if defined(CONFIG_SCHED_LPWORK)
-/* The state of the kernel mode, low priority work queue(s). */
-
-struct lp_wqueue_s g_lpwork;
-#endif /* CONFIG_SCHED_LPWORK */
 
 /****************************************************************************
  * Private Functions
  ****************************************************************************/
 
 /****************************************************************************
- * Name: work_thread
+ * Name: work_hpthread
  *
  * Description:
  *   These are the worker threads that performs the actions placed on the
@@ -82,14 +74,26 @@ struct lp_wqueue_s g_lpwork;
  *
  ****************************************************************************/
 
-static int work_thread(int argc, char *argv[])
+static int work_hpthread(int argc, char *argv[])
 {
-  FAR struct kwork_wqueue_s *queue;
-  FAR struct kworker_s      *worker;
+  int wndx = 0;
+#if CONFIG_SCHED_HPNTHREADS > 1
+  pid_t me = getpid();
+  int i;
 
-  queue  = (FAR struct kwork_wqueue_s *)
-           ((uintptr_t)strtoul(argv[1], NULL, 0));
-  worker = (FAR struct kworker_s *)((uintptr_t)strtoul(argv[2], NULL, 0));
+  /* Find out thread index by search the workers in g_hpwork */
+
+  for (wndx = 0, i = 0; i < CONFIG_SCHED_HPNTHREADS; i++)
+    {
+      if (g_hpwork.worker[i].pid == me)
+        {
+          wndx = i;
+          break;
+        }
+    }
+
+  DEBUGASSERT(i < CONFIG_SCHED_HPNTHREADS);
+#endif
 
   /* Loop forever */
 
@@ -100,73 +104,12 @@ static int work_thread(int argc, char *argv[])
        * triggered, or delayed work expires.
        */
 
-      work_process(queue, worker);
+      work_process((FAR struct kwork_wqueue_s *)&g_hpwork, wndx);
     }
 
   return OK; /* To keep some compilers happy */
 }
 
-/****************************************************************************
- * Name: work_thread_create
- *
- * Description:
- *   This function creates and activates a work thread task with kernel-
- *   mode privileges.
- *
- * Input Parameters:
- *   name       - Name of the new task
- *   priority   - Priority of the new task
- *   stack_size - size (in bytes) of the stack needed
- *   nthread    - Number of work thread should be created
- *   wqueue     - Work queue instance
- *
- * Returned Value:
- *   A negated errno value is returned on failure.
- *
- ****************************************************************************/
-
-static int work_thread_create(FAR const char *name, int priority,
-                              int stack_size, int nthread,
-                              FAR struct kwork_wqueue_s *wqueue)
-{
-  FAR char *argv[3];
-  char args[2][16];
-  int wndx;
-  int pid;
-
-  snprintf(args[0], 16, "0x%" PRIxPTR, (uintptr_t)wqueue);
-  argv[0] = args[0];
-  argv[2] = NULL;
-
-  for (wndx = 0; wndx < nthread; wndx++)
-    {
-      nxsem_init(&wqueue->worker[wndx].sem, 0, 0);
-      nxsem_set_protocol(&wqueue->worker[wndx].sem, SEM_PRIO_NONE);
-
-      wqueue->worker[wndx].busy = true;
-
-      snprintf(args[1], 16, "0x%" PRIxPTR, (uintptr_t)&wqueue->worker[wndx]);
-      argv[1] = args[1];
-
-      pid = kthread_create(name, priority, stack_size,
-                           (main_t)work_thread, argv);
-
-      DEBUGASSERT(pid > 0);
-      if (pid < 0)
-        {
-          serr("ERROR: work_thread_create %d failed: %d\n", wndx, pid);
-          sched_unlock();
-          return pid;
-        }
-
-#ifdef CONFIG_PRIORITY_INHERITANCE
-      wqueue->worker[wndx].pid  = pid;
-#endif
-    }
-
-  return OK;
-}
-
 /****************************************************************************
  * Public Functions
  ****************************************************************************/
@@ -181,50 +124,47 @@ static int work_thread_create(FAR const char *name, int priority,
  *   None
  *
  * Returned Value:
- *   A negated errno value is returned on failure.
+ *   The task ID of the worker thread is returned on success.  A negated
+ *   errno value is returned on failure.
  *
  ****************************************************************************/
 
-#if defined(CONFIG_SCHED_HPWORK)
 int work_start_highpri(void)
 {
+  pid_t pid;
+  int wndx;
+
+  /* Don't permit any of the threads to run until we have fully initialized
+   * g_hpwork.
+   */
+
+  sched_lock();
+
   /* Start the high-priority, kernel mode worker thread(s) */
 
   sinfo("Starting high-priority kernel worker thread(s)\n");
 
-  return work_thread_create(HPWORKNAME, CONFIG_SCHED_HPWORKPRIORITY,
-                            CONFIG_SCHED_HPWORKSTACKSIZE,
-                            CONFIG_SCHED_HPNTHREADS,
-                            (FAR struct kwork_wqueue_s *)&g_hpwork);
+  for (wndx = 0; wndx < CONFIG_SCHED_HPNTHREADS; wndx++)
+    {
+      pid = kthread_create(HPWORKNAME, CONFIG_SCHED_HPWORKPRIORITY,
+                           CONFIG_SCHED_HPWORKSTACKSIZE,
+                           (main_t)work_hpthread,
+                           (FAR char * const *)NULL);
+
+      DEBUGASSERT(pid > 0);
+      if (pid < 0)
+        {
+          serr("ERROR: kthread_create %d failed: %d\n", wndx, (int)pid);
+          sched_unlock();
+          return (int)pid;
+        }
+
+      g_hpwork.worker[wndx].pid  = pid;
+      g_hpwork.worker[wndx].busy = true;
+    }
+
+  sched_unlock();
+  return g_hpwork.worker[0].pid;
 }
+
 #endif /* CONFIG_SCHED_HPWORK */
-
-/****************************************************************************
- * Name: work_start_lowpri
- *
- * Description:
- *   Start the low-priority, kernel-mode worker thread(s)
- *
- * Input Parameters:
- *   None
- *
- * Returned Value:
- *   A negated errno value is returned on failure.
- *
- ****************************************************************************/
-
-#if defined(CONFIG_SCHED_LPWORK)
-int work_start_lowpri(void)
-{
-  /* Start the low-priority, kernel mode worker thread(s) */
-
-  sinfo("Starting low-priority kernel worker thread(s)\n");
-
-  return work_thread_create(LPWORKNAME, CONFIG_SCHED_LPWORKPRIORITY,
-                            CONFIG_SCHED_LPWORKSTACKSIZE,
-                            CONFIG_SCHED_LPNTHREADS,
-                            (FAR struct kwork_wqueue_s *)&g_lpwork);
-}
-#endif /* CONFIG_SCHED_LPWORK */
-
-#endif /* CONFIG_SCHED_WORKQUEUE */

sched/wqueue/kwork_notifier.c

@@ -33,6 +33,7 @@
 #include <assert.h>
 
 #include <nuttx/kmalloc.h>
+#include <nuttx/semaphore.h>
 #include <nuttx/wqueue.h>
 
 #include "wqueue/wqueue.h"

sched/wqueue/kwork_process.c

@@ -98,16 +98,14 @@
  *   be called from application level logic.
  *
  * Input Parameters:
- *   wqueue  - Describes the work queue to be processed
- *   kworker - Describes a worker thread
+ *   wqueue - Describes the work queue to be processed
  *
  * Returned Value:
  *   None
  *
  ****************************************************************************/
 
-void work_process(FAR struct kwork_wqueue_s *wqueue,
-                  FAR struct kworker_s *kworker)
+void work_process(FAR struct kwork_wqueue_s *wqueue, int wndx)
 {
   volatile FAR struct work_s *work;
   worker_t  worker;
@@ -231,9 +229,16 @@ void work_process(FAR struct kwork_wqueue_s *wqueue,
 
   if (next == WORK_DELAY_MAX)
     {
-      kworker->busy = false;
-      nxsem_wait_uninterruptible(&kworker->sem);
-      kworker->busy = true;
+      sigset_t set;
+
+      /* Wait indefinitely until signalled with SIGWORK */
+
+      sigemptyset(&set);
+      nxsig_addset(&set, SIGWORK);
+
+      wqueue->worker[wndx].busy = false;
+      DEBUGVERIFY(nxsig_waitinfo(&set, NULL));
+      wqueue->worker[wndx].busy = true;
     }
   else
     {
@@ -242,11 +247,9 @@ void work_process(FAR struct kwork_wqueue_s *wqueue,
        * Interrupts will be re-enabled while we wait.
        */
 
-      kworker->busy = false;
-      nxsem_tickwait_uninterruptible(&kworker->sem,
-                                     clock_systime_ticks(),
-                                     next * USEC_PER_TICK);
-      kworker->busy = true;
+      wqueue->worker[wndx].busy = false;
+      nxsig_usleep(next * USEC_PER_TICK);
+      wqueue->worker[wndx].busy = true;
     }
 
   leave_critical_section(flags);

sched/wqueue/kwork_signal.c

@@ -57,7 +57,6 @@
 int work_signal(int qid)
 {
   FAR struct kwork_wqueue_s *work;
-  int semcount;
   int threads;
   int i;
 
@@ -106,13 +105,7 @@ int work_signal(int qid)
 
   /* Otherwise, signal the first IDLE thread found */
 
-  nxsem_get_value(&work->worker[i].sem, &semcount);
-  if (semcount < 1)
-    {
-      nxsem_post(&work->worker[i].sem);
-    }
-
-  return OK;
+  return nxsig_kill(work->worker[i].pid, SIGWORK);
 }
 
 #endif /* CONFIG_SCHED_WORKQUEUE */

sched/wqueue/wqueue.h

@@ -31,7 +31,6 @@
 #include <queue.h>
 
 #include <nuttx/clock.h>
-#include <nuttx/semaphore.h>
 
 #ifdef CONFIG_SCHED_WORKQUEUE
 
@@ -52,10 +51,7 @@
 
 struct kworker_s
 {
-#ifdef CONFIG_PRIORITY_INHERITANCE
   pid_t             pid;    /* The task ID of the worker thread */
-#endif
-  sem_t             sem;    /* The counting semaphore of the worker thread */
   volatile bool     busy;   /* True: Worker is not available */
 };
 
@@ -165,16 +161,15 @@ int work_start_lowpri(void);
  *   be called from application level logic.
  *
  * Input Parameters:
- *   wqueue  - Describes the work queue to be processed
- *   kworker - Describes a worker thread
+ *   wqueue - Describes the work queue to be processed
+ *   wndx   - The worker thread index
  *
  * Returned Value:
  *   None
  *
  ****************************************************************************/
 
-void work_process(FAR struct kwork_wqueue_s *wqueue,
-                  FAR struct kworker_s *kworker);
+void work_process(FAR struct kwork_wqueue_s *wqueue, int wndx);
 
 /****************************************************************************
  * Name: work_initialize_notifier