acpi: Tasks: Size the number of tasks with the actual number of CPUs
Needs ReviewPublic
Actions

Authored by olce on Mon, Oct 5, 5:04 PM.

Details

Reviewers

obiwac
jkim
emaste

Summary

With MAXCPU being 1024 on amd64 and arm64, the number of allocated task
slots (4096) looks too high for any practical use (barring bugs filling
all slots, in which case the actual limit does not matter). For
example, on a 24-CPU machine with a long uptime,
'debug.acpi.tasks_hiwater' is only 23.

We do not seem to have received a lot of complaints by people getting
"AcpiOsExecute: failed to enqueue task, consider increasing the
debug.acpi.max_tasks tunable". There is such report in PR 282241 but as
symptom of another problem that was fixed. Such an error is mentioned
in PR 160838, which references a bump of ACPI_MAX_TASKS fixing it
(scaling from 2 * MAXCPU to 4 * MAXCPU), and in PR 176591 (of before the
last ACPI_MAX_TASKS bump), the MAXCPU bump from 64 to 256 having
intervened some years after the reports, in 2014 (and the last bump to
1024 in 2023). It also appears in a trace in the old PR 144956 (MAXCPU
was only 32 back then, as well as ACPI_MAX_TASKS).

So, scale ACPI_MAX_TASKS with the actual number of CPUs rather than
MAXCPU, as this has probably been intended from the start, keeping the
same 4 factor but providing some more breathing room for machines with
a low number of CPUs by adding a fixed amount of 64. The old value of
4096 slots is now reached only on machines with 1008 cores, and typical
16 CPUs machines only allocate 128 slots.

In acpi_task_enqueue() when no task slot is available, suggest in the
diagnostic message to report the value of 'debug.acpi.tasks_hiwater'
after having raised 'debug.acpi.max_tasks'.

While here, sort the headers in 'acpivar.h'.

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 77728
Build 74611: arc lint + arc unit

Event Timeline

olce created this revision.Mon, Oct 5, 5:04 PM

Herald added a subscriber: imp. · View Herald TranscriptMon, Oct 5, 5:04 PM

olce requested review of this revision.Mon, Oct 5, 5:04 PM

Harbormaster completed remote builds in B77728: Diff 188736.Mon, Oct 5, 5:04 PM

olce added a parent revision: D60373: acpi: Tasks: Ensure sane values for number of tasks and threads.Mon, Oct 5, 5:04 PM

olce added a child revision: D60375: acpi: Tasks: Precisely report the maximum number of slots used.

This change seems to make sense, but might be a little bit risky. Thoughts?

I think scaling by the actual number of CPUs rather than MAXCPU is indeed reasonable, and agree that 4*1024 is somewhat ridiculously high.

I wonder in general how many ACPI tasks are correlated with # of CPUs and how many are not. 64 seems like a larger constant than is likely needed but there's not really a concern.

Maybe worth asking people (on current@?) to check debug.acpi.tasks_hiwater, and if it's higher than 32 reply with some information about their system?

FWIW on my Framework 11th gen Intel (nproc=8) tasks_hiwater is 15.

In D60374#1386537, @emaste wrote:

I think scaling by the actual number of CPUs rather than MAXCPU is indeed reasonable, and agree that 4*1024 is somewhat ridiculously high.

Yeah. And if we really need a lot of slots concurrently after boot time, which I doubt, we might want to revise the current synchronization mechanism, as it is not designed for such volume.

I wonder in general how many ACPI tasks are correlated with # of CPUs and how many are not. 64 seems like a larger constant than is likely needed but there's not really a concern.

Probably, yes, just did that out of caution.

Maybe worth asking people (on current@?) to check debug.acpi.tasks_hiwater, and if it's higher than 32 reply with some information about their system?

I agree. To potentially make that even more useful, I'm going first to separate statistics from boot time (where tasks can be enqueued but cannot be processed yet) from run time, as the first may lead to a much higher maximum. This additional information most probably won't be relevant for the needed allocation per se, but will make apparent the actual concurrency needs in normal operation.

FWIW on my Framework 11th gen Intel (nproc=8) tasks_hiwater is 15.

These cores are hyper-threaded, aren't they? Prematurely generalizing from this and my own data point in the commit message, it looks like the max is 2*CPUs-1, and I suspect it is reached at boot time.