Received-SPF: pass (google.com: domain of linux-kernel+bounces-9474-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Precedence: bulk
MIME-Version: 1.0
References: <20231220072529.1036099-1-tj@kernel.org> <CAJhGHyACBBW5wnYTTz7S+AgLMhLYOsfxTnN6VHodCgECqEFeEg@mail.gmail.com>
 <ZYTDz10ohj0kokum@mtj.duckdns.org>
In-Reply-To: <ZYTDz10ohj0kokum@mtj.duckdns.org>
From: Lai Jiangshan <jiangshanlai@gmail.com>
Date: Fri, 22 Dec 2023 16:04:27 +0800
Message-ID: <CAJhGHyDyWBCSH_41cU7TokbDTE=vknLjLKYMQUtN8LyUsvg9dw@mail.gmail.com>
Subject: Re: [PATCHSET wq/for-6.8] workqueue: Implement system-wide max_active
 for unbound workqueues
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 Fri, Dec 22, 2023 at 7:01=E2=80=AFAM Tejun Heo <tj@kernel.org> wrote:
>
> Hello, Lai.
>
> On Wed, Dec 20, 2023 at 05:20:18PM +0800, Lai Jiangshan wrote:
> > The patchset seems complicated to me.  For me, reverting a bit to the b=
ehavior
> > of 636b927eba5b ("workqueue: Make unbound workqueues to use per-cpu
> > pool_workqueues"), like the following code (untested, just for showing
> > the idea),
> > seems simpler.
> >
> > max_active will have the same behavior as before if the wq is configure=
d
> > with WQ_AFFN_NUMA. For WQ_AFFN_{CPU|SMT|CACHE}, the problem
> > isn't fixed.
>
> Yeah, it is complicated but the complications come from the fact that the
> domain we count nr_active can't match the worker_pools, and that's becaus=
e
> unbound workqueue behavior is noticeably worse if we let them roam across=
 L3
> boundaries on modern processors with multiple chiplets or otherwise
> segmented L3 caches.
>
> We need WQ_AFFN_CACHE to behave well on these chips and max_active
> enforcement breaks if we keep them tied to pool in such cases, so I'm afr=
aid
> our hands are tied here. The hardware has changed and we have to adapt to
> it. In this case, that comes at the cost of extra complexity to divorce
> max_active enforcement domain from worker_pool boundaries.
>

Hello, Tejun

The current "max_active enforcement domain" is just a historical
accident.  IMO, it is
better to change the semantics of max_active and the related alloc_workqueu=
e()
callers rather than add a large bulk of complicated code. Most of
alloc_workqueue()
are called with max_active=3D0, so it is feasible.

Thanks
Lai