From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4FC63175A85 for ; Mon, 2 Mar 2026 12:19:54 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=90.155.50.34 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772453996; cv=none; b=U6xYTBERreGc2Ks/UTKqA9Lk5a4/fO6PDSTrk3TzVHb4NTSFi2R9UacHQnExs+msStoRpKbzTQiB3mmtZDE3+OO5WhDg/mBke8sN4hBfjZp4NZYPgj7OMBwJZI07fSXpow/m3aD4HBwbyCGXJRcIwJ8IZorISQxdCUmxAlD6Wgg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1772453996; c=relaxed/simple; bh=AbyuqU99t7NnWc1E7r9ryHv60Uq/YjIIwo1oLXuJeBU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=JzWl8SwR/oav+eGjoGjxDDcjfwB/Pr2ItGQY1SRD+MjvEoDUlLx71GGOA+Y45f4Xic1hOgucuLbjfrHR+Sas28S2yjd25SjrC1kW9PTJfq8/QGYws2QRzrf9yzrP/oYzBij9Tsxz3MEgJj2iLmiAZIVKT95kY0IA1zQomHcHMCg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org; spf=none smtp.mailfrom=infradead.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b=ENeXVlwk; arc=none smtp.client-ip=90.155.50.34 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=infradead.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="ENeXVlwk" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=In-Reply-To:Content-Type:MIME-Version: References:Message-ID:Subject:Cc:To:From:Date:Sender:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=oW0YuFii2QpVbdOdwI0iosM7dONgoX9kVKLypEjdLWg=; b=ENeXVlwkIcodVOQmVYZxzdJcgw Xg3X3SYXuwPUlqC70VCwlPmolv6NHDOl8WjCEafz8U3phdlLRXVzV7Dw0iYzTW7WBjR2G+oSjpf9k 3Bzs31o4Rezvuk8d0WbHvmf2ap+mvZ9ljnh7rePL9O7wf34mHviS+Bwf6dqMB7EhKaTaw4Al/Hv88 8f0i6qT8B1xJCdxjIoGYvYT7PO+WMBXMQyn5+JlPyejYLg37jqHv6ZsAaYg61fsLk68xWYX8A+Ou7 OQ2hN9yowbcra22mJZNtWXTnmQOCc4bR/97gY5d2SduYzU9HiP1BX4CHdHSscV59NDVvmaV3pC6QP QK6jmPTQ==; Received: from 77-249-17-252.cable.dynamic.v4.ziggo.nl ([77.249.17.252] helo=noisy.programming.kicks-ass.net) by casper.infradead.org with esmtpsa (Exim 4.98.2 #2 (Red Hat Linux)) id 1vx2Fb-00000009ODM-18ce; Mon, 02 Mar 2026 12:19:51 +0000 Received: by noisy.programming.kicks-ass.net (Postfix, from userid 1000) id 926B3300B40; Mon, 02 Mar 2026 13:19:50 +0100 (CET) Date: Mon, 2 Mar 2026 13:19:50 +0100 From: Peter Zijlstra To: Thomas Gleixner Cc: LKML , "Rafael J. Wysocki" , Frederic Weisbecker , Christian Loehle Subject: Re: [patch 2/2] sched/idle: Make default_idle_call() NOHZ aware Message-ID: <20260302121950.GZ1395416@noisy.programming.kicks-ass.net> References: <20260301191959.406218221@kernel.org> <20260301192915.171574741@kernel.org> <20260302121755.GF1395266@noisy.programming.kicks-ass.net> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20260302121755.GF1395266@noisy.programming.kicks-ass.net> On Mon, Mar 02, 2026 at 01:17:55PM +0100, Peter Zijlstra wrote: > On Sun, Mar 01, 2026 at 08:30:51PM +0100, Thomas Gleixner wrote: > > Guests fall back to default_idle_call() as there is no cpuidle driver > > available to them by default. That causes a problem in fully loaded > > scenarios where CPUs go briefly idle for a couple of microseconds: > > > > tick_nohz_idle_stop_tick() is invoked unconditionally which means unless > > there is timer pending in the next tick, the tick is stopped and a couple > > of microseconds later when the idle condition goes away restarted. That > > requires to program the clockevent device twice which implies a VM exit for > > each reprogramming. > > > > It was suggested to remove the tick_nohz_idle_stop_tick() invocation from > > the default idle code, but would be counterproductive. It would not allow > > the host to go into deeper idle states when the guest CPU is fully idle as > > it has to maintain the periodic tick. > > > > Cure this by implementing a trivial moving average filter which keeps track > > of the recent idle recidency time and only stop the tick when the average > > is larger than a tick. > > > > Signed-off-by: Thomas Gleixner > > How about so? No reason to not also pass this into the idle governors. > This way it becomes a common least functionality. Governor can override, > but it had better have a good reason. > > --- > --- a/kernel/sched/idle.c > +++ b/kernel/sched/idle.c > @@ -6,10 +6,12 @@ > * (NOTE: these are not related to SCHED_IDLE batch scheduled > * tasks which are handled in sched/fair.c ) > */ > +#include > #include > #include > #include > #include "sched.h" > +#include "pelt.h" > #include "smp.h" > > /* Linker adds these: start and end of __cpuidle functions */ > @@ -105,12 +107,7 @@ static inline void cond_tick_broadcast_e > static inline void cond_tick_broadcast_exit(void) { } > #endif /* !CONFIG_GENERIC_CLOCKEVENTS_BROADCAST_IDLE */ > > -/** > - * default_idle_call - Default CPU idle routine. > - * > - * To use when the cpuidle framework cannot be used. > - */ > -static void __cpuidle default_idle_call(void) > +static void __cpuidle __default_idle_call(void) > { > instrumentation_begin(); > if (!current_clr_polling_and_test()) { > @@ -130,6 +127,63 @@ static void __cpuidle default_idle_call( > instrumentation_end(); > } > > +#ifdef CONFIG_NO_HZ_COMMON > + > +/* Limit to 4 entries so it fits in a cache line */ > +#define IDLE_DUR_ENTRIES 4 > +#define IDLE_DUR_MASK (IDLE_DUR_ENTRIES - 1) > + > +struct idle_nohz_data { > + u64 duration[IDLE_DUR_ENTRIES]; > + u64 entry_time; > + u64 sum; > + unsigned int idx; > +}; > + > +static DEFINE_PER_CPU_ALIGNED(struct idle_nohz_data, nohz_data); > + > +static void default_idle_enter(void) > +{ > + this_cpu_write(nohz_data.entry_time, sched_clock()); > +} > + > +static inline bool default_stop_tick(void) > +{ > + struct idle_nohz_data *nd = this_cpu_ptr(&nohz_data); > + return nd->sum > TICK_NSEC * IDLE_DUR_ENTRIES; > +} > + > +static void default_reflect(void) > +{ > + struct idle_nohz_data *nd = this_cpu_ptr(&nohz_data); > + unsigned int idx = nd->idx; > + s64 delta; > + > + /* > + * Build a moving average of the time spent in idle to prevent stopping > + * the tick on a loaded system which only goes idle briefly. > + */ > + delta = max(sched_clock() - nd->entry_time, 0); > + nd->sum += delta - nd->duration[idx]; > + nd->duration[idx] = delta; > + nd->idx = (idx + 1) & IDLE_DUR_MASK; > +} > +#else /* CONFIG_NO_HZ_COMMON */ > +static inline void default_idle_enter(void) { } > +static inline bool default_stop_tick(void) { return false; } > +static inline void default_reflect(void) { } > +#endif /* !CONFIG_NO_HZ_COMMON */ > + > +static inline void default_idle_call(void) > +{ > + if (default_stop_tick()) > + tick_nohz_idle_stop_tick(); > + > + __default_idle_call(); > + > + default_reflect(); > +} > + > static int call_cpuidle_s2idle(struct cpuidle_driver *drv, > struct cpuidle_device *dev, > u64 max_latency_ns) > @@ -186,8 +240,6 @@ static void cpuidle_idle_call(void) > } > > if (cpuidle_not_available(drv, dev)) { > - tick_nohz_idle_stop_tick(); > - > default_idle_call(); > goto exit_idle; > } > @@ -222,7 +274,7 @@ static void cpuidle_idle_call(void) > next_state = cpuidle_find_deepest_state(drv, dev, max_latency_ns); > call_cpuidle(drv, dev, next_state); > } else { > - bool stop_tick = true; > + bool stop_tick = default_stop_tick(); > > /* > * Ask the cpuidle framework to choose a convenient idle state. > @@ -238,6 +290,7 @@ static void cpuidle_idle_call(void) > /* > * Give the governor an opportunity to reflect on the outcome > */ > + default_reflect(); > cpuidle_reflect(dev, entered_state); > } > > @@ -276,6 +329,7 @@ static void do_idle(void) > > __current_set_polling(); > tick_nohz_idle_enter(); > + default_idle_enter(); > > while (!need_resched()) { > Damn, lost hunk: diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c index 65fbb8e807b9..c7876e9e024f 100644 --- a/drivers/cpuidle/cpuidle.c +++ b/drivers/cpuidle/cpuidle.c @@ -359,16 +359,6 @@ noinstr int cpuidle_enter_state(struct cpuidle_device *dev, int cpuidle_select(struct cpuidle_driver *drv, struct cpuidle_device *dev, bool *stop_tick) { - /* - * If there is only a single idle state (or none), there is nothing - * meaningful for the governor to choose. Skip the governor and - * always use state 0 with the tick running. - */ - if (drv->state_count <= 1) { - *stop_tick = false; - return 0; - } - return cpuidle_curr_governor->select(drv, dev, stop_tick); } Also, I suppose menu wants this? diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c index 899ff16ff1fe..a75fe1fca65d 100644 --- a/drivers/cpuidle/governors/menu.c +++ b/drivers/cpuidle/governors/menu.c @@ -290,7 +290,8 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev, * it right away and keep the tick running if state[0] is a * polling one. */ - *stop_tick = !(drv->states[0].flags & CPUIDLE_FLAG_POLLING); + if (drv->states[0].flags & CPUIDLE_FLAG_POLLING) + *stop_tick = false; return 0; }