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

take maximum processor Mhz #16740

Merged
merged 3 commits into from
Mar 31, 2023
Merged

take maximum processor Mhz #16740

merged 3 commits into from
Mar 31, 2023

Conversation

gburanov
Copy link
Contributor

Currently we take just first processor Mhz, and exit, assuming they are all the same.

We depend on "github.com/shoenig/go-m1cpu" to get the cpu data

On linux platform this either uses maximum frequency https://github.com/shoenig/gopsutil/blob/master/cpu/cpu_linux.go#L88 from cpuinfo_max_freq (that would be correct)

However some machines lack this file, and then we fallback to real frequency https://github.com/shoenig/gopsutil/blob/master/cpu/cpu_linux.go#L175-L179

Real frequency can be much lower than maximal, and taking just first processor value can give incorrect data.

For example on my machine

[georgy@kf06 ~]$ cat /proc/cpuinfo  | grep "cpu MHz"
cpu MHz		: 2200.000
cpu MHz		: 2685.956
cpu MHz		: 2200.000
cpu MHz		: 2252.721
cpu MHz		: 2200.000
cpu MHz		: 2200.000
cpu MHz		: 2200.000
cpu MHz		: 2700.009
cpu MHz		: 2200.000
cpu MHz		: 2592.342
cpu MHz		: 1906.243
cpu MHz		: 2200.000
cpu MHz		: 2200.000
cpu MHz		: 2200.000
cpu MHz		: 2042.605
cpu MHz		: 2200.000
cpu MHz		: 1423.054
cpu MHz		: 2200.000
cpu MHz		: 2200.000
cpu MHz		: 2552.760
cpu MHz		: 1645.222
cpu MHz		: 2591.675
cpu MHz		: 1858.395
cpu MHz		: 2699.999
cpu MHz		: 1231.719
cpu MHz		: 2688.291
cpu MHz		: 2236.831
cpu MHz		: 2200.000
cpu MHz		: 1244.091
cpu MHz		: 2200.000
cpu MHz		: 2200.000
cpu MHz		: 2200.000
cpu MHz		: 2200.000
cpu MHz		: 2200.000
cpu MHz		: 2200.000
cpu MHz		: 2200.000
cpu MHz		: 2200.000
cpu MHz		: 2631.418
cpu MHz		: 2200.000
cpu MHz		: 2200.000
cpu MHz		: 2200.000
cpu MHz		: 2200.000
cpu MHz		: 1789.280
cpu MHz		: 2200.000
cpu MHz		: 1754.116
cpu MHz		: 2586.694
cpu MHz		: 1209.627
cpu MHz		: 2700.003

Would not taking the maximum value be better here?

@hashicorp-cla
Copy link

hashicorp-cla commented Mar 31, 2023

CLA assistant check
All committers have signed the CLA.

@shoenig
Copy link
Member

shoenig commented Mar 31, 2023

Hi @gburanov! Indeed I think your patch is the correct thing to do in the short-term (in fact, I thought this is what we were doing already 🤦‍♂️).

In the longer term though we need to start accounting for P vs. E cores (which is what go-m1cpu does, but specifically for Apple M1/M2 chips only), which I think is going to require more changes to gopsutil, or another bit of custom code in Nomad.

And if this is for linux/arm64 there is also the idea of using dmidecode to extract the maximum CPU performance data: #4233

@shoenig shoenig added the backport/1.5.x backport to 1.5.x release line label Mar 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport/1.5.x backport to 1.5.x release line
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants