From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752482AbdKFJaP (ORCPT ); Mon, 6 Nov 2017 04:30:15 -0500 Received: from mail-lf0-f52.google.com ([209.85.215.52]:50261 "EHLO mail-lf0-f52.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750939AbdKFJaM (ORCPT ); Mon, 6 Nov 2017 04:30:12 -0500 X-Google-Smtp-Source: ABhQp+Q0nSOwRwMT/IFm0AWPgZBUhcjNoY4/C8yU+yNMezgmQTLIoEOZPt8N5pVlJenz2TJcV1prYA== From: Uladzislau Rezki X-Google-Original-From: Uladzislau Rezki Date: Mon, 6 Nov 2017 10:29:52 +0100 To: Vincent Guittot Cc: Joel Fernandes , Steve Muckle , Viresh Kumar , LKML , "Rafael J . Wysocki" , Ingo Molnar , Peter Zijlstra , "Cc: Srinivas Pandruvada" , "Cc: Len Brown" , "Cc: Juri Lelli" , "Cc: Patrick Bellasi" , "Cc: Brendan Jackman" , "Cc: Chris Redpath" , "Cc: Atish Patra" , "Cc: Dietmar Eggemann" , "Cc: Morten Ramussen" , "Cc: Frederic Weisbecker" , "Cc: Thomas Gleixner" , "Cc: EAS Dev" , "Cc: Android Kernel" , Steven Rostedt Subject: Re: [PATCH RFC 2/5] sched/fair: Skip frequency update if CPU about to idle Message-ID: <20171106092952.5nybexs2zdwuizgs@pc636> References: <20171028095941.4773-1-joelaf@google.com> <20171028095941.4773-3-joelaf@google.com> <20171030120709.GO4240@vireshk-i7> <6ed7d641-4fa2-77e1-9294-64d51193c786@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, Nov 06, 2017 at 09:00:45AM +0100, Vincent Guittot wrote: > Hi Joel, > > On 4 November 2017 at 06:44, Joel Fernandes wrote: > > On Wed, Nov 1, 2017 at 12:35 PM, Steve Muckle wrote: > >> On 10/30/2017 12:02 PM, Joel Fernandes wrote: > >>>> > >>>> Also, this more looks like a policy decision. Will it be better to > >>>> put that directly into schedutil? Like this: > >>>> > >>>> if (cpu_idle()) > >>>> "Don't change the freq"; > >>>> > >>>> Will something like that work? > >>> > >>> > >>> I thought about this and I think it wont work very well. In the > >>> dequeue path we're still running the task being dequeued so the CPU is > >>> not yet idle. What is needed here IMO is a notion that the CPU is > >>> possibly about to idle and we can get predict that from the dequeue > >>> path of the CFS class. > >>> > >>> Also just looking at whether the CPU is currently idle or not in the > >>> governor doesn't help to differentiate between say the dequeue path / > >>> tick path. Both of which can occur when the CPU is not idle. > >>> > >>> Any thoughts about this? > >> > >> > >> Also if it really is the case that this bit of policy is universally > >> desirable, I'd think it is better to do this in the scheduler and avoid a > >> needless trip through a fn pointer out to schedutil for performance reasons. > > > > I agree. > > > > Peter, what do you think? Are you Ok with the approach of this patch > > (preventing of the cpufreq update call during dequeue)? > > I'm not really convinced that we should do change OPP at dequeue. > Although i agree that it makes perfect sense to prevent increasing OPP > just before going idle for mp3 playback, i'm not sure that this is > always the case especially for performance oriented use case because > we delay the OPP increase and as a result the responsiveness of the > CPU > In fact this decision really depends about how long we are going to > sleep. If the cpu will wakes up in few ms, it's worth already > increasing the frequency when utilization is above the threshold > because it will not decreases enough to go back to lower OPP. At the > opposite, if the cpu will not wake up shortly skipping OPP change can > make sense. > > Regarding the reduction of the number of OPP changes, will the > util_est feature provide the same amount of reduction by directly > providing the estimated max utilization ? > > Just to say that IMHO skipping or not OPP change at dequeue is a > policy decision and not a generic one > Indeed. Otherwise we may end up in a situation of handling corner cases in a scheduler when it comes to OPP updates. I also agree that it is up to policy to do update or not. -- Uladzislau Rezki