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/07/01 14:45:47 UTC

[GitHub] [incubator-nuttx] Donny9 opened a new pull request #4035: work_queue: schedule the work queue using the timer mechanism

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


   ## Summary
   work_queue: schedule the work queue using the timer mechanism.
   ## Impact
   
   ## Testing
   daily test.
   


-- 
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] hartmannathan commented on pull request #4035: work_queue: schedule the work queue using the timer mechanism

Posted by GitBox <gi...@apache.org>.
hartmannathan commented on pull request #4035:
URL: https://github.com/apache/incubator-nuttx/pull/4035#issuecomment-874330013


   Hi @Donny9 do you intend to continue refining this PR?
   
   Currently there are some compile errors:
   
   ```
   wqueue/work_usrthread.c: In function 'work_process':
   352
   Error: wqueue/work_usrthread.c:160:29: error: 'volatile struct work_s' has no member named 'qtime'
   353
     160 |       elapsed = ctick - work->qtime;
   354
         |                             ^~
   355
   Error: wqueue/work_usrthread.c:161:26: error: 'volatile struct work_s' has no member named 'delay'
   356
     161 |       if (elapsed >= work->delay)
   357
         |                          ^~
   358
   Error: wqueue/work_usrthread.c:215:49: error: 'volatile struct work_s' has no member named 'dq'; did you mean 'sq'?
   359
     215 |               work = (FAR struct work_s *)work->dq.flink;
   360
         |                                                 ^~
   361
         |                                                 sq
   ```
   
   It looks like that's because you changed only the kernel worker queues, not the user work queue that can be used by applications in Kernel or Protected builds.
   


-- 
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] hartmannathan commented on pull request #4035: work_queue: schedule the work queue using the timer mechanism

Posted by GitBox <gi...@apache.org>.
hartmannathan commented on pull request #4035:
URL: https://github.com/apache/incubator-nuttx/pull/4035#issuecomment-874330013






-- 
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] Donny9 commented on a change in pull request #4035: work_queue: schedule the work queue using the timer mechanism

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



##########
File path: sched/wqueue/kwork_cancel.c
##########
@@ -77,18 +77,30 @@ static int work_qcancel(FAR struct kwork_wqueue_s *wqueue,
   flags = enter_critical_section();
   if (work->worker != NULL)
     {
-      /* A little test of the integrity of the work queue */
-
-      DEBUGASSERT(work->dq.flink != NULL ||
-                  (FAR dq_entry_t *)work == wqueue->q.tail);
-      DEBUGASSERT(work->dq.blink != NULL ||
-                  (FAR dq_entry_t *)work == wqueue->q.head);
-
       /* Remove the entry from the work queue and make sure that it is
        * marked as available (i.e., the worker field is nullified).
        */
 
-      dq_rem((FAR dq_entry_t *)work, &wqueue->q);
+      if (WDOG_ISACTIVE(&work->timer))
+        {
+          wd_cancel(&work->timer);
+        }
+      else
+        {
+          /* The sem_wait() can't call from interrupt handlers, so worker
+           * thread will still be awakened even if the work cancel is
+           * called in interrupt handlers, but work thread can handle
+           * this case with work empty in the work queue.
+           */
+
+          if (up_interrupt_context() == false)
+            {
+              nxsem_wait(&wqueue->sem);
+            }
+
+          sq_rem((FAR sq_entry_t *)work, &wqueue->q);
+        }
+

Review comment:
       the item need to remove but the semaphore can't be reduced by one.




-- 
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] davids5 commented on pull request #4035: work_queue: schedule the work queue using the timer mechanism

Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4035:
URL: https://github.com/apache/incubator-nuttx/pull/4035#issuecomment-876515801


   @hartmannathan it was on an F7. 


-- 
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] Donny9 commented on a change in pull request #4035: work_queue: schedule the work queue using the timer mechanism

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



##########
File path: sched/wqueue/kwork_thread.c
##########
@@ -0,0 +1,297 @@
+/****************************************************************************
+ * sched/wqueue/kwork_thread.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <unistd.h>
+#include <sched.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <assert.h>
+#include <queue.h>
+#include <debug.h>
+
+#include <nuttx/wqueue.h>
+#include <nuttx/kthread.h>
+#include <nuttx/semaphore.h>
+
+#include "wqueue/wqueue.h"
+
+#if defined(CONFIG_SCHED_WORKQUEUE)
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#if defined(CONFIG_SCHED_CRITMONITOR_MAXTIME_WQUEUE) && CONFIG_SCHED_CRITMONITOR_MAXTIME_WQUEUE > 0
+#  define CALL_WORKER(worker, arg) \
+     do \
+       { \
+         uint32_t start; \
+         uint32_t elapsed; \
+         start = up_critmon_gettime(); \
+         worker(arg); \
+         elapsed = up_critmon_gettime() - start; \
+         if (elapsed > CONFIG_SCHED_CRITMONITOR_MAXTIME_WQUEUE) \
+           { \
+             serr("WORKER %p execute too long %"PRIu32"\n", \
+                   worker, elapsed); \
+           } \
+       } \
+     while (0)
+#else
+#  define CALL_WORKER(worker, arg) worker(arg)
+#endif
+
+/****************************************************************************
+ * 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
+ *
+ * Description:
+ *   These are the worker threads that performs the actions placed on the
+ *   high priority work queue.
+ *
+ *   These, along with the lower priority worker thread(s) are the kernel
+ *   mode work queues (also build in the flat build).
+ *
+ *   All kernel mode worker threads are started by the OS during normal
+ *   bring up.  This entry point is referenced by OS internally and should
+ *   not be accessed by application logic.
+ *
+ * Input Parameters:
+ *   argc, argv (not used)
+ *
+ * Returned Value:
+ *   Does not return
+ *
+ ****************************************************************************/
+
+static int work_thread(int argc, FAR char *argv[])
+{
+  FAR struct kwork_wqueue_s *wqueue;
+  FAR struct work_s *work;
+  worker_t  worker;
+  irqstate_t flags;
+  FAR void *arg;
+
+  wqueue = (FAR struct kwork_wqueue_s *)
+           ((uintptr_t)strtoul(argv[1], NULL, 0));
+
+  flags = enter_critical_section();
+
+  /* Loop forever */
+
+  for (; ; )
+    {
+      /* Then process queued work.  work_process will not return until: (1)
+       * there is no further work in the work queue, and (2) semaphore is
+       * posted.
+       */
+
+      nxsem_wait_uninterruptible(&wqueue->sem);
+
+      /* And check each 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 */
+
+      work = (FAR struct work_s *)sq_remfirst(&wqueue->q);
+      if (work && work->worker)
+        {
+          /* Extract the work description from the entry (in case the work
+           * instance by the re-used after it has been de-queued).

Review comment:
       > This comment doesn't quite make sense. Perhaps you mean something like:
   > 
   > "Extract the work description from the entry (in case the work instance will be re-used after it has been de-queued)."
   
   Done, Thank you.




-- 
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] Donny9 commented on pull request #4035: work_queue: schedule the work queue using the timer mechanism

Posted by GitBox <gi...@apache.org>.
Donny9 commented on pull request #4035:
URL: https://github.com/apache/incubator-nuttx/pull/4035#issuecomment-873018030


   > @Donny9 0 I need more time to review this.
   
   Yes. Thank you.


-- 
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] hartmannathan commented on a change in pull request #4035: work_queue: schedule the work queue using the timer mechanism

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



##########
File path: sched/wqueue/kwork_queue.c
##########
@@ -148,16 +107,39 @@ static void work_qqueue(FAR struct kwork_wqueue_s *wqueue,
 int work_queue(int qid, FAR struct work_s *work, worker_t worker,
                FAR void *arg, clock_t delay)
 {
+  irqstate_t flags;
+
+  /* Remove the entry from the timer and work queue. */
+
+  work_cancel(qid, work);
+
+  /* Interrupts are disabled so that this logic can be called from with
+   * task logic or ifrom nterrupt handling logic.

Review comment:
       typo:
   
   `ifrom nterrupt handling logic`
   
   should be
   
   `from interrupt handling logic`




-- 
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] Donny9 commented on a change in pull request #4035: work_queue: schedule the work queue using the timer mechanism

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



##########
File path: sched/wqueue/kwork_queue.c
##########
@@ -148,16 +107,39 @@ static void work_qqueue(FAR struct kwork_wqueue_s *wqueue,
 int work_queue(int qid, FAR struct work_s *work, worker_t worker,
                FAR void *arg, clock_t delay)
 {
+  irqstate_t flags;
+
+  /* Remove the entry from the timer and work queue. */
+
+  work_cancel(qid, work);
+
+  /* Interrupts are disabled so that this logic can be called from with
+   * task logic or ifrom nterrupt handling logic.

Review comment:
       > typo:
   > 
   > `ifrom nterrupt handling logic`
   > 
   > should be
   > 
   > `from interrupt handling logic`
   
   Done. Thank you.




-- 
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] hartmannathan commented on pull request #4035: work_queue: schedule the work queue using the timer mechanism

Posted by GitBox <gi...@apache.org>.
hartmannathan commented on pull request #4035:
URL: https://github.com/apache/incubator-nuttx/pull/4035#issuecomment-876477240


   > @Donny9 @xiaoxiang781216
   > 
   > I have had a look at this in detail.
   
   @davids5 thank you for doing this. May I ask which micro you tested with?
   


-- 
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] hartmannathan commented on pull request #4035: work_queue: schedule the work queue using the timer mechanism

Posted by GitBox <gi...@apache.org>.
hartmannathan commented on pull request #4035:
URL: https://github.com/apache/incubator-nuttx/pull/4035#issuecomment-874330082


   Also the file `arch/renesas/include/rx65n/inttypes.h` is being changed, but I think it's a different subject and so should be in its own separate PR?
   


-- 
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 pull request #4035: work_queue: schedule the work queue using the timer mechanism

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4035:
URL: https://github.com/apache/incubator-nuttx/pull/4035#issuecomment-876485440


   
   > So we use more resources and couple work-queues with watch dogs, but the performances is superior.
   > 
   
   work_s has 6 int before modification, need 7 int, so each work requre additional 4 bytes. but we can use union like this to achieve the same size as before:
   ```
   work_s
   {
     union
     {
        struct sq_entry_s sq;
        struct wdog_s timer;
     };
     worker_t  worker;
     FAR void *arg;    
   };
   ```
   since sq and timer is never used at the same time.


-- 
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] Donny9 commented on a change in pull request #4035: work_queue: schedule the work queue using the timer mechanism

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



##########
File path: sched/wqueue/kwork_thread.c
##########
@@ -0,0 +1,297 @@
+/****************************************************************************
+ * sched/wqueue/kwork_thread.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <unistd.h>
+#include <sched.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <assert.h>
+#include <queue.h>
+#include <debug.h>
+
+#include <nuttx/wqueue.h>
+#include <nuttx/kthread.h>
+#include <nuttx/semaphore.h>
+
+#include "wqueue/wqueue.h"
+
+#if defined(CONFIG_SCHED_WORKQUEUE)
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#if defined(CONFIG_SCHED_CRITMONITOR_MAXTIME_WQUEUE) && CONFIG_SCHED_CRITMONITOR_MAXTIME_WQUEUE > 0
+#  define CALL_WORKER(worker, arg) \
+     do \
+       { \
+         uint32_t start; \
+         uint32_t elapsed; \
+         start = up_critmon_gettime(); \
+         worker(arg); \
+         elapsed = up_critmon_gettime() - start; \
+         if (elapsed > CONFIG_SCHED_CRITMONITOR_MAXTIME_WQUEUE) \
+           { \
+             serr("WORKER %p execute too long %"PRIu32"\n", \
+                   worker, elapsed); \
+           } \
+       } \
+     while (0)
+#else
+#  define CALL_WORKER(worker, arg) worker(arg)
+#endif
+
+/****************************************************************************
+ * 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
+ *
+ * Description:
+ *   These are the worker threads that performs the actions placed on the
+ *   high priority work queue.
+ *
+ *   These, along with the lower priority worker thread(s) are the kernel
+ *   mode work queues (also build in the flat build).
+ *
+ *   All kernel mode worker threads are started by the OS during normal
+ *   bring up.  This entry point is referenced by OS internally and should
+ *   not be accessed by application logic.
+ *
+ * Input Parameters:
+ *   argc, argv (not used)
+ *
+ * Returned Value:
+ *   Does not return
+ *
+ ****************************************************************************/
+
+static int work_thread(int argc, FAR char *argv[])

Review comment:
       Done. Thank you.




-- 
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] davids5 commented on pull request #4035: work_queue: schedule the work queue using the timer mechanism

Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4035:
URL: https://github.com/apache/incubator-nuttx/pull/4035#issuecomment-876470352


   @Donny9 @xiaoxiang781216 
   
   I have had a look at this in detail.
   
   Contrasting the resources and performance.
   
     | master | wd
   -- | -- | --
   thread | same | same
   signaling | single signal per thread | semaphore per queue
   delay | sleep(min delay in queue) | Watchdog per item
   Performaces   enque | 7.37 us | 2.32us
   Delay 300us | 301.315 us | 300.594 us
   
   So we use more resources and couple work-queues with watch dogs, but the performances is superior.
   
   @patacongo - do you see any downside issues with this?
   
   If there are no objections let'c clean it up (separate out https://github.com/apache/incubator-nuttx/pull/4035#issuecomment-874330082 and rebase ). 
   


-- 
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] Donny9 closed pull request #4035: work_queue: schedule the work queue using the timer mechanism

Posted by GitBox <gi...@apache.org>.
Donny9 closed pull request #4035:
URL: https://github.com/apache/incubator-nuttx/pull/4035


   


-- 
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] gustavonihei commented on a change in pull request #4035: work_queue: schedule the work queue using the timer mechanism

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



##########
File path: sched/wqueue/kwork_thread.c
##########
@@ -0,0 +1,297 @@
+/****************************************************************************
+ * sched/wqueue/kwork_thread.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <unistd.h>
+#include <sched.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <assert.h>
+#include <queue.h>
+#include <debug.h>
+
+#include <nuttx/wqueue.h>
+#include <nuttx/kthread.h>
+#include <nuttx/semaphore.h>
+
+#include "wqueue/wqueue.h"
+
+#if defined(CONFIG_SCHED_WORKQUEUE)
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#if defined(CONFIG_SCHED_CRITMONITOR_MAXTIME_WQUEUE) && CONFIG_SCHED_CRITMONITOR_MAXTIME_WQUEUE > 0
+#  define CALL_WORKER(worker, arg) \
+     do \
+       { \
+         uint32_t start; \
+         uint32_t elapsed; \
+         start = up_critmon_gettime(); \
+         worker(arg); \
+         elapsed = up_critmon_gettime() - start; \
+         if (elapsed > CONFIG_SCHED_CRITMONITOR_MAXTIME_WQUEUE) \
+           { \
+             serr("WORKER %p execute too long %"PRIu32"\n", \
+                   worker, elapsed); \
+           } \
+       } \
+     while (0)
+#else
+#  define CALL_WORKER(worker, arg) worker(arg)
+#endif
+
+/****************************************************************************
+ * 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
+ *
+ * Description:
+ *   These are the worker threads that performs the actions placed on the
+ *   high priority work queue.
+ *
+ *   These, along with the lower priority worker thread(s) are the kernel
+ *   mode work queues (also build in the flat build).
+ *
+ *   All kernel mode worker threads are started by the OS during normal
+ *   bring up.  This entry point is referenced by OS internally and should
+ *   not be accessed by application logic.
+ *
+ * Input Parameters:
+ *   argc, argv (not used)
+ *
+ * Returned Value:
+ *   Does not return

Review comment:
       ```suggestion
    *   These are the worker threads that perform the actions placed on the
    *   high priority work queue.
    *
    *   These, along with the lower priority worker thread(s), are the kernel
    *   mode work queues (also built in the flat build).
    *
    *   All kernel mode worker threads are started by the OS during normal
    *   bring up.  This entry point is referenced by OS internally and should
    *   not be accessed by application logic.
    *
    * Input Parameters:
    *   argc, argv (not used)
    *
    * Returned Value:
    *   Does not return
   ```




-- 
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] Donny9 commented on pull request #4035: work_queue: schedule the work queue using the timer mechanism

Posted by GitBox <gi...@apache.org>.
Donny9 commented on pull request #4035:
URL: https://github.com/apache/incubator-nuttx/pull/4035#issuecomment-876478748


   > Also the file `arch/renesas/include/rx65n/inttypes.h` is being changed, but I think it's a different subject and so should be in its own separate PR?
   
   


-- 
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] hartmannathan commented on a change in pull request #4035: work_queue: schedule the work queue using the timer mechanism

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



##########
File path: sched/wqueue/kwork_thread.c
##########
@@ -0,0 +1,297 @@
+/****************************************************************************
+ * sched/wqueue/kwork_thread.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <unistd.h>
+#include <sched.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <assert.h>
+#include <queue.h>
+#include <debug.h>
+
+#include <nuttx/wqueue.h>
+#include <nuttx/kthread.h>
+#include <nuttx/semaphore.h>
+
+#include "wqueue/wqueue.h"
+
+#if defined(CONFIG_SCHED_WORKQUEUE)
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#if defined(CONFIG_SCHED_CRITMONITOR_MAXTIME_WQUEUE) && CONFIG_SCHED_CRITMONITOR_MAXTIME_WQUEUE > 0
+#  define CALL_WORKER(worker, arg) \
+     do \
+       { \
+         uint32_t start; \
+         uint32_t elapsed; \
+         start = up_critmon_gettime(); \
+         worker(arg); \
+         elapsed = up_critmon_gettime() - start; \
+         if (elapsed > CONFIG_SCHED_CRITMONITOR_MAXTIME_WQUEUE) \
+           { \
+             serr("WORKER %p execute too long %"PRIu32"\n", \
+                   worker, elapsed); \
+           } \
+       } \
+     while (0)
+#else
+#  define CALL_WORKER(worker, arg) worker(arg)
+#endif
+
+/****************************************************************************
+ * 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
+ *
+ * Description:
+ *   These are the worker threads that performs the actions placed on the
+ *   high priority work queue.
+ *
+ *   These, along with the lower priority worker thread(s) are the kernel
+ *   mode work queues (also build in the flat build).
+ *
+ *   All kernel mode worker threads are started by the OS during normal
+ *   bring up.  This entry point is referenced by OS internally and should
+ *   not be accessed by application logic.
+ *
+ * Input Parameters:
+ *   argc, argv (not used)
+ *
+ * Returned Value:
+ *   Does not return
+ *
+ ****************************************************************************/
+
+static int work_thread(int argc, FAR char *argv[])
+{
+  FAR struct kwork_wqueue_s *wqueue;
+  FAR struct work_s *work;
+  worker_t  worker;
+  irqstate_t flags;
+  FAR void *arg;
+
+  wqueue = (FAR struct kwork_wqueue_s *)
+           ((uintptr_t)strtoul(argv[1], NULL, 0));
+
+  flags = enter_critical_section();
+
+  /* Loop forever */
+
+  for (; ; )
+    {
+      /* Then process queued work.  work_process will not return until: (1)
+       * there is no further work in the work queue, and (2) semaphore is
+       * posted.
+       */
+
+      nxsem_wait_uninterruptible(&wqueue->sem);
+
+      /* And check each 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 */
+
+      work = (FAR struct work_s *)sq_remfirst(&wqueue->q);
+      if (work && work->worker)
+        {
+          /* Extract the work description from the entry (in case the work
+           * instance by the re-used after it has been de-queued).

Review comment:
       This comment doesn't quite make sense. Perhaps you mean something like:
   
   "Extract the work description from the entry (in case the work instance will be re-used after it has been de-queued)."




-- 
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 pull request #4035: work_queue: schedule the work queue using the timer mechanism

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #4035:
URL: https://github.com/apache/incubator-nuttx/pull/4035#issuecomment-886830237


   The memory consumption should be same as before, @davids5 please take a look.
   The next optimization is remove arg from both wdog_s(-4B) and work_s(-8B), since both are always directly embedded inside the big context struct, we can restore the context struct by contain_of macro without helping from arg pointer.


-- 
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] Donny9 commented on a change in pull request #4035: work_queue: schedule the work queue using the timer mechanism

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



##########
File path: sched/wqueue/kwork_thread.c
##########
@@ -0,0 +1,297 @@
+/****************************************************************************
+ * sched/wqueue/kwork_thread.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <unistd.h>
+#include <sched.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <assert.h>
+#include <queue.h>
+#include <debug.h>
+
+#include <nuttx/wqueue.h>
+#include <nuttx/kthread.h>
+#include <nuttx/semaphore.h>
+
+#include "wqueue/wqueue.h"
+
+#if defined(CONFIG_SCHED_WORKQUEUE)
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#if defined(CONFIG_SCHED_CRITMONITOR_MAXTIME_WQUEUE) && CONFIG_SCHED_CRITMONITOR_MAXTIME_WQUEUE > 0
+#  define CALL_WORKER(worker, arg) \
+     do \
+       { \
+         uint32_t start; \
+         uint32_t elapsed; \
+         start = up_critmon_gettime(); \
+         worker(arg); \
+         elapsed = up_critmon_gettime() - start; \
+         if (elapsed > CONFIG_SCHED_CRITMONITOR_MAXTIME_WQUEUE) \
+           { \
+             serr("WORKER %p execute too long %"PRIu32"\n", \
+                   worker, elapsed); \
+           } \
+       } \
+     while (0)
+#else
+#  define CALL_WORKER(worker, arg) worker(arg)
+#endif
+
+/****************************************************************************
+ * 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
+ *
+ * Description:
+ *   These are the worker threads that performs the actions placed on the
+ *   high priority work queue.
+ *
+ *   These, along with the lower priority worker thread(s) are the kernel
+ *   mode work queues (also build in the flat build).
+ *
+ *   All kernel mode worker threads are started by the OS during normal
+ *   bring up.  This entry point is referenced by OS internally and should
+ *   not be accessed by application logic.
+ *
+ * Input Parameters:
+ *   argc, argv (not used)
+ *
+ * Returned Value:
+ *   Does not return

Review comment:
       Done! Thank you.




-- 
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] gustavonihei commented on a change in pull request #4035: work_queue: schedule the work queue using the timer mechanism

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



##########
File path: sched/wqueue/kwork_thread.c
##########
@@ -0,0 +1,297 @@
+/****************************************************************************
+ * sched/wqueue/kwork_thread.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <unistd.h>
+#include <sched.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <assert.h>
+#include <queue.h>
+#include <debug.h>
+
+#include <nuttx/wqueue.h>
+#include <nuttx/kthread.h>
+#include <nuttx/semaphore.h>
+
+#include "wqueue/wqueue.h"
+
+#if defined(CONFIG_SCHED_WORKQUEUE)
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#if defined(CONFIG_SCHED_CRITMONITOR_MAXTIME_WQUEUE) && CONFIG_SCHED_CRITMONITOR_MAXTIME_WQUEUE > 0
+#  define CALL_WORKER(worker, arg) \
+     do \
+       { \
+         uint32_t start; \
+         uint32_t elapsed; \
+         start = up_critmon_gettime(); \
+         worker(arg); \
+         elapsed = up_critmon_gettime() - start; \
+         if (elapsed > CONFIG_SCHED_CRITMONITOR_MAXTIME_WQUEUE) \
+           { \
+             serr("WORKER %p execute too long %"PRIu32"\n", \
+                   worker, elapsed); \
+           } \
+       } \
+     while (0)
+#else
+#  define CALL_WORKER(worker, arg) worker(arg)
+#endif
+
+/****************************************************************************
+ * 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
+ *
+ * Description:
+ *   These are the worker threads that performs the actions placed on the
+ *   high priority work queue.
+ *
+ *   These, along with the lower priority worker thread(s) are the kernel
+ *   mode work queues (also build in the flat build).
+ *
+ *   All kernel mode worker threads are started by the OS during normal
+ *   bring up.  This entry point is referenced by OS internally and should
+ *   not be accessed by application logic.
+ *
+ * Input Parameters:
+ *   argc, argv (not used)
+ *
+ * Returned Value:
+ *   Does not return
+ *
+ ****************************************************************************/
+
+static int work_thread(int argc, FAR char *argv[])

Review comment:
       ```suggestion
   static noreturn_function int work_thread(int argc, FAR char *argv[])
   ```
   Since it does not return, should we add the `noreturn` attribute?




-- 
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] Donny9 commented on pull request #4035: work_queue: schedule the work queue using the timer mechanism

Posted by GitBox <gi...@apache.org>.
Donny9 commented on pull request #4035:
URL: https://github.com/apache/incubator-nuttx/pull/4035#issuecomment-872674683


   > If I understand correctly, this PR does 2 things:
   > 
   >     1. Unifies the HP and LP work queue logic
   > 
   >     2. Modifies the "delay" feature so it uses a WD timer instead of clock_systime_ticks(). If I understand correctly, this eliminates some polling and improves performance?
   > 
   > 
   > Did I understand correctly?
   
   right.


-- 
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] davids5 commented on a change in pull request #4035: work_queue: schedule the work queue using the timer mechanism

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



##########
File path: sched/wqueue/kwork_cancel.c
##########
@@ -77,18 +77,30 @@ static int work_qcancel(FAR struct kwork_wqueue_s *wqueue,
   flags = enter_critical_section();
   if (work->worker != NULL)
     {
-      /* A little test of the integrity of the work queue */
-
-      DEBUGASSERT(work->dq.flink != NULL ||
-                  (FAR dq_entry_t *)work == wqueue->q.tail);
-      DEBUGASSERT(work->dq.blink != NULL ||
-                  (FAR dq_entry_t *)work == wqueue->q.head);
-
       /* Remove the entry from the work queue and make sure that it is
        * marked as available (i.e., the worker field is nullified).
        */
 
-      dq_rem((FAR dq_entry_t *)work, &wqueue->q);
+      if (WDOG_ISACTIVE(&work->timer))
+        {
+          wd_cancel(&work->timer);
+        }
+      else
+        {
+          /* The sem_wait() can't call from interrupt handlers, so worker
+           * thread will still be awakened even if the work cancel is
+           * called in interrupt handlers, but work thread can handle
+           * this case with work empty in the work queue.
+           */
+
+          if (up_interrupt_context() == false)
+            {
+              nxsem_wait(&wqueue->sem);
+            }
+
+          sq_rem((FAR sq_entry_t *)work, &wqueue->q);
+        }
+

Review comment:
       This is running with interrupts off.
   Why is this not a remove item from queue and cancel if active?  




-- 
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] Donny9 commented on pull request #4035: work_queue: schedule the work queue using the timer mechanism

Posted by GitBox <gi...@apache.org>.
Donny9 commented on pull request #4035:
URL: https://github.com/apache/incubator-nuttx/pull/4035#issuecomment-872674331


   @hartmannathan @davids5 Hello, I did have some problems using the previous Work Queue, Then, I done this PR with reference to the delay work implementation of Linux. 
   
   PR1:  Unifies the kwork_lpwork.c and kwork_hpwork.c to kwork_thread.c, because LP and HP worker thread are same, just the handle of wqueue is different and we can fix the difference by pass parameter to worker thread when they were created.
   
   PR2: Using wd timer to keep work queue with delay are ordered and can remove the complicated timing update logic. 
   The PR can fix flollowing issue:
   1. It can reduce the number of awakenings of worker thread. For Previous work queue, If one work with delay is queued and multiple workers thread wait at the same time have just been awakened,  Then each of them will update the work, this is wrong. If using wd timer, it will start a timer and post a semaphore when timer expired, then only one of worker thread will wake up.
   2. In this PR, we need to ensure that the POST and WAIT operations are equal. 
       Post occurs when the work queue.
       Wait occurs when worker thread waiting and work cancel. but if we cancel work in interrupt handlers, wait can't run so that worker thread will be awakened and work is empty, The PR has covered this case.
   


-- 
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 #4035: work_queue: schedule the work queue using the timer mechanism

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


   


-- 
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 #4035: work_queue: schedule the work queue using the timer mechanism

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



##########
File path: include/nuttx/wqueue.h
##########
@@ -244,11 +245,14 @@ typedef CODE void (*worker_t)(FAR void *arg);
 
 struct work_s
 {
-  struct dq_entry_s dq;  /* Implements a doubly linked list */
+  struct sq_entry_s sq;  /* Implements a single linked list */

Review comment:
       let's union this field to reduce work_s size




-- 
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] gustavonihei commented on a change in pull request #4035: work_queue: schedule the work queue using the timer mechanism

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



##########
File path: sched/wqueue/kwork_thread.c
##########
@@ -0,0 +1,297 @@
+/****************************************************************************
+ * sched/wqueue/kwork_thread.c
+ *
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.  The
+ * ASF licenses this file to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.  See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ *
+ ****************************************************************************/
+
+/****************************************************************************
+ * Included Files
+ ****************************************************************************/
+
+#include <nuttx/config.h>
+
+#include <unistd.h>
+#include <sched.h>
+#include <stdio.h>
+#include <string.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <assert.h>
+#include <queue.h>
+#include <debug.h>
+
+#include <nuttx/wqueue.h>
+#include <nuttx/kthread.h>
+#include <nuttx/semaphore.h>
+
+#include "wqueue/wqueue.h"
+
+#if defined(CONFIG_SCHED_WORKQUEUE)
+
+/****************************************************************************
+ * Pre-processor Definitions
+ ****************************************************************************/
+
+#if defined(CONFIG_SCHED_CRITMONITOR_MAXTIME_WQUEUE) && CONFIG_SCHED_CRITMONITOR_MAXTIME_WQUEUE > 0
+#  define CALL_WORKER(worker, arg) \
+     do \
+       { \
+         uint32_t start; \
+         uint32_t elapsed; \
+         start = up_critmon_gettime(); \
+         worker(arg); \
+         elapsed = up_critmon_gettime() - start; \
+         if (elapsed > CONFIG_SCHED_CRITMONITOR_MAXTIME_WQUEUE) \
+           { \
+             serr("WORKER %p execute too long %"PRIu32"\n", \
+                   worker, elapsed); \
+           } \
+       } \
+     while (0)
+#else
+#  define CALL_WORKER(worker, arg) worker(arg)
+#endif
+
+/****************************************************************************
+ * 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
+ *
+ * Description:
+ *   These are the worker threads that performs the actions placed on the
+ *   high priority work queue.
+ *
+ *   These, along with the lower priority worker thread(s) are the kernel
+ *   mode work queues (also build in the flat build).
+ *
+ *   All kernel mode worker threads are started by the OS during normal
+ *   bring up.  This entry point is referenced by OS internally and should
+ *   not be accessed by application logic.
+ *
+ * Input Parameters:
+ *   argc, argv (not used)
+ *
+ * Returned Value:
+ *   Does not return
+ *
+ ****************************************************************************/
+
+static int work_thread(int argc, FAR char *argv[])

Review comment:
       I don't know if you've missed this change or if it has been reverted somehow, but `noreturn` attribute is missing.




-- 
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] hartmannathan commented on pull request #4035: work_queue: schedule the work queue using the timer mechanism

Posted by GitBox <gi...@apache.org>.
hartmannathan commented on pull request #4035:
URL: https://github.com/apache/incubator-nuttx/pull/4035#issuecomment-872362903


   If I understand correctly, this PR does 2 things:
   
   1) Unifies the HP and LP work queue logic
   
   2) Modifies the "delay" feature so it uses a WD timer instead of clock_systime_ticks(). If I understand correctly, this eliminates some polling and improves performance?
   
   Did I understand correctly?


-- 
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] Donny9 commented on pull request #4035: work_queue: schedule the work queue using the timer mechanism

Posted by GitBox <gi...@apache.org>.
Donny9 commented on pull request #4035:
URL: https://github.com/apache/incubator-nuttx/pull/4035#issuecomment-876478453


   > ould be
   
   Ok, i will done them tomorrow. Thank you.


-- 
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] davids5 commented on pull request #4035: work_queue: schedule the work queue using the timer mechanism

Posted by GitBox <gi...@apache.org>.
davids5 commented on pull request #4035:
URL: https://github.com/apache/incubator-nuttx/pull/4035#issuecomment-873012925


   @Donny9 0 I need more time to review this.


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