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.

Sign up for GitHub

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

Jump to bottom

[cpu][linux] Fix #849 implement giampaolo/psutil#1727 and test Counts against lscpu #944

Merged
merged 2 commits into from
Sep 19, 2020
Merged

[cpu][linux] Fix #849 implement giampaolo/psutil#1727 and test Counts against lscpu #944

merged 2 commits into from
Sep 19, 2020

Conversation

Lomanic
Copy link
Collaborator

@Lomanic Lomanic commented Sep 15, 2020

[cpu][linux] Fix #849 implement giampaolo/psutil#1727 and test Counts against lscpu

@Lomanic
Copy link
Collaborator Author

Lomanic commented Sep 16, 2020

This was only tested against amd64, if people watching the repo and having a linux box at hand could test it with go test github.com/shirou/gopsutil/cpu with this patch applied and report back, this would be extremely appreciated. Especially on multi-socketed machines. Thanks in advance!

@shirou
Copy link
Owner

shirou commented Sep 19, 2020

I confirmed it works on my amd64 Linux. Thank you so much!

@shirou shirou merged commit c5b7357 into master Sep 19, 2020
@shirou shirou deleted the issue849 branch September 19, 2020 01:24
@Lomanic
Copy link
Collaborator Author

Lomanic commented Sep 19, 2020

Did you run the tests on a multi-socket machine?

@shirou
Copy link
Owner

shirou commented Sep 20, 2020

Sorry, Actually, no. Should I revert this?

@Lomanic
Copy link
Collaborator Author

Lomanic commented Sep 20, 2020

No, but I wanted more samples for the test I wrote that compares values from lscpu and gopsutil.

@Lomanic Lomanic mentioned this pull request Sep 20, 2020
Closed
@Lomanic
Copy link
Collaborator Author

Lomanic commented Sep 20, 2020

BTW today I could finally confirm that the tests run successfully on an armv7l host. This PR fixed this in fact (cpu.Counts(false) would return 0 physical cores on arm without error), as I think the initial bug report in #849 conflated cpu sockets/packages and cpu physical cores.

@shirou
Copy link
Owner

shirou commented Sep 21, 2020

Oh, awesome! Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

cpu count not correct
2 participants
@Lomanic @shirou