[GitHub] [incubator-nuttx] pkarashchenko commented on a diff in pull request #7018: wqueue: change single queue to double queue to improve speed

[GitBox <gi...@apache.org>]

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


##########
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:
   why do we need changes in this file? I see only function reordering, but no functional changes



##########
include/nuttx/wdog.h:
##########
@@ -61,17 +61,22 @@ typedef uint32_t  wdparm_t;
 
 typedef CODE void (*wdentry_t)(wdparm_t arg);
 
-/* This is the internal representation of the watchdog timer structure. */
+/* This is the internal representation of the watchdog timer structure.
+ * Notice !!!
+ * Carefully with the struct wdog_s order, you may not directly modify
+ * this. This struct will combine in struct work_s in union type, and,
+ * wqueue will modify/check this struct in work_qcancel().
+ */
 
 struct wdog_s
 {
   FAR struct wdog_s *next;       /* Support for singly linked lists. */
+  wdparm_t           arg;        /* Callback argument */

Review Comment:
   How ordering here bring an error in combination with union?



##########
include/nuttx/wdog.h:
##########
@@ -61,17 +61,22 @@ typedef uint32_t  wdparm_t;
 
 typedef CODE void (*wdentry_t)(wdparm_t arg);
 
-/* This is the internal representation of the watchdog timer structure. */
+/* This is the internal representation of the watchdog timer structure.
+ * Notice !!!
+ * Carefully with the struct wdog_s order, you may not directly modify
+ * this. This struct will combine in struct work_s in union type, and,
+ * wqueue will modify/check this struct in work_qcancel().

Review Comment:
   ```suggestion
    * wqueue will modify/check this struct in kwork work_qcancel().
   ```



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