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

Add real and cores to CPU plugin on FreeBSD to match Linux #674

Merged
merged 2 commits into from
Dec 9, 2015

Conversation

tas50
Copy link
Contributor

@tas50 tas50 commented Dec 5, 2015

Use dmesg to correctly determine the number of real CPUs, the number of
real cores, and the number of HT cores. This aligns CPU count data with
what we collect on Linux.

@arachnist
Copy link

dmesg contents are volatile.
Relying on them is bound to produce errors or undefined behaviour sooner or later.

@kev009
Copy link

kev009 commented Dec 6, 2015

It should be fairly safe between majors

Use dmesg to correctly determine the number of real CPUs, the number of
real cores, and the number of HT cores.  This aligns CPU count data with
what we collect on Linux.
@lamont-granquist
Copy link
Contributor

@arachnist i raised that question when we saw the same patch for dragonflybsd, but bsd users couldn't come up with a better API to query than dmesg...

cpuinfo["total"] = $1.to_i
when /FreeBSD\/SMP: (\d*) package\(s\) x (\d*) core\(s\)/
cpuinfo["real"] = $1.to_i
cpuinfo["cores"] = $2.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.

doesn't this need to be $1.to_i * $2.to_i?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lamont-granquist do you mean the total should be real * cores? The line in dmesg above actually gives the number of cores including HT cores so we don't need to do the math.

Copy link
Contributor

Choose a reason for hiding this comment

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

No, I'm talking about the 'cores' computation. If this is a dual-socket quad-proc shouldn't that look like:

FreeBSD/SMP: Multiprocessor System Detected: 16 CPUs
FreeBSD/SMP: 2 package(s) x 4 core(s) x 2 SMT threads

and cpuinfo["cores"] should be 8 (2 x 4)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep. let me fix that one too

@lamont-granquist
Copy link
Contributor

👍

@mcquin
Copy link
Contributor

mcquin commented Dec 9, 2015

LGTM

tas50 added a commit that referenced this pull request Dec 9, 2015
Determine real, total, and cores for BSD
@tas50 tas50 merged commit 37d0b81 into chef:master Dec 9, 2015
@tas50 tas50 changed the title Determine real, total, and cores for BSD Add real and cores to CPU plugin on FreeBSD to match Linux Dec 9, 2015
@tas50 tas50 deleted the freebsd_cores branch December 10, 2015 03:28
@thommay thommay added Type: Enhancement Adds new functionality. and removed Enhancement 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: Enhancement Adds new functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants