Skip to content

Conversation

etcwilde
Copy link
Member

This improves the CPU core count handling for FreeBSD. This uses sysconf to grab the number of configured processors when logical and physical CPU counts are requested, and then pthread_getaffinity_np and the number of active processors. This takes into account the number of available cores when the process is restricted to a subset of the processors.

This also fixes the dispatch_workqueye test, which has its own implementation to extract the core count. FreeBSD does not have an hw.activecpu syscall, so the syscallbyname was failing and the activecpu count was garbage from the stack. I've updated the test to initialize the value with 0xa1a1a1 to make the garbage clearer. Also update the test for FreeBSD to take advantage of sysconf and pthread_getaffinity_np in the test.

Fixes: #897

@etcwilde
Copy link
Member Author

CC @kevans91

@etcwilde
Copy link
Member Author

@swift-ci please test

@etcwilde etcwilde moved this to In Progress in Swift on FreeBSD Aug 26, 2025
Copy link

@kevans91 kevans91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks reasonable, thanks!

This improves the CPU core count handling for FreeBSD. This uses sysconf
to grab the number of configured processors when logical and physical
CPU counts are requested, and then pthread_getaffinity_np and the number
of active processors. This takes into account the number of available
cores when the process is restricted to a subset of the processors.

This also fixes the `dispatch_workqueye` test, which has its own
implementation to extract the core count. FreeBSD does not have an
`hw.activecpu` syscall, so the syscallbyname was failing and the
activecpu count was garbage from the stack. I've updated the test to
initialize the value with `0xa1a1a1` to make the garbage clearer.
Also update the test for FreeBSD to take advantage of sysconf and
pthread_getaffinity_np in the test.

Fixes: swiftlang#897
@etcwilde etcwilde force-pushed the ewilde/freebsd-cpu-integration branch from 558886c to ea88133 Compare August 26, 2025 23:10
@etcwilde
Copy link
Member Author

@swift-ci please test

Copy link

