From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.1 required=3.0 tests=DKIM_INVALID,DKIM_SIGNED, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 50ACCC432C3 for ; Fri, 15 Nov 2019 17:44:26 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 273DC2072B for ; Fri, 15 Nov 2019 17:44:26 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=fail reason="signature verification failed" (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="QHwa1471" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725907AbfKORoZ (ORCPT ); Fri, 15 Nov 2019 12:44:25 -0500 Received: from bombadil.infradead.org ([198.137.202.133]:49488 "EHLO bombadil.infradead.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725848AbfKORoZ (ORCPT ); Fri, 15 Nov 2019 12:44:25 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20170209; h=In-Reply-To:Content-Type:MIME-Version :References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=Elg7Tj8yjS/b/ScYw/Zf/Uu5gZwsbcJit98EjXWYU08=; b=QHwa1471lA8KkwjDIvNIB3sRN HRVpVP0oiyqHlfm/pRAycgcBULkejAuVLVsv/za4wK6oe7fFSiyCYxtA08V6Ps2efmGqKZzX//t4F wEso6OHL4yD/69r4T9Ixj6snzrB8bAK5B0mDOi/RAd3rSOvmAVRe5D2Wi/JcTaFgcFg6YC/DyImFb dpA0GBz6XRDZzZzod2xAc/YEpwA3OolGB0XyVpnTLSp2Oh6TJIVF9EhoRt0aYP+UHKiJwcoJDdPDf VRe11Q7oVDQ3I8/F7Je+uc07+02jmIzBGR9pIe0b4Znp2ojwyXLjYMcPvsrjuh+5fJeoEcAv9ycdc wexEl+qpw==; Received: from j217100.upc-j.chello.nl ([24.132.217.100] helo=noisy.programming.kicks-ass.net) by bombadil.infradead.org with esmtpsa (Exim 4.92.3 #3 (Red Hat Linux)) id 1iVfdX-0008VF-1i; Fri, 15 Nov 2019 17:43:59 +0000 Received: from hirez.programming.kicks-ass.net (hirez.programming.kicks-ass.net [192.168.1.225]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by noisy.programming.kicks-ass.net (Postfix) with ESMTPS id 91517303D9F; Fri, 15 Nov 2019 18:42:47 +0100 (CET) Received: by hirez.programming.kicks-ass.net (Postfix, from userid 1000) id A520B2B130F61; Fri, 15 Nov 2019 18:43:55 +0100 (CET) Date: Fri, 15 Nov 2019 18:43:55 +0100 From: Peter Zijlstra To: Vincent Guittot Cc: linux-kernel , Ingo Molnar , Dietmar Eggemann , Juri Lelli , Steven Rostedt , Mel Gorman , Doug Smythies , "open list:THERMAL" , Linus Torvalds , Thomas Gleixner , Sargun Dhillon , Tejun Heo , Xie XiuQi , xiezhipeng1@huawei.com, Srinivas Pandruvada Subject: Re: [PATCH v4] sched/freq: move call to cpufreq_update_util Message-ID: <20191115174355.GP4131@hirez.programming.kicks-ass.net> References: <1573751251-3505-1-git-send-email-vincent.guittot@linaro.org> <20191115132520.GJ4131@hirez.programming.kicks-ass.net> <20191115151220.GO4131@hirez.programming.kicks-ass.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org On Fri, Nov 15, 2019 at 04:31:35PM +0100, Vincent Guittot wrote: > > @@ -7476,10 +7477,14 @@ static void update_blocked_averages(int cpu) > > * list_add_leaf_cfs_rq() for details. > > */ > > for_each_leaf_cfs_rq_safe(rq, cfs_rq, pos) { > > + bool last = cfs_rq == &rq->cfs; > > struct sched_entity *se; > > > > - if (update_cfs_rq_load_avg(cfs_rq_clock_pelt(cfs_rq), cfs_rq)) > > + if (update_cfs_rq_load_avg(cfs_rq_clock_pelt(cfs_rq), cfs_rq)) { > > update_tg_load_avg(cfs_rq, 0); > > + if (last) > > using this last make code more readable > > > + decayed = true; > > + } > > > > /* Propagate pending load changes to the parent, if any: */ > > se = cfs_rq->tg->se[cpu]; > > @@ -7490,7 +7495,7 @@ static void update_blocked_averages(int cpu) > > * There can be a lot of idle CPU cgroups. Don't let fully > > * decayed cfs_rqs linger on the list. > > */ > > - if (cfs_rq_is_decayed(cfs_rq)) > > + if (!last && cfs_rq_is_decayed(cfs_rq)) > > list_del_leaf_cfs_rq(cfs_rq); > > Keeping root cfs in the list will not change anything now that > cfs_rq_util_change is in update_load_avg() > cfs_rq_util_change will not be called Oh but it does, since it will then keep triggering that hunk above on every period. > > > > /* Don't need periodic decay once load/util_avg are null */ > > @@ -7498,6 +7503,9 @@ static void update_blocked_averages(int cpu) > > done = false; > > } > > > > + if (decayed || done) > > I'm not sure to get why you want to call cpufreq when done is true > which means that everything reaches 0 > Why do you prefer to use done instead of ORing the decay of rt, dl, > irq and cfs ? > > > + cpufreq_update_util(rq, 0); Because we don't care about the rt,dl,irq decay anywhere else either. We only call cpufreq_update_util() for rq->cfs changes. Also, as I argued, realistically rt,dl and cfs decay on the same edge, so aside from some fuzz on the first period, they're all the same. But even if they were not, why would we care about their exact edges here when we do no anywhere else. Not caring reduces the number of cpufreq_update_util() calls to one per period, instead of potentially many more. Doing the || done ensures never miss the all 0 case.