mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Kajetan Puchalski <kajetan.puchalski@arm.com>
To: Peter Zijlstra <peterz@infradead.org>
Cc: anna-maria@linutronix.de, rafael@kernel.org, tglx@linutronix.de,
	frederic@kernel.org, gautham.shenoy@amd.com,
	linux-kernel@vger.kernel.org, daniel.lezcano@linaro.org,
	linux-pm@vger.kernel.org, mingo@redhat.com,
	juri.lelli@redhat.com, vincent.guittot@linaro.org,
	dietmar.eggemann@arm.com, rostedt@goodmis.org,
	bsegall@google.com, mgorman@suse.de, bristot@redhat.com,
	vschneid@redhat.com, kajetan.puchalski@arm.com
Subject: Re: [RFC][PATCH v2 3/5] cpuidle/teo: Simplify a little
Date: Thu, 3 Aug 2023 11:12:30 +0100	[thread overview]
Message-ID: <ZMt9jooB4HV/5uXp@e126311.manchester.arm.com> (raw)
In-Reply-To: <20230802132925.686745535@infradead.org>

On Wed, Aug 02, 2023 at 03:24:34PM +0200, Peter Zijlstra wrote:
> Remove some of the early exit cases that rely on state_count, since we
> have the additional tick state. Declutters some of the next patches, can
> possibly be re-instated later if desired.

How does having that added tick state compensate for not checking the
amount of states that the governor can choose from in the first place?

[...]

> -	/* Check if there is any choice in the first place. */
> -	if (drv->state_count < 2) {
> -		idx = 0;
> -		goto end;
> -	}
> -	if (!dev->states_usage[0].disable) {
> -		idx = 0;
> -		if (drv->states[1].target_residency_ns > duration_ns)
> -			goto end;
> -	}
> -
> -	cpu_data->utilized = teo_cpu_is_utilized(dev->cpu, cpu_data);
> -	/*
> -	 * If the CPU is being utilized over the threshold and there are only 2
> -	 * states to choose from, the metrics need not be considered, so choose
> -	 * the shallowest non-polling state and exit.
> -	 */
> -	if (drv->state_count < 3 && cpu_data->utilized) {
> -		for (i = 0; i < drv->state_count; ++i) {
> -			if (!dev->states_usage[i].disable &&
> -			    !(drv->states[i].flags & CPUIDLE_FLAG_POLLING)) {
> -				idx = i;
> -				goto end;
> -			}
> -		}
> -	}

What exactly is the benefit of removing this part? On systems with 2
idle states this will just make the governor pointlessly execute the
metrics code when the result is already known regardless. Seems like
pure added overhead.

> -
>  	/*
>  	 * Find the deepest idle state whose target residency does not exceed
>  	 * the current sleep length and the deepest idle state not deeper than
> @@ -541,7 +512,7 @@ static int teo_select(struct cpuidle_dri
>  	 * If the CPU is being utilized over the threshold, choose a shallower
>  	 * non-polling state to improve latency
>  	 */
> -	if (cpu_data->utilized)
> +	if (teo_cpu_is_utilized(dev->cpu, cpu_data))
>  		idx = teo_find_shallower_state(drv, dev, idx, duration_ns, true);
>  
>  end:
> 
> 

  reply	other threads:[~2023-08-03 10:12 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-02 13:24 [RFC][PATCH v2 0/5] cpuidle,teo: Improve TEO vs NOHZ Peter Zijlstra
2023-08-02 13:24 ` [RFC][PATCH v2 1/5] tick/nohz: Introduce tick_get_sleep_length() Peter Zijlstra
2023-08-02 13:24 ` [RFC][PATCH v2 2/5] cpuidle: Inject tick boundary state Peter Zijlstra
2023-08-02 13:24 ` [RFC][PATCH v2 3/5] cpuidle/teo: Simplify a little Peter Zijlstra
2023-08-03 10:12   ` Kajetan Puchalski [this message]
2023-08-02 13:24 ` [RFC][PATCH v2 4/5] cpuidle/teo: Avoid tick_nohz_next_event() Peter Zijlstra
2023-08-02 13:24 ` [RFC][PATCH v2 5/5] cpuidle,teo: Improve state selection Peter Zijlstra

Reply instructions:

You may reply publicly 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=ZMt9jooB4HV/5uXp@e126311.manchester.arm.com \
    --to=kajetan.puchalski@arm.com \
    --cc=anna-maria@linutronix.de \
    --cc=bristot@redhat.com \
    --cc=bsegall@google.com \
    --cc=daniel.lezcano@linaro.org \
    --cc=dietmar.eggemann@arm.com \
    --cc=frederic@kernel.org \
    --cc=gautham.shenoy@amd.com \
    --cc=juri.lelli@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=peterz@infradead.org \
    --cc=rafael@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=tglx@linutronix.de \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®