Messages in this thread |  | | Date | Fri, 17 Jan 2025 23:39:28 +0530 | | Subject | Re: [PATCH 4/6] cpuidle: Handle TIF_NR_POLLING on behalf of CPUIDLE_FLAG_MWAIT states | | From | K Prateek Nayak <> |
| |
Hello Frederic,
On 1/2/2025 8:31 PM, Frederic Weisbecker wrote: > [..snip..] > diff --git a/kernel/sched/idle.c b/kernel/sched/idle.c > index 621696269584..9eece3df1080 100644 > --- a/kernel/sched/idle.c > +++ b/kernel/sched/idle.c > @@ -114,12 +114,13 @@ void __cpuidle default_idle_call(void) > stop_critical_timings(); > > ct_cpuidle_enter(); > - arch_cpu_idle(); > + arch_cpu_idle(); // XXX assumes !polling > ct_cpuidle_exit(); > > start_critical_timings(); > trace_cpu_idle(PWR_EVENT_EXIT, smp_processor_id()); > cond_tick_broadcast_exit(); > + __current_set_polling(); > } > local_irq_enable(); > instrumentation_end(); > @@ -128,31 +129,14 @@ void __cpuidle default_idle_call(void) > static int call_cpuidle_s2idle(struct cpuidle_driver *drv, > struct cpuidle_device *dev) > { > + int ret; > + > if (current_clr_polling_and_test()) > return -EBUSY; > > - return cpuidle_enter_s2idle(drv, dev); > -} > - > -static int call_cpuidle(struct cpuidle_driver *drv, struct cpuidle_device *dev, > - int next_state) > -{ > - /* > - * The idle task must be scheduled, it is pointless to go to idle, just > - * update no idle residency and return. > - */ > - if (current_clr_polling_and_test()) {
nit.
Previously, if TIF_NEED_RESCHED was set by this point, the CPU would bail out early before entering the idle state but with this change, I believe only at need_resched() in mwait_idle_with_hints() do we realize we have a pending IPI / task wakeup. Is this a concern?
In my testing with ipistorm I could not see any difference in IPI throughput to polling idle CPUs but a bailout before the entry method for need_resched() can save a few cycles.
-- Thanks and Regards, Prateek
> - dev->last_residency_ns = 0; > - local_irq_enable(); > - return -EBUSY; > - } > - > - /* > - * Enter the idle state previously returned by the governor decision. > - * This function will block until an interrupt occurs and will take > - * care of re-enabling the local interrupts > - */ > - return cpuidle_enter(drv, dev, next_state); > + ret = cpuidle_enter_s2idle(drv, dev); > + __current_set_polling(); > + return ret; > } > > /** > @@ -213,7 +197,7 @@ static void cpuidle_idle_call(void) > tick_nohz_idle_stop_tick(); > > next_state = cpuidle_find_deepest_state(drv, dev, max_latency_ns); > - call_cpuidle(drv, dev, next_state); > + cpuidle_enter(drv, dev, next_state); > } else { > bool stop_tick = true; > > @@ -227,7 +211,12 @@ static void cpuidle_idle_call (void) > else > tick_nohz_idle_retain_tick(); > > - entered_state = call_cpuidle(drv, dev, next_state); > + /* > + * Enter the idle state previously returned by the governor decision. > + * This function will block until an interrupt occurs and will take > + * care of re-enabling the local interrupts. > + */ > + entered_state = cpuidle_enter(drv, dev, next_state); > /* > * Give the governor an opportunity to reflect on the outcome > */ > @@ -235,7 +224,6 @@ static void cpuidle_idle_call(void) > } > > exit_idle: > - __current_set_polling(); > > /* > * It is up to the idle functions to re-enable local interrupts
|  |