Received-SPF: pass (google.com: domain of linux-kernel+bounces-9145-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Sender: Tejun Heo <htejun@gmail.com>
Date: Fri, 22 Dec 2023 08:01:35 +0900
From: Tejun Heo <tj@kernel.org>
To: Lai Jiangshan <jiangshanlai@gmail.com>
Cc: linux-kernel@vger.kernel.org, Naohiro.Aota@wdc.com,
	kernel-team@meta.com
Subject: Re: [PATCHSET wq/for-6.8] workqueue: Implement system-wide
 max_active for unbound workqueues
Message-ID: <ZYTDz10ohj0kokum@mtj.duckdns.org>
References: <20231220072529.1036099-1-tj@kernel.org>
 <CAJhGHyACBBW5wnYTTz7S+AgLMhLYOsfxTnN6VHodCgECqEFeEg@mail.gmail.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAJhGHyACBBW5wnYTTz7S+AgLMhLYOsfxTnN6VHodCgECqEFeEg@mail.gmail.com>

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 behavior
> 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 configured
> 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 because
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 afraid
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.

Thanks.

-- 
tejun