Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <CA+G9fYud2YH1c9Hxekd-pi8VsD4HmA4v9g-mr-vRPqaRQd2F5w@mail.gmail.com>
 <CALm+0cXew-QX9gkHkL7majJuENP1C_WLVG=pQ_6RGZ1UgQ79Eg@mail.gmail.com>
 <ZGuydp0KToLf55vY@slm.duckdns.org> <ZGvap5qi0OKAOfqX@slm.duckdns.org>
In-Reply-To: <ZGvap5qi0OKAOfqX@slm.duckdns.org>
From:   Naresh Kamboju <naresh.kamboju@linaro.org>
Date:   Tue, 23 May 2023 14:55:27 +0530
Message-ID: <CA+G9fYs8Oa8sS97H8LdAOv=k9tJVuikekbqh_E57vEUVcvk_4A@mail.gmail.com>
Subject: Re: next: WARNING: CPU: 0 PID: 63 at kernel/workqueue.c:1999 worker_enter_idle+0xb2/0xc0
To:     Tejun Heo <tj@kernel.org>, Anders Roxell <anders.roxell@linaro.org>
Cc:     Z qiang <qiang.zhang1211@gmail.com>,
        open list <linux-kernel@vger.kernel.org>,
        Linux-Next Mailing List <linux-next@vger.kernel.org>,
        lkft-triage@lists.linaro.org,
        clang-built-linux <llvm@lists.linux.dev>,
        Nathan Chancellor <nathan@kernel.org>,
        Arnd Bergmann <arnd@arndb.de>,
        Dan Carpenter <dan.carpenter@linaro.org>,
        Nick Desaulniers <ndesaulniers@google.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Tue, 23 May 2023 at 02:42, Tejun Heo <tj@kernel.org> wrote:
>
> On Mon, May 22, 2023 at 08:20:38AM -1000, Tejun Heo wrote:
> > Hello,
> >
> > On Mon, May 22, 2023 at 09:24:09PM +0800, Z qiang wrote:
> > > diff --git a/kernel/workqueue.c b/kernel/workqueue.c
> > > index 9c5c1cfa478f..f8d739fef311 100644
> > > --- a/kernel/workqueue.c
> > > +++ b/kernel/workqueue.c
> > > @@ -1060,10 +1060,9 @@ void wq_worker_running(struct task_struct *task)
> > >          * and leave with an unexpected pool->nr_running == 1 on the newly
> > > unbound
> > >          * pool. Protect against such race.
> > >          */
> > > -       preempt_disable();
> > > +       local_irq_disable();
> > >         if (!(worker->flags & WORKER_NOT_RUNNING))
> > >                 worker->pool->nr_running++;
> > > -       preempt_enable();
> > >
> > >         /*
> > >          * CPU intensive auto-detection cares about how long a work item
> > > hogged
> > > @@ -1072,6 +1071,7 @@ void wq_worker_running(struct task_struct *task)
> > >         worker->current_at = worker->task->se.sum_exec_runtime;
> > >
> > >         worker->sleeping = 0;
> > > +       local_irq_enable();
> > >  }
> >
> > Ah, yeah, this is correct. Now we're modifying nr_running from timer tick
> > too, so if don't block irq, the timer tick can ruin the not-irq-protected
> > read-write-modify nr_running update from wq_worker_running(). Naresh, can
> > you please confirm the fix?
>
> Z qiang, while waiting for Naresh's test result, can you send the fix as a
> proper signed-off-patch?


The proposed patch applied on top of Linux next and boot tested for
more than 100 times and is still running in a loop by Anders.

The reported warning did not reproduce again.

Tested-by: Anders Roxell <anders.roxell@linaro.org>
Reported-by: Linux Kernel Functional Testing <lkft@linaro.org>


- Naresh


>
> Thanks.


>
> --
> tejun