Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752752AbcC3LbU (ORCPT ); Wed, 30 Mar 2016 07:31:20 -0400 Received: from mail-lf0-f65.google.com ([209.85.215.65]:34067 "EHLO mail-lf0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751756AbcC3LbS (ORCPT ); Wed, 30 Mar 2016 07:31:18 -0400 MIME-Version: 1.0 In-Reply-To: <20160330053011.GF8773@vireshk-i7> References: <7262976.zPkLj56ATU@vostro.rjw.lan> <6666532.7ULg06hQ7e@vostro.rjw.lan> <145931680.Kk1xSBT0Ro@vostro.rjw.lan> <20160330053011.GF8773@vireshk-i7> Date: Wed, 30 Mar 2016 13:31:16 +0200 X-Google-Sender-Auth: O4-eqjU9Oq-qEPCi-7EUxnFbs50 Message-ID: Subject: Re: [Update][PATCH v7 7/7] cpufreq: schedutil: New governor based on scheduler utilization data From: "Rafael J. Wysocki" To: Viresh Kumar Cc: "Rafael J. Wysocki" , Linux PM list , Juri Lelli , Steve Muckle , ACPI Devel Maling List , Linux Kernel Mailing List , Peter Zijlstra , Srinivas Pandruvada , Vincent Guittot , Michael Turquette , Ingo Molnar Content-Type: text/plain; charset=UTF-8 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1200 Lines: 39 [cut] > The current version of this looks good to me and takes care of all the issues I > raised earlier. Thanks. > >> +static int sugov_limits(struct cpufreq_policy *policy) >> +{ >> + struct sugov_policy *sg_policy = policy->governor_data; >> + >> + if (!policy->fast_switch_enabled) { >> + mutex_lock(&sg_policy->work_lock); >> + >> + if (policy->max < policy->cur) >> + __cpufreq_driver_target(policy, policy->max, >> + CPUFREQ_RELATION_H); >> + else if (policy->min > policy->cur) >> + __cpufreq_driver_target(policy, policy->min, >> + CPUFREQ_RELATION_L); >> + >> + mutex_unlock(&sg_policy->work_lock); >> + } >> + >> + sg_policy->need_freq_update = true; > > I am wondering why we need to do this for !fast_switch_enabled case? That will cause the rate limit to be ignored in the utilization update handler which may be necessary if it is set to a relatively large value (like 1 s). >> + return 0; >> +} > > Apart from that: > > Acked-by: Viresh Kumar Thanks, Rafael