| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJZ5v0iy6oN67o2uH_j_weCqAb0CNBJhOLFaxv-Bexmf0Mybaw@mail.gmail.com>
Date: Tue, 14 Jan 2025 15:56:48 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Frederic Weisbecker <frederic@...nel.org>
Cc: LKML <linux-kernel@...r.kernel.org>, Peter Zijlstra <peterz@...radead.org>,
Daniel Lezcano <daniel.lezcano@...aro.org>, linux-pm@...r.kernel.org,
Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>
Subject: Re: [PATCH 6/6] cpuidle: Handle TIF_NR_POLLING on behalf of software
polling idle states
On Thu, Jan 2, 2025 at 4:02 PM Frederic Weisbecker <frederic@...nel.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 monitoring states.
>
> Signed-off-by: Frederic Weisbecker <frederic@...nel.org>
So this is the part I was wondering about when I was looking at patch [4/6].
The code changes below look good to me, so feel free to add
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
to this patch when ready.
> ---
> drivers/cpuidle/cpuidle-powernv.c | 10 ----------
> drivers/cpuidle/cpuidle-pseries.c | 11 -----------
> drivers/cpuidle/cpuidle.c | 2 +-
> drivers/cpuidle/poll_state.c | 30 ++++++++++++------------------
> 4 files changed, 13 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/cpuidle/cpuidle-powernv.c b/drivers/cpuidle/cpuidle-powernv.c
> index 9ebedd972df0..1bf0d2234016 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 f68c65f1d023..704bb01d9e9e 100644
> --- a/drivers/cpuidle/cpuidle-pseries.c
> +++ b/drivers/cpuidle/cpuidle-pseries.c
> @@ -40,8 +40,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;
> @@ -51,21 +49,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 46c0a2726f67..fecc50c2860e 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -237,7 +237,7 @@ noinstr int cpuidle_enter_state(struct cpuidle_device *dev,
> broadcast = false;
> }
>
> - polling = target_state->flags & CPUIDLE_FLAG_MWAIT;
> + polling = target_state->flags & (CPUIDLE_FLAG_MWAIT | CPUIDLE_FLAG_POLLING);
>
> /*
> * If the target state doesn't poll on need_resched(), this is
> diff --git a/drivers/cpuidle/poll_state.c b/drivers/cpuidle/poll_state.c
> index 9b6d90a72601..d69936e2517e 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;
> }
>
> --
> 2.46.0
>
Powered by blists - more mailing lists