mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Peter Zijlstra <peterz@infradead.org>
To: "Rafael J. Wysocki" <rafael@kernel.org>
Cc: anna-maria@linutronix.de, 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
Subject: Re: [RFC][PATCH 1/3] cpuidle: Inject tick boundary state
Date: Mon, 31 Jul 2023 13:38:50 +0200	[thread overview]
Message-ID: <20230731113850.GE29590@hirez.programming.kicks-ass.net> (raw)
In-Reply-To: <CAJZ5v0jh5oozZm7OvN9j1iHtzYQzPMOJ=Nt0HaJKYyJ218Cezw@mail.gmail.com>

On Mon, Jul 31, 2023 at 12:35:20PM +0200, Rafael J. Wysocki wrote:

> > So I agree with 1.
> >
> > I do not agree with 2. Disabling the tick is costly, doubly so with the
> > timer-pull thing, but even today. Simply disabling it because we picked
> > the deepest idle state, irrespective of the expected duration is wrong
> > as it will incur this significant cost.
> >
> > With 3 there is the question of how we get the expected sleep duration;
> > this is especially important with timer-pull, where we have this
> > chicken-and-egg thing.
> >
> > Notably: tick_nohz_get_sleep_length() wants to know if the tick gets
> > disabled
> 
> Well, it shouldn't.  Or at least it didn't before.

Correct, this is new in the timer-pull thing.

> It is expected to produce two values, one with the tick stopped (this
> is the return value of the function) and the other with the tick
> ticking (this is the one written under the address passed as the arg).
> This cannot depend on whether or not the tick will be stopped.  Both
> are good to know.
> 
> Now, I understand that getting these two values may be costly, so
> there is an incentive to avoid calling it, but then the governor needs
> to figure this out from its crystal ball and so care needs to be taken
> to limit the possible damage in case the crystal ball is not right.

If we can get the governor to decide the tick state up-front we can
avoid a lot of the expensive parts.

> > and cpuilde wants to use tick_nohz_get_sleep_length() to
> > determine if to disable the tick. This cycle needs to be broken for
> > timer-pull.
> >
> > Hence my proposal to introduce the extra tick state, that allows fixing
> > both 2 and 3.
> 
> I'm not sure about 3 TBH.
> 
> Say there are 2 idle states, one shallow (say its target residency is
> 10 us) and one deep (say its target residency is T = 2 * TICK_NSEC).

This is the easy case and that actually 'works' today. The
interesting case is where your deepest state has a target residency that
is below the tick (because for HZ=100, we have a 10ms tick and pretty
much all idle states are below that).

In that case you cannot tell the difference between I'm good to use this
state and I'm good to disable the tick and still use this state.



  parent reply	other threads:[~2023-07-31 11:39 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-28 14:55 [RFC][PATCH 0/3] cpuidle,teo: Improve TEO tick decisions Peter Zijlstra
2023-07-28 14:55 ` [RFC][PATCH 1/3] cpuidle: Inject tick boundary state Peter Zijlstra
2023-07-28 15:36   ` Rafael J. Wysocki
2023-07-29  8:44     ` Peter Zijlstra
2023-07-31  8:01       ` Rafael J. Wysocki
2023-07-31  9:09         ` Peter Zijlstra
2023-07-31 10:35           ` Rafael J. Wysocki
2023-07-31 11:00             ` Rafael J. Wysocki
2023-07-31 11:38             ` Peter Zijlstra [this message]
2023-07-31 16:55               ` Rafael J. Wysocki
2023-07-31 17:27                 ` Rafael J. Wysocki
2023-07-31 18:06                   ` Peter Zijlstra
2023-08-02 10:34                 ` Peter Zijlstra
2023-08-02 12:44                   ` Rafael J. Wysocki
2023-08-02 13:23                     ` Peter Zijlstra
2023-07-28 14:55 ` [RFC][PATCH 2/3] cpuidle,teo: Improve NOHZ management Peter Zijlstra
2023-07-28 16:56   ` Rafael J. Wysocki
2023-07-28 22:01     ` Peter Zijlstra
2023-07-31 10:17       ` Rafael J. Wysocki
2023-07-31 12:02         ` Peter Zijlstra
2023-07-31 17:20           ` Rafael J. Wysocki
2023-07-28 14:55 ` [RFC][PATCH 3/3] cpuidle,teo: Improve state selection Peter Zijlstra
2023-07-28 17:07   ` Rafael J. Wysocki
2023-07-29  8:46     ` Peter Zijlstra
2023-07-31 11:46 ` [RFC][PATCH 0/3] cpuidle,teo: Improve TEO tick decisions Anna-Maria Behnsen
2023-07-31 17:51   ` Rafael J. Wysocki
2023-08-02  8:12     ` Anna-Maria Behnsen

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=20230731113850.GE29590@hirez.programming.kicks-ass.net \
    --to=peterz@infradead.org \
    --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=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®