Skip to content

Commit 62849a9

Browse files
Sebastian Andrzej SiewiorIngo Molnar
authored andcommitted
workqueue: Remove the warning in wq_worker_sleeping()
The kernel test robot triggered a warning with the following race: task-ctx A interrupt-ctx B worker -> process_one_work() -> work_item() -> schedule(); -> sched_submit_work() -> wq_worker_sleeping() -> ->sleeping = 1 atomic_dec_and_test(nr_running) __schedule(); *interrupt* async_page_fault() -> local_irq_enable(); -> schedule(); -> sched_submit_work() -> wq_worker_sleeping() -> if (WARN_ON(->sleeping)) return -> __schedule() -> sched_update_worker() -> wq_worker_running() -> atomic_inc(nr_running); -> ->sleeping = 0; -> sched_update_worker() -> wq_worker_running() if (!->sleeping) return In this context the warning is pointless everything is fine. An interrupt before wq_worker_sleeping() will perform the ->sleeping assignment (0 -> 1 > 0) twice. An interrupt after wq_worker_sleeping() will trigger the warning and nr_running will be decremented (by A) and incremented once (only by B, A will skip it). This is the case until the ->sleeping is zeroed again in wq_worker_running(). Remove the WARN statement because this condition may happen. Document that preemption around wq_worker_sleeping() needs to be disabled to protect ->sleeping and not just as an optimisation. Fixes: 6d25be5 ("sched/core, workqueues: Distangle worker accounting from rq lock") Reported-by: kernel test robot <[email protected]> Signed-off-by: Sebastian Andrzej Siewior <[email protected]> Signed-off-by: Peter Zijlstra (Intel) <[email protected]> Signed-off-by: Ingo Molnar <[email protected]> Cc: Tejun Heo <[email protected]> Link: https://lkml.kernel.org/r/20200327074308.GY11705@shao2-debian
1 parent 111688c commit 62849a9

File tree

2 files changed

+6
-3
lines changed

2 files changed

+6
-3
lines changed

kernel/sched/core.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4120,7 +4120,8 @@ static inline void sched_submit_work(struct task_struct *tsk)
41204120
* it wants to wake up a task to maintain concurrency.
41214121
* As this function is called inside the schedule() context,
41224122
* we disable preemption to avoid it calling schedule() again
4123-
* in the possible wakeup of a kworker.
4123+
* in the possible wakeup of a kworker and because wq_worker_sleeping()
4124+
* requires it.
41244125
*/
41254126
if (tsk->flags & (PF_WQ_WORKER | PF_IO_WORKER)) {
41264127
preempt_disable();

kernel/workqueue.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -858,7 +858,8 @@ void wq_worker_running(struct task_struct *task)
858858
* @task: task going to sleep
859859
*
860860
* This function is called from schedule() when a busy worker is
861-
* going to sleep.
861+
* going to sleep. Preemption needs to be disabled to protect ->sleeping
862+
* assignment.
862863
*/
863864
void wq_worker_sleeping(struct task_struct *task)
864865
{
@@ -875,7 +876,8 @@ void wq_worker_sleeping(struct task_struct *task)
875876

876877
pool = worker->pool;
877878

878-
if (WARN_ON_ONCE(worker->sleeping))
879+
/* Return if preempted before wq_worker_running() was reached */
880+
if (worker->sleeping)
879881
return;
880882

881883
worker->sleeping = 1;

0 commit comments

Comments
 (0)