Page MenuHomeFreeBSD

LinuxKPI: account for delayed work cancelled after taskqueue dequeue
Needs ReviewPublic

Authored by oleglelchuk_gmail.com on Sat, Sep 26, 2:35 PM.
Tags
None
Referenced Files
F173591873: D60046.diff
Sun, Sep 27, 1:22 AM
F173581744: D60046.id187762.diff
Sat, Sep 26, 11:48 PM
F173581726: D60046.id187746.diff
Sat, Sep 26, 11:48 PM
F173576040: D60046.id187746.diff
Sat, Sep 26, 10:57 PM
F173575373: D60046.id187762.diff
Sat, Sep 26, 10:51 PM
F173575285: D60046.diff
Sat, Sep 26, 10:50 PM
F173573190: D60046.id187762.diff
Sat, Sep 26, 10:32 PM
F173572350: D60046.id187746.diff
Sat, Sep 26, 10:25 PM

Details

Reviewers
wulf
Group Reviewers
linuxkpi
Summary

I encounter intermittent shutdown/restart hangs with i915/GuC and the normal 34 ms scheduling-disable delay. Setting that delay to zero has avoided the hang in my testing. This review fixes a reproducible LinuxKPI cancellation-accounting defect that can leave the corresponding context reference unreleased, rather than changing the driver's delay.

Commit 79e290d967862bef1adcf39f0bfcf1b4993a8202 corrected the ordinary queued and already-executing return values of cancel_delayed_work_sync(). There is still a gap between native taskqueue dequeue and LinuxKPI callback ownership:

  1. In sys/kern/subr_taskqueue.c, taskqueue_run_locked() removes the task, clears ta_pending and sets tb_running before calling its handler.
  2. Before linux_work_fn() claims the callback, linux_cancel_delayed_work_sync_int() atomically changes WORK_ST_TASK to WORK_ST_IDLE.
  3. taskqueue_cancel() can now return EBUSY with pending == 0: the native task is busy, but the Linux callback has not started.
  4. linux_work_fn() subsequently sees IDLE and skips the callback. The cancelling thread drains the task and retries, but returns false because neither the timer-stop result nor the native pending count records what it cancelled.

The task can also finish consuming IDLE before taskqueue_cancel() is called, giving success with pending == 0 and the same incorrect false return. This affects zero-delay WORK_ST_TIMER and WORK_ST_CANCEL left by a failed nonblocking cancellation as well. A zero-delay self-requeue handled by linux_work_fn()'s executor loop can similarly be pending without a native queue entry.

Use the old state returned by the atomic transition to account for pending work suppressed by this cancellation: TIMER, TASK and CANCEL contribute a true result. EXEC does not: the callback has already been claimed, or has finished with that sticky state. Keep the native pending-count aggregation, drain operations and retry decisions unchanged; use a separate local variable for the callout-stop result. In particular, EBUSY alone is not treated as successful cancellation, which would regress the already-running-callback case fixed by 79e290d96786. Correct the helper's comment to distinguish its retry return value from its cancellation output.

One affected consumer is guc_request_alloc() in drm-kmod's drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c. It calls intel_context_sched_disable_unpin() when cancel_delayed_work_sync() returns true. Suppressing the delayed callback while returning false can leave neither path responsible for releasing that reference, retaining the engine wakeref that shutdown later waits for.

My failed-shutdown diagnostic capture showed a GT wakeref count of 1, an RCS0 wakeref count of 2, no pending GuC replies and a shutdown wakeref wait lasting at least 51 seconds. This is consistent with the mechanism above, but the diagnostic history dropped some observations and did not capture the decisive atomic transition. The capture came from a kernel based on b9811d13572b with unrelated local changes; linux_work.c and subr_taskqueue.c were unmodified. The controlled tests establish the source defect independently; they do not prove that every hardware hang has this cause.

The patch is based on vanilla main f492ef8318f580081047da41905c3b339e924387, verified as the current tip on September 26. It changes only linux_work.c, with no drm-kmod, GuC-delay, PFN, Wi-Fi, Bluetooth or diagnostic changes. An LLM prepared the patch and the source-extraction test harness. I have now booted a kernel containing this fix on the affected laptop, alongside my unrelated local kernel changes, with vanilla drm-kmod and the normal 34 ms GuC delay. This confirms basic integration; repeated shutdown/restart trials and an in-kernel race test are still required.

Related earlier fix: https://cgit.freebsd.org/src/commit/?id=79e290d967862bef1adcf39f0bfcf1b4993a8202
Related discussion: https://github.com/freebsd/freebsd-src/pull/2268
Driver call site: https://github.com/freebsd/drm-kmod/blob/f252a30f27d157d9c763cd408850775096a6263f/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c#L3895

