Skip to content

Allow clock field in /proc/cpuinfo as cpu MHz fallback value#269

Merged
shirou merged 1 commit intoshirou:masterfrom
andhe:ppc64le
Oct 15, 2016
Merged

Allow clock field in /proc/cpuinfo as cpu MHz fallback value#269
shirou merged 1 commit intoshirou:masterfrom
andhe:ppc64le

Conversation

@andhe
Copy link
Copy Markdown
Contributor

@andhe andhe commented Oct 14, 2016

Needed on ppc64le debian porter boxes atleast.

See #230

This avoids getting MHz from both sysfs cpufreq and /proc/cpuinfo failing. Previously an error was thrown. With this patch we have MHz value in fallback.

Needed on ppc64le debian porter boxes atleast.

See #230
@andhe andhe mentioned this pull request Oct 14, 2016
Copy link
Copy Markdown

@dvusboy dvusboy left a comment

Choose a reason for hiding this comment

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

Probably a non-blocking issue.

Comment thread cpu/cpu_linux.go
case "cpu MHz", "clock":
// treat this as the fallback value, thus we ignore error
if t, err := strconv.ParseFloat(value, 64); err == nil {
if t, err := strconv.ParseFloat(strings.Replace(value, "MHz", "", 1), 64); err == nil {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What happens if the unit is not "MHz", but, say, "kHz" or "GHz"? Since I don't have any experience with PPC/Power architecture, I don't know. Of course, "cpu kHz" can be a problem too, but I have not seen that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your review! Given the current value I'm seeing on the Debian ppc64le porterbox would have been more suitably given in GHz I just assumed the value was always in MHz, but this is just an assumption on my side.... (The real answer is to be found in the kernel source somewhere.)

Copy link
Copy Markdown
Contributor Author

@andhe andhe Oct 14, 2016

Choose a reason for hiding this comment

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

I believe here's the answer to the used unit (-> always MHz): http://sources.debian.net/src/linux/4.5.5-1/arch/powerpc/kernel/setup-common.c/?hl=279#L279

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

👍

@shirou
Copy link
Copy Markdown
Owner

shirou commented Oct 15, 2016

wonderful! Thank you for joining discussion and contribution!

@shirou shirou merged commit c2dd8ca into shirou:master Oct 15, 2016
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