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

cpu: support for DragonflyBSD #893

Merged
merged 6 commits into from
Jun 30, 2020

Conversation

gballet
Copy link
Contributor

@gballet gballet commented Jun 18, 2020

No description provided.

var featuresMatch = regexp.MustCompile(`Features=.+<(.+)>`)
var featuresMatch2 = regexp.MustCompile(`Features2=[a-f\dx]+<(.+)>`)
var cpuEnd = regexp.MustCompile(`^Trying to mount root`)
var cpuCores = regexp.MustCompile(`FreeBSD/SMP: (\d*) package\(s\) x (\d*) core\(s\)`)
Copy link
Owner

Choose a reason for hiding this comment

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

sorry, I don't have a Dragonfly, so I can not check but is this FreeBSD correct ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch. I just checked, and parsing the dmesg output is unlikely to work on FreeBSD, too, as regular users do not have access to dmesg. I'll remove this regexp because it will never match, and the correct value is eventually extracted in InfoWithContext.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok, I see why you parse the output of dmesg: to differentiate cpu vs. cores.

Copy link
Owner

Choose a reason for hiding this comment

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

Thank you for the information. I found it is not worked correctly in FreeBSD... it should be use kern.sched.topology_spec to getting how many psychical core it has. I will fix it, so we can left this at this time.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have rewritten the PR to use the equivalent hw.cpu_topology.tree in DragonflyBSD. Looking forward to seeing what you come up with on the FreeBSD side.

cpu/cpu_dragonfly.go Outdated Show resolved Hide resolved
cpu/cpu_dragonfly.go Outdated Show resolved Hide resolved
cpu/cpu_dragonfly.go Outdated Show resolved Hide resolved
cpu/cpu_dragonfly.go Outdated Show resolved Hide resolved
@shirou
Copy link
Owner

shirou commented Jun 21, 2020

Thank you for your contribution!. However, this file seems exactly same as cpu_freebsd.go. I don't know the DragonflyBSD but I'm curious to see if this PR really works...

If this works, it's ok to add, but could you change the FreeBSD word in a error? Thank you!

@gballet
Copy link
Contributor Author

gballet commented Jun 21, 2020

The PR works, at least on my machines ;) There are indeed some issues with parsing the number of cores, I'm going to check if it's possible to extract that information without parsing the output of dmesg because this is unlikely to remain stable over the releases.

@shirou
Copy link
Owner

shirou commented Jun 30, 2020

I can not confirm this can works, but I merge it. Thank you for your great contribution!

@shirou shirou merged commit 01afd76 into shirou:master Jun 30, 2020
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.

2 participants