Received-SPF: pass (google.com: domain of linux-kernel+bounces-11433-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20231220072529.1036099-1-tj@kernel.org> <20231220072529.1036099-5-tj@kernel.org>
In-Reply-To: <20231220072529.1036099-5-tj@kernel.org>
From: Lai Jiangshan <jiangshanlai@gmail.com>
Date: Tue, 26 Dec 2023 17:12:55 +0800
Message-ID: <CAJhGHyC4Qj7t=Emd65556r_g0fUbf9LWEA-1OKuwaDFTz8O50g@mail.gmail.com>
Subject: Re: [PATCH 04/10] workqueue: Move nr_active handling into helpers
To: Tejun Heo <tj@kernel.org>
Cc: linux-kernel@vger.kernel.org, Naohiro.Aota@wdc.com, kernel-team@meta.com
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Dec 20, 2023 at 3:25=E2=80=AFPM Tejun Heo <tj@kernel.org> wrote:

> +static bool pwq_tryinc_nr_active(struct pool_workqueue *pwq)
>  {
> -       struct work_struct *work =3D list_first_entry(&pwq->inactive_work=
s,
> -                                                   struct work_struct, e=
ntry);
> +       struct workqueue_struct *wq =3D pwq->wq;
> +       struct worker_pool *pool =3D pwq->pool;
> +       bool obtained;
>
> -       pwq_activate_work(pwq, work);
> +       lockdep_assert_held(&pool->lock);
> +
> +       obtained =3D pwq->nr_active < wq->max_active;

It is better to use READ_ONCE(wq->max_active) here in case
the compiler issues code to calculate "obtained" multi-time.

Theoretically, READ_ONCE(wq->max_active) is recommended
from the patch1 in pwq_dec_nr_in_flight() and __queue_work()
since there is no wq->mutex for them.


> +
> +       if (obtained)
> +               pwq->nr_active++;
> +       return obtained;
> +}
> +