btrfs thread_pool_size logic out of sync with workqueue

btrfs thread_pool_size logic out of sync with workqueue

[Junxuan Liao]

Hi all,

Commit 636b927eba5bc63375 (workqueue: Make unbound workqueues to use
per-cpu pool_workqueues) makes max_active per CPU for unbounded
workqueues as well, but thread_pool_size in btrfs_fs_info and the mount
option thread_pool still assume the limit is global.

e.g. The default value of 8 allows a total of 64 workers on an 8-CPU
machine.

As far as I know, this means that on the Btrfs side we can no longer
control the concurrency level at the same granularity as before. We
should rewrite the auto-scaling logic, change the default
thread_pool_size value, and update the documentation. 

Am I missing something?

-- 
Thanks,
Junxuan

Re: btrfs thread_pool_size logic out of sync with workqueue

[Tejun Heo]

On Mon, Apr 28, 2025 at 06:23:39PM -0500, Junxuan Liao wrote:
> Hi all,
> 
> Commit 636b927eba5bc63375 (workqueue: Make unbound workqueues to use
> per-cpu pool_workqueues) makes max_active per CPU for unbounded
> workqueues as well, but thread_pool_size in btrfs_fs_info and the mount
> option thread_pool still assume the limit is global.
> 
> e.g. The default value of 8 allows a total of 64 workers on an 8-CPU
> machine.
> 
> As far as I know, this means that on the Btrfs side we can no longer
> control the concurrency level at the same granularity as before. We
> should rewrite the auto-scaling logic, change the default
> thread_pool_size value, and update the documentation. 
> 
> Am I missing something?

5797b1c18919 ("workqueue: Implement system-wide nr_active enforcement for
unbound workqueues") turned max_active system-wide. The count is now split
across nodes proportional to the number of cpus each node has. This is still
a different behavior from before where max_active was applied per node, but
no behavior change on single node machines and the new behavior is easier to
work with.

Thanks.

-- 
tejun

Re: btrfs thread_pool_size logic out of sync with workqueue

[Junxuan Liao]

On 4/29/25 1:12 PM, Tejun Heo wrote:
> 5797b1c18919 ("workqueue: Implement system-wide nr_active enforcement for
> unbound workqueues") turned max_active system-wide. The count is now split
> across nodes proportional to the number of cpus each node has. This is still
> a different behavior from before where max_active was applied per node, but
> no behavior change on single node machines and the new behavior is easier to
> work with.

Thanks! I missed that. workqueue.rst doesn't reflect the change though.

-- 
Junxuan

Re: btrfs thread_pool_size logic out of sync with workqueue

[Tejun Heo]

On Tue, Apr 29, 2025 at 01:28:46PM -0500, Junxuan Liao wrote:
> 
> 
> On 4/29/25 1:12 PM, Tejun Heo wrote:
> > 5797b1c18919 ("workqueue: Implement system-wide nr_active enforcement for
> > unbound workqueues") turned max_active system-wide. The count is now split
> > across nodes proportional to the number of cpus each node has. This is still
> > a different behavior from before where max_active was applied per node, but
> > no behavior change on single node machines and the new behavior is easier to
> > work with.
> 
> Thanks! I missed that. workqueue.rst doesn't reflect the change though.

Yeah, I really should but patches welcome. :-)

Thanks.

-- 
tejun

Re: btrfs thread_pool_size logic out of sync with workqueue

[Junxuan Liao]

> 5797b1c18919 ("workqueue: Implement system-wide nr_active enforcement for
> unbound workqueues") turned max_active system-wide. 

So versions from 6.6 to 6.8 do have the bug, right? I guess the
performance regression isn't easy to trigger so no one noticed and
reported it.

-- 
Thanks,
Junxuan

Re: btrfs thread_pool_size logic out of sync with workqueue

[Junxuan Liao]

On 4/29/25 4:15 PM, Junxuan Liao wrote:
> So versions from 6.6 to 6.8 do have the bug, right? I guess the
> performance regression isn't easy to trigger so no one noticed and
> reported it.
> 

My bad. I missed that it has already been reported in 2023.
https://lore.kernel.org/all/dbu6wiwu3sdhmhikb2w6lns7b27gbobfavhjj57kwi2quafgwl@htjcc5oikcr3/

-- 
Junxuan

Re: btrfs thread_pool_size logic out of sync with workqueue

[Tejun Heo]

On Tue, Apr 29, 2025 at 04:26:52PM -0500, Junxuan Liao wrote:
> On 4/29/25 4:15 PM, Junxuan Liao wrote:
> > So versions from 6.6 to 6.8 do have the bug, right? I guess the
> > performance regression isn't easy to trigger so no one noticed and
> > reported it.
> 
> My bad. I missed that it has already been reported in 2023.
> https://lore.kernel.org/all/dbu6wiwu3sdhmhikb2w6lns7b27gbobfavhjj57kwi2quafgwl@htjcc5oikcr3/

Yeah, the changes were too invasive to backport through -stable especially
at the time as I didn't know how well the new scheme would work. There have
been some updates afterwards but it seemed to have held up fine, so if some
distros wanna backport them, this should be pretty safe now. However, I
think the window has already passed for -stable backports.

Thanks.

-- 
tejun