Skip to content

Fix CPU_COUNT check for FreeBSD 13.1+#77867

Merged
akoeplinger merged 4 commits intodotnet:mainfrom
Thefrank:fbsd_cpucount
Jan 11, 2023
Merged

Fix CPU_COUNT check for FreeBSD 13.1+#77867
akoeplinger merged 4 commits intodotnet:mainfrom
Thefrank:fbsd_cpucount

Conversation

@Thefrank
Copy link
Contributor

@Thefrank Thefrank commented Nov 3, 2022

HAVE_GNU_CPU_COUNT has always incorrectly failed for FreeBSD.

This has not been an issue until FreeBSD 13.1 with its addition of sched_getaffinity and sched_setaffinity.

@Thefrank Thefrank requested a review from marek-safar as a code owner November 3, 2022 22:54
@ghost ghost added area-Build-mono community-contribution Indicates that the PR has been added by a community member labels Nov 3, 2022
@Thefrank
Copy link
Contributor Author

Thefrank commented Nov 3, 2022 •
edited
Loading

more info: With the two sched checks passing but CPU_COUNT failing it will try and make its own CPU_COUNT...and fail because FreeBSD already has a macro it uses for that.

Thinking about it more, the other option would be to opt FreeBSD out of HAVE_SCHED_GETAFFINITY and HAVE_GNU_CPU_COUNT

@build-analysis build-analysis bot mentioned this pull request Nov 4, 2022
Closed
2 tasks
@marek-safar marek-safar added the os-freebsd FreeBSD OS label Nov 4, 2022
@marek-safar marek-safar requested a review from vargaz November 4, 2022 08:32
am11
am11 reviewed Nov 4, 2022
View reviewed changes
@Thefrank
Copy link
Contributor Author

Thefrank commented Nov 4, 2022 •
edited
Loading

Switched to check_symbol_exists as per @am11 suggestion.
I left it in the section with the other "GNU" items but for readability it might be better with the other check_symbol_exists items

For those wondering:
FreeBSD 13.1:
CPU_COUNT sys/cpuset.h:74:#define CPU_COUNT(p) ((int)__BIT_COUNT(CPU_SETSIZE, p))
Ubuntu 20.04.02 + kernel 5.18.4
CPU_COUNT sched.h:88: # define CPU_COUNT(cpusetp) __CPU_COUNT_S (sizeof (cpu_set_t), cpusetp)

This was referenced Dec 28, 2022
Closed
Closed
@Thefrank Thefrank mentioned this pull request Jan 7, 2023
Merged
@am11 am11 requested review from akoeplinger and lambdageek January 7, 2023 17:50
am11
am11 approved these changes Jan 7, 2023
View reviewed changes
Copy link
Member

@am11 am11 left a comment

Choose a reason for hiding this comment

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

LGTM

@akoeplinger
Copy link
Member

akoeplinger commented Jan 11, 2023

Thank you!

@akoeplinger akoeplinger merged commit 97cb762 into dotnet:main Jan 11, 2023
@Thefrank Thefrank deleted the fbsd_cpucount branch January 23, 2023 22:20
@Thefrank Thefrank mentioned this pull request Feb 2, 2023
Closed
@ghost ghost locked as resolved and limited conversation to collaborators Feb 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@akoeplinger akoeplinger akoeplinger approved these changes

@marek-safar marek-safar Awaiting requested review from marek-safar

@vargaz vargaz Awaiting requested review from vargaz

@lambdageek lambdageek Awaiting requested review from lambdageek

+1 more reviewer

@am11 am11 am11 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

area-Build-mono community-contribution Indicates that the PR has been added by a community member os-freebsd FreeBSD OS

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@Thefrank @akoeplinger @am11 @marek-safar