Messages in this thread | | | From | Lai Jiangshan <> | Date | Thu, 2 Apr 2020 08:07:35 +0800 | Subject | Re: [PATCH] workqueue: Don't double assign worker->sleeping |
| |
On Wed, Apr 1, 2020 at 9:03 PM Sebastian Andrzej Siewior <bigeasy@linutronix.de> wrote: > > On 2020-04-01 11:44:06 [+0800], Lai Jiangshan wrote: > > On Wed, Apr 1, 2020 at 11:22 AM Lai Jiangshan <jiangshanlai@gmail.com> wrote: > > > > > > Hello > Hi Lai, > > … > > > 2) wq_worker_running() can be interrupted(async-page-faulted in virtual machine) > > > and nr_running would be decreased twice. > > > > would be *increased* twice > > > > I just saw the V2 patch, this issue is not listed, but need to be fixed too. > > | void wq_worker_running(struct task_struct *task) > | { > | struct worker *worker = kthread_data(task); > | > | if (!worker->sleeping) > | return; > | if (!(worker->flags & WORKER_NOT_RUNNING)) > | atomic_inc(&worker->pool->nr_running); > *0 > | worker->sleeping = 0; > *1 > | } > > So an interrupt > - before *0, the preempting caller drop early in wq_worker_sleeping(), only one > atomic_inc()
If it is preempted on *0, the preempting caller drop early in wq_worker_sleeping() so there is no atomic decreasing, only one atomic_inc() in the preempting caller. The preempted point here, wq_worker_running(), has already just done atomic_inc(), the total number of atomic_inc() is two, while the number of atomic decreasing is one.
> > - after *1, the preempting task will invoke wq_worker_sleeping() and do > dec() + inc(). > > What did I miss here? > > Sebastian
| |