Linux-PM Archive on lore.kernel.org
 help / color / Atom feed
From: Dietmar Eggemann <dietmar.eggemann@arm.com>
To: Douglas RAILLARD <douglas.raillard@arm.com>,
	linux-kernel@vger.kernel.org
Cc: linux-pm@vger.kernel.org, mingo@redhat.com, peterz@infradead.org,
	rjw@rjwysocki.net, viresh.kumar@linaro.org,
	juri.lelli@redhat.com, vincent.guittot@linaro.org,
	qperret@qperret.net, patrick.bellasi@matbug.net,
	dh.han@samsung.com
Subject: Re: [RFC PATCH v3 5/6] sched/cpufreq: Boost schedutil frequency ramp up
Date: Thu, 17 Oct 2019 11:21:21 +0200
Message-ID: <4235a01d-26ee-418c-b78a-6c6109deda1d@arm.com> (raw)
In-Reply-To: <20191011134500.235736-6-douglas.raillard@arm.com>

On 11/10/2019 15:44, Douglas RAILLARD wrote:

[...]

> @@ -539,6 +543,7 @@ static void sugov_update_single(struct update_util_data *hook, u64 time,
>  	unsigned long util, max;
>  	unsigned int next_f;
>  	bool busy;
> +	unsigned long ramp_boost = 0;

Shouldn't always order local variable declarations from longest to
shortest line?

>  	sugov_iowait_boost(sg_cpu, time, flags);
>  	sg_cpu->last_update = time;
> @@ -552,10 +557,10 @@ static void sugov_update_single(struct update_util_data *hook, u64 time,
>  	busy = !sg_policy->need_freq_update && sugov_cpu_is_busy(sg_cpu);
>  
>  	util = sugov_get_util(sg_cpu);
> -	sugov_cpu_ramp_boost_update(sg_cpu);
> +	ramp_boost = sugov_cpu_ramp_boost_update(sg_cpu);
>  	max = sg_cpu->max;
>  	util = sugov_iowait_apply(sg_cpu, time, util, max);
> -	next_f = get_next_freq(sg_policy, util, max);
> +	next_f = get_next_freq(sg_policy, util, max, ramp_boost);
>  	/*
>  	 * Do not reduce the frequency if the CPU has not been idle
>  	 * recently, as the reduction is likely to be premature then.
> @@ -587,6 +592,8 @@ static unsigned int sugov_next_freq_shared(struct sugov_cpu *sg_cpu, u64 time)
>  	struct cpufreq_policy *policy = sg_policy->policy;
>  	unsigned long util = 0, max = 1;
>  	unsigned int j;
> +	unsigned long ramp_boost = 0;
> +	unsigned long j_ramp_boost = 0;

Shouldn't always order local variable declarations from longest to
shortest line?

>  	for_each_cpu(j, policy->cpus) {
>  		struct sugov_cpu *j_sg_cpu = &per_cpu(sugov_cpu, j);
> @@ -594,7 +601,11 @@ static unsigned int sugov_next_freq_shared(struct sugov_cpu *sg_cpu, u64 time)
>  
>  		j_util = sugov_get_util(j_sg_cpu);
>  		if (j_sg_cpu == sg_cpu)
> -			sugov_cpu_ramp_boost_update(sg_cpu);
> +			j_ramp_boost = sugov_cpu_ramp_boost_update(sg_cpu);
> +		else
> +			j_ramp_boost = sugov_cpu_ramp_boost(j_sg_cpu);
> +		ramp_boost = max(ramp_boost, j_ramp_boost);

Ah, that's why you call sugov_cpu_ramp_boost_update() from
sugov_next_freq_shared() in 4/6. So sugov_cpu_ramp_boost_update() is
more a sugov_cpu_ramp_boost(..., int update)? Minor detail though.

[...]

  reply index

Thread overview: 35+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-11 13:44 [RFC PATCH v3 0/6] sched/cpufreq: Make schedutil energy aware Douglas RAILLARD
2019-10-11 13:44 ` [RFC PATCH v3 1/6] PM: Introduce em_pd_get_higher_freq() Douglas RAILLARD
2019-10-17  8:57   ` Dietmar Eggemann
2019-10-17  9:58   ` Dietmar Eggemann
2019-10-17 11:09     ` Douglas Raillard
2019-10-11 13:44 ` [RFC PATCH v3 2/6] sched/cpufreq: Attach perf domain to sugov policy Douglas RAILLARD
2019-10-17  8:57   ` Dietmar Eggemann
2019-10-17 10:22     ` Douglas Raillard
2019-10-11 13:44 ` [RFC PATCH v3 3/6] sched/cpufreq: Hook em_pd_get_higher_power() into get_next_freq() Douglas RAILLARD
2019-10-11 13:44 ` [RFC PATCH v3 4/6] sched/cpufreq: Introduce sugov_cpu_ramp_boost Douglas RAILLARD
2019-10-14 14:33   ` Peter Zijlstra
2019-10-14 15:32     ` Douglas Raillard
2019-10-17  8:57   ` Dietmar Eggemann
2019-10-17 11:19     ` Douglas Raillard
2019-10-11 13:44 ` [RFC PATCH v3 5/6] sched/cpufreq: Boost schedutil frequency ramp up Douglas RAILLARD
2019-10-17  9:21   ` Dietmar Eggemann [this message]
2019-10-11 13:45 ` [RFC PATCH v3 6/6] sched/cpufreq: Add schedutil_em_tp tracepoint Douglas RAILLARD
2019-10-14 14:53 ` [RFC PATCH v3 0/6] sched/cpufreq: Make schedutil energy aware Peter Zijlstra
2019-10-14 15:50   ` Douglas Raillard
2019-10-17  9:50     ` Peter Zijlstra
2019-10-17 11:11       ` Quentin Perret
2019-10-17 14:11         ` Peter Zijlstra
2019-10-18  7:44           ` Dietmar Eggemann
2019-10-18  7:59             ` Peter Zijlstra
2019-10-18 17:24               ` Douglas Raillard
2019-10-18  8:11             ` Peter Zijlstra
2019-10-17 14:23       ` Douglas Raillard
2019-10-17 14:53         ` Peter Zijlstra
2019-10-17 19:07         ` Peter Zijlstra
2019-10-18 11:46           ` Douglas Raillard
2019-10-18 12:07             ` Peter Zijlstra
2019-10-18 14:44               ` Douglas Raillard
2019-10-18 15:15                 ` Vincent Guittot
2019-10-18 16:03                   ` Douglas Raillard
2019-10-18 15:20                 ` Vincent Guittot

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4235a01d-26ee-418c-b78a-6c6109deda1d@arm.com \
    --to=dietmar.eggemann@arm.com \
    --cc=dh.han@samsung.com \
    --cc=douglas.raillard@arm.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=patrick.bellasi@matbug.net \
    --cc=peterz@infradead.org \
    --cc=qperret@qperret.net \
    --cc=rjw@rjwysocki.net \
    --cc=vincent.guittot@linaro.org \
    --cc=viresh.kumar@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-PM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-pm/0 linux-pm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-pm linux-pm/ https://lore.kernel.org/linux-pm \
		linux-pm@vger.kernel.org
	public-inbox-index linux-pm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-pm


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git