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

Correctly count real vs. total vs. cores on darwin #672

Merged
merged 1 commit into from
Dec 9, 2015

Conversation

tas50
Copy link
Contributor

@tas50 tas50 commented Dec 5, 2015

We were counting CPUs wrong. See #671 for details

@mcquin
Copy link
Contributor

mcquin commented Dec 7, 2015

👍

ping @chef/client-core

cpu[:total] = so.stdout.to_i
so = shell_out("sysctl -n hw.logicalcpu")
cpu[:cores] = so.stdout.to_i
Copy link
Contributor

Choose a reason for hiding this comment

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

well, this is confusing. at first glance it seems backwards from what i'd expect.

"cores" should be physical cores -- i would expect '4' here
"total" should be the logical cores things like the scheduler sees - i would expect '8' here
"real" should be how many packages i have plugged into the motherboard - i would expect '1' here

i haven't looked at how we define these on linux or what the APIs return on darwin, but your cores and total read backwards from what i would expect.

(and definitely include some comments if things really need to be confusing).

Copy link
Contributor

Choose a reason for hiding this comment

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

checked the darwin APIs and they're the way i assumed they be from the names... so the only remaining question is if our ohai APIs are backwards from what i think they should mean...

@lamont-granquist
Copy link
Contributor

So on linux the 'total' is the thing that the scheduler sees, so all the hyperthreaded cores aka all the entries in /proc/cpuinfo. So i'm pretty certain 'total' here should match and be hw.logicalcpu

@lamont-granquist
Copy link
Contributor

And linux only defines 'real' as the sum of the unique physical_id's which should be the number of sockets.

So i'm pretty sure linux behaves the way that I outlined above.

@tas50
Copy link
Contributor Author

tas50 commented Dec 8, 2015

Updated to swap the cores vs. total logic

@lamont-granquist
Copy link
Contributor

👍

tas50 added a commit that referenced this pull request Dec 9, 2015
Correctly count real vs. total vs. cores on darwin
@tas50 tas50 merged commit 84f5550 into chef:master Dec 9, 2015
@tas50 tas50 deleted the osx_cpu branch December 10, 2015 03:27
@thommay thommay added Type: Bug Does not work as expected. and removed Bug labels Jan 24, 2017
@chef chef locked and limited conversation to collaborators Nov 16, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Type: Bug Does not work as expected.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants