This is an automated email from the ASF dual-hosted git repository.
xiaoxiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx.git
commit 900b1c19ddfaece35df7306ad368db72c6a32a78
Author: ouyangxiangzhen <ouyangxiangz...@xiaomi.com>
AuthorDate: Tue May 6 15:05:40 2025 +0800
wqueue: improve the robustness of the work
struct work_s
{
union
{
struct
{
struct dq_entry_s dq; /* Implements a double linked list */
clock_t qtime; /* Time work queued */
} s;
struct wdog_s timer; /* Delay expiry timer */
struct wdog_period_s ptimer; /* Period expiry timer */
} u;
worker_t worker; /* Work callback */
FAR void *arg; /* Callback argument */
FAR struct kwork_wqueue_s *wq; /* Work queue */
};
work_cancel() should determine whether the current work is
in the timer or has already entered the queue.
This judgment is indispensable because the structure is a union.
Whether it is interpreted as a timer or as a dq needs to be determined.
But this judgment seriously depends on the order of struct wdog_s and
struct dq_entry_s, once someone change the order of any, there is a bug.
So we decide remove the union, to improve the robustness.
For the work_s structure size will grow bigger, then we will provide a
another optimization patch
Signed-off-by: ligd <liguidi...@xiaomi.com>
Signed-off-by: ouyangxiangzhen <ouyangxiangz...@xiaomi.com>
---
include/nuttx/wdog.h | 11 +++--------
include/nuttx/wqueue.h | 13 +++----------
libs/libc/wqueue/work_cancel.c | 2 +-
libs/libc/wqueue/work_queue.c | 14 +++++++-------
libs/libc/wqueue/work_usrthread.c | 4 ++--
5 files changed, 16 insertions(+), 28 deletions(-)
diff --git a/include/nuttx/wdog.h b/include/nuttx/wdog.h
index 90ba3fc85f..02f18b1b30 100644
--- a/include/nuttx/wdog.h
+++ b/include/nuttx/wdog.h
@@ -72,12 +72,7 @@ struct wdlist_node
FAR struct wdlist_node *next;
};
-/* 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 kwork work_qcancel().
- */
+/* Support a doubly linked list of watchdog timers */
struct wdog_s
{
@@ -87,7 +82,7 @@ struct wdog_s
#ifdef CONFIG_PIC
FAR void *picbase; /* PIC base address */
#endif
- clock_t expired; /* Timer associated with the absoulute time */
+ clock_t expired; /* Timer associated with the absolute time */
};
struct wdog_period_s
@@ -168,7 +163,7 @@ int wd_start(FAR struct wdog_s *wdog, sclock_t delay,
*
* Input Parameters:
* wdog - Watchdog ID
- * ticks - Absoulute time in clock ticks
+ * ticks - Absolute time in clock ticks
* wdentry - Function to call on timeout
* arg - Parameter to pass to wdentry.
*
diff --git a/include/nuttx/wqueue.h b/include/nuttx/wqueue.h
index 6155015540..f964608fbb 100644
--- a/include/nuttx/wqueue.h
+++ b/include/nuttx/wqueue.h
@@ -249,13 +249,10 @@ typedef CODE void (*worker_t)(FAR void *arg);
struct work_s
{
+ struct dq_entry_s dq; /* Implements a double linked list */
+ clock_t qtime; /* Time work queued */
union
{
- struct
- {
- struct dq_entry_s dq; /* Implements a double linked list */
- clock_t qtime; /* Time work queued */
- } s;
struct wdog_s timer; /* Delay expiry timer */
struct wdog_period_s ptimer; /* Period expiry timer */
} u;
@@ -560,11 +557,7 @@ int work_cancel_sync_wq(FAR struct kwork_wqueue_s *wqueue,
*
****************************************************************************/
-#ifdef __KERNEL__
-# define work_timeleft(work) wd_gettime(&((work)->u.timer))
-#else
-# define work_timeleft(work) ((sclock_t)((work)->u.s.qtime - clock()))
-#endif
+#define work_timeleft(work) ((sclock_t)((work)->qtime - clock()))
/****************************************************************************
* Name: lpwork_boostpriority
diff --git a/libs/libc/wqueue/work_cancel.c b/libs/libc/wqueue/work_cancel.c
index 4a86335c2e..195f781a02 100644
--- a/libs/libc/wqueue/work_cancel.c
+++ b/libs/libc/wqueue/work_cancel.c
@@ -89,7 +89,7 @@ static int work_qcancel(FAR struct usr_wqueue_s *wqueue,
*/
curr = wqueue->q.head;
- while (curr && curr != &work->u.s.dq)
+ while (curr && curr != &work->dq)
{
prev = curr;
curr = curr->flink;
diff --git a/libs/libc/wqueue/work_queue.c b/libs/libc/wqueue/work_queue.c
index 0a00bf6f2e..26b4acf9ff 100644
--- a/libs/libc/wqueue/work_queue.c
+++ b/libs/libc/wqueue/work_queue.c
@@ -87,9 +87,9 @@ static int work_qqueue(FAR struct usr_wqueue_s *wqueue,
/* Initialize the work structure */
- work->worker = worker; /* Work callback. non-NULL means queued */
- work->arg = arg; /* Callback argument */
- work->u.s.qtime = clock() + delay; /* Delay until work performed */
+ work->worker = worker; /* Work callback. non-NULL means queued */
+ work->arg = arg; /* Callback argument */
+ work->qtime = clock() + delay; /* Delay until work performed */
/* Do the easy case first -- when the work queue is empty. */
@@ -97,7 +97,7 @@ static int work_qqueue(FAR struct usr_wqueue_s *wqueue,
{
/* Add the watchdog to the head == tail of the queue. */
- dq_addfirst(&work->u.s.dq, &wqueue->q);
+ dq_addfirst(&work->dq, &wqueue->q);
nxsem_post(&wqueue->wake);
}
@@ -111,7 +111,7 @@ static int work_qqueue(FAR struct usr_wqueue_s *wqueue,
do
{
- delta = work->u.s.qtime - ((FAR struct work_s *)curr)->u.s.qtime;
+ delta = work->qtime - ((FAR struct work_s *)curr)->qtime;
if (delta < 0)
{
break;
@@ -128,7 +128,7 @@ static int work_qqueue(FAR struct usr_wqueue_s *wqueue,
{
/* Insert the watchdog at the head of the list */
- dq_addfirst(&work->u.s.dq, &wqueue->q);
+ dq_addfirst(&work->dq, &wqueue->q);
nxsem_get_value(&wqueue->wake, &semcount);
if (semcount < 1)
{
@@ -139,7 +139,7 @@ static int work_qqueue(FAR struct usr_wqueue_s *wqueue,
{
/* Insert the watchdog in mid- or end-of-queue */
- dq_addafter(prev, &work->u.s.dq, &wqueue->q);
+ dq_addafter(prev, &work->dq, &wqueue->q);
}
}
diff --git a/libs/libc/wqueue/work_usrthread.c
b/libs/libc/wqueue/work_usrthread.c
index 8e4c679217..791cfd3e3e 100644
--- a/libs/libc/wqueue/work_usrthread.c
+++ b/libs/libc/wqueue/work_usrthread.c
@@ -125,7 +125,7 @@ static void work_process(FAR struct usr_wqueue_s *wqueue)
* zero will always execute immediately.
*/
- elapsed = clock() - work->u.s.qtime;
+ elapsed = clock() - work->qtime;
/* Is this delay work ready? */
@@ -180,7 +180,7 @@ static void work_process(FAR struct usr_wqueue_s *wqueue)
}
else
{
- next = work->u.s.qtime - clock();
+ next = work->qtime - clock();
break;
}
}
