This is an automated email from the ASF dual-hosted git repository. archer pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/nuttx.git
commit f442a41102deaabbd06d78ea4c9736f33635678f Author: ouyangxiangzhen <ouyangxiangz...@xiaomi.com> AuthorDate: Thu May 8 11:17:46 2025 +0800 sched/wqueue: Fix work_cancel_sync. This commit fixed work_cancel_sync at a very rare boundary case. When a worker thread re-enqueues the work data structure during the execution of work, the user thread cannot directly dequeue the work in work_cancel_sync. Instead, it should wait until all workers' references to the work data structure have been eliminated after dequeuing. Signed-off-by: ouyangxiangzhen <ouyangxiangz...@xiaomi.com> --- sched/wqueue/kwork_cancel.c | 53 +++++++++++++++--------------------- sched/wqueue/wqueue.h | 66 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 31 deletions(-) diff --git a/sched/wqueue/kwork_cancel.c b/sched/wqueue/kwork_cancel.c index 369730d6a0..4a2019ce53 100644 --- a/sched/wqueue/kwork_cancel.c +++ b/sched/wqueue/kwork_cancel.c @@ -46,7 +46,7 @@ static int work_qcancel(FAR struct kwork_wqueue_s *wqueue, bool sync, FAR struct work_s *work) { irqstate_t flags; - int ret = OK; + FAR sem_t *sync_wait = NULL; if (wqueue == NULL || work == NULL) { @@ -60,56 +60,49 @@ static int work_qcancel(FAR struct kwork_wqueue_s *wqueue, bool sync, flags = spin_lock_irqsave(&wqueue->lock); - /* Check whether we own the work structure. */ - if (!work_available(work)) { - bool is_head = list_is_head(&wqueue->pending, &work->node); - - /* Seize the ownership from the work thread. */ - - work->worker = NULL; - - list_delete(&work->node); - /* If the head of the pending queue has changed, we should reset * the wqueue timer. */ - if (is_head) + if (work_remove(wqueue, work)) { - if (!list_is_empty(&wqueue->pending)) - { - work = list_first_entry(&wqueue->pending, struct work_s, node); - - ret = wd_start_abstick(&wqueue->timer, work->qtime, - work_timer_expired, (wdparm_t)wqueue); - } - else - { - wd_cancel(&wqueue->timer); - } + work_timer_reset(wqueue); } } - else if (!up_interrupt_context() && !sched_idletask() && sync) + + /* Note that cancel_sync can not be called in the interrupt + * context and the idletask context. + */ + + if (sync) { int wndx; + pid_t pid = nxsched_gettid(); + + /* Wait until the worker thread finished the work. */ for (wndx = 0; wndx < wqueue->nthreads; wndx++) { if (wqueue->worker[wndx].work == work && - wqueue->worker[wndx].pid != nxsched_gettid()) + wqueue->worker[wndx].pid != pid) { wqueue->worker[wndx].wait_count++; - spin_unlock_irqrestore(&wqueue->lock, flags); - nxsem_wait_uninterruptible(&wqueue->worker[wndx].wait); - return 1; + sync_wait = &wqueue->worker[wndx].wait; + break; } } } spin_unlock_irqrestore(&wqueue->lock, flags); - return ret; + + if (sync_wait) + { + nxsem_wait_uninterruptible(sync_wait); + } + + return 0; } /**************************************************************************** @@ -163,8 +156,6 @@ int work_cancel_wq(FAR struct kwork_wqueue_s *wqueue, * * Returned Value: * Zero means the work was successfully cancelled. - * One means the work was not cancelled because it is currently being - * processed by work thread, but wait for it to finish. * A negated errno value is returned on any failure: * * -ENOENT - There is no such work queued. diff --git a/sched/wqueue/wqueue.h b/sched/wqueue/wqueue.h index c6ecb95883..57177ad98d 100644 --- a/sched/wqueue/wqueue.h +++ b/sched/wqueue/wqueue.h @@ -203,6 +203,39 @@ bool work_insert_pending(FAR struct kwork_wqueue_s *wqueue, return curr == head; } +/**************************************************************************** + * Name: work_remove + * + * Description: + * Internal public function to remove the work from the workqueue. + * Require wqueue != NULL and work != NULL. + * + * Input Parameters: + * wqueue - The work queue. + * work - The work to be inserted. + * + * Returned Value: + * Return whether the head of the pending queue has changed. + * + ****************************************************************************/ + +static inline_function +bool work_remove(FAR struct kwork_wqueue_s *wqueue, + FAR struct work_s *work) +{ + FAR struct work_s *head; + + head = list_first_entry(&wqueue->pending, struct work_s, node); + + /* Seize the ownership from the work thread. */ + + work->worker = NULL; + + list_delete(&work->node); + + return head == work; +} + /**************************************************************************** * Name: work_timer_expired * @@ -216,6 +249,39 @@ bool work_insert_pending(FAR struct kwork_wqueue_s *wqueue, void work_timer_expired(wdparm_t arg); +/**************************************************************************** + * Name: work_timer_reset + * + * Description: + * Internal public function to reset the timer of the workqueue. + * Require wqueue != NULL. + * + * Input Parameters: + * wqueue - The work queue. + * + * Returned Value: + * None. + * + ****************************************************************************/ + +static inline_function +void work_timer_reset(FAR struct kwork_wqueue_s *wqueue) +{ + if (!list_is_empty(&wqueue->pending)) + { + FAR struct work_s *work; + + work = list_first_entry(&wqueue->pending, struct work_s, node); + + wd_start_abstick(&wqueue->timer, work->qtime, + work_timer_expired, (wdparm_t)wqueue); + } + else + { + wd_cancel(&wqueue->timer); + } +} + /**************************************************************************** * Name: work_start_highpri *