History

workqueue: Release PENDING in __queue_work() drain/destroy reject path

The caller of __queue_work() owns WORK_STRUCT_PENDING, won via test_and_set_bit() in queue_work_on()/__queue_delayed_work(). The state machine documented above __queue_work() requires that owner to either hand the token to a pwq (insert_work() -> set_work_pwq()), hand it to a timer, or release it via set_work_pool_and_clear_pending(). try_to_grab_pending() relies on this: when it observes "PENDING && off-queue" it busy-loops, trusting the current owner to make progress. The (__WQ_DESTROYING | __WQ_DRAINING) early-return path violates that contract. It WARN_ONCE()s and bare-returns, leaving work->data with PENDING set, WORK_STRUCT_PWQ clear, and work->entry empty. The path is reachable without explicit API abuse: queue_delayed_work() arms a timer with PENDING set; if drain_workqueue() runs while the timer is still pending, delayed_work_timer_fn() -> __queue_work() in softirq context hits the WARN, current is not a wq worker so is_chained_work() is false, and the work is silently dropped with PENDING leaked. Mirror what clear_pending_if_disabled() already does on its analogous reject path: unpack the off-queue data and call set_work_pool_and_clear_pending() to release the token before returning. I was able to reproduce this by queueing several slow works on a max_active=1 wq, arm a delayed_work whose timer fires while drain_workqueue() is blocked, then call cancel_delayed_work_sync(). Without this patch the cancel livelocks at 100% CPU; with it the cancel returns immediately.

Breno Leitao committed 6 months ago

Commit a7488f01 parent 20e81c61 file · +12 −0Browse files

Showing the changes to kernel/workqueue.c (1 of 1 file). Show all

Unified · Split

kernel/workqueue.c+12−0
@@ -2296,6 +2296,18 @@ static void __queue_work(int cpu, struct workqueue_struct *wq, · 2295 unmodified lines
22962296 if (unlikely(wq->flags & (__WQ_DESTROYING | __WQ_DRAINING) &&
22972297 WARN_ONCE(!is_chained_work(wq), "workqueue: cannot queue %ps on wq %s\n",
22982298 work->func, wq->name))) {
2299+ struct work_offq_data offqd;
2300+
2301+ /*
2302+ * State on entry: PENDING is set, work is off-queue (no
2303+ * insert_work() has run).
2304+ *
2305+ * Returning without clearing PENDING would leave the work
2306+ * in a weird state (PENDING=1, PWQ=0, entry empty)
2307+ */
2308+ work_offqd_unpack(&offqd, *work_data_bits(work));
2309+ set_work_pool_and_clear_pending(work, offqd.pool_id,
2310+ work_offqd_pack_flags(&offqd));
22992311 return;
23002312 }
23012313 rcu_read_lock();