From patchwork Mon May 30 10:18:30 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Viresh Kumar X-Patchwork-Id: 68837 Delivered-To: patch@linaro.org Received: by 10.140.92.199 with SMTP id b65csp1326133qge; Mon, 30 May 2016 03:18:37 -0700 (PDT) X-Received: by 10.66.66.234 with SMTP id i10mr45631508pat.114.1464603517251; Mon, 30 May 2016 03:18:37 -0700 (PDT) Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id 187si4422994pff.129.2016.05.30.03.18.36; Mon, 30 May 2016 03:18:37 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-pm-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org; spf=pass (google.com: best guess record for domain of linux-pm-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-pm-owner@vger.kernel.org; dmarc=fail (p=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754541AbcE3KSf (ORCPT + 14 others); Mon, 30 May 2016 06:18:35 -0400 Received: from mail-pf0-f169.google.com ([209.85.192.169]:36408 "EHLO mail-pf0-f169.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754519AbcE3KSe (ORCPT ); Mon, 30 May 2016 06:18:34 -0400 Received: by mail-pf0-f169.google.com with SMTP id f144so50822119pfa.3 for ; Mon, 30 May 2016 03:18:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=utBobfcHpXnf7x5jJ4KxnOlyU5DLhHYnU+9BzRPkT5w=; b=WjdF4JQOaI4FV3939qayz/qHUJ4T/WK9Mg/uctbHffn1Lx/7xcNZIoYld4GDakZG3+ cRVdUECYfESuPY4I0zx1h7T5ZSDtVFWn1Cmxb/9x6NLfXbp9FDmX8NIbCfLt8ATz7yMn 8CVnpvc53i9rKATea5Q5A4BuAuoT/E+/eXSpw= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=utBobfcHpXnf7x5jJ4KxnOlyU5DLhHYnU+9BzRPkT5w=; b=DKxF8X55NbMQhJj4SjQfXH0Tiu/9PP2gWPVTK5XzstZC8NztbbT62c8VHwoYbyL/nF TfTblqU0m27f2U5+fCEK2RZU6Wn399RtHyH55m5Q/RzfzhcEP+2Vj8rYW2V20G4zRKFu HINHcEJJbnl35ATUCD6q42IsoDzYm0scuE/7JVmI4OyIcDAT4HfdIk5hH8IgbHlZehiP tIaHuFMh1xd4GLUumjdQ2JdqRRsRCnwn15TPmBtupr9r+D4WLOHOxNWDbrZf0kFDTcJ/ CpJJ5HTx1/Uz7OAZ1fGNukg51tm7EX8J+xtC79ciAZ0A4bPxH6ys+mZ0dBDyI99w8BLt iK6g== X-Gm-Message-State: ALyK8tKNRCXF5hnoXJG1lZPOzX1oUz6Wg0do9+PzaRf9HsV21jTWVdcMKSDvT8bCluCdK3yv X-Received: by 10.98.30.132 with SMTP id e126mr45881940pfe.109.1464603513642; Mon, 30 May 2016 03:18:33 -0700 (PDT) Received: from localhost ([122.167.19.26]) by smtp.gmail.com with ESMTPSA id a19sm31781393pfc.57.2016.05.30.03.18.32 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 30 May 2016 03:18:32 -0700 (PDT) Date: Mon, 30 May 2016 15:48:30 +0530 From: Viresh Kumar To: "Rafael J. Wysocki" Cc: Steve Muckle , Peter Zijlstra , Ingo Molnar , Linux Kernel Mailing List , "linux-pm@vger.kernel.org" , Vincent Guittot , Morten Rasmussen , Dietmar Eggemann , Juri Lelli , Patrick Bellasi , Michael Turquette Subject: Re: [PATCH v2 3/3] cpufreq: schedutil: map raw required frequency to driver frequency Message-ID: <20160530101830.GE30066@vireshk-i7> References: <1464231181-30741-1-git-send-email-smuckle@linaro.org> <1464231181-30741-4-git-send-email-smuckle@linaro.org> <20160526071629.GW17585@vireshk-i7> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org On 29-05-16, 02:40, Rafael J. Wysocki wrote: > I can't really parse the above question, so I'm not going to try to > answer it. :-) Sorry about that :( IOW, I think that we should make this change into the sched-governor (I will send a patch separately if you agree to this): And here is my reasoning behind this. Can you show me any case, where the above code (as present in mainline today) leads to a freq-change? I couldn't find any. Let me demonstrate. Following only talks about the fast-switch path, the other path is same as well. Suppose this is the current range of frequencies supported by a driver: 200, 400, 600, 800, 1000 (in MHz). And policy->cur = next_freq = 400 MHz. A.) Suppose that we change policy->min to 400 MHz from userspace. -> sugov_limits() This will find everything in order and simply set need_freq_update, without updating the frequency. On next util-callback, we will forcefully return true from sugov_should_update_freq() and reach sugov_update_commit(). We calculate next_freq and that comes to 400 MHz again (that's the case we are trying to target with the above code). With the current code, we will forcefully end up calling cpufreq_driver_fast_switch(). Because the new and current frequencies are same, cpufreq_driver->fast_switch() will simply return. NOTE: I also think that cpufreq_driver_fast_switch() should have a check like (policy->cur == target_freq). I will add that too, in case you agree. So, forcefully updating next_freq to UINT_MAX will end up wasting some cycles, but wouldn't do any useful stuff. B.) Suppose that we change policy->min to 600 MHz from userspace. -> sugov_limits() This will find that policy->cur is less than 600 and will set that to 600 MHz by calling __cpufreq_driver_target(). We will also set need_freq_update. Note that next_freq and policy->cur are not in sync anymore and perhaps this is the most important case for the above code. On next util-callback, we will forcefully return true from sugov_should_update_freq() and reach sugov_update_commit(). We calculate next_freq and lets say that comes to 400 MHz again (as that's the case we are trying to target with the above code). With the current code, we will forcefully end up calling cpufreq_driver_fast_switch(). Because next_freq() is not part of the new range, we will clamp it and set it to 600 MHz eventually. Again, next and current frequencies are same, cpufreq_driver->fast_switch() will simply return. So, forcefully updating next_freq to UINT_MAX will end up wasting some cycles here as well, but would do any useful stuff. What am I missing ? -- viresh -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c index 14c4aa25cc45..5934b14aa21c 100644 --- a/kernel/sched/cpufreq_schedutil.c +++ b/kernel/sched/cpufreq_schedutil.c @@ -66,11 +66,6 @@ static bool sugov_should_update_freq(struct sugov_policy *sg_policy, u64 time) if (unlikely(sg_policy->need_freq_update)) { sg_policy->need_freq_update = false; - /* - * This happens when limits change, so forget the previous - * next_freq value and force an update. - */ - sg_policy->next_freq = UINT_MAX; return true; }