p71924506
/
linux_old1
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

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: 6d25be5782 ("sched/core, workqueues: Distangle worker accounting from rq lock")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Cc: Tejun Heo <tj@kernel.org>
Link: https://lkml.kernel.org/r/20200327074308.GY11705@shao2-debian
This commit is contained in:
Sebastian Andrzej Siewior 2020-03-28 00:29:59 +01:00 committed by Ingo Molnar
parent 111688ca1c
commit 62849a9612
2 changed files with 6 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
kernel/sched/core.c
View File

@ -4120,7 +4120,8 @@ static inline void sched_submit_work(struct task_struct *tsk)
* it wants to wake up a task to maintain concurrency. * it wants to wake up a task to maintain concurrency.
* As this function is called inside the schedule() context, * As this function is called inside the schedule() context,
* we disable preemption to avoid it calling schedule() again * we disable preemption to avoid it calling schedule() again
* in the possible wakeup of a kworker. * in the possible wakeup of a kworker and because wq_worker_sleeping()
* requires it.
*/ */
if (tsk->flags & (PF_WQ_WORKER | PF_IO_WORKER)) { if (tsk->flags & (PF_WQ_WORKER | PF_IO_WORKER)) {
preempt_disable(); preempt_disable();

6
kernel/workqueue.c
View File

@ -858,7 +858,8 @@ void wq_worker_running(struct task_struct *task)
* @task: task going to sleep * @task: task going to sleep
* *
* This function is called from schedule() when a busy worker is * This function is called from schedule() when a busy worker is
* going to sleep. * going to sleep. Preemption needs to be disabled to protect ->sleeping
* assignment.
*/ */
void wq_worker_sleeping(struct task_struct *task) void wq_worker_sleeping(struct task_struct *task)
{ {
@ -875,7 +876,8 @@ void wq_worker_sleeping(struct task_struct *task)
pool = worker->pool; pool = worker->pool;
if (WARN_ON_ONCE(worker->sleeping)) /* Return if preempted before wq_worker_running() was reached */
if (worker->sleeping)
return; return;
worker->sleeping = 1; worker->sleeping = 1;