Full-context patch file:


Source-extraction test sources, manifests and results:

Test Plan

Completed on FreeBSD amd64:

  • Verified clean application to vanilla main f492ef8318f580081047da41905c3b339e924387 using a separate Git index, compared the resulting blob with the tested source, and checked reversal back to the exact base tree. git diff --check passes. Only sys/compat/linuxkpi/common/src/linux_work.c changes.
  • Built the complete sys/modules/linuxkpi module against that source with base Clang 21.1.8, the normal -Werror flags and six build jobs. The build completed without warnings or errors. That standalone module build was not installed or loaded.
  • Ran a user-space harness which extracts the actual LinuxKPI and native taskqueue function bodies from the source, retaining their original license notices. Pthreads/C11 atomics substitute for kernel primitives; callout expiry is driven explicitly by the test. Gates outside the extracted functions force the native-dequeued/callback-not-claimed interleaving.
  • The unpatched main source fails seven deterministic checks: TIMER/TASK/CANCEL dispatched with EBUSY and zero pending count, the same three states consumed before native cancellation observes the task, and a pending zero-delay self-requeue handled by the executor loop. In each failing cancellation case, the relevant callback is suppressed but cancellation returns false.
  • The patched source passes all 24 result/callback checks per iteration. Controls cover idle and completed work, armed timers, native queued work, repeated cancellation, draining an already-running callback without reporting it cancelled, delayed self-requeue before cancellation and self-requeue during the drain. An unforced worker/canceller race checks that exactly one side owns completion. Extra assertions check queue/executor cleanup and that cancellation has not returned while a running callback is blocked.
  • 1,000 complete iterations passed: 24,000 result/callback checks, zero failures. AddressSanitizer plus UndefinedBehaviorSanitizer passed 100 iterations: 2,400 checks, zero failures. LeakSanitizer was disabled because it is unsupported by the FreeBSD runtime used here.

The attached test sources include a runner, extraction manifests and results so reviewers can reproduce these checks. This is source-extraction testing, not an in-kernel scheduler/callout test; the sanitizer result applies to the harness, not to a running kernel. It does not cover arbitrary external producer/canceller concurrency, object destruction, other architectures or firmware behavior.

Still required before claiming the laptop's intermittent hang is eliminated:

  • Review the cancellation ownership argument and the unchanged drain/retry paths.
  • Repeat ordinary graphics use plus shutdown/restart trials on the corrected kernel with the normal 34 ms GuC delay.
  • Exercise the relevant races with real kernel workqueues/callouts, including running callbacks and self-requeue, under appropriate kernel diagnostics.

Update, September 26: I built and successfully booted a complete kernel based on f492ef8318f580081047da41905c3b339e924387 containing this exact fix alongside my unrelated local kernel changes. The loaded Intel DRM modules and firmware were built from unmodified upstream drm-kmod f252a30f27d157d9c763cd408850775096a6263f and drm-kmod-firmware 2c12915d69954673c782a9a55c6ec39c19cfea93. The loaded module hashes were verified, and the GuC scheduling-disable delay remains 34 ms. This is basic boot/integration validation, not a controlled in-kernel race test or a post-fix shutdown comparison. Those tests remain outstanding.

Reproducible test bundle:

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Passed

Event Timeline

oleglelchuk_gmail.com created this object with visibility "Public (No Login Required)".
oleglelchuk_gmail.com changed the edit policy from "All Users" to "Administrators".Sat, Sep 26, 3:44 PM

Can you please upload this with either arc or a patch from git diff -U9999 so context is available.

Also "Edit Policy" to Administrators only is not really helpful.

This definitively needs review from other people but we cannot adjust reviewers anymore.

oleglelchuk_gmail.com changed the edit policy from "Administrators" to "All Users".
oleglelchuk_gmail.com edited the summary of this revision. (Show Details)
oleglelchuk_gmail.com edited the test plan for this revision. (Show Details)

I have re-uploaded the same fix using git diff --full-index -U9999, so the complete linux_work.c context is available. I also changed the edit policy from Administrators to All Users (logged-in users), so other developers can adjust the reviewers.

The code is unchanged: applying this diff produces exactly the same source blob as the previously tested patch. I updated the test status to record a successful boot with this fix, my other local kernel changes, vanilla drm-kmod and the normal 34 ms GuC delay. Repeated shutdown/restart trials and controlled in-kernel race testing remain outstanding.