| 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
| ||
|
Date: Tue, 12 Dec 2023 14:27:38 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Frederic Weisbecker <frederic@...nel.org>
Cc: LKML <linux-kernel@...r.kernel.org>,
"Rafael J . Wysocki" <rafael@...nel.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>,
Peter Zijlstra <peterz@...radead.org>
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@...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 polling states.
>
> Signed-off-by: Frederic Weisbecker <frederic@...nel.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;
> }
>
> --
Powered by blists - more mailing lists