Skip to content

Commit de35994

Browse files
Tvrtko Ursulinhtejun
authored andcommitted
workqueue: Do not warn when cancelling WQ_MEM_RECLAIM work from !WQ_MEM_RECLAIM worker
After commit 746ae46 ("drm/sched: Mark scheduler work queues with WQ_MEM_RECLAIM") amdgpu started seeing the following warning: [ ] workqueue: WQ_MEM_RECLAIM sdma0:drm_sched_run_job_work [gpu_sched] is flushing !WQ_MEM_RECLAIM events:amdgpu_device_delay_enable_gfx_off [amdgpu] ... [ ] Workqueue: sdma0 drm_sched_run_job_work [gpu_sched] ... [ ] Call Trace: [ ] <TASK> ... [ ] ? check_flush_dependency+0xf5/0x110 ... [ ] cancel_delayed_work_sync+0x6e/0x80 [ ] amdgpu_gfx_off_ctrl+0xab/0x140 [amdgpu] [ ] amdgpu_ring_alloc+0x40/0x50 [amdgpu] [ ] amdgpu_ib_schedule+0xf4/0x810 [amdgpu] [ ] ? drm_sched_run_job_work+0x22c/0x430 [gpu_sched] [ ] amdgpu_job_run+0xaa/0x1f0 [amdgpu] [ ] drm_sched_run_job_work+0x257/0x430 [gpu_sched] [ ] process_one_work+0x217/0x720 ... [ ] </TASK> The intent of the verifcation done in check_flush_depedency is to ensure forward progress during memory reclaim, by flagging cases when either a memory reclaim process, or a memory reclaim work item is flushed from a context not marked as memory reclaim safe. This is correct when flushing, but when called from the cancel(_delayed)_work_sync() paths it is a false positive because work is either already running, or will not be running at all. Therefore cancelling it is safe and we can relax the warning criteria by letting the helper know of the calling context. Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@igalia.com> Fixes: fca839c ("workqueue: warn if memory reclaim tries to flush !WQ_MEM_RECLAIM workqueue") References: 746ae46 ("drm/sched: Mark scheduler work queues with WQ_MEM_RECLAIM") Cc: Tejun Heo <tj@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Lai Jiangshan <jiangshanlai@gmail.com> Cc: Alex Deucher <alexander.deucher@amd.com> Cc: Christian König <christian.koenig@amd.com Cc: Matthew Brost <matthew.brost@intel.com> Cc: <stable@vger.kernel.org> # v4.5+ Signed-off-by: Tejun Heo <tj@kernel.org>
1 parent b03917e commit de35994

File tree

1 file changed

+13
-9
lines changed

1 file changed

+13
-9
lines changed

kernel/workqueue.c

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3680,23 +3680,27 @@ void workqueue_softirq_dead(unsigned int cpu)
36803680
* check_flush_dependency - check for flush dependency sanity
36813681
* @target_wq: workqueue being flushed
36823682
* @target_work: work item being flushed (NULL for workqueue flushes)
3683+
* @from_cancel: are we called from the work cancel path
36833684
*
36843685
* %current is trying to flush the whole @target_wq or @target_work on it.
3685-
* If @target_wq doesn't have %WQ_MEM_RECLAIM, verify that %current is not
3686-
* reclaiming memory or running on a workqueue which doesn't have
3687-
* %WQ_MEM_RECLAIM as that can break forward-progress guarantee leading to
3688-
* a deadlock.
3686+
* If this is not the cancel path (which implies work being flushed is either
3687+
* already running, or will not be at all), check if @target_wq doesn't have
3688+
* %WQ_MEM_RECLAIM and verify that %current is not reclaiming memory or running
3689+
* on a workqueue which doesn't have %WQ_MEM_RECLAIM as that can break forward-
3690+
* progress guarantee leading to a deadlock.
36893691
*/
36903692
static void check_flush_dependency(struct workqueue_struct *target_wq,
3691-
struct work_struct *target_work)
3693+
struct work_struct *target_work,
3694+
bool from_cancel)
36923695
{
3693-
work_func_t target_func = target_work ? target_work->func : NULL;
3696+
work_func_t target_func;
36943697
struct worker *worker;
36953698

3696-
if (target_wq->flags & WQ_MEM_RECLAIM)
3699+
if (from_cancel || target_wq->flags & WQ_MEM_RECLAIM)
36973700
return;
36983701

36993702
worker = current_wq_worker();
3703+
target_func = target_work ? target_work->func : NULL;
37003704

37013705
WARN_ONCE(current->flags & PF_MEMALLOC,
37023706
"workqueue: PF_MEMALLOC task %d(%s) is flushing !WQ_MEM_RECLAIM %s:%ps",
@@ -3980,7 +3984,7 @@ void __flush_workqueue(struct workqueue_struct *wq)
39803984
list_add_tail(&this_flusher.list, &wq->flusher_overflow);
39813985
}
39823986

3983-
check_flush_dependency(wq, NULL);
3987+
check_flush_dependency(wq, NULL, false);
39843988

39853989
mutex_unlock(&wq->mutex);
39863990

@@ -4155,7 +4159,7 @@ static bool start_flush_work(struct work_struct *work, struct wq_barrier *barr,
41554159
}
41564160

41574161
wq = pwq->wq;
4158-
check_flush_dependency(wq, work);
4162+
check_flush_dependency(wq, work, from_cancel);
41594163

41604164
insert_wq_barrier(pwq, barr, work, worker);
41614165
raw_spin_unlock_irq(&pool->lock);

0 commit comments

Comments
 (0)