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

UCS/ARCH: Fix reading ppc timebase from /proc/cpuinfo. #1623

Merged
merged 1 commit into from
Jun 21, 2017
Merged

UCS/ARCH: Fix reading ppc timebase from /proc/cpuinfo. #1623

merged 1 commit into from
Jun 21, 2017

Conversation

yosefe
Copy link
Contributor

@yosefe yosefe commented Jun 20, 2017 •
edited
Loading

Fixes #1588

@swx-jenkins1
Copy link

Can one of the admins verify this patch?

@yosefe
Copy link
Contributor Author

yosefe commented Jun 20, 2017

ok to test

@yosefe
Copy link
Contributor Author

yosefe commented Jun 20, 2017

bot:bgate:retest

@swx-jenkins1
Copy link

Test PASSed.
See http://bgate.mellanox.com/jenkins/job/gh-ucx-pr/1924/ for details.

@mellanox-github
Copy link
Contributor

Test FAILed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/3892/ for details (Mellanox internal link).

@mellanox-github
Copy link
Contributor

Test PASSed.
See http://hpc-master.lab.mtl.com:8080/job/hpc-ucx-pr/3893/ for details (Mellanox internal link).

@yosefe
Copy link
Contributor Author

yosefe commented Jun 20, 2017

@shamisp pls review

@yosefe yosefe merged commit 8a1596f into openucx:master Jun 21, 2017
@shamisp
Copy link
Contributor

shamisp commented Jun 21, 2017

You need to add test for the new API. Proc is a not reliable way of reading frequency. Linux does not require to report freq. I would suggest to rollback the patch.

@shamisp shamisp mentioned this pull request Jun 21, 2017
Closed
@yosefe yosefe deleted the topic/fix-ppc-timebase-cpuinfo branch June 21, 2017 22:12
@yosefe
Copy link
Contributor Author

yosefe commented Jun 21, 2017

it's done in case glibc does not support reading timebase.
We already have test for it (the one which tests ucs_get_time()) but didn't actually run it on older system so far.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mike-dubman mike-dubman mike-dubman approved these changes

Assignees
No one assigned
Labels
Bugfix Portability
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@yosefe @swx-jenkins1 @mellanox-github @shamisp @mike-dubman