Messages in this thread | | | From | "Rafael J. Wysocki" <> | Date | Tue, 12 Dec 2023 14:27:38 +0100 | Subject | Re: [PATCH 7/7] cpuidle: Handle TIF_NR_POLLING on behalf of software polling idle states |
| |
On Fri, Nov 24, 2023 at 11:33 PM Frederic Weisbecker <frederic@kernel.org> wrote: > > Software polling idle states set again TIF_NR_POLLING and clear it upon > exit. This involves error prone duplicated code and wasted cycles > performing atomic operations, sometimes RmW fully ordered. > > To avoid this, benefit instead from the same generic TIF_NR_POLLING > handling that is currently in use for hardware polling states. > > Signed-off-by: Frederic Weisbecker <frederic@kernel.org> > --- > drivers/cpuidle/cpuidle-powernv.c | 10 ---------- > drivers/cpuidle/cpuidle-pseries.c | 11 ----------- > drivers/cpuidle/cpuidle.c | 4 ++-- > drivers/cpuidle/poll_state.c | 30 ++++++++++++------------------ > 4 files changed, 14 insertions(+), 41 deletions(-) > > diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c > index 675b8eb81ebd..b88bbf7ead41 100644 > --- a/drivers/cpuidle/cpuidle-powernv.c > +++ b/drivers/cpuidle/cpuidle-powernv.c > @@ -71,8 +71,6 @@ static int snooze_loop(struct cpuidle_device *dev, > { > u64 snooze_exit_time; > > - set_thread_flag(TIF_POLLING_NRFLAG); > - > local_irq_enable(); > > snooze_exit_time = get_tb() + get_snooze_timeout(dev, drv, index); > @@ -81,21 +79,13 @@ static int snooze_loop(struct cpuidle_device *dev, > HMT_very_low(); > while (!need_resched()) { > if (likely(snooze_timeout_en) && get_tb() > snooze_exit_time) { > - /* > - * Task has not woken up but we are exiting the polling > - * loop anyway. Require a barrier after polling is > - * cleared to order subsequent test of need_resched(). > - */ > - clear_thread_flag(TIF_POLLING_NRFLAG); > dev->poll_time_limit = true; > - smp_mb(); > break; > } > } > > HMT_medium(); > ppc64_runlatch_on(); > - clear_thread_flag(TIF_POLLING_NRFLAG); > > local_irq_disable(); > > diff --git a/drivers/cpuidle/cpuidle-pseries.c b/drivers/cpuidle/cpuidle-pseries.c > index 4e08c9a39172..0ae76512b740 100644 > --- a/drivers/cpuidle/cpuidle-pseries.c > +++ b/drivers/cpuidle/cpuidle-pseries.c > @@ -39,8 +39,6 @@ int snooze_loop(struct cpuidle_device *dev, struct cpuidle_driver *drv, > { > u64 snooze_exit_time; > > - set_thread_flag(TIF_POLLING_NRFLAG); > - > pseries_idle_prolog(); > raw_local_irq_enable(); > snooze_exit_time = get_tb() + snooze_timeout; > @@ -50,21 +48,12 @@ int snooze_loop(struct cpuidle_device *dev, struct cpuidle_driver *drv, > HMT_low(); > HMT_very_low(); > if (likely(snooze_timeout_en) && get_tb() > snooze_exit_time) { > - /* > - * Task has not woken up but we are exiting the polling > - * loop anyway. Require a barrier after polling is > - * cleared to order subsequent test of need_resched(). > - */ > dev->poll_time_limit = true; > - clear_thread_flag(TIF_POLLING_NRFLAG); > - smp_mb(); > break; > } > } > > HMT_medium(); > - clear_thread_flag(TIF_POLLING_NRFLAG); > - > raw_local_irq_disable(); > > pseries_idle_epilog(); > diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c > index 49078cc83f4a..9eb811b5d8b6 100644 > --- a/drivers/cpuidle/cpuidle.c > +++ b/drivers/cpuidle/cpuidle.c > @@ -236,8 +236,8 @@ noinstr int cpuidle_enter_state(struct cpuidle_device *dev, > broadcast = false; > } > > - polling = target_state->flags & CPUIDLE_FLAG_POLLING_HARD; > - > + polling = (target_state->flags & (CPUIDLE_FLAG_POLLING_SOFT | > + CPUIDLE_FLAG_POLLING_HARD));
The outer parens are not needed on the right-hand side, or apply !! to it.
> /* > * If the target state doesn't poll on need_resched(), this is > * the last check after which further TIF_NEED_RESCHED remote setting > diff --git a/drivers/cpuidle/poll_state.c b/drivers/cpuidle/poll_state.c > index a2fe173de117..3bfa251b344a 100644 > --- a/drivers/cpuidle/poll_state.c > +++ b/drivers/cpuidle/poll_state.c > @@ -13,35 +13,29 @@ > static int __cpuidle poll_idle(struct cpuidle_device *dev, > struct cpuidle_driver *drv, int index) > { > - u64 time_start; > - > - time_start = local_clock_noinstr(); > + u64 time_start = local_clock_noinstr(); > + unsigned int loop_count = 0; > + u64 limit; > > dev->poll_time_limit = false; > > raw_local_irq_enable(); > - if (!current_set_polling_and_test()) { > - unsigned int loop_count = 0; > - u64 limit; > > - limit = cpuidle_poll_time(drv, dev); > + limit = cpuidle_poll_time(drv, dev); > > - while (!need_resched()) { > - cpu_relax(); > - if (loop_count++ < POLL_IDLE_RELAX_COUNT) > - continue; > + while (!need_resched()) { > + cpu_relax(); > + if (loop_count++ < POLL_IDLE_RELAX_COUNT) > + continue; > > - loop_count = 0; > - if (local_clock_noinstr() - time_start > limit) { > - dev->poll_time_limit = true; > - break; > - } > + loop_count = 0; > + if (local_clock_noinstr() - time_start > limit) { > + dev->poll_time_limit = true; > + break; > } > } > raw_local_irq_disable(); > > - current_clr_polling(); > - > return index; > } > > --
| |