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.

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

Fixed incorrect threads count on Linux #339

Merged
merged 1 commit into from
Dec 29, 2022
Merged

Fixed incorrect threads count on Linux #339

merged 1 commit into from
Dec 29, 2022

Conversation

emoon
Copy link
Contributor

@emoon emoon commented Dec 20, 2022

On my Linux system (ArchLinux) using sysconf(_SC_NPROCESSORS_CONF); will return 128 while my actual thread count is 32. This will cause Tundra to print an warning that number of threads gets clamped to 64, but after that will then crash.

This PR doesn't fix the underlying crash, but instead uses C++11 std::thread::hardware_concurrency(); to get the number of hw threads in the system and this returns the correct value and now everything works as it should, but there is still another issue that makes tundra crash when thread count ends up being 128.

I know we use C++11 for Linux, but I'm unsure about the other targets. If that is true in all cases then it might be worth considering using this code path on all targets.

@deplinenoise deplinenoise merged commit c3416af into deplinenoise:master Dec 29, 2022
@emoon emoon deleted the linux-cpu-count-fix branch December 29, 2022 08:46
@deplinenoise
Copy link
Owner

According to a new issue, this is now again returning 128 on machines where it shouldn't. No idea why.

@deplinenoise
Copy link
Owner

See #347

The root issue seems to be that on some Linux kernels there is some "hotswap" CPU path that allocates a massive number of fake CPUs in case you add some later. And for whatever reason that's what both the std::thread concurrency number and sysconf reports.

However there is a way to ask sysconf for "online" CPUs, and that should be a sane number.

@questor
Copy link

questor commented Aug 4, 2024

sorry for confusion, I have my own tundra-fork with a small thing for myself added (honestly: currently I have forgotten were I used my patch) and it might be that I used the code before emoons patch applied with my machine reporting 128 cores. I will test your latest fix tomorrow on my machine and will report if it's working (for me) or not.

@questor
Copy link

questor commented Aug 5, 2024

integrated your fixes and can confirm that it does not crash anymore and detects the correct amount of cores here in my environment with the latest (current) version. thanks for your help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants