-
Notifications
You must be signed in to change notification settings - Fork 15
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
Fixes for the next maintenance update #126
Conversation
…e supported in the get-Functions too. This should fix the problem with some special Azure VMs (E20d_v5) on newer SLES SPs (jsc#SAPSOL-110)
system/cpu.go
Outdated
state, _ := GetSysString(path.Join(cpuDirSys, entry.Name(), "cpuidle", centry.Name(), "disable")) | ||
// save latency states for 'revert' | ||
// savedStates = "cpu1:state0:0 cpu1:state1:0" | ||
savedStates = savedStates + " " + entry.Name() + ":" + centry.Name() + ":" + state |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The first time that savedStates will be overwrite there will have at first a empty string than a space, is this produce something wrong with the result? If so is possible to remove the first whitespace on the return at line 357 with savedStates[1:]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the whitespace at the beginning of the string is not a problem as the string will be later used in function SetForceLatency(..) with strings.Fields(savedStates), which will split the string into pieces around consecutive white space characters.
system/cpu.go
Outdated
} | ||
for _, entry := range dirCont { | ||
// cpu0 ... cpuXY | ||
if isCPU.MatchString(entry.Name()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't create a variable for entry.Name()
? entry.Name()
will be called multiple time and this can increase the time computing of the block because entry.Name() have is own time to compute. We are probably talking about insignificant nanoseconds, but you never know :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, good idea. See new commit
system/cpu.go
Outdated
supported = true | ||
for _, centry := range cpudirCont { | ||
// state0 ... stateXY | ||
if isState.MatchString(centry.Name()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as entry.Name()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done with new commit, thanks
Co-authored-by: GabrielePuliti <[email protected]>
change handling of the performance options (cpu governor, perf bias and force latency)
Check, if the settings are supported, in the get-Functions too.
This should fix the problem with some special Azure VMs (E20d_v5) on newer SLES SPs (jsc#SAPSOL-110)
support inline comments in /etc/sysconfig/saptune
fix formating and typos