From 011c686e3dc4fd63f3fd506817dda0414066957c Mon Sep 17 00:00:00 2001 From: Ryan Fahy Date: Sun, 7 Jun 2026 20:32:28 -0400 Subject: [PATCH] linuxkpi: Fix cancel_delayed_work_sync() return value Align behavior between the Linux cancel_delayed_work_sync() function return value and the LinuxKPI equivalent. Linux cancel_delayed_work_sync() returns whether delayed work was pending, even if canceled before executing. This includes the case where the timer fired and work was queued but the callback had not yet started. The LinuxKPI version used the return value from taskqueue_cancel() as the return value of the public facing API, which inverted the behavior of two cases, violating the Linux API contract. Queued work which was removed before running would return false, and work whose callback was already executing would return true. Track the taskqueue pending count separately from the taskqueue_cancel() return value. Use the pending count for the public return value. Use taskqueue_cancel() return value only to decide whether state needs to be re-checked. This fixes behavior in consumers which use the return value of cancel_delayed_work_sync() to distinguish canceled pending work from work that was already running or idle. Signed-off-by: Ryan Fahy --- sys/compat/linuxkpi/common/src/linux_work.c | 42 +++++++++++++++------ 1 file changed, 30 insertions(+), 12 deletions(-) diff --git a/sys/compat/linuxkpi/common/src/linux_work.c b/sys/compat/linuxkpi/common/src/linux_work.c index b1975d16025e23..2bf2179f0e1358 100644 --- a/sys/compat/linuxkpi/common/src/linux_work.c +++ b/sys/compat/linuxkpi/common/src/linux_work.c @@ -506,12 +506,16 @@ linux_cancel_delayed_work(struct delayed_work *dwork) } /* - * This function cancels the given work structure in a synchronous - * fashion. It returns true if the work was successfully - * cancelled. Else the work was already cancelled. + * This function cancels the given delayed work structure in a + * synchronous fashion. It returns true if pending delayed work was + * cancelled. Else the work was not pending. + * + * If the work restarted itself or was busy while being cancelled, + * retry_needed is set to true so the caller can re-check the state. */ static bool -linux_cancel_delayed_work_sync_int(struct delayed_work *dwork) +linux_cancel_delayed_work_sync_int(struct delayed_work *dwork, + bool *retry_needed) { static const uint8_t states[WORK_ST_MAX] __aligned(8) = { [WORK_ST_IDLE] = WORK_ST_IDLE, /* NOP */ @@ -523,6 +527,9 @@ linux_cancel_delayed_work_sync_int(struct delayed_work *dwork) struct taskqueue *tq; int ret, state; bool cancelled; + u_int pending = 0; + + *retry_needed = false; WITNESS_WARN(WARN_GIANTOK | WARN_SLEEPOK, NULL, "linux_cancel_delayed_work_sync() might sleep"); @@ -538,19 +545,23 @@ linux_cancel_delayed_work_sync_int(struct delayed_work *dwork) cancelled = (callout_stop(&dwork->timer.callout) == 1); tq = dwork->work.work_queue->taskqueue; - ret = taskqueue_cancel(tq, &dwork->work.work_task, NULL); + ret = taskqueue_cancel(tq, &dwork->work.work_task, &pending); mtx_unlock(&dwork->timer.mtx); callout_drain(&dwork->timer.callout); taskqueue_drain(tq, &dwork->work.work_task); - return (cancelled || (ret != 0)); + if (ret != 0) + *retry_needed = true; + return (cancelled || pending != 0); default: tq = dwork->work.work_queue->taskqueue; - ret = taskqueue_cancel(tq, &dwork->work.work_task, NULL); + ret = taskqueue_cancel(tq, &dwork->work.work_task, &pending); mtx_unlock(&dwork->timer.mtx); - if (ret != 0) + if (ret != 0) { taskqueue_drain(tq, &dwork->work.work_task); - return (ret != 0); + *retry_needed = true; + } + return (pending != 0); } } @@ -558,10 +569,17 @@ bool linux_cancel_delayed_work_sync(struct delayed_work *dwork) { bool res; + bool ret; + bool retry_needed = false; + + ret = linux_cancel_delayed_work_sync_int(dwork, &retry_needed); + res = ret; + + while (ret || retry_needed) { + ret = linux_cancel_delayed_work_sync_int(dwork, &retry_needed); + res = res || ret; + } - res = false; - while (linux_cancel_delayed_work_sync_int(dwork)) - res = true; return (res); }