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

Fix #7: use intel_pstate/no_turbo to disable turboboost #29

Closed
wants to merge 1 commit into from

Conversation

jinankjain
Copy link
Contributor

@jinankjain jinankjain commented Jan 24, 2018

Using this would resolve the dependency on msr-tools to disable turbo boost.

Copy link
Owner

@travisdowns travisdowns left a comment

Choose a reason for hiding this comment

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

Thank you very much for this help!

There is one main problem, which is mostly a problem of specification in the issue that I filed for this. The issue is that no_turbo is the cleanest way of disabling turbo only for users who are using the intel_pstate driver. The user of the acpi_cpufreq driver, which was the default up until a couple of years ago for Intel, is still very common and this approach will fail there (but the MSR will work, at least for hardware that supports it).

Is there any chance you can make this change see if the intel_pstate path exists, and then uses this code, and only then otherwise falls back to the old msr approach?

If not I'll try to integration your changes with that approach myself.

uarch-bench.sh Outdated
}


function is_even {
Copy link
Owner

Choose a reason for hiding this comment

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

Oops, thanks for deleting this, this was just spuriously left over from some testing I was doing :).

uarch-bench.sh Outdated
export VENDOR_ID=$(lscpu | grep 'Vendor ID' | egrep -o '[^ ]*$')
export MODEL_NAME=$(lscpu | grep 'Model name' | sed -n 's/Model name:\s*\(.*\)$/\1/p')
export SCALING_DRIVER=$(cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_driver)
export SCALING_GOVERNOR=$(cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_governor)
export NO_TURBO=$(cat /sys/devices/system/cpu/intel_pstate/no_turbo)
Copy link
Owner

Choose a reason for hiding this comment

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

See the review overall comments, but we should handle the case where this path doesn't exist.

@jinankjain
Copy link
Contributor Author

@travisdowns Updated the Pull Request with the suggested changes 😄

@travisdowns
Copy link
Owner

@jinankjain - thanks! I rebased and merged this in 277ff77. I think this PR won't close automatically due to the rebase (different SHAs), so closing this now.

Thanks again for your help and I look forward to any further contributions!

@jinankjain jinankjain deleted the intel_pstate branch February 19, 2018 21:36
@jinankjain
Copy link
Contributor Author

Thanks @travisdowns for merging this. I was thinking of working on porting this MacOSX. I saw that there is an open issue.

@travisdowns
Copy link
Owner

@jinankjain - that would be awesome!

The primary difficulty for "full functionality", I think, would be porting over the libpfc stuff which allows you to read the performance counters. If one were to simply use the default ClockTimer implementation, then the code is pretty much portable C++ (plus nasm asm which no doubt is available on OSX - the asm stuff should pretty much work "as is" since the sysv calling convention is used there also), and so the initial port should be straightforward. Beyond that there is the matter of disabling turbo and other frequency scaling (the stuff in uarch-bench.sh basically), where the details are no doubt different.

@jinankjain
Copy link
Contributor Author

@travisdowns How about using https://github.com/opcm/pcm instead of libpfc as it provides much generic interface for Mac, Windows or Linux etc?

@travisdowns
Copy link
Owner

travisdowns commented Feb 19, 2018

@jinankjain yes, sure. In fact I recall now that this was mentioned on the associated issue. I'm not sure if you can completely "replace" libpfc with pcm because it isn't clear if pcm offers an API to read counters directly from user-space using rdpmc[1], which is kind of the key feature of the libpfc integration, but you could certainly use the kernel driver parts to enable the counters to avoid re-inventing the wheel on the counter-programming side which is where the vast majority of the complexity lies.

In principle, the user-mode parts of libpfc (basically the PFC_END macro) should carry over pretty much unchanged in OSX since they are just very small inline assembly wrappers around the rdpmc instruction, which should be OS-independent (as long as the bit in CR4 is set that enables user-mode rdpmc access).


[1] Correct me if I'm wrong though! My current assumption is that reading the counters goes though an API which ultimately calls into the kernel and uses the functionality exposed by the kernel driver (or on Linux I think it can use the existing perf_events stuff).

@travisdowns travisdowns mentioned this pull request Feb 19, 2018
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