Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Regression in threads tests on FreeBSD #54817

Closed
ararslan opened this issue Jun 15, 2024 · 0 comments · Fixed by #55261
Closed

Regression in threads tests on FreeBSD #54817

ararslan opened this issue Jun 15, 2024 · 0 comments · Fixed by #55261
Labels
multithreading Base.Threads and related functionality regression Regression in behavior compared to a previous version system:freebsd Affects only FreeBSD test This change adds or pertains to unit tests

Comments

@ararslan
Copy link
Member

#53402 added thread affinity functions jl_getaffinity and jl_setaffinity. The added test fails on FreeBSD:

Error in testset threads:
Test Failed at /usr/home/julia/.buildkite-agent/builds/freebsd13-amdci6-3/julialang/julia-master/julia-58927d885a/share/julia/test/threads.jl:366
  Expression: jl_setaffinity(1, mask, cpumasksize) == 0
   Evaluated: -22 == 0

The return code there in the failing test is EINVAL, meaning an argument is invalid. We could just not run that test on FreeBSD, as is currently done with Windows, but ideally we'd be able to fix the test (or implementation, I guess) to support FreeBSD.
@ararslan ararslan added test This change adds or pertains to unit tests regression Regression in behavior compared to a previous version system:freebsd Affects only FreeBSD multithreading Base.Threads and related functionality labels Jun 15, 2024
@ararslan ararslan mentioned this issue Jul 26, 2024
Merged
KristofferC pushed a commit that referenced this issue Aug 2, 2024
@ararslan @KristofferC
Make jl_*affinity tests more portable (#55261)
e7ddd62 
Changes made:
- Use 0 for the thread ID to ensure it's always valid. The function
expects `0 <= tid < jl_n_threads` so 1 is incorrect if `jl_n_threads` is
1.
- After retrieving the affinity mask with `jl_getaffinity`, pass that
same mask back to `jl_setaffinity`. This ensures that the mask is always
valid. Using a mask of all ones results in `EINVAL` on FreeBSD. Based on
the discussion in #53402, this change may also fix Windows, so I've
tried reenabling it here.
- To check whether `jl_getaffinity` actually did something, we can check
that the mask is no longer all zeros after the call.

Fixes #54817

(cherry picked from commit 8a7e23d)
lazarusA pushed a commit to lazarusA/julia that referenced this issue Aug 17, 2024
@ararslan @lazarusA
Make jl_*affinity tests more portable (JuliaLang#55261)
72762a6 
Changes made:
- Use 0 for the thread ID to ensure it's always valid. The function
expects `0 <= tid < jl_n_threads` so 1 is incorrect if `jl_n_threads` is
1.
- After retrieving the affinity mask with `jl_getaffinity`, pass that
same mask back to `jl_setaffinity`. This ensures that the mask is always
valid. Using a mask of all ones results in `EINVAL` on FreeBSD. Based on
the discussion in JuliaLang#53402, this change may also fix Windows, so I've
tried reenabling it here.
- To check whether `jl_getaffinity` actually did something, we can check
that the mask is no longer all zeros after the call.

Fixes JuliaLang#54817
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
multithreading Base.Threads and related functionality regression Regression in behavior compared to a previous version system:freebsd Affects only FreeBSD test This change adds or pertains to unit tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Make jl_*affinity tests more portable JuliaLang/julia
1 participant
@ararslan