From patchwork Tue Feb 6 10:55:27 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Claudio Scordino X-Patchwork-Id: 10202759 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 02DD2605BA for ; Tue, 6 Feb 2018 10:55:44 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0C4E528A31 for ; Tue, 6 Feb 2018 10:55:44 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 0091228A38; Tue, 6 Feb 2018 10:55:43 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4877628A35 for ; Tue, 6 Feb 2018 10:55:43 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752691AbeBFKzb (ORCPT ); Tue, 6 Feb 2018 05:55:31 -0500 Received: from mail-wr0-f196.google.com ([209.85.128.196]:43548 "EHLO mail-wr0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752379AbeBFKz2 (ORCPT ); Tue, 6 Feb 2018 05:55:28 -0500 Received: by mail-wr0-f196.google.com with SMTP id b52so1396855wrd.10 for ; Tue, 06 Feb 2018 02:55:28 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=evidence-eu-com.20150623.gappssmtp.com; s=20150623; h=subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=HpOfaSgKp1Ox44Q6RYulN+5Q8PEqBZ/Gat4Bg5+nD4s=; b=dV6bVdpZtv/lqK+6TBFWMgT4E7xSG52uYstHEj+/n2jLIG8Dfu2K44Oo6spaRGJq31 HL1kqkOiJHXF/MqMhIh0/i+4T8nj9cPwcgvrL4EEETzs6XiPTCMS8NErOs+Oqq9/pO5E SXfNWV1XGEnl/gEL767hCNKpzrm2tnLFC+jELnE8Gxtl4SgoY5NY1gM6yDZCiE/ob8pB i0fuLt3pBiQqISWT0H+bXLABChR6/DqRjQmcIZ1VwYIi/eg++tNSa0+BnOj2IPEilrtI mahM3nen9DSShZLRRqSsrj4REg9VroAvziyA4TCfOMF7N3siwXB6NrwJDu4z6nENVKQ7 qSIg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=HpOfaSgKp1Ox44Q6RYulN+5Q8PEqBZ/Gat4Bg5+nD4s=; b=XF6GcYCES/OffIfD5tOKtKN5m1VPCobK+K6dWNaWNg2/luwU0bR5H7mNGiwjQ7G00p ypFjjkzYxlJSbK2/FvHuU0OClIsf1o2twXkjVI6R/l0G1anxzmrHSbzzYWZ7MZvjENHA C4s2Fx1pz+FcN9U+5AiaxY1cX9mPdmxyZ0WPNLYZmnyrFPBmvZHOtv4AaWrXj3nYZANe i3Dy8WmfHUcQPDwtcyE+pIjzhHZ8hxEBT3BpMpRzLzj55UpnGx4XZizGm3Y2VQdmokgH uqKyc2Mdwu+hFktxkElqIOUIKKR0QkQBlIU/O2Dw5v0f4AIL8NnkNdMi+UzdE3h4syIK kkeQ== X-Gm-Message-State: APf1xPBosZz8kbkoC8L4gL085saob1oOOfiiFH7b1R5d4OaZZHiGZfiS 3XEoE/T8ywf/92hVi8hk+GgDgg== X-Google-Smtp-Source: AH8x225wqktLGhNH6yM1dDpEzW19OGzLymKktLH/68oNDsZ6kImok/ygbmnzKX0UlF2BXhPPJgggLQ== X-Received: by 10.223.132.67 with SMTP id 61mr1831251wrf.120.1517914527396; Tue, 06 Feb 2018 02:55:27 -0800 (PST) Received: from [192.168.10.160] (host92-93-static.8-79-b.business.telecomitalia.it. [79.8.93.92]) by smtp.gmail.com with ESMTPSA id y55sm17066693wry.87.2018.02.06.02.55.26 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 06 Feb 2018 02:55:26 -0800 (PST) Subject: Re: [PATCH v3 0/6] cpufreq: schedutil: fixes for flags updates To: Peter Zijlstra Cc: Patrick Bellasi , linux-kernel@vger.kernel.org, linux-pm@vger.kernel.org, Ingo Molnar , "Rafael J . Wysocki" , Viresh Kumar , Vincent Guittot , Dietmar Eggemann , Morten Rasmussen , Juri Lelli , Todd Kjos , Joel Fernandes References: <20171130114723.29210-1-patrick.bellasi@arm.com> <20171220153029.dqrtjbyowhqdl56r@hirez.programming.kicks-ass.net> From: Claudio Scordino Message-ID: Date: Tue, 6 Feb 2018 11:55:27 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: <20171220153029.dqrtjbyowhqdl56r@hirez.programming.kicks-ass.net> Content-Language: en-US Sender: linux-pm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Peter, Il 20/12/2017 16:30, Peter Zijlstra ha scritto: > > So I ended up with the below (on top of Juri's cpufreq-dl patches). > > It compiles, but that's about all the testing it had. > > --- a/include/linux/sched/cpufreq.h > +++ b/include/linux/sched/cpufreq.h > @@ -8,9 +8,7 @@ > * Interface between cpufreq drivers and the scheduler: > */ > > -#define SCHED_CPUFREQ_RT (1U << 0) > -#define SCHED_CPUFREQ_DL (1U << 1) > -#define SCHED_CPUFREQ_IOWAIT (1U << 2) > +#define SCHED_CPUFREQ_IOWAIT (1U << 0) > > #ifdef CONFIG_CPU_FREQ > struct update_util_data { > --- a/kernel/sched/cpufreq_schedutil.c > +++ b/kernel/sched/cpufreq_schedutil.c > @@ -63,7 +63,6 @@ struct sugov_cpu { > unsigned long util_cfs; > unsigned long util_dl; > unsigned long max; > - unsigned int flags; > > /* The field below is for single-CPU policies only. */ > #ifdef CONFIG_NO_HZ_COMMON > @@ -188,17 +187,23 @@ static void sugov_get_util(struct sugov_ > > static unsigned long sugov_aggregate_util(struct sugov_cpu *sg_cpu) > { > + unsigned long util = sg_cpu->util_cfs + sg_cpu->util_dl; > + struct rq *rq = cpu_rq(sg_cpu->cpu); > + > + if (rq->rt.rt_nr_running) > + util = sg_cpu->max; > + > /* > * Ideally we would like to set util_dl as min/guaranteed freq and > * util_cfs + util_dl as requested freq. However, cpufreq is not yet > * ready for such an interface. So, we only do the latter for now. > */ > - return min(sg_cpu->util_cfs + sg_cpu->util_dl, sg_cpu->max); > + return min(util, sg_cpu->max); > } > > -static void sugov_set_iowait_boost(struct sugov_cpu *sg_cpu, u64 time) > +static void sugov_set_iowait_boost(struct sugov_cpu *sg_cpu, u64 time, unsigned int flags) > { > - if (sg_cpu->flags & SCHED_CPUFREQ_IOWAIT) { > + if (flags & SCHED_CPUFREQ_IOWAIT) { > if (sg_cpu->iowait_boost_pending) > return; > > @@ -267,12 +272,11 @@ static void sugov_update_single(struct u > { > struct sugov_cpu *sg_cpu = container_of(hook, struct sugov_cpu, update_util); > struct sugov_policy *sg_policy = sg_cpu->sg_policy; > - struct cpufreq_policy *policy = sg_policy->policy; > unsigned long util, max; > unsigned int next_f; > bool busy; > > - sugov_set_iowait_boost(sg_cpu, time); > + sugov_set_iowait_boost(sg_cpu, time, flags); > sg_cpu->last_update = time; > > if (!sugov_should_update_freq(sg_policy, time)) > @@ -280,25 +284,22 @@ static void sugov_update_single(struct u > > busy = sugov_cpu_is_busy(sg_cpu); > > - if (flags & SCHED_CPUFREQ_RT) { > - next_f = policy->cpuinfo.max_freq; > - } else { > - sugov_get_util(sg_cpu); > - max = sg_cpu->max; > - util = sugov_aggregate_util(sg_cpu); > - sugov_iowait_boost(sg_cpu, &util, &max); > - next_f = get_next_freq(sg_policy, util, max); > - /* > - * Do not reduce the frequency if the CPU has not been idle > - * recently, as the reduction is likely to be premature then. > - */ > - if (busy && next_f < sg_policy->next_freq) { > - next_f = sg_policy->next_freq; > + sugov_get_util(sg_cpu); > + max = sg_cpu->max; > + util = sugov_aggregate_util(sg_cpu); > + sugov_iowait_boost(sg_cpu, &util, &max); > + next_f = get_next_freq(sg_policy, util, max); > + /* > + * Do not reduce the frequency if the CPU has not been idle > + * recently, as the reduction is likely to be premature then. > + */ > + if (busy && next_f < sg_policy->next_freq) { > + next_f = sg_policy->next_freq; > > - /* Reset cached freq as next_freq has changed */ > - sg_policy->cached_raw_freq = 0; > - } > + /* Reset cached freq as next_freq has changed */ > + sg_policy->cached_raw_freq = 0; > } > + > sugov_update_commit(sg_policy, time, next_f); > } > > @@ -314,6 +315,9 @@ static unsigned int sugov_next_freq_shar > unsigned long j_util, j_max; > s64 delta_ns; > > + if (j_sg_cpu != sg_cpu) > + sugov_get_util(j_sg_cpu); > + > /* > * If the CFS CPU utilization was last updated before the > * previous frequency update and the time elapsed between the > @@ -327,12 +331,7 @@ static unsigned int sugov_next_freq_shar > if (delta_ns > TICK_NSEC) { > j_sg_cpu->iowait_boost = 0; > j_sg_cpu->iowait_boost_pending = false; > - j_sg_cpu->util_cfs = 0; > - if (j_sg_cpu->util_dl == 0) > - continue; > } > - if (j_sg_cpu->flags & SCHED_CPUFREQ_RT) > - return policy->cpuinfo.max_freq; > > j_max = j_sg_cpu->max; > j_util = sugov_aggregate_util(j_sg_cpu); > @@ -357,17 +356,11 @@ static void sugov_update_shared(struct u > raw_spin_lock(&sg_policy->update_lock); > > sugov_get_util(sg_cpu); > - sg_cpu->flags = flags; > - > - sugov_set_iowait_boost(sg_cpu, time); > + sugov_set_iowait_boost(sg_cpu, time, flags); > sg_cpu->last_update = time; > > if (sugov_should_update_freq(sg_policy, time)) { > - if (flags & SCHED_CPUFREQ_RT) > - next_f = sg_policy->policy->cpuinfo.max_freq; > - else > - next_f = sugov_next_freq_shared(sg_cpu, time); > - > + next_f = sugov_next_freq_shared(sg_cpu, time); > sugov_update_commit(sg_policy, time, next_f); > } > > @@ -678,7 +671,6 @@ static int sugov_start(struct cpufreq_po > memset(sg_cpu, 0, sizeof(*sg_cpu)); > sg_cpu->cpu = cpu; > sg_cpu->sg_policy = sg_policy; > - sg_cpu->flags = 0; > sg_cpu->iowait_boost_max = policy->cpuinfo.max_freq; > } > > --- a/kernel/sched/deadline.c > +++ b/kernel/sched/deadline.c > @@ -87,7 +87,7 @@ void __add_running_bw(u64 dl_bw, struct > SCHED_WARN_ON(dl_rq->running_bw < old); /* overflow */ > SCHED_WARN_ON(dl_rq->running_bw > dl_rq->this_bw); > /* kick cpufreq (see the comment in kernel/sched/sched.h). */ > - cpufreq_update_util(rq_of_dl_rq(dl_rq), SCHED_CPUFREQ_DL); > + cpufreq_update_util(rq_of_dl_rq(dl_rq), 0); > } > > static inline > @@ -101,7 +101,7 @@ void __sub_running_bw(u64 dl_bw, struct > if (dl_rq->running_bw > old) > dl_rq->running_bw = 0; > /* kick cpufreq (see the comment in kernel/sched/sched.h). */ > - cpufreq_update_util(rq_of_dl_rq(dl_rq), SCHED_CPUFREQ_DL); > + cpufreq_update_util(rq_of_dl_rq(dl_rq), 0); > } > > static inline > --- a/kernel/sched/rt.c > +++ b/kernel/sched/rt.c > @@ -959,9 +959,6 @@ static void update_curr_rt(struct rq *rq > if (unlikely((s64)delta_exec <= 0)) > return; > > - /* Kick cpufreq (see the comment in kernel/sched/sched.h). */ > - cpufreq_update_util(rq, SCHED_CPUFREQ_RT); > - > schedstat_set(curr->se.statistics.exec_max, > max(curr->se.statistics.exec_max, delta_exec)); > > @@ -1003,6 +1000,9 @@ dequeue_top_rt_rq(struct rt_rq *rt_rq) > > sub_nr_running(rq, rt_rq->rt_nr_running); > rt_rq->rt_queued = 0; > + > + /* Kick cpufreq (see the comment in kernel/sched/sched.h). */ > + cpufreq_update_util(rq, 0); > } > > static void > @@ -1019,6 +1019,9 @@ enqueue_top_rt_rq(struct rt_rq *rt_rq) > > add_nr_running(rq, rt_rq->rt_nr_running); > rt_rq->rt_queued = 1; > + > + /* Kick cpufreq (see the comment in kernel/sched/sched.h). */ > + cpufreq_update_util(rq, 0); > } > > #if defined CONFIG_SMP > What is the status of this patch ? I couldn't find it on the tip/queue repositories. BTW, I wonder if we actually want to remove also the information about the scheduling class who triggered the frequency change. This prevents us from adopting class-specific behaviors. For example, we might want to skip the rate limits when deadline asks for an increase of frequency, as shown in the patch below. In this case, we could just remove the flags from sugov_cpu, but leave the defines and the argument for sugov_update_*() Best regards, Claudio From ed13fa5a8f93a43f8ff8f7d354b18c0031df482c Mon Sep 17 00:00:00 2001 From: Claudio Scordino Date: Wed, 27 Sep 2017 17:16:36 +0200 Subject: [PATCH RFC] cpufreq: schedutil: rate limits for SCHED_DEADLINE When the SCHED_DEADLINE scheduling class asks to increase CPU frequency, we should not wait the rate limit, otherwise we may miss some deadline. The patch just ignores the limit whenever SCHED_DEADLINE asks for a higher CPU frequency. Signed-off-by: Claudio Scordino --- kernel/sched/cpufreq_schedutil.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/kernel/sched/cpufreq_schedutil.c b/kernel/sched/cpufreq_schedutil.c index dd062a1..5027ab1 100644 --- a/kernel/sched/cpufreq_schedutil.c +++ b/kernel/sched/cpufreq_schedutil.c @@ -75,7 +75,8 @@ static DEFINE_PER_CPU(struct sugov_cpu, sugov_cpu); /************************ Governor internals ***********************/ -static bool sugov_should_update_freq(struct sugov_policy *sg_policy, u64 time) +static bool sugov_should_update_freq(struct sugov_policy *sg_policy, u64 time, + unsigned int next_freq, unsigned int flags) { s64 delta_ns; @@ -112,6 +113,10 @@ static bool sugov_should_update_freq(struct sugov_policy *sg_policy, u64 time) return true; } + /* Ignore rate limit if DL asked to increase CPU frequency */ + if ((flags & SCHED_CPUFREQ_DL) && (next_freq > sg_policy->next_freq)) + return true; + delta_ns = time - sg_policy->last_freq_update_time; return delta_ns >= sg_policy->freq_update_delay_ns; } @@ -275,9 +280,6 @@ static void sugov_update_single(struct update_util_data *hook, u64 time, sugov_set_iowait_boost(sg_cpu, time); sg_cpu->last_update = time; - if (!sugov_should_update_freq(sg_policy, time)) - return; - busy = sugov_cpu_is_busy(sg_cpu); if (flags & SCHED_CPUFREQ_RT) { @@ -299,7 +301,8 @@ static void sugov_update_single(struct update_util_data *hook, u64 time, sg_policy->cached_raw_freq = 0; } } - sugov_update_commit(sg_policy, time, next_f); + if (sugov_should_update_freq(sg_policy, time, next_f, flags)) + sugov_update_commit(sg_policy, time, next_f); } static unsigned int sugov_next_freq_shared(struct sugov_cpu *sg_cpu, u64 time) @@ -362,14 +365,13 @@ static void sugov_update_shared(struct update_util_data *hook, u64 time, sugov_set_iowait_boost(sg_cpu, time); sg_cpu->last_update = time; - if (sugov_should_update_freq(sg_policy, time)) { - if (flags & SCHED_CPUFREQ_RT) - next_f = sg_policy->policy->cpuinfo.max_freq; - else - next_f = sugov_next_freq_shared(sg_cpu, time); + if (flags & SCHED_CPUFREQ_RT) + next_f = sg_policy->policy->cpuinfo.max_freq; + else + next_f = sugov_next_freq_shared(sg_cpu, time); + if (sugov_should_update_freq(sg_policy, time, next_f, flags)) sugov_update_commit(sg_policy, time, next_f); - } raw_spin_unlock(&sg_policy->update_lock); }