On Tue, Jul 11, 2017 at 7:14 AM, Joel Fernandes <joe...@google.com> wrote: [..] >>> + } >>> } else if (sg_cpu->iowait_boost) { >>> s64 delta_ns = time - sg_cpu->last_update; >>> >>> /* Clear iowait_boost if the CPU apprears to have been idle. >>> */ >>> if (delta_ns > TICK_NSEC) >>> sg_cpu->iowait_boost = 0; >>> + >>> + /* >>> + * Since we don't decay iowait_boost when its consumed during >>> + * the previous SCHED_CPUFREQ_IOWAIT update, decay it now. >>> + */ >>> + if (sg_cpu->prev_iowait_boost) { >> >> SCHED_CPUFREQ_IOWAIT flag is set only by CFS from the enqueue_task() and in >> many >> cases we call the util hook twice from the same enqueue_task() instance >> before >> returning (2nd one after updating util). And in such cases we will set >> iowait_boost as 0 on the second call. >> >> Have you ever seen two consecutive calls to sugov_set_iowait_boost() with >> IOWAIT >> flag set ? Can we get the ratio of that against the other case where we have >> IOWAIT flag set in first call, followed by one or more non-IOWAIT calls and >> then >> IOWAIT again ? >> >> I am asking because if the calls with IOWAIT flag aren't consecutive then we >> will make iowait_boost as 0 in the next non-IOWAIT call. > > Yes, I've seen that happen in my testing (consecutive iowait). I > haven't seen the other case where you have IOWAIT followed by > non-IOWAIT for a repeated set of IOWAIT requests. Would you more > comfortable if we moved sugov_set_iowait_boost() after the > sugov_should_update_freq() ? That way if there are consecutive > requests in the same path, then it most likely rate-limiting will > prevent such updates. I will also try to collect some stats as you > suggested to see if how often if at all this can happen.
Just to be more clear, I was saying that I've only seen repeated IOWAIT requests in the update path, not IOWAIT followed by non-IOWAIT cpufreq updates for a repeated sequence of IOWAIT wakeups. thanks, -Joel