@kevans91 kevans91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, for what that's worth. Happy to look a little bit closer if there's still something off with the test (I haven't tried to run it yet, personally).

@etcwilde
Copy link
Member Author

Thanks for your help. I’ve checked on my FreeBSD box, and all tests are passing with this change.

@etcwilde
Copy link
Member Author

@swift-ci please test Windows

@etcwilde etcwilde requested a review from rokhinip August 27, 2025 17:37
@al45tair
Copy link
Contributor

I'm assuming FreeBSD doesn't have anything like the problem Linux has with cgroups (namely, on Linux you can use cgroups to restrict the number of CPU cores available, but the usual ways of querying CPU core availability still report the number of cores on the system, not the number of cores set in the cgroup).

@github-project-automation github-project-automation bot moved this from In Progress to Merge in Swift on FreeBSD Aug 29, 2025
@kevans91
Copy link

kevans91 commented Aug 29, 2025

I'm assuming FreeBSD doesn't have anything like the problem Linux has with cgroups (namely, on Linux you can use cgroups to restrict the number of CPU cores available, but the usual ways of querying CPU core availability still report the number of cores on the system, not the number of cores set in the cgroup).

As in: sched_getaffinity/pthread_getafffinity_np won't report these restrictions either and you need cgroup-specific bits, or this is the reason for checking the affinity?

You can cpuset(8) away CPUs entirely per-process or per-jail without visibility to sysconf(3) or via a sysctl. If it's done at the jail level you're pretty much restricted to that subset, if it's per-process you have a chance of lifting the restriction.

It would be true on FreeBSD to say that the current implementation of _dispatch_hw_config_logical_cpus and _dispatch_hw_config_physical_cpus may be inaccurate if you're in a jail that has a more limited root set because you wouldn't be able to cpuset_setaffinity(2) your way up to the currently reported values -- you would need to grab your root cpuset id and check the affinity of that to understand the most a process has available to it beyond what's currently configured.

I don't think it's a problem that FreeBSD couldn't fix, but it is a little awkward to do so.

@kevans91
Copy link

I don't think it's a problem that FreeBSD couldn't fix, but it is a little awkward to do so.

or not; from a user thread perspective, I don't see any reason we couldn't just consider cores not available to the thread as offline. You would need the same level of third-party (i.e. outside of the jail) intervention as you would for, e.g., cpu hotplug.

@al45tair
Copy link
Contributor

al45tair commented Sep 1, 2025

As in: sched_getaffinity/pthread_getafffinity_np won't report these restrictions either and you need cgroup-specific bits, or this is the reason for checking the affinity?

I'm not sure the sched_getaffinity/pthread_getaffinity_np() APIs will actually solve the problem completely for the cgroups case either — CPU restrictions in cgroups are done on the basis of the fraction of time a process can run as well as processor affinity, so we might need to fix that as well (in principle there's no reason the affinity couldn't be broad, but have a small fraction of the total available). The *affinity* APIs do handle the case where someone has actually used processor affinity to schedule the process on particular cores though.

or not; from a user thread perspective, I don't see any reason we couldn't just consider cores not available to the thread as offline. You would need the same level of third-party (i.e. outside of the jail) intervention as you would for, e.g., cpu hotplug.

Personally I think that's a sensible change :-)

@kevans91
Copy link

kevans91 commented Sep 1, 2025

As in: sched_getaffinity/pthread_getafffinity_np won't report these restrictions either and you need cgroup-specific bits, or this is the reason for checking the affinity?

I'm not sure the sched_getaffinity/pthread_getaffinity_np() APIs will actually solve the problem completely for the cgroups case either — CPU restrictions in cgroups are done on the basis of the fraction of time a process can run as well as processor affinity, so we might need to fix that as well (in principle there's no reason the affinity couldn't be broad, but have a small fraction of the total available). The *affinity* APIs do handle the case where someone has actually used processor affinity to schedule the process on particular cores though.

Oh, that's a good point. We do independently have the same problem - there's actually two ways to limit CPU usage:

  • cpu affinity (thread, process, jail) which my FreeBSD patch tries to fix
  • rctl(8) to limit cputime/pcpu (process, user, loginclass or jail), which it does not

rctl(8) is a little shady[0] here, so I'm not 100% certain it's worth trying to accommodate for in the general case at the moment.

or not; from a user thread perspective, I don't see any reason we couldn't just consider cores not available to the thread as offline. You would need the same level of third-party (i.e. outside of the jail) intervention as you would for, e.g., cpu hotplug.

Personally I think that's a sensible change :-)

Thanks for the sanity check!

[0]

rctl(8) offers two cpu limitation option: cputime, pcpu. For cputime you can only opt to signal the process after the allocated runtime, but with pcpu you can either signal or throttle the process (and it's specified in terms of % of a single core). My recollection here is that pcpu limitations are funky in the sense that all processes are measured and throttled at the same point rather than some more reasonable distribution, so you can end up with a system where a lot of processes run for the first 30% of a second then end up throttled for the remainder, rather than trying to smooth things out.

With a limitation like cputime, obviously that's not a CPU restriction in the same sense like we care about here, but I guess my question would be if this is something that Dispatch would take into consideration elsewhere on other platforms. In particular, I'm thinking of a scenario (that we don't really do in FreeBSD, but it's a valid use) where a process wants to slap an upper bound on its runtime in advance and just do 'whatever it can' before it's signalled. Writing it out, that doesn't sound like a Dispatch problem at all.

For pcpu, I suspect that we wouldn't want _SC_NPROCESSORS_ONLN to report that. Maybe another sysconf(3)? My concern here would primarily be that the # processors online is a significant fact in itself to estimate the maximum concurrency you can expect, so (in general) you probably want to have some estimate of the best-case and average levels of concurrency independently.

Copy link

@akmorrison akmorrison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would have slightly preferred checking for errors from sysctlbyname/pthread_get_affinity and aborting the test on failure instead of fixing the stack garbage thing, but it’s a test and it’ll likely fail either way, so not worth changing after this has passed CI. Approved

@etcwilde
Copy link
Member Author

etcwilde commented Sep 2, 2025

Would have slightly preferred checking for errors from sysctlbyname/pthread_get_affinity and aborting the test on failure instead of fixing the stack garbage thing, but it’s a test and it’ll likely fail either way, so not worth changing after this has passed CI.

I considered doing that, but since this PR is for FreeBSD and we no longer use that codepath, I decided not to put the check here. Here is a separate PR to check the result of sysctlbyname: #906

@etcwilde etcwilde merged commit f086778 into swiftlang:main Sep 2, 2025
2 checks passed
@github-project-automation github-project-automation bot moved this from Merge to Done in Swift on FreeBSD Sep 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

FreeBSD: Test Failed: dispatch_workqueue

4 participants