All of lore.kernel.org
 help / color / mirror / Atom feed
From: Frederic Weisbecker <frederic@kernel.org>
To: Thomas Gleixner <tglx@linutronix.de>
Cc: Huacai Chen <chenhuacai@kernel.org>,
	WANG Xuerui <kernel@xen0n.name>,
	Peter Zijlstra <peterz@infradead.org>,
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	Anna-Maria Behnsen <anna-maria@linutronix.de>,
	LKML <linux-kernel@vger.kernel.org>
Subject: Re: Loongson (and other $ARCHs?) idle VS timer enqueue
Date: Fri, 21 Apr 2023 18:55:40 +0200	[thread overview]
Message-ID: <ZELADFhjWR2Swn3l@lothringen> (raw)
In-Reply-To: <87leil2r7v.ffs@tglx>

On Fri, Apr 21, 2023 at 05:24:36PM +0200, Thomas Gleixner wrote:
> On Fri, Apr 21 2023 at 14:36, Frederic Weisbecker wrote:
> The check for TIF_NEED_RESCHED as loop termination condition is simply
> wrong. The architecture is not to supposed to loop in arch_cpu_idle().
> 
> That loop is from Linux 0.9 days. Seems MIPS::__r4k_wait() and
> loongarch, which copied that muck are still stuck in the 1990'ies.
> 
> It has to return when an interrupt brings it out of the "idle wait"
> instruction.
> 
> The special case are mwait() alike mechanisms which also return when a
> monitored cacheline is written to. x86 uses that to spare the reseched
> IPI as MWAIT will return when TIF_NEED_RESCHED is set by a remote CPU.

Right.

> 
> > More generally IRQs must _not_ be re-enabled between cpuidle_select()
> > (or just tick_nohz_idle_stop_tick() if no cpuidle support) and the
> > last halting ASM instruction. If that happens there must be
> > a mechanism to cope with that and make sure we return to the main
> > idle loop.
> 
> No. arch_cpu_idle() can safely reenable interrupts when the "wait"
> instruction requires that. It has then to disable interrupts before
> returning.
> 
> x86 default_idle() does: STI; HLT; CLI; That's perfectly fine because
> the effect of STI is delayed to the HLT instruction boundary.

Right, I implicitly included sti;mwait and sti;hlt
The point is that if interrupts are enabled too early before the
idling instruction then we are screwed.

> 
> > Another way to cope with this would be to have:
> >
> > #define TIF_IDLE_TIMER	 ...
> > #define TIF_IDLE_EXIT	 (TIF_NEED_RESCHED | TIF_IDLE_TIMER)
> 
> There is absolutely no need for this. arch_cpu_idle() has to return
> after an interrupt, period. If MIPS/loongarch cannot do that then they
> can have their private interrupt counter in that magic rollback ASM to
> check for. But we really don't need a TIF flag which makes the (hr)timer
> enqueue path more complex.

Then I'm relieved :)  (well sort-of, given the risk for an accident somewhere
on an arch or a cpuidle driver I may have overlooked).

> 
> > I'm trying to find an automated way to debug this kind of issue but
> > it's not easy...
> 
> It's far from trivial because you'd need correlation between the
> interrupt entry and the enter to and return from arch_cpu_idle().
> 
> I fear manual inspection is the main tool here :(

I thought so :)

I'm already halfway through the architectures, then will come the cpuidle drivers...

Thanks.

  reply	other threads:[~2023-04-21 16:57 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-21 12:36 Loongson (and other $ARCHs?) idle VS timer enqueue Frederic Weisbecker
2023-04-21 14:24 ` Peter Zijlstra
2023-04-21 16:47   ` Frederic Weisbecker
2023-04-22  8:08     ` Peter Zijlstra
2023-04-22 11:38       ` Peter Zijlstra
2023-04-22 14:48         ` Frederic Weisbecker
2023-04-21 15:24 ` Thomas Gleixner
2023-04-21 16:55   ` Frederic Weisbecker [this message]
2023-04-21 20:28     ` Thomas Gleixner
2023-04-22  8:17   ` Peter Zijlstra
2023-04-22  8:22     ` Peter Zijlstra
2023-04-22 14:21     ` Frederic Weisbecker
2023-04-22 15:04       ` Peter Zijlstra
2023-04-23 13:52         ` bibo, mao
2023-04-24  8:26           ` Frederic Weisbecker
2023-04-24 11:23             ` maobibo
2023-04-25 11:49           ` Peter Zijlstra
2023-04-25 13:25             ` maobibo
2023-04-25 13:28               ` WANG Xuerui
2023-04-26  0:46                 ` maobibo
2023-04-26  2:10                   ` WANG Xuerui
2023-04-26  2:23                     ` maobibo
2023-06-06 22:07             ` Frederic Weisbecker

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=ZELADFhjWR2Swn3l@lothringen \
    --to=frederic@kernel.org \
    --cc=anna-maria@linutronix.de \
    --cc=chenhuacai@kernel.org \
    --cc=kernel@xen0n.name \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=rafael.j.wysocki@intel.com \
    --cc=tglx@linutronix.de \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.