Received: by 2002:a25:8b91:0:0:0:0:0 with SMTP id j17csp9457966ybl; Fri, 17 Jan 2020 12:21:35 -0800 (PST) X-Google-Smtp-Source: APXvYqyYiiu6jHdzXyNBbGnMfJCmPLwGsCUpGeBBVmCzkZJYFqzwlw9+8qABWonbMKinqhYUJXx+ X-Received: by 2002:a9d:6745:: with SMTP id w5mr7589990otm.52.1579292495417; Fri, 17 Jan 2020 12:21:35 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1579292495; cv=none; d=google.com; s=arc-20160816; b=nh3kr9SFy2VpgKWkfU0IV0G8ydvWEqPKHA9FRY6nDRf4KqZUM47tJBCOBwUoF/x+ed vtDk/EqfSRTXPkXQ9Bm5CBa9OoUYNY70+qWWsRdErwSF6SCK7sA65LajQKmjfEqMnAsE WRgtryh/o8tpLPpckTRt+6n4tBiMv7/aGSD9SHTuYUVmcU7drwl0TPtTQesbOljEjN2t qEGM9/UtC2oCyrHt27CXVb81JU3xp2FrhapNfqTpFmgNzBegIzht4sAO98bNZu4LEKA+ Bb+O2QguLZ4/19njzgiJbJEAWSnaaXAE+kgSInAkNnxuZixyThw/pjvqc659qwLhZkHU gDfA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:content-transfer-encoding:in-reply-to :mime-version:user-agent:date:message-id:from:cc:references:to :subject:dkim-signature; bh=NlHGXeYVtUWpsui+87kvKh+ZFIYbyJG1G/B6RvZSgI4=; b=EfhljMc8IdY0cqtgqtB3v0FWf2VT/0Zs3Vv27r0wq+vOz675u7dtl50taot41I5fGG +etWR4xhXtG5qaTipFNsks+hE1BSeoRZ/o1NpHQhLPZ4i8o0TVCv2kmuSBJkUy3M+R0b jXHUQRQ2ctR1bZMjLiGcakGhRyJuE7hGmXhGAT9MWKcH6VrlfPm08Pd5GeX5brz6mpFj 0yyvWj7Jg5xn+VeE4+FziYOYtpWgH42yMeOmo6qSsao34m1JjRy6f/5+lkQU8JNzH8wU 3Iwv3I+kLXScmZUFkKQnJCm2QseZhCSlHKHXS+KnlZKuP4cxn1MQbDuvC8gBAdryqTAR ySFw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=BVelXQe0; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id 194si13727826oii.2.2020.01.17.12.21.20; Fri, 17 Jan 2020 12:21:35 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@linaro.org header.s=google header.b=BVelXQe0; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729612AbgAQUUS (ORCPT + 99 others); Fri, 17 Jan 2020 15:20:18 -0500 Received: from mail-qk1-f196.google.com ([209.85.222.196]:36381 "EHLO mail-qk1-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727519AbgAQUUR (ORCPT ); Fri, 17 Jan 2020 15:20:17 -0500 Received: by mail-qk1-f196.google.com with SMTP id a203so23992810qkc.3 for ; Fri, 17 Jan 2020 12:20:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=subject:to:references:cc:from:message-id:date:user-agent :mime-version:in-reply-to:content-transfer-encoding; bh=NlHGXeYVtUWpsui+87kvKh+ZFIYbyJG1G/B6RvZSgI4=; b=BVelXQe0sD8MLWCLMIL1DKUz1Xw93+yH6cJlCQrZoFBfXPuJ53w7HCF1JsIYWv6Zvg Xi5DWaD1do/bggiQDpAs3islaycY3yCg7AIwsDNl7UeNuajzUlnnNfvPRzM7gKV5IPzQ IuxbiP4DJUQW93trU8b6RN2pA9yiIsfivN+E7JERSjl83ImVowtrDgrDz5qzoX1E/rWW TOCnC+00iyM0Fkm+nNhu8nAD7IExOP9DDrW18xLwkonekEYvFchYBpJ9NsfzVzi+O160 c2SqoqDlr41mIFhV7yW95DrCFwLt77lD3Go9Asu6RY4L+clzMXTFW30CsJlGpB5Ocvsu Wr8A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:cc:from:message-id:date :user-agent:mime-version:in-reply-to:content-transfer-encoding; bh=NlHGXeYVtUWpsui+87kvKh+ZFIYbyJG1G/B6RvZSgI4=; b=OKS8V+nq1GmVuzL5ne3bjex9DjZgrsrugcj+2kWaJn6NngyTZvidxCZjdiO7xYc4pg Ut8ni1ry37nZQLk1V++jWk37YD0skKdknwS8VLyT/SGKZnFYUgT+QmjsmzRm3Xe9sRxh Mp4+JH1v4g4pLD+dlfO3STXockS5TO3Fwus3YpFrmpNhQGXkYgzBNR0Hbh3oqegIBM9k BncUgq1k/qeE4Z4QlpDdYfDrnl3sI9MzLSF7UMDLWg6zwTD7a8eidLJtJrC1HOjvXVw9 EGIACI8Nb1vy8FqCZxP3aWPmdnaXhnq+aWETnPS+Gk9fbww9TMauUxJrZf84kEpUTQGB ebGg== X-Gm-Message-State: APjAAAUK6AwoAHebmhy3Pg/2JZFkk9MO9zpCLVnEvnNaLsYbscyGhI+j TNZlLXLwUXk9b0Oee+PX3a6IwcgMPaE= X-Received: by 2002:ae9:ef50:: with SMTP id d77mr34398666qkg.71.1579292416084; Fri, 17 Jan 2020 12:20:16 -0800 (PST) Received: from [192.168.1.169] (pool-71-255-246-27.washdc.fios.verizon.net. [71.255.246.27]) by smtp.gmail.com with ESMTPSA id j15sm13152574qtn.37.2020.01.17.12.20.14 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 17 Jan 2020 12:20:15 -0800 (PST) Subject: Re: [Patch v8 4/7] sched/fair: Enable periodic update of average thermal pressure To: Peter Zijlstra References: <1579031859-18692-1-git-send-email-thara.gopinath@linaro.org> <1579031859-18692-5-git-send-email-thara.gopinath@linaro.org> <20200116151502.GQ2827@hirez.programming.kicks-ass.net> Cc: mingo@redhat.com, ionela.voinescu@arm.com, vincent.guittot@linaro.org, dietmar.eggemann@arm.com, rui.zhang@intel.com, qperret@google.com, daniel.lezcano@linaro.org, viresh.kumar@linaro.org, linux-kernel@vger.kernel.org, amit.kachhap@gmail.com, javi.merino@kernel.org, amit.kucheria@verdurent.com From: Thara Gopinath Message-ID: <5E2216FD.5040903@linaro.org> Date: Fri, 17 Jan 2020 15:20:13 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.5.1 MIME-Version: 1.0 In-Reply-To: <20200116151502.GQ2827@hirez.programming.kicks-ass.net> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 01/16/2020 10:15 AM, Peter Zijlstra wrote: > On Tue, Jan 14, 2020 at 02:57:36PM -0500, Thara Gopinath wrote: >> Introduce support in CFS periodic tick and other bookkeeping apis >> to trigger the process of computing average thermal pressure for a >> cpu. Also consider avg_thermal.load_avg in others_have_blocked >> which allows for decay of pelt signals. >> >> Signed-off-by: Thara Gopinath >> --- >> kernel/sched/fair.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c >> index 8da0222..311bb0b 100644 >> --- a/kernel/sched/fair.c >> +++ b/kernel/sched/fair.c >> @@ -7470,6 +7470,9 @@ static inline bool others_have_blocked(struct rq *rq) >> if (READ_ONCE(rq->avg_dl.util_avg)) >> return true; >> >> + if (READ_ONCE(rq->avg_thermal.load_avg)) >> + return true; >> + > > Given that struct sched_avg is 1 cacheline, the above is a pointless > guaranteed cacheline miss if the arch doesn't > CONFIG_HAVE_SCHED_THERMAL_PRESSURE. Thanks for the review Peter. I see your suggestion in Patch 1 to fix this issue. Will send out the next version implementing it. > >> #ifdef CONFIG_HAVE_SCHED_AVG_IRQ >> if (READ_ONCE(rq->avg_irq.util_avg)) >> return true; >> @@ -7495,6 +7498,7 @@ static bool __update_blocked_others(struct rq *rq, bool *done) >> { >> const struct sched_class *curr_class; >> u64 now = rq_clock_pelt(rq); >> + unsigned long thermal_pressure = arch_cpu_thermal_pressure(cpu_of(rq)); >> bool decayed; >> >> /* >> @@ -7505,6 +7509,8 @@ static bool __update_blocked_others(struct rq *rq, bool *done) >> >> decayed = update_rt_rq_load_avg(now, rq, curr_class == &rt_sched_class) | >> update_dl_rq_load_avg(now, rq, curr_class == &dl_sched_class) | >> + update_thermal_load_avg(rq_clock_task(rq), rq, >> + thermal_pressure) | >> update_irq_load_avg(rq, 0); >> >> if (others_have_blocked(rq)) > > That there indentation trainwreck is a reason to rename the function. > > decayed = update_rt_rq_load_avg(now, rq, curr_class == &rt_sched_class) | > update_dl_rq_load_avg(now, rq, curr_class == &dl_sched_class) | > update_thermal_load_avg(rq_clock_task(rq), rq, thermal_pressure) | > update_irq_load_avg(rq, 0); > > Is much better. Did you intend to say here to rename update_thermal_load_avg to something else ? > > But now that you made me look at that, I noticed it's using a different > clock -- it is _NOT_ using now/rq_clock_pelt(), which means it'll not be > in sync with the other averages. > > Is there a good reason for that? So I guess as Vincent has replied in his email, rq_clock_pelt adjusts clock for frequency and cpu capacity invariance. Thermal pressure signal is already accounted for it when updated from the thermal framework. > >> @@ -10275,6 +10281,7 @@ static void task_tick_fair(struct rq *rq, struct task_struct *curr, int queued) >> { >> struct cfs_rq *cfs_rq; >> struct sched_entity *se = &curr->se; >> + unsigned long thermal_pressure = arch_cpu_thermal_pressure(cpu_of(rq)); >> >> for_each_sched_entity(se) { >> cfs_rq = cfs_rq_of(se); >> @@ -10286,6 +10293,7 @@ static void task_tick_fair(struct rq *rq, struct task_struct *curr, int queued) >> >> update_misfit_status(curr, rq); >> update_overutilized_status(task_rq(curr)); >> + update_thermal_load_avg(rq_clock_task(rq), rq, thermal_pressure); >> } > > I'm thinking this is the wrong place; should this not be in > scheduler_tick(), right before calling sched_class::task_tick() ? Surely > any execution will affect thermals, not only fair class execution. I read all other comments from others on this. I agree. I will move this to scheduler_tick. > -- Warm Regards Thara