2022-09-19 10:19:57

by Peter Zijlstra

Subject: [PATCH v2 03/44] cpuidle/poll: Ensure IRQ state is invariant

cpuidle_state::enter() methods should be IRQ invariant

Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Reviewed-by: Rafael J. Wysocki <[email protected]>
---
drivers/cpuidle/poll_state.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

--- a/drivers/cpuidle/poll_state.c
+++ b/drivers/cpuidle/poll_state.c
@@ -17,7 +17,7 @@ static int __cpuidle poll_idle(struct cp

dev->poll_time_limit = false;

- local_irq_enable();
+ raw_local_irq_enable();
if (!current_set_polling_and_test()) {
unsigned int loop_count = 0;
u64 limit;
@@ -36,6 +36,8 @@ static int __cpuidle poll_idle(struct cp
}
}
}
+ raw_local_irq_disable();
+
current_clr_polling();

return index;

2022-09-19 14:07:52

by Frederic Weisbecker

Subject: Re: [PATCH v2 03/44] cpuidle/poll: Ensure IRQ state is invariant

On Mon, Sep 19, 2022 at 11:59:42AM +0200, Peter Zijlstra wrote:
> cpuidle_state::enter() methods should be IRQ invariant

Got a bit confused with the invariant thing since the first chunck I
see in this patch is a conversion to an non-traceable local_irq_enable().

Maybe just add a short mention about that and why?

Thanks.

>
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> Reviewed-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/cpuidle/poll_state.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> --- a/drivers/cpuidle/poll_state.c
> +++ b/drivers/cpuidle/poll_state.c
> @@ -17,7 +17,7 @@ static int __cpuidle poll_idle(struct cp
>
> dev->poll_time_limit = false;
>
> - local_irq_enable();
> + raw_local_irq_enable();
> if (!current_set_polling_and_test()) {
> unsigned int loop_count = 0;
> u64 limit;
> @@ -36,6 +36,8 @@ static int __cpuidle poll_idle(struct cp
> }
> }
> }
> + raw_local_irq_disable();
> +
> current_clr_polling();
>
> return index;
>
>

2022-09-20 09:29:44

by Peter Zijlstra

Subject: Re: [PATCH v2 03/44] cpuidle/poll: Ensure IRQ state is invariant

On Mon, Sep 19, 2022 at 03:19:27PM +0200, Frederic Weisbecker wrote:
> On Mon, Sep 19, 2022 at 11:59:42AM +0200, Peter Zijlstra wrote:
> > cpuidle_state::enter() methods should be IRQ invariant
>
> Got a bit confused with the invariant thing since the first chunck I
> see in this patch is a conversion to an non-traceable local_irq_enable().
>
> Maybe just add a short mention about that and why?

Changelog now reads:

---
Subject: cpuidle/poll: Ensure IRQ state is invariant
From: Peter Zijlstra <[email protected]>
Date: Tue May 31 15:43:32 CEST 2022

cpuidle_state::enter() methods should be IRQ invariant.

Additionally make sure to use raw_local_irq_*() methods since this
cpuidle callback will be called with RCU already disabled.

Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Reviewed-by: Rafael J. Wysocki <[email protected]>
---

2022-09-20 11:12:38

by Frederic Weisbecker

Subject: Re: [PATCH v2 03/44] cpuidle/poll: Ensure IRQ state is invariant

On Tue, Sep 20, 2022 at 10:57:00AM +0200, Peter Zijlstra wrote:
> On Mon, Sep 19, 2022 at 03:19:27PM +0200, Frederic Weisbecker wrote:
> > On Mon, Sep 19, 2022 at 11:59:42AM +0200, Peter Zijlstra wrote:
> > > cpuidle_state::enter() methods should be IRQ invariant
> >
> > Got a bit confused with the invariant thing since the first chunck I
> > see in this patch is a conversion to an non-traceable local_irq_enable().
> >
> > Maybe just add a short mention about that and why?
>
> Changelog now reads:
>
> ---
> Subject: cpuidle/poll: Ensure IRQ state is invariant
> From: Peter Zijlstra <[email protected]>
> Date: Tue May 31 15:43:32 CEST 2022
>
> cpuidle_state::enter() methods should be IRQ invariant.
>
> Additionally make sure to use raw_local_irq_*() methods since this
> cpuidle callback will be called with RCU already disabled.
>
> Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
> Reviewed-by: Rafael J. Wysocki <[email protected]>

Reviewed-by: Frederic Weisbecker <[email protected]>

Thanks!