Hi Rafael, On Thu, May 18, 2017 at 6:08 PM, Rafael J. Wysocki <[email protected]> wrote: [..] >> static struct governor_attr rate_limit_us = __ATTR_RW(rate_limit_us); >> +static struct governor_attr iowait_boost_enable = >> __ATTR_RW(iowait_boost_enable); >> >> static struct attribute *sugov_attributes[] = { >> &rate_limit_us.attr, >> + &iowait_boost_enable.attr, >> NULL >> }; >> >> @@ -543,6 +574,9 @@ static int sugov_init(struct cpufreq_policy *policy) >> tunables->rate_limit_us *= lat; >> } >> >> + if (policy->iowait_boost_enable) >> + tunables->iowait_boost_enable = policy->iowait_boost_enable; > > Well, that's just > > tunables->iowait_boost_enable = policy->iowait_boost_enable; > > so I'm not sure about the role of the if ().
Yes you're right, will fix. > > Also, do we only need the policy field as an initial value here? > That's sort of confusing, because intel_pstate does iowait boosting in > the active mode too and then it is unconditional. > I didn't get your comment. Could you clarify what you meant by 'intel_pstate does iowait boosting in the active mode' ? Is there another path outside the governor where intel_pstate does iowait boosting, or are you referring to the hardware behavior? thanks, -Joel
