* [RFC][PATCH 1/4] acpi: Use CPUIDLE_FLAG_TIMER_STOP
2020-09-15 10:31 [RFC][PATCH 0/4] Fix up ACPI processor idle vs RCU Peter Zijlstra
@ 2020-09-15 10:31 ` Peter Zijlstra
2020-09-15 16:26 ` Rafael J. Wysocki
2020-09-22 3:26 ` Guenter Roeck
2020-09-15 10:31 ` [RFC][PATCH 2/4] acpi: Use CPUIDLE_FLAG_TLB_FLUSHED Peter Zijlstra
` (3 subsequent siblings)
4 siblings, 2 replies; 22+ messages in thread
From: Peter Zijlstra @ 2020-09-15 10:31 UTC (permalink / raw)
To: rjw, bp
Cc: x86, tony.luck, lenb, daniel.lezcano, linux-kernel, linux-acpi,
linux-pm, ulf.hansson, paulmck, tglx, naresh.kamboju, peterz
Make acpi_processor_idle use the common broadcast code, there's no
reason not to. This also removes some RCU usage after
rcu_idle_enter().
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
drivers/acpi/processor_idle.c | 49 +++++++++++++-----------------------------
1 file changed, 16 insertions(+), 33 deletions(-)
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -161,18 +161,10 @@ static void lapic_timer_propagate_broadc
}
/* Power(C) State timer broadcast control */
-static void lapic_timer_state_broadcast(struct acpi_processor *pr,
- struct acpi_processor_cx *cx,
- int broadcast)
-{
- int state = cx - pr->power.states;
-
- if (state >= pr->power.timer_broadcast_on_state) {
- if (broadcast)
- tick_broadcast_enter();
- else
- tick_broadcast_exit();
- }
+static bool lapic_timer_needs_broadcast(struct acpi_processor *pr,
+ struct acpi_processor_cx *cx)
+{
+ return cx - pr->power.states >= pr->power.timer_broadcast_on_state;
}
#else
@@ -180,9 +172,9 @@ static void lapic_timer_state_broadcast(
static void lapic_timer_check_state(int state, struct acpi_processor *pr,
struct acpi_processor_cx *cstate) { }
static void lapic_timer_propagate_broadcast(struct acpi_processor *pr) { }
-static void lapic_timer_state_broadcast(struct acpi_processor *pr,
- struct acpi_processor_cx *cx,
- int broadcast)
+
+static bool lapic_timer_needs_broadcast(struct acpi_processor *pr,
+ struct acpi_processor_cx *cx)
{
}
@@ -568,21 +560,13 @@ static DEFINE_RAW_SPINLOCK(c3_lock);
* acpi_idle_enter_bm - enters C3 with proper BM handling
* @pr: Target processor
* @cx: Target state context
- * @timer_bc: Whether or not to change timer mode to broadcast
*/
static void acpi_idle_enter_bm(struct acpi_processor *pr,
- struct acpi_processor_cx *cx, bool timer_bc)
+ struct acpi_processor_cx *cx)
{
acpi_unlazy_tlb(smp_processor_id());
/*
- * Must be done before busmaster disable as we might need to
- * access HPET !
- */
- if (timer_bc)
- lapic_timer_state_broadcast(pr, cx, 1);
-
- /*
* disable bus master
* bm_check implies we need ARB_DIS
* bm_control implies whether we can do ARB_DIS
@@ -609,9 +593,6 @@ static void acpi_idle_enter_bm(struct ac
c3_cpu_count--;
raw_spin_unlock(&c3_lock);
}
-
- if (timer_bc)
- lapic_timer_state_broadcast(pr, cx, 0);
}
static int acpi_idle_enter(struct cpuidle_device *dev,
@@ -630,7 +611,7 @@ static int acpi_idle_enter(struct cpuidl
cx = per_cpu(acpi_cstate[index], dev->cpu);
} else if (cx->type == ACPI_STATE_C3 && pr->flags.bm_check) {
if (cx->bm_sts_skip || !acpi_idle_bm_check()) {
- acpi_idle_enter_bm(pr, cx, true);
+ acpi_idle_enter_bm(pr, cx);
return index;
} else if (drv->safe_state_index >= 0) {
index = drv->safe_state_index;
@@ -642,15 +623,11 @@ static int acpi_idle_enter(struct cpuidl
}
}
- lapic_timer_state_broadcast(pr, cx, 1);
-
if (cx->type == ACPI_STATE_C3)
ACPI_FLUSH_CPU_CACHE();
acpi_idle_do_entry(cx);
- lapic_timer_state_broadcast(pr, cx, 0);
-
return index;
}
@@ -666,7 +643,7 @@ static int acpi_idle_enter_s2idle(struct
return 0;
if (pr->flags.bm_check) {
- acpi_idle_enter_bm(pr, cx, false);
+ acpi_idle_enter_bm(pr, cx);
return 0;
} else {
ACPI_FLUSH_CPU_CACHE();
@@ -682,6 +659,7 @@ static int acpi_processor_setup_cpuidle_
{
int i, count = ACPI_IDLE_STATE_START;
struct acpi_processor_cx *cx;
+ struct cpuidle_state *state;
if (max_cstate == 0)
max_cstate = 1;
@@ -694,6 +672,11 @@ static int acpi_processor_setup_cpuidle_
per_cpu(acpi_cstate[count], dev->cpu) = cx;
+ if (lapic_timer_needs_broadcast(pr, cx)) {
+ state = &acpi_idle_driver.states[count];
+ state->flags |= CPUIDLE_FLAG_TIMER_STOP;
+ }
+
count++;
if (count == CPUIDLE_STATE_MAX)
break;
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC][PATCH 1/4] acpi: Use CPUIDLE_FLAG_TIMER_STOP
2020-09-15 10:31 ` [RFC][PATCH 1/4] acpi: Use CPUIDLE_FLAG_TIMER_STOP Peter Zijlstra
@ 2020-09-15 16:26 ` Rafael J. Wysocki
2020-09-16 15:42 ` peterz
2020-09-22 3:26 ` Guenter Roeck
1 sibling, 1 reply; 22+ messages in thread
From: Rafael J. Wysocki @ 2020-09-15 16:26 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Rafael J. Wysocki, Borislav Petkov, the arch/x86 maintainers,
Tony Luck, Len Brown, Daniel Lezcano, Linux Kernel Mailing List,
ACPI Devel Maling List, Linux PM, Ulf Hansson, Paul E. McKenney,
Thomas Gleixner, Naresh Kamboju
On Tue, Sep 15, 2020 at 12:44 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> Make acpi_processor_idle use the common broadcast code, there's no
> reason not to. This also removes some RCU usage after
> rcu_idle_enter().
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
The whole series looks good to me, so please feel free to add
Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
to all of the four patches.
Alternatively, please let me know if you want me to take the patches.
> ---
> drivers/acpi/processor_idle.c | 49 +++++++++++++-----------------------------
> 1 file changed, 16 insertions(+), 33 deletions(-)
>
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -161,18 +161,10 @@ static void lapic_timer_propagate_broadc
> }
>
> /* Power(C) State timer broadcast control */
> -static void lapic_timer_state_broadcast(struct acpi_processor *pr,
> - struct acpi_processor_cx *cx,
> - int broadcast)
> -{
> - int state = cx - pr->power.states;
> -
> - if (state >= pr->power.timer_broadcast_on_state) {
> - if (broadcast)
> - tick_broadcast_enter();
> - else
> - tick_broadcast_exit();
> - }
> +static bool lapic_timer_needs_broadcast(struct acpi_processor *pr,
> + struct acpi_processor_cx *cx)
> +{
> + return cx - pr->power.states >= pr->power.timer_broadcast_on_state;
> }
>
> #else
> @@ -180,9 +172,9 @@ static void lapic_timer_state_broadcast(
> static void lapic_timer_check_state(int state, struct acpi_processor *pr,
> struct acpi_processor_cx *cstate) { }
> static void lapic_timer_propagate_broadcast(struct acpi_processor *pr) { }
> -static void lapic_timer_state_broadcast(struct acpi_processor *pr,
> - struct acpi_processor_cx *cx,
> - int broadcast)
> +
> +static bool lapic_timer_needs_broadcast(struct acpi_processor *pr,
> + struct acpi_processor_cx *cx)
> {
> }
>
> @@ -568,21 +560,13 @@ static DEFINE_RAW_SPINLOCK(c3_lock);
> * acpi_idle_enter_bm - enters C3 with proper BM handling
> * @pr: Target processor
> * @cx: Target state context
> - * @timer_bc: Whether or not to change timer mode to broadcast
> */
> static void acpi_idle_enter_bm(struct acpi_processor *pr,
> - struct acpi_processor_cx *cx, bool timer_bc)
> + struct acpi_processor_cx *cx)
> {
> acpi_unlazy_tlb(smp_processor_id());
>
> /*
> - * Must be done before busmaster disable as we might need to
> - * access HPET !
> - */
> - if (timer_bc)
> - lapic_timer_state_broadcast(pr, cx, 1);
> -
> - /*
> * disable bus master
> * bm_check implies we need ARB_DIS
> * bm_control implies whether we can do ARB_DIS
> @@ -609,9 +593,6 @@ static void acpi_idle_enter_bm(struct ac
> c3_cpu_count--;
> raw_spin_unlock(&c3_lock);
> }
> -
> - if (timer_bc)
> - lapic_timer_state_broadcast(pr, cx, 0);
> }
>
> static int acpi_idle_enter(struct cpuidle_device *dev,
> @@ -630,7 +611,7 @@ static int acpi_idle_enter(struct cpuidl
> cx = per_cpu(acpi_cstate[index], dev->cpu);
> } else if (cx->type == ACPI_STATE_C3 && pr->flags.bm_check) {
> if (cx->bm_sts_skip || !acpi_idle_bm_check()) {
> - acpi_idle_enter_bm(pr, cx, true);
> + acpi_idle_enter_bm(pr, cx);
> return index;
> } else if (drv->safe_state_index >= 0) {
> index = drv->safe_state_index;
> @@ -642,15 +623,11 @@ static int acpi_idle_enter(struct cpuidl
> }
> }
>
> - lapic_timer_state_broadcast(pr, cx, 1);
> -
> if (cx->type == ACPI_STATE_C3)
> ACPI_FLUSH_CPU_CACHE();
>
> acpi_idle_do_entry(cx);
>
> - lapic_timer_state_broadcast(pr, cx, 0);
> -
> return index;
> }
>
> @@ -666,7 +643,7 @@ static int acpi_idle_enter_s2idle(struct
> return 0;
>
> if (pr->flags.bm_check) {
> - acpi_idle_enter_bm(pr, cx, false);
> + acpi_idle_enter_bm(pr, cx);
> return 0;
> } else {
> ACPI_FLUSH_CPU_CACHE();
> @@ -682,6 +659,7 @@ static int acpi_processor_setup_cpuidle_
> {
> int i, count = ACPI_IDLE_STATE_START;
> struct acpi_processor_cx *cx;
> + struct cpuidle_state *state;
>
> if (max_cstate == 0)
> max_cstate = 1;
> @@ -694,6 +672,11 @@ static int acpi_processor_setup_cpuidle_
>
> per_cpu(acpi_cstate[count], dev->cpu) = cx;
>
> + if (lapic_timer_needs_broadcast(pr, cx)) {
> + state = &acpi_idle_driver.states[count];
> + state->flags |= CPUIDLE_FLAG_TIMER_STOP;
> + }
> +
> count++;
> if (count == CPUIDLE_STATE_MAX)
> break;
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC][PATCH 1/4] acpi: Use CPUIDLE_FLAG_TIMER_STOP
2020-09-15 16:26 ` Rafael J. Wysocki
@ 2020-09-16 15:42 ` peterz
2020-09-16 16:01 ` Borislav Petkov
2020-09-16 17:38 ` Rafael J. Wysocki
0 siblings, 2 replies; 22+ messages in thread
From: peterz @ 2020-09-16 15:42 UTC (permalink / raw)
To: Rafael J. Wysocki
Cc: Rafael J. Wysocki, Borislav Petkov, the arch/x86 maintainers,
Tony Luck, Len Brown, Daniel Lezcano, Linux Kernel Mailing List,
ACPI Devel Maling List, Linux PM, Ulf Hansson, Paul E. McKenney,
Thomas Gleixner, Naresh Kamboju
On Tue, Sep 15, 2020 at 06:26:52PM +0200, Rafael J. Wysocki wrote:
> On Tue, Sep 15, 2020 at 12:44 PM Peter Zijlstra <peterz@infradead.org> wrote:
> >
> > Make acpi_processor_idle use the common broadcast code, there's no
> > reason not to. This also removes some RCU usage after
> > rcu_idle_enter().
> >
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>
> The whole series looks good to me, so please feel free to add
>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
>
> to all of the four patches.
>
> Alternatively, please let me know if you want me to take the patches.
Feel free to take them. All the prerequisite borkage is in linus' tree
already.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC][PATCH 1/4] acpi: Use CPUIDLE_FLAG_TIMER_STOP
2020-09-16 15:42 ` peterz
@ 2020-09-16 16:01 ` Borislav Petkov
2020-09-16 17:38 ` Rafael J. Wysocki
2020-09-16 17:38 ` Rafael J. Wysocki
1 sibling, 1 reply; 22+ messages in thread
From: Borislav Petkov @ 2020-09-16 16:01 UTC (permalink / raw)
To: peterz
Cc: Rafael J. Wysocki, Rafael J. Wysocki, the arch/x86 maintainers,
Tony Luck, Len Brown, Daniel Lezcano, Linux Kernel Mailing List,
ACPI Devel Maling List, Linux PM, Ulf Hansson, Paul E. McKenney,
Thomas Gleixner, Naresh Kamboju
On Wed, Sep 16, 2020 at 05:42:12PM +0200, peterz@infradead.org wrote:
> On Tue, Sep 15, 2020 at 06:26:52PM +0200, Rafael J. Wysocki wrote:
> > On Tue, Sep 15, 2020 at 12:44 PM Peter Zijlstra <peterz@infradead.org> wrote:
> > >
> > > Make acpi_processor_idle use the common broadcast code, there's no
> > > reason not to. This also removes some RCU usage after
> > > rcu_idle_enter().
> > >
> > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> >
> > The whole series looks good to me, so please feel free to add
> >
> > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > to all of the four patches.
> >
> > Alternatively, please let me know if you want me to take the patches.
>
> Feel free to take them. All the prerequisite borkage is in linus' tree
> already.
You can add:
Reported-by: Borislav Petkov <bp@suse.de>
for this one and for this whole series:
Tested-by: Borislav Petkov <bp@suse.de>
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC][PATCH 1/4] acpi: Use CPUIDLE_FLAG_TIMER_STOP
2020-09-16 16:01 ` Borislav Petkov
@ 2020-09-16 17:38 ` Rafael J. Wysocki
0 siblings, 0 replies; 22+ messages in thread
From: Rafael J. Wysocki @ 2020-09-16 17:38 UTC (permalink / raw)
To: Borislav Petkov
Cc: Peter Zijlstra, Rafael J. Wysocki, Rafael J. Wysocki,
the arch/x86 maintainers, Tony Luck, Len Brown, Daniel Lezcano,
Linux Kernel Mailing List, ACPI Devel Maling List, Linux PM,
Ulf Hansson, Paul E. McKenney, Thomas Gleixner, Naresh Kamboju
On Wed, Sep 16, 2020 at 6:01 PM Borislav Petkov <bp@alien8.de> wrote:
>
> On Wed, Sep 16, 2020 at 05:42:12PM +0200, peterz@infradead.org wrote:
> > On Tue, Sep 15, 2020 at 06:26:52PM +0200, Rafael J. Wysocki wrote:
> > > On Tue, Sep 15, 2020 at 12:44 PM Peter Zijlstra <peterz@infradead.org> wrote:
> > > >
> > > > Make acpi_processor_idle use the common broadcast code, there's no
> > > > reason not to. This also removes some RCU usage after
> > > > rcu_idle_enter().
> > > >
> > > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > >
> > > The whole series looks good to me, so please feel free to add
> > >
> > > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > >
> > > to all of the four patches.
> > >
> > > Alternatively, please let me know if you want me to take the patches.
> >
> > Feel free to take them. All the prerequisite borkage is in linus' tree
> > already.
>
> You can add:
>
> Reported-by: Borislav Petkov <bp@suse.de>
>
> for this one and for this whole series:
>
> Tested-by: Borislav Petkov <bp@suse.de>
Done.
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC][PATCH 1/4] acpi: Use CPUIDLE_FLAG_TIMER_STOP
2020-09-16 15:42 ` peterz
2020-09-16 16:01 ` Borislav Petkov
@ 2020-09-16 17:38 ` Rafael J. Wysocki
1 sibling, 0 replies; 22+ messages in thread
From: Rafael J. Wysocki @ 2020-09-16 17:38 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Rafael J. Wysocki, Rafael J. Wysocki, Borislav Petkov,
the arch/x86 maintainers, Tony Luck, Len Brown, Daniel Lezcano,
Linux Kernel Mailing List, ACPI Devel Maling List, Linux PM,
Ulf Hansson, Paul E. McKenney, Thomas Gleixner, Naresh Kamboju
On Wed, Sep 16, 2020 at 5:42 PM <peterz@infradead.org> wrote:
>
> On Tue, Sep 15, 2020 at 06:26:52PM +0200, Rafael J. Wysocki wrote:
> > On Tue, Sep 15, 2020 at 12:44 PM Peter Zijlstra <peterz@infradead.org> wrote:
> > >
> > > Make acpi_processor_idle use the common broadcast code, there's no
> > > reason not to. This also removes some RCU usage after
> > > rcu_idle_enter().
> > >
> > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> >
> > The whole series looks good to me, so please feel free to add
> >
> > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > to all of the four patches.
> >
> > Alternatively, please let me know if you want me to take the patches.
>
> Feel free to take them. All the prerequisite borkage is in linus' tree
> already.
OK
All applied (with some minor edits in the subjects) for 5.9-rc6.
Thanks!
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC][PATCH 1/4] acpi: Use CPUIDLE_FLAG_TIMER_STOP
2020-09-15 10:31 ` [RFC][PATCH 1/4] acpi: Use CPUIDLE_FLAG_TIMER_STOP Peter Zijlstra
2020-09-15 16:26 ` Rafael J. Wysocki
@ 2020-09-22 3:26 ` Guenter Roeck
2020-09-22 19:03 ` Rafael J. Wysocki
1 sibling, 1 reply; 22+ messages in thread
From: Guenter Roeck @ 2020-09-22 3:26 UTC (permalink / raw)
To: Peter Zijlstra
Cc: rjw, bp, x86, tony.luck, lenb, daniel.lezcano, linux-kernel,
linux-acpi, linux-pm, ulf.hansson, paulmck, tglx, naresh.kamboju
On Tue, Sep 15, 2020 at 12:31:58PM +0200, Peter Zijlstra wrote:
> Make acpi_processor_idle use the common broadcast code, there's no
> reason not to. This also removes some RCU usage after
> rcu_idle_enter().
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> Reported-by: Borislav Petkov <bp@suse.de>
> Tested-by: Borislav Petkov <bp@suse.de>
> ---
> drivers/acpi/processor_idle.c | 49 +++++++++++++-----------------------------
> 1 file changed, 16 insertions(+), 33 deletions(-)
>
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -161,18 +161,10 @@ static void lapic_timer_propagate_broadc
> }
>
> /* Power(C) State timer broadcast control */
> -static void lapic_timer_state_broadcast(struct acpi_processor *pr,
> - struct acpi_processor_cx *cx,
> - int broadcast)
> -{
> - int state = cx - pr->power.states;
> -
> - if (state >= pr->power.timer_broadcast_on_state) {
> - if (broadcast)
> - tick_broadcast_enter();
> - else
> - tick_broadcast_exit();
> - }
> +static bool lapic_timer_needs_broadcast(struct acpi_processor *pr,
> + struct acpi_processor_cx *cx)
> +{
> + return cx - pr->power.states >= pr->power.timer_broadcast_on_state;
> }
>
> #else
> @@ -180,9 +172,9 @@ static void lapic_timer_state_broadcast(
> static void lapic_timer_check_state(int state, struct acpi_processor *pr,
> struct acpi_processor_cx *cstate) { }
> static void lapic_timer_propagate_broadcast(struct acpi_processor *pr) { }
> -static void lapic_timer_state_broadcast(struct acpi_processor *pr,
> - struct acpi_processor_cx *cx,
> - int broadcast)
> +
> +static bool lapic_timer_needs_broadcast(struct acpi_processor *pr,
> + struct acpi_processor_cx *cx)
> {
> }
drivers/acpi/processor_idle.c: In function 'lapic_timer_needs_broadcast':
drivers/acpi/processor_idle.c:179:1: warning: no return statement in function returning non-void [-Wreturn-type]
Should this return true or false ?
Guenter
>
> @@ -568,21 +560,13 @@ static DEFINE_RAW_SPINLOCK(c3_lock);
> * acpi_idle_enter_bm - enters C3 with proper BM handling
> * @pr: Target processor
> * @cx: Target state context
> - * @timer_bc: Whether or not to change timer mode to broadcast
> */
> static void acpi_idle_enter_bm(struct acpi_processor *pr,
> - struct acpi_processor_cx *cx, bool timer_bc)
> + struct acpi_processor_cx *cx)
> {
> acpi_unlazy_tlb(smp_processor_id());
>
> /*
> - * Must be done before busmaster disable as we might need to
> - * access HPET !
> - */
> - if (timer_bc)
> - lapic_timer_state_broadcast(pr, cx, 1);
> -
> - /*
> * disable bus master
> * bm_check implies we need ARB_DIS
> * bm_control implies whether we can do ARB_DIS
> @@ -609,9 +593,6 @@ static void acpi_idle_enter_bm(struct ac
> c3_cpu_count--;
> raw_spin_unlock(&c3_lock);
> }
> -
> - if (timer_bc)
> - lapic_timer_state_broadcast(pr, cx, 0);
> }
>
> static int acpi_idle_enter(struct cpuidle_device *dev,
> @@ -630,7 +611,7 @@ static int acpi_idle_enter(struct cpuidl
> cx = per_cpu(acpi_cstate[index], dev->cpu);
> } else if (cx->type == ACPI_STATE_C3 && pr->flags.bm_check) {
> if (cx->bm_sts_skip || !acpi_idle_bm_check()) {
> - acpi_idle_enter_bm(pr, cx, true);
> + acpi_idle_enter_bm(pr, cx);
> return index;
> } else if (drv->safe_state_index >= 0) {
> index = drv->safe_state_index;
> @@ -642,15 +623,11 @@ static int acpi_idle_enter(struct cpuidl
> }
> }
>
> - lapic_timer_state_broadcast(pr, cx, 1);
> -
> if (cx->type == ACPI_STATE_C3)
> ACPI_FLUSH_CPU_CACHE();
>
> acpi_idle_do_entry(cx);
>
> - lapic_timer_state_broadcast(pr, cx, 0);
> -
> return index;
> }
>
> @@ -666,7 +643,7 @@ static int acpi_idle_enter_s2idle(struct
> return 0;
>
> if (pr->flags.bm_check) {
> - acpi_idle_enter_bm(pr, cx, false);
> + acpi_idle_enter_bm(pr, cx);
> return 0;
> } else {
> ACPI_FLUSH_CPU_CACHE();
> @@ -682,6 +659,7 @@ static int acpi_processor_setup_cpuidle_
> {
> int i, count = ACPI_IDLE_STATE_START;
> struct acpi_processor_cx *cx;
> + struct cpuidle_state *state;
>
> if (max_cstate == 0)
> max_cstate = 1;
> @@ -694,6 +672,11 @@ static int acpi_processor_setup_cpuidle_
>
> per_cpu(acpi_cstate[count], dev->cpu) = cx;
>
> + if (lapic_timer_needs_broadcast(pr, cx)) {
> + state = &acpi_idle_driver.states[count];
> + state->flags |= CPUIDLE_FLAG_TIMER_STOP;
> + }
> +
> count++;
> if (count == CPUIDLE_STATE_MAX)
> break;
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC][PATCH 1/4] acpi: Use CPUIDLE_FLAG_TIMER_STOP
2020-09-22 3:26 ` Guenter Roeck
@ 2020-09-22 19:03 ` Rafael J. Wysocki
0 siblings, 0 replies; 22+ messages in thread
From: Rafael J. Wysocki @ 2020-09-22 19:03 UTC (permalink / raw)
To: Guenter Roeck
Cc: Peter Zijlstra, bp, x86, tony.luck, lenb, daniel.lezcano,
linux-kernel, linux-acpi, linux-pm, ulf.hansson, paulmck, tglx,
naresh.kamboju
On Tuesday, September 22, 2020 5:26:51 AM CEST Guenter Roeck wrote:
> On Tue, Sep 15, 2020 at 12:31:58PM +0200, Peter Zijlstra wrote:
> > Make acpi_processor_idle use the common broadcast code, there's no
> > reason not to. This also removes some RCU usage after
> > rcu_idle_enter().
> >
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> > Reported-by: Borislav Petkov <bp@suse.de>
> > Tested-by: Borislav Petkov <bp@suse.de>
> > ---
> > drivers/acpi/processor_idle.c | 49 +++++++++++++-----------------------------
> > 1 file changed, 16 insertions(+), 33 deletions(-)
> >
> > --- a/drivers/acpi/processor_idle.c
> > +++ b/drivers/acpi/processor_idle.c
> > @@ -161,18 +161,10 @@ static void lapic_timer_propagate_broadc
> > }
> >
> > /* Power(C) State timer broadcast control */
> > -static void lapic_timer_state_broadcast(struct acpi_processor *pr,
> > - struct acpi_processor_cx *cx,
> > - int broadcast)
> > -{
> > - int state = cx - pr->power.states;
> > -
> > - if (state >= pr->power.timer_broadcast_on_state) {
> > - if (broadcast)
> > - tick_broadcast_enter();
> > - else
> > - tick_broadcast_exit();
> > - }
> > +static bool lapic_timer_needs_broadcast(struct acpi_processor *pr,
> > + struct acpi_processor_cx *cx)
> > +{
> > + return cx - pr->power.states >= pr->power.timer_broadcast_on_state;
> > }
> >
> > #else
> > @@ -180,9 +172,9 @@ static void lapic_timer_state_broadcast(
> > static void lapic_timer_check_state(int state, struct acpi_processor *pr,
> > struct acpi_processor_cx *cstate) { }
> > static void lapic_timer_propagate_broadcast(struct acpi_processor *pr) { }
> > -static void lapic_timer_state_broadcast(struct acpi_processor *pr,
> > - struct acpi_processor_cx *cx,
> > - int broadcast)
> > +
> > +static bool lapic_timer_needs_broadcast(struct acpi_processor *pr,
> > + struct acpi_processor_cx *cx)
> > {
> > }
>
> drivers/acpi/processor_idle.c: In function 'lapic_timer_needs_broadcast':
> drivers/acpi/processor_idle.c:179:1: warning: no return statement in function returning non-void [-Wreturn-type]
>
> Should this return true or false ?
false - if the lapic timer doesn't stop, it doesn't need broadcast.
FWIW, patch appended.
Cheers!
---
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Subject: [PATCH] ACPI: processor: Fix build for ARCH_APICTIMER_STOPS_ON_C3 unset
Fix the lapic_timer_needs_broadcast() stub for
ARCH_APICTIMER_STOPS_ON_C3 unset to actually return
a value.
Fixes: aa6b43d57f99 ("ACPI: processor: Use CPUIDLE_FLAG_TIMER_STOP")
Reported-by: Guenter Roeck <linux@roeck-us.net>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
drivers/acpi/processor_idle.c | 1 +
1 file changed, 1 insertion(+)
Index: linux-pm/drivers/acpi/processor_idle.c
===================================================================
--- linux-pm.orig/drivers/acpi/processor_idle.c
+++ linux-pm/drivers/acpi/processor_idle.c
@@ -176,6 +176,7 @@ static void lapic_timer_propagate_broadc
static bool lapic_timer_needs_broadcast(struct acpi_processor *pr,
struct acpi_processor_cx *cx)
{
+ return false;
}
#endif
^ permalink raw reply [flat|nested] 22+ messages in thread
* [RFC][PATCH 2/4] acpi: Use CPUIDLE_FLAG_TLB_FLUSHED
2020-09-15 10:31 [RFC][PATCH 0/4] Fix up ACPI processor idle vs RCU Peter Zijlstra
2020-09-15 10:31 ` [RFC][PATCH 1/4] acpi: Use CPUIDLE_FLAG_TIMER_STOP Peter Zijlstra
@ 2020-09-15 10:31 ` Peter Zijlstra
2020-09-15 10:32 ` [RFC][PATCH 3/4] cpuidle: Allow cpuidle drivers to take over RCU-idle Peter Zijlstra
` (2 subsequent siblings)
4 siblings, 0 replies; 22+ messages in thread
From: Peter Zijlstra @ 2020-09-15 10:31 UTC (permalink / raw)
To: rjw, bp
Cc: x86, tony.luck, lenb, daniel.lezcano, linux-kernel, linux-acpi,
linux-pm, ulf.hansson, paulmck, tglx, naresh.kamboju, peterz
Make acpi_processor_idle() use the generic TLB flushing code.
This again removes RCU usage after rcu_idle_enter().
(XXX make every C3 invalidate TLBs, not just C3-BM)
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
arch/ia64/include/asm/acpi.h | 2 --
arch/x86/include/asm/acpi.h | 2 --
drivers/acpi/processor_idle.c | 10 +++++-----
3 files changed, 5 insertions(+), 9 deletions(-)
--- a/arch/ia64/include/asm/acpi.h
+++ b/arch/ia64/include/asm/acpi.h
@@ -74,8 +74,6 @@ static inline void arch_acpi_set_pdc_bit
buf[2] |= ACPI_PDC_EST_CAPABILITY_SMP;
}
-#define acpi_unlazy_tlb(x)
-
#ifdef CONFIG_ACPI_NUMA
extern cpumask_t early_cpu_possible_map;
#define for_each_possible_early_cpu(cpu) \
--- a/arch/x86/include/asm/acpi.h
+++ b/arch/x86/include/asm/acpi.h
@@ -159,8 +159,6 @@ static inline u64 x86_default_get_root_p
extern int x86_acpi_numa_init(void);
#endif /* CONFIG_ACPI_NUMA */
-#define acpi_unlazy_tlb(x) leave_mm(x)
-
#ifdef CONFIG_ACPI_APEI
static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)
{
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -565,8 +565,6 @@ static DEFINE_RAW_SPINLOCK(c3_lock);
static void acpi_idle_enter_bm(struct acpi_processor *pr,
struct acpi_processor_cx *cx)
{
- acpi_unlazy_tlb(smp_processor_id());
-
/*
* disable bus master
* bm_check implies we need ARB_DIS
@@ -666,6 +664,7 @@ static int acpi_processor_setup_cpuidle_
max_cstate = 1;
for (i = 1; i < ACPI_PROCESSOR_MAX_POWER && i <= max_cstate; i++) {
+ state = &acpi_idle_driver.states[count];
cx = &pr->power.states[i];
if (!cx->valid)
@@ -673,10 +672,11 @@ static int acpi_processor_setup_cpuidle_
per_cpu(acpi_cstate[count], dev->cpu) = cx;
- if (lapic_timer_needs_broadcast(pr, cx)) {
- state = &acpi_idle_driver.states[count];
+ if (lapic_timer_needs_broadcast(pr, cx))
state->flags |= CPUIDLE_FLAG_TIMER_STOP;
- }
+
+ if (cx->type == ACPI_STATE_C3)
+ state->flags |= CPUIDLE_FLAG_TLB_FLUSHED;
count++;
if (count == CPUIDLE_STATE_MAX)
^ permalink raw reply [flat|nested] 22+ messages in thread
* [RFC][PATCH 3/4] cpuidle: Allow cpuidle drivers to take over RCU-idle
2020-09-15 10:31 [RFC][PATCH 0/4] Fix up ACPI processor idle vs RCU Peter Zijlstra
2020-09-15 10:31 ` [RFC][PATCH 1/4] acpi: Use CPUIDLE_FLAG_TIMER_STOP Peter Zijlstra
2020-09-15 10:31 ` [RFC][PATCH 2/4] acpi: Use CPUIDLE_FLAG_TLB_FLUSHED Peter Zijlstra
@ 2020-09-15 10:32 ` Peter Zijlstra
2020-09-15 13:20 ` Ulf Hansson
2020-09-15 10:32 ` [RFC][PATCH 4/4] acpi: Take over RCU-idle for C3-BM idle Peter Zijlstra
2020-09-15 18:31 ` [RFC][PATCH 0/4] Fix up ACPI processor idle vs RCU Borislav Petkov
4 siblings, 1 reply; 22+ messages in thread
From: Peter Zijlstra @ 2020-09-15 10:32 UTC (permalink / raw)
To: rjw, bp
Cc: x86, tony.luck, lenb, daniel.lezcano, linux-kernel, linux-acpi,
linux-pm, ulf.hansson, paulmck, tglx, naresh.kamboju, peterz
Some drivers have to do significant work, some of which relies on RCU
still being active. Instead of using RCU_NONIDLE in the drivers and
flipping RCU back on, allow drivers to take over RCU-idle duty.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
drivers/cpuidle/cpuidle.c | 15 ++++++++++-----
include/linux/cpuidle.h | 1 +
2 files changed, 11 insertions(+), 5 deletions(-)
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -138,6 +138,7 @@ static void enter_s2idle_proper(struct c
struct cpuidle_device *dev, int index)
{
ktime_t time_start, time_end;
+ struct cpuidle_state *target_state = &drv->states[index];
time_start = ns_to_ktime(local_clock());
@@ -153,8 +154,9 @@ static void enter_s2idle_proper(struct c
* suspended is generally unsafe.
*/
stop_critical_timings();
- rcu_idle_enter();
- drv->states[index].enter_s2idle(dev, drv, index);
+ if (!(target_state->flags & CPUIDLE_FLAG_RCU_IDLE))
+ rcu_idle_enter();
+ target_state->enter_s2idle(dev, drv, index);
if (WARN_ON_ONCE(!irqs_disabled()))
local_irq_disable();
/*
@@ -162,7 +164,8 @@ static void enter_s2idle_proper(struct c
* first CPU executing it calls functions containing RCU read-side
* critical sections, so tell RCU about that.
*/
- rcu_idle_exit();
+ if (!(target_state->flags & CPUIDLE_FLAG_RCU_IDLE))
+ rcu_idle_exit();
tick_unfreeze();
start_critical_timings();
@@ -239,9 +242,11 @@ int cpuidle_enter_state(struct cpuidle_d
time_start = ns_to_ktime(local_clock());
stop_critical_timings();
- rcu_idle_enter();
+ if (!(target_state->flags & CPUIDLE_FLAG_RCU_IDLE))
+ rcu_idle_enter();
entered_state = target_state->enter(dev, drv, index);
- rcu_idle_exit();
+ if (!(target_state->flags & CPUIDLE_FLAG_RCU_IDLE))
+ rcu_idle_exit();
start_critical_timings();
sched_clock_idle_wakeup_event();
--- a/include/linux/cpuidle.h
+++ b/include/linux/cpuidle.h
@@ -82,6 +82,7 @@ struct cpuidle_state {
#define CPUIDLE_FLAG_UNUSABLE BIT(3) /* avoid using this state */
#define CPUIDLE_FLAG_OFF BIT(4) /* disable this state by default */
#define CPUIDLE_FLAG_TLB_FLUSHED BIT(5) /* idle-state flushes TLBs */
+#define CPUIDLE_FLAG_RCU_IDLE BIT(6) /* idle-state takes care of RCU */
struct cpuidle_device_kobj;
struct cpuidle_state_kobj;
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC][PATCH 3/4] cpuidle: Allow cpuidle drivers to take over RCU-idle
2020-09-15 10:32 ` [RFC][PATCH 3/4] cpuidle: Allow cpuidle drivers to take over RCU-idle Peter Zijlstra
@ 2020-09-15 13:20 ` Ulf Hansson
0 siblings, 0 replies; 22+ messages in thread
From: Ulf Hansson @ 2020-09-15 13:20 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Rafael J. Wysocki, Borislav Petkov, x86, Tony Luck, Len Brown,
Daniel Lezcano, Linux Kernel Mailing List,
ACPI Devel Maling List, Linux PM, Paul E. McKenney,
Thomas Gleixner, Naresh Kamboju
On Tue, 15 Sep 2020 at 12:44, Peter Zijlstra <peterz@infradead.org> wrote:
>
> Some drivers have to do significant work, some of which relies on RCU
> still being active. Instead of using RCU_NONIDLE in the drivers and
> flipping RCU back on, allow drivers to take over RCU-idle duty.
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>
Kind regards
Uffe
> ---
> drivers/cpuidle/cpuidle.c | 15 ++++++++++-----
> include/linux/cpuidle.h | 1 +
> 2 files changed, 11 insertions(+), 5 deletions(-)
>
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -138,6 +138,7 @@ static void enter_s2idle_proper(struct c
> struct cpuidle_device *dev, int index)
> {
> ktime_t time_start, time_end;
> + struct cpuidle_state *target_state = &drv->states[index];
>
> time_start = ns_to_ktime(local_clock());
>
> @@ -153,8 +154,9 @@ static void enter_s2idle_proper(struct c
> * suspended is generally unsafe.
> */
> stop_critical_timings();
> - rcu_idle_enter();
> - drv->states[index].enter_s2idle(dev, drv, index);
> + if (!(target_state->flags & CPUIDLE_FLAG_RCU_IDLE))
> + rcu_idle_enter();
> + target_state->enter_s2idle(dev, drv, index);
> if (WARN_ON_ONCE(!irqs_disabled()))
> local_irq_disable();
> /*
> @@ -162,7 +164,8 @@ static void enter_s2idle_proper(struct c
> * first CPU executing it calls functions containing RCU read-side
> * critical sections, so tell RCU about that.
> */
> - rcu_idle_exit();
> + if (!(target_state->flags & CPUIDLE_FLAG_RCU_IDLE))
> + rcu_idle_exit();
> tick_unfreeze();
> start_critical_timings();
>
> @@ -239,9 +242,11 @@ int cpuidle_enter_state(struct cpuidle_d
> time_start = ns_to_ktime(local_clock());
>
> stop_critical_timings();
> - rcu_idle_enter();
> + if (!(target_state->flags & CPUIDLE_FLAG_RCU_IDLE))
> + rcu_idle_enter();
> entered_state = target_state->enter(dev, drv, index);
> - rcu_idle_exit();
> + if (!(target_state->flags & CPUIDLE_FLAG_RCU_IDLE))
> + rcu_idle_exit();
> start_critical_timings();
>
> sched_clock_idle_wakeup_event();
> --- a/include/linux/cpuidle.h
> +++ b/include/linux/cpuidle.h
> @@ -82,6 +82,7 @@ struct cpuidle_state {
> #define CPUIDLE_FLAG_UNUSABLE BIT(3) /* avoid using this state */
> #define CPUIDLE_FLAG_OFF BIT(4) /* disable this state by default */
> #define CPUIDLE_FLAG_TLB_FLUSHED BIT(5) /* idle-state flushes TLBs */
> +#define CPUIDLE_FLAG_RCU_IDLE BIT(6) /* idle-state takes care of RCU */
>
> struct cpuidle_device_kobj;
> struct cpuidle_state_kobj;
>
>
^ permalink raw reply [flat|nested] 22+ messages in thread
* [RFC][PATCH 4/4] acpi: Take over RCU-idle for C3-BM idle
2020-09-15 10:31 [RFC][PATCH 0/4] Fix up ACPI processor idle vs RCU Peter Zijlstra
` (2 preceding siblings ...)
2020-09-15 10:32 ` [RFC][PATCH 3/4] cpuidle: Allow cpuidle drivers to take over RCU-idle Peter Zijlstra
@ 2020-09-15 10:32 ` Peter Zijlstra
2020-09-21 9:12 ` Sven Joachim
2020-09-25 15:20 ` [RFC][PATCH 4/4] acpi: Take over RCU-idle for C3-BM idle Guenter Roeck
2020-09-15 18:31 ` [RFC][PATCH 0/4] Fix up ACPI processor idle vs RCU Borislav Petkov
4 siblings, 2 replies; 22+ messages in thread
From: Peter Zijlstra @ 2020-09-15 10:32 UTC (permalink / raw)
To: rjw, bp
Cc: x86, tony.luck, lenb, daniel.lezcano, linux-kernel, linux-acpi,
linux-pm, ulf.hansson, paulmck, tglx, naresh.kamboju, peterz
The C3 BusMaster idle code takes lock in a number of places, some deep
inside the ACPI code. Instead of wrapping it all in RCU_NONIDLE, have
the driver take over RCU-idle duty and avoid flipping RCU state back
and forth a lot.
( by marking 'C3 && bm_check' as RCU_IDLE, we _must_ call enter_bm() for
that combination, otherwise we'll loose RCU-idle, this requires
shuffling some code around )
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
drivers/acpi/processor_idle.c | 69 +++++++++++++++++++++++++++++-------------
1 file changed, 49 insertions(+), 20 deletions(-)
--- a/drivers/acpi/processor_idle.c
+++ b/drivers/acpi/processor_idle.c
@@ -558,22 +558,43 @@ static DEFINE_RAW_SPINLOCK(c3_lock);
/**
* acpi_idle_enter_bm - enters C3 with proper BM handling
+ * @drv: cpuidle driver
* @pr: Target processor
* @cx: Target state context
+ * @index: index of target state
*/
-static void acpi_idle_enter_bm(struct acpi_processor *pr,
- struct acpi_processor_cx *cx)
+static int acpi_idle_enter_bm(struct cpuidle_driver *drv,
+ struct acpi_processor *pr,
+ struct acpi_processor_cx *cx,
+ int index)
{
+ static struct acpi_processor_cx safe_cx = {
+ .entry_method = ACPI_CSTATE_HALT,
+ };
+
/*
* disable bus master
* bm_check implies we need ARB_DIS
* bm_control implies whether we can do ARB_DIS
*
- * That leaves a case where bm_check is set and bm_control is
- * not set. In that case we cannot do much, we enter C3
- * without doing anything.
+ * That leaves a case where bm_check is set and bm_control is not set.
+ * In that case we cannot do much, we enter C3 without doing anything.
*/
- if (pr->flags.bm_control) {
+ bool dis_bm = pr->flags.bm_control;
+
+ /* If we can skip BM, demote to a safe state. */
+ if (!cx->bm_sts_skip && acpi_idle_bm_check()) {
+ dis_bm = false;
+ index = drv->safe_state_index;
+ if (index >= 0) {
+ cx = this_cpu_read(acpi_cstate[index]);
+ } else {
+ cx = &safe_cx;
+ index = -EBUSY;
+ }
+ }
+
+ if (dis_bm) {
raw_spin_lock(&c3_lock);
c3_cpu_count++;
/* Disable bus master arbitration when all CPUs are in C3 */
@@ -582,15 +603,21 @@ static void acpi_idle_enter_bm(struct ac
raw_spin_unlock(&c3_lock);
}
+ rcu_idle_enter();
+
acpi_idle_do_entry(cx);
+ rcu_idle_exit();
+
/* Re-enable bus master arbitration */
- if (pr->flags.bm_control) {
+ if (dis_bm) {
raw_spin_lock(&c3_lock);
acpi_write_bit_register(ACPI_BITREG_ARB_DISABLE, 0);
c3_cpu_count--;
raw_spin_unlock(&c3_lock);
}
+
+ return index;
}
static int acpi_idle_enter(struct cpuidle_device *dev,
@@ -604,20 +631,13 @@ static int acpi_idle_enter(struct cpuidl
return -EINVAL;
if (cx->type != ACPI_STATE_C1) {
+ if (cx->type == ACPI_STATE_C3 && pr->flags.bm_check)
+ return acpi_idle_enter_bm(drv, pr, cx, index);
+
+ /* C2 to C1 demotion. */
if (acpi_idle_fallback_to_c1(pr) && num_online_cpus() > 1) {
index = ACPI_IDLE_STATE_START;
cx = per_cpu(acpi_cstate[index], dev->cpu);
- } else if (cx->type == ACPI_STATE_C3 && pr->flags.bm_check) {
- if (cx->bm_sts_skip || !acpi_idle_bm_check()) {
- acpi_idle_enter_bm(pr, cx);
- return index;
- } else if (drv->safe_state_index >= 0) {
- index = drv->safe_state_index;
- cx = per_cpu(acpi_cstate[index], dev->cpu);
- } else {
- acpi_safe_halt();
- return -EBUSY;
- }
}
}
@@ -641,7 +661,13 @@ static int acpi_idle_enter_s2idle(struct
return 0;
if (pr->flags.bm_check) {
- acpi_idle_enter_bm(pr, cx);
+ u8 bm_sts_skip = cx->bm_sts_skip;
+
+ /* Don't check BM_STS, do an unconditional ARB_DIS for S2IDLE */
+ cx->bm_sts_skip = 1;
+ acpi_idle_enter_bm(drv, pr, cx, index);
+ cx->bm_sts_skip = bm_sts_skip;
+
return 0;
} else {
ACPI_FLUSH_CPU_CACHE();
@@ -674,8 +700,11 @@ static int acpi_processor_setup_cpuidle_
if (lapic_timer_needs_broadcast(pr, cx))
state->flags |= CPUIDLE_FLAG_TIMER_STOP;
- if (cx->type == ACPI_STATE_C3)
+ if (cx->type == ACPI_STATE_C3) {
state->flags |= CPUIDLE_FLAG_TLB_FLUSHED;
+ if (pr->flags.bm_check)
+ state->flags |= CPUIDLE_FLAG_RCU_IDLE;
+ }
count++;
if (count == CPUIDLE_STATE_MAX)
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC][PATCH 4/4] acpi: Take over RCU-idle for C3-BM idle
2020-09-15 10:32 ` [RFC][PATCH 4/4] acpi: Take over RCU-idle for C3-BM idle Peter Zijlstra
@ 2020-09-21 9:12 ` Sven Joachim
2020-09-21 10:37 ` [PATCH] rcu/tree: Export rcu_idle_{enter,exit} to module Borislav Petkov
2020-09-25 15:20 ` [RFC][PATCH 4/4] acpi: Take over RCU-idle for C3-BM idle Guenter Roeck
1 sibling, 1 reply; 22+ messages in thread
From: Sven Joachim @ 2020-09-21 9:12 UTC (permalink / raw)
To: Peter Zijlstra
Cc: rjw, bp, x86, tony.luck, lenb, daniel.lezcano, linux-kernel,
linux-acpi, linux-pm, ulf.hansson, paulmck, tglx, naresh.kamboju
On 2020-09-15 12:32 +0200, Peter Zijlstra wrote:
> The C3 BusMaster idle code takes lock in a number of places, some deep
> inside the ACPI code. Instead of wrapping it all in RCU_NONIDLE, have
> the driver take over RCU-idle duty and avoid flipping RCU state back
> and forth a lot.
>
> ( by marking 'C3 && bm_check' as RCU_IDLE, we _must_ call enter_bm() for
> that combination, otherwise we'll loose RCU-idle, this requires
> shuffling some code around )
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
I got modpost errors in 5.9-rc6 after this patch:
ERROR: modpost: "rcu_idle_enter" [drivers/acpi/processor.ko] undefined!
ERROR: modpost: "rcu_idle_exit" [drivers/acpi/processor.ko] undefined!
Reverting commit 1fecfdbb7acc made them go away. Notably my
configuration had CONFIG_ACPI_PROCESSOR=m, changing that
to CONFIG_ACPI_PROCESSOR=y let the build succeed as well.
Cheers,
Sven
> ---
> drivers/acpi/processor_idle.c | 69 +++++++++++++++++++++++++++++-------------
> 1 file changed, 49 insertions(+), 20 deletions(-)
>
> --- a/drivers/acpi/processor_idle.c
> +++ b/drivers/acpi/processor_idle.c
> @@ -558,22 +558,43 @@ static DEFINE_RAW_SPINLOCK(c3_lock);
>
> /**
> * acpi_idle_enter_bm - enters C3 with proper BM handling
> + * @drv: cpuidle driver
> * @pr: Target processor
> * @cx: Target state context
> + * @index: index of target state
> */
> -static void acpi_idle_enter_bm(struct acpi_processor *pr,
> - struct acpi_processor_cx *cx)
> +static int acpi_idle_enter_bm(struct cpuidle_driver *drv,
> + struct acpi_processor *pr,
> + struct acpi_processor_cx *cx,
> + int index)
> {
> + static struct acpi_processor_cx safe_cx = {
> + .entry_method = ACPI_CSTATE_HALT,
> + };
> +
> /*
> * disable bus master
> * bm_check implies we need ARB_DIS
> * bm_control implies whether we can do ARB_DIS
> *
> - * That leaves a case where bm_check is set and bm_control is
> - * not set. In that case we cannot do much, we enter C3
> - * without doing anything.
> + * That leaves a case where bm_check is set and bm_control is not set.
> + * In that case we cannot do much, we enter C3 without doing anything.
> */
> - if (pr->flags.bm_control) {
> + bool dis_bm = pr->flags.bm_control;
> +
> + /* If we can skip BM, demote to a safe state. */
> + if (!cx->bm_sts_skip && acpi_idle_bm_check()) {
> + dis_bm = false;
> + index = drv->safe_state_index;
> + if (index >= 0) {
> + cx = this_cpu_read(acpi_cstate[index]);
> + } else {
> + cx = &safe_cx;
> + index = -EBUSY;
> + }
> + }
> +
> + if (dis_bm) {
> raw_spin_lock(&c3_lock);
> c3_cpu_count++;
> /* Disable bus master arbitration when all CPUs are in C3 */
> @@ -582,15 +603,21 @@ static void acpi_idle_enter_bm(struct ac
> raw_spin_unlock(&c3_lock);
> }
>
> + rcu_idle_enter();
> +
> acpi_idle_do_entry(cx);
>
> + rcu_idle_exit();
> +
> /* Re-enable bus master arbitration */
> - if (pr->flags.bm_control) {
> + if (dis_bm) {
> raw_spin_lock(&c3_lock);
> acpi_write_bit_register(ACPI_BITREG_ARB_DISABLE, 0);
> c3_cpu_count--;
> raw_spin_unlock(&c3_lock);
> }
> +
> + return index;
> }
>
> static int acpi_idle_enter(struct cpuidle_device *dev,
> @@ -604,20 +631,13 @@ static int acpi_idle_enter(struct cpuidl
> return -EINVAL;
>
> if (cx->type != ACPI_STATE_C1) {
> + if (cx->type == ACPI_STATE_C3 && pr->flags.bm_check)
> + return acpi_idle_enter_bm(drv, pr, cx, index);
> +
> + /* C2 to C1 demotion. */
> if (acpi_idle_fallback_to_c1(pr) && num_online_cpus() > 1) {
> index = ACPI_IDLE_STATE_START;
> cx = per_cpu(acpi_cstate[index], dev->cpu);
> - } else if (cx->type == ACPI_STATE_C3 && pr->flags.bm_check) {
> - if (cx->bm_sts_skip || !acpi_idle_bm_check()) {
> - acpi_idle_enter_bm(pr, cx);
> - return index;
> - } else if (drv->safe_state_index >= 0) {
> - index = drv->safe_state_index;
> - cx = per_cpu(acpi_cstate[index], dev->cpu);
> - } else {
> - acpi_safe_halt();
> - return -EBUSY;
> - }
> }
> }
>
> @@ -641,7 +661,13 @@ static int acpi_idle_enter_s2idle(struct
> return 0;
>
> if (pr->flags.bm_check) {
> - acpi_idle_enter_bm(pr, cx);
> + u8 bm_sts_skip = cx->bm_sts_skip;
> +
> + /* Don't check BM_STS, do an unconditional ARB_DIS for S2IDLE */
> + cx->bm_sts_skip = 1;
> + acpi_idle_enter_bm(drv, pr, cx, index);
> + cx->bm_sts_skip = bm_sts_skip;
> +
> return 0;
> } else {
> ACPI_FLUSH_CPU_CACHE();
> @@ -674,8 +700,11 @@ static int acpi_processor_setup_cpuidle_
> if (lapic_timer_needs_broadcast(pr, cx))
> state->flags |= CPUIDLE_FLAG_TIMER_STOP;
>
> - if (cx->type == ACPI_STATE_C3)
> + if (cx->type == ACPI_STATE_C3) {
> state->flags |= CPUIDLE_FLAG_TLB_FLUSHED;
> + if (pr->flags.bm_check)
> + state->flags |= CPUIDLE_FLAG_RCU_IDLE;
> + }
>
> count++;
> if (count == CPUIDLE_STATE_MAX)
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH] rcu/tree: Export rcu_idle_{enter,exit} to module
2020-09-21 9:12 ` Sven Joachim
@ 2020-09-21 10:37 ` Borislav Petkov
2020-09-21 12:15 ` Rafael J. Wysocki
2020-09-21 13:32 ` Paul E. McKenney
0 siblings, 2 replies; 22+ messages in thread
From: Borislav Petkov @ 2020-09-21 10:37 UTC (permalink / raw)
To: Sven Joachim
Cc: Peter Zijlstra, rjw, x86, tony.luck, lenb, daniel.lezcano,
linux-kernel, linux-acpi, linux-pm, ulf.hansson, paulmck, tglx,
naresh.kamboju, Josh Triplett, Steven Rostedt, Mathieu Desnoyers,
Lai Jiangshan, Joel Fernandes, rcu
Lemme add whatever get_maintainer spits, to Cc.
On Mon, Sep 21, 2020 at 11:12:33AM +0200, Sven Joachim wrote:
> On 2020-09-15 12:32 +0200, Peter Zijlstra wrote:
>
> > The C3 BusMaster idle code takes lock in a number of places, some deep
> > inside the ACPI code. Instead of wrapping it all in RCU_NONIDLE, have
> > the driver take over RCU-idle duty and avoid flipping RCU state back
> > and forth a lot.
> >
> > ( by marking 'C3 && bm_check' as RCU_IDLE, we _must_ call enter_bm() for
> > that combination, otherwise we'll loose RCU-idle, this requires
> > shuffling some code around )
> >
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>
> I got modpost errors in 5.9-rc6 after this patch:
>
> ERROR: modpost: "rcu_idle_enter" [drivers/acpi/processor.ko] undefined!
> ERROR: modpost: "rcu_idle_exit" [drivers/acpi/processor.ko] undefined!
>
> Reverting commit 1fecfdbb7acc made them go away. Notably my
> configuration had CONFIG_ACPI_PROCESSOR=m, changing that
> to CONFIG_ACPI_PROCESSOR=y let the build succeed as well.
I guess this. Running randconfigs on it for a while, to see what else
breaks.
---
From: Borislav Petkov <bp@suse.de>
Date: Mon, 21 Sep 2020 12:31:36 +0200
Fix this link error:
ERROR: modpost: "rcu_idle_enter" [drivers/acpi/processor.ko] undefined!
ERROR: modpost: "rcu_idle_exit" [drivers/acpi/processor.ko] undefined!
when CONFIG_ACPI_PROCESSOR is built as module. PeterZ says that in light
of ARM needing those soon too, they should simply be exported.
Fixes: 1fecfdbb7acc ("ACPI: processor: Take over RCU-idle for C3-BM idle")
Reported-by: Sven Joachim <svenjoac@gmx.de>
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Borislav Petkov <bp@suse.de>
---
kernel/rcu/tree.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 8ce77d9ac716..f78ee759af9c 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -673,6 +673,7 @@ void rcu_idle_enter(void)
lockdep_assert_irqs_disabled();
rcu_eqs_enter(false);
}
+EXPORT_SYMBOL_GPL(rcu_idle_enter);
#ifdef CONFIG_NO_HZ_FULL
/**
@@ -886,6 +887,7 @@ void rcu_idle_exit(void)
rcu_eqs_exit(false);
local_irq_restore(flags);
}
+EXPORT_SYMBOL_GPL(rcu_idle_exit);
#ifdef CONFIG_NO_HZ_FULL
/**
--
2.21.0
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH] rcu/tree: Export rcu_idle_{enter,exit} to module
2020-09-21 10:37 ` [PATCH] rcu/tree: Export rcu_idle_{enter,exit} to module Borislav Petkov
@ 2020-09-21 12:15 ` Rafael J. Wysocki
2020-09-21 13:32 ` Paul E. McKenney
1 sibling, 0 replies; 22+ messages in thread
From: Rafael J. Wysocki @ 2020-09-21 12:15 UTC (permalink / raw)
To: Borislav Petkov, Paul E. McKenney
Cc: Sven Joachim, Peter Zijlstra, Rafael J. Wysocki,
the arch/x86 maintainers, Tony Luck, Len Brown, Daniel Lezcano,
Linux Kernel Mailing List, ACPI Devel Maling List, Linux PM,
Ulf Hansson, Thomas Gleixner, Naresh Kamboju, Josh Triplett,
Steven Rostedt, Mathieu Desnoyers, Lai Jiangshan, Joel Fernandes,
rcu
On Mon, Sep 21, 2020 at 12:37 PM Borislav Petkov <bp@alien8.de> wrote:
>
> Lemme add whatever get_maintainer spits, to Cc.
>
> On Mon, Sep 21, 2020 at 11:12:33AM +0200, Sven Joachim wrote:
> > On 2020-09-15 12:32 +0200, Peter Zijlstra wrote:
> >
> > > The C3 BusMaster idle code takes lock in a number of places, some deep
> > > inside the ACPI code. Instead of wrapping it all in RCU_NONIDLE, have
> > > the driver take over RCU-idle duty and avoid flipping RCU state back
> > > and forth a lot.
> > >
> > > ( by marking 'C3 && bm_check' as RCU_IDLE, we _must_ call enter_bm() for
> > > that combination, otherwise we'll loose RCU-idle, this requires
> > > shuffling some code around )
> > >
> > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> >
> > I got modpost errors in 5.9-rc6 after this patch:
> >
> > ERROR: modpost: "rcu_idle_enter" [drivers/acpi/processor.ko] undefined!
> > ERROR: modpost: "rcu_idle_exit" [drivers/acpi/processor.ko] undefined!
> >
> > Reverting commit 1fecfdbb7acc made them go away. Notably my
> > configuration had CONFIG_ACPI_PROCESSOR=m, changing that
> > to CONFIG_ACPI_PROCESSOR=y let the build succeed as well.
>
> I guess this. Running randconfigs on it for a while, to see what else
> breaks.
>
> ---
> From: Borislav Petkov <bp@suse.de>
> Date: Mon, 21 Sep 2020 12:31:36 +0200
>
> Fix this link error:
>
> ERROR: modpost: "rcu_idle_enter" [drivers/acpi/processor.ko] undefined!
> ERROR: modpost: "rcu_idle_exit" [drivers/acpi/processor.ko] undefined!
>
> when CONFIG_ACPI_PROCESSOR is built as module. PeterZ says that in light
> of ARM needing those soon too, they should simply be exported.
>
> Fixes: 1fecfdbb7acc ("ACPI: processor: Take over RCU-idle for C3-BM idle")
> Reported-by: Sven Joachim <svenjoac@gmx.de>
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Borislav Petkov <bp@suse.de>
Well, I guess I should take this through cpuidle?
Any concerns about doing that? Paul?
> ---
> kernel/rcu/tree.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 8ce77d9ac716..f78ee759af9c 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -673,6 +673,7 @@ void rcu_idle_enter(void)
> lockdep_assert_irqs_disabled();
> rcu_eqs_enter(false);
> }
> +EXPORT_SYMBOL_GPL(rcu_idle_enter);
>
> #ifdef CONFIG_NO_HZ_FULL
> /**
> @@ -886,6 +887,7 @@ void rcu_idle_exit(void)
> rcu_eqs_exit(false);
> local_irq_restore(flags);
> }
> +EXPORT_SYMBOL_GPL(rcu_idle_exit);
>
> #ifdef CONFIG_NO_HZ_FULL
> /**
> --
> 2.21.0
>
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] rcu/tree: Export rcu_idle_{enter,exit} to module
2020-09-21 10:37 ` [PATCH] rcu/tree: Export rcu_idle_{enter,exit} to module Borislav Petkov
2020-09-21 12:15 ` Rafael J. Wysocki
@ 2020-09-21 13:32 ` Paul E. McKenney
2020-09-21 13:38 ` Rafael J. Wysocki
1 sibling, 1 reply; 22+ messages in thread
From: Paul E. McKenney @ 2020-09-21 13:32 UTC (permalink / raw)
To: Borislav Petkov
Cc: Sven Joachim, Peter Zijlstra, rjw, x86, tony.luck, lenb,
daniel.lezcano, linux-kernel, linux-acpi, linux-pm, ulf.hansson,
tglx, naresh.kamboju, Josh Triplett, Steven Rostedt,
Mathieu Desnoyers, Lai Jiangshan, Joel Fernandes, rcu
On Mon, Sep 21, 2020 at 12:37:41PM +0200, Borislav Petkov wrote:
> Lemme add whatever get_maintainer spits, to Cc.
>
> On Mon, Sep 21, 2020 at 11:12:33AM +0200, Sven Joachim wrote:
> > On 2020-09-15 12:32 +0200, Peter Zijlstra wrote:
> >
> > > The C3 BusMaster idle code takes lock in a number of places, some deep
> > > inside the ACPI code. Instead of wrapping it all in RCU_NONIDLE, have
> > > the driver take over RCU-idle duty and avoid flipping RCU state back
> > > and forth a lot.
> > >
> > > ( by marking 'C3 && bm_check' as RCU_IDLE, we _must_ call enter_bm() for
> > > that combination, otherwise we'll loose RCU-idle, this requires
> > > shuffling some code around )
> > >
> > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> >
> > I got modpost errors in 5.9-rc6 after this patch:
> >
> > ERROR: modpost: "rcu_idle_enter" [drivers/acpi/processor.ko] undefined!
> > ERROR: modpost: "rcu_idle_exit" [drivers/acpi/processor.ko] undefined!
> >
> > Reverting commit 1fecfdbb7acc made them go away. Notably my
> > configuration had CONFIG_ACPI_PROCESSOR=m, changing that
> > to CONFIG_ACPI_PROCESSOR=y let the build succeed as well.
>
> I guess this. Running randconfigs on it for a while, to see what else
> breaks.
>
> ---
> From: Borislav Petkov <bp@suse.de>
> Date: Mon, 21 Sep 2020 12:31:36 +0200
>
> Fix this link error:
>
> ERROR: modpost: "rcu_idle_enter" [drivers/acpi/processor.ko] undefined!
> ERROR: modpost: "rcu_idle_exit" [drivers/acpi/processor.ko] undefined!
>
> when CONFIG_ACPI_PROCESSOR is built as module. PeterZ says that in light
> of ARM needing those soon too, they should simply be exported.
>
> Fixes: 1fecfdbb7acc ("ACPI: processor: Take over RCU-idle for C3-BM idle")
> Reported-by: Sven Joachim <svenjoac@gmx.de>
> Suggested-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Paul E. McKenney <paulmckrcu@kernel.org>
> ---
> kernel/rcu/tree.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 8ce77d9ac716..f78ee759af9c 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -673,6 +673,7 @@ void rcu_idle_enter(void)
> lockdep_assert_irqs_disabled();
> rcu_eqs_enter(false);
> }
> +EXPORT_SYMBOL_GPL(rcu_idle_enter);
>
> #ifdef CONFIG_NO_HZ_FULL
> /**
> @@ -886,6 +887,7 @@ void rcu_idle_exit(void)
> rcu_eqs_exit(false);
> local_irq_restore(flags);
> }
> +EXPORT_SYMBOL_GPL(rcu_idle_exit);
>
> #ifdef CONFIG_NO_HZ_FULL
> /**
> --
> 2.21.0
>
> --
> Regards/Gruss,
> Boris.
>
> https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH] rcu/tree: Export rcu_idle_{enter,exit} to module
2020-09-21 13:32 ` Paul E. McKenney
@ 2020-09-21 13:38 ` Rafael J. Wysocki
0 siblings, 0 replies; 22+ messages in thread
From: Rafael J. Wysocki @ 2020-09-21 13:38 UTC (permalink / raw)
To: Paul E. McKenney, Borislav Petkov
Cc: Sven Joachim, Peter Zijlstra, Rafael J. Wysocki,
the arch/x86 maintainers, Tony Luck, Len Brown, Daniel Lezcano,
Linux Kernel Mailing List, ACPI Devel Maling List, Linux PM,
Ulf Hansson, Thomas Gleixner, Naresh Kamboju, Josh Triplett,
Steven Rostedt, Mathieu Desnoyers, Lai Jiangshan, Joel Fernandes,
rcu
On Mon, Sep 21, 2020 at 3:32 PM Paul E. McKenney <paulmck@kernel.org> wrote:
>
> On Mon, Sep 21, 2020 at 12:37:41PM +0200, Borislav Petkov wrote:
> > Lemme add whatever get_maintainer spits, to Cc.
> >
> > On Mon, Sep 21, 2020 at 11:12:33AM +0200, Sven Joachim wrote:
> > > On 2020-09-15 12:32 +0200, Peter Zijlstra wrote:
> > >
> > > > The C3 BusMaster idle code takes lock in a number of places, some deep
> > > > inside the ACPI code. Instead of wrapping it all in RCU_NONIDLE, have
> > > > the driver take over RCU-idle duty and avoid flipping RCU state back
> > > > and forth a lot.
> > > >
> > > > ( by marking 'C3 && bm_check' as RCU_IDLE, we _must_ call enter_bm() for
> > > > that combination, otherwise we'll loose RCU-idle, this requires
> > > > shuffling some code around )
> > > >
> > > > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> > >
> > > I got modpost errors in 5.9-rc6 after this patch:
> > >
> > > ERROR: modpost: "rcu_idle_enter" [drivers/acpi/processor.ko] undefined!
> > > ERROR: modpost: "rcu_idle_exit" [drivers/acpi/processor.ko] undefined!
> > >
> > > Reverting commit 1fecfdbb7acc made them go away. Notably my
> > > configuration had CONFIG_ACPI_PROCESSOR=m, changing that
> > > to CONFIG_ACPI_PROCESSOR=y let the build succeed as well.
> >
> > I guess this. Running randconfigs on it for a while, to see what else
> > breaks.
> >
> > ---
> > From: Borislav Petkov <bp@suse.de>
> > Date: Mon, 21 Sep 2020 12:31:36 +0200
> >
> > Fix this link error:
> >
> > ERROR: modpost: "rcu_idle_enter" [drivers/acpi/processor.ko] undefined!
> > ERROR: modpost: "rcu_idle_exit" [drivers/acpi/processor.ko] undefined!
> >
> > when CONFIG_ACPI_PROCESSOR is built as module. PeterZ says that in light
> > of ARM needing those soon too, they should simply be exported.
> >
> > Fixes: 1fecfdbb7acc ("ACPI: processor: Take over RCU-idle for C3-BM idle")
> > Reported-by: Sven Joachim <svenjoac@gmx.de>
> > Suggested-by: Peter Zijlstra <peterz@infradead.org>
> > Signed-off-by: Borislav Petkov <bp@suse.de>
>
> Reviewed-by: Paul E. McKenney <paulmckrcu@kernel.org>
OK
Applied as 5.9-rc7 material, thanks!
> > ---
> > kernel/rcu/tree.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index 8ce77d9ac716..f78ee759af9c 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -673,6 +673,7 @@ void rcu_idle_enter(void)
> > lockdep_assert_irqs_disabled();
> > rcu_eqs_enter(false);
> > }
> > +EXPORT_SYMBOL_GPL(rcu_idle_enter);
> >
> > #ifdef CONFIG_NO_HZ_FULL
> > /**
> > @@ -886,6 +887,7 @@ void rcu_idle_exit(void)
> > rcu_eqs_exit(false);
> > local_irq_restore(flags);
> > }
> > +EXPORT_SYMBOL_GPL(rcu_idle_exit);
> >
> > #ifdef CONFIG_NO_HZ_FULL
> > /**
> > --
> > 2.21.0
> >
> > --
> > Regards/Gruss,
> > Boris.
> >
> > https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC][PATCH 4/4] acpi: Take over RCU-idle for C3-BM idle
2020-09-15 10:32 ` [RFC][PATCH 4/4] acpi: Take over RCU-idle for C3-BM idle Peter Zijlstra
2020-09-21 9:12 ` Sven Joachim
@ 2020-09-25 15:20 ` Guenter Roeck
2020-09-25 15:24 ` Rafael J. Wysocki
2020-09-25 15:29 ` Paul E. McKenney
1 sibling, 2 replies; 22+ messages in thread
From: Guenter Roeck @ 2020-09-25 15:20 UTC (permalink / raw)
To: Peter Zijlstra
Cc: rjw, bp, x86, tony.luck, lenb, daniel.lezcano, linux-kernel,
linux-acpi, linux-pm, ulf.hansson, paulmck, tglx, naresh.kamboju
On Tue, Sep 15, 2020 at 12:32:01PM +0200, Peter Zijlstra wrote:
> The C3 BusMaster idle code takes lock in a number of places, some deep
> inside the ACPI code. Instead of wrapping it all in RCU_NONIDLE, have
> the driver take over RCU-idle duty and avoid flipping RCU state back
> and forth a lot.
>
> ( by marking 'C3 && bm_check' as RCU_IDLE, we _must_ call enter_bm() for
> that combination, otherwise we'll loose RCU-idle, this requires
> shuffling some code around )
>
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
ia64:defconfig:
ERROR: modpost: "rcu_idle_enter" [drivers/acpi/processor.ko] undefined!
ERROR: modpost: "rcu_idle_exit" [drivers/acpi/processor.ko] undefined!
I realize that this has already been reported more than a week ago, with
no visible reaction. Another problem introduced in the same file, resulting
in
drivers/acpi/processor_idle.c: In function 'lapic_timer_needs_broadcast':
drivers/acpi/processor_idle.c:179:1: warning:
no return statement in function returning non-void
may cause ia64 boot problems since a non-zero return value will trigger
a function call. AFAICS that is not supposed to happen on ia64.
This makes me wonder - if no one cares about buiding (much less running)
ia64 images with the upstream kernel, is it possibly time to remove it ?
Guenter
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC][PATCH 4/4] acpi: Take over RCU-idle for C3-BM idle
2020-09-25 15:20 ` [RFC][PATCH 4/4] acpi: Take over RCU-idle for C3-BM idle Guenter Roeck
@ 2020-09-25 15:24 ` Rafael J. Wysocki
2020-09-25 15:29 ` Paul E. McKenney
1 sibling, 0 replies; 22+ messages in thread
From: Rafael J. Wysocki @ 2020-09-25 15:24 UTC (permalink / raw)
To: Guenter Roeck
Cc: Peter Zijlstra, Rafael J. Wysocki, Borislav Petkov,
the arch/x86 maintainers, Tony Luck, Len Brown, Daniel Lezcano,
Linux Kernel Mailing List, ACPI Devel Maling List, Linux PM,
Ulf Hansson, Paul E. McKenney, Thomas Gleixner, Naresh Kamboju
On Fri, Sep 25, 2020 at 5:20 PM Guenter Roeck <linux@roeck-us.net> wrote:
>
> On Tue, Sep 15, 2020 at 12:32:01PM +0200, Peter Zijlstra wrote:
> > The C3 BusMaster idle code takes lock in a number of places, some deep
> > inside the ACPI code. Instead of wrapping it all in RCU_NONIDLE, have
> > the driver take over RCU-idle duty and avoid flipping RCU state back
> > and forth a lot.
> >
> > ( by marking 'C3 && bm_check' as RCU_IDLE, we _must_ call enter_bm() for
> > that combination, otherwise we'll loose RCU-idle, this requires
> > shuffling some code around )
> >
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>
> ia64:defconfig:
>
> ERROR: modpost: "rcu_idle_enter" [drivers/acpi/processor.ko] undefined!
> ERROR: modpost: "rcu_idle_exit" [drivers/acpi/processor.ko] undefined!
>
> I realize that this has already been reported more than a week ago, with
> no visible reaction. Another problem introduced in the same file, resulting
> in
>
> drivers/acpi/processor_idle.c: In function 'lapic_timer_needs_broadcast':
> drivers/acpi/processor_idle.c:179:1: warning:
> no return statement in function returning non-void
>
> may cause ia64 boot problems since a non-zero return value will trigger
> a function call. AFAICS that is not supposed to happen on ia64.
There are fixes for the above in my tree, they will go to Linus shortly.
Thanks!
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC][PATCH 4/4] acpi: Take over RCU-idle for C3-BM idle
2020-09-25 15:20 ` [RFC][PATCH 4/4] acpi: Take over RCU-idle for C3-BM idle Guenter Roeck
2020-09-25 15:24 ` Rafael J. Wysocki
@ 2020-09-25 15:29 ` Paul E. McKenney
1 sibling, 0 replies; 22+ messages in thread
From: Paul E. McKenney @ 2020-09-25 15:29 UTC (permalink / raw)
To: Guenter Roeck
Cc: Peter Zijlstra, rjw, bp, x86, tony.luck, lenb, daniel.lezcano,
linux-kernel, linux-acpi, linux-pm, ulf.hansson, tglx,
naresh.kamboju
On Fri, Sep 25, 2020 at 08:20:00AM -0700, Guenter Roeck wrote:
> On Tue, Sep 15, 2020 at 12:32:01PM +0200, Peter Zijlstra wrote:
> > The C3 BusMaster idle code takes lock in a number of places, some deep
> > inside the ACPI code. Instead of wrapping it all in RCU_NONIDLE, have
> > the driver take over RCU-idle duty and avoid flipping RCU state back
> > and forth a lot.
> >
> > ( by marking 'C3 && bm_check' as RCU_IDLE, we _must_ call enter_bm() for
> > that combination, otherwise we'll loose RCU-idle, this requires
> > shuffling some code around )
> >
> > Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
>
> ia64:defconfig:
>
> ERROR: modpost: "rcu_idle_enter" [drivers/acpi/processor.ko] undefined!
> ERROR: modpost: "rcu_idle_exit" [drivers/acpi/processor.ko] undefined!
>
> I realize that this has already been reported more than a week ago, with
> no visible reaction. Another problem introduced in the same file, resulting
> in
>
> drivers/acpi/processor_idle.c: In function 'lapic_timer_needs_broadcast':
> drivers/acpi/processor_idle.c:179:1: warning:
> no return statement in function returning non-void
>
> may cause ia64 boot problems since a non-zero return value will trigger
> a function call. AFAICS that is not supposed to happen on ia64.
>
> This makes me wonder - if no one cares about buiding (much less running)
> ia64 images with the upstream kernel, is it possibly time to remove it ?
Rafael is taking a fix up his cpuidle tree:
https://lkml.kernel.org/lkml/CAJZ5v0jVerU92WxL4qCoU6NC0KCyszmRNhpL3tu5LYtMqALd9A@mail.gmail.com/
Thanx, Paul
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [RFC][PATCH 0/4] Fix up ACPI processor idle vs RCU
2020-09-15 10:31 [RFC][PATCH 0/4] Fix up ACPI processor idle vs RCU Peter Zijlstra
` (3 preceding siblings ...)
2020-09-15 10:32 ` [RFC][PATCH 4/4] acpi: Take over RCU-idle for C3-BM idle Peter Zijlstra
@ 2020-09-15 18:31 ` Borislav Petkov
4 siblings, 0 replies; 22+ messages in thread
From: Borislav Petkov @ 2020-09-15 18:31 UTC (permalink / raw)
To: Peter Zijlstra
Cc: rjw, x86, tony.luck, lenb, daniel.lezcano, linux-kernel,
linux-acpi, linux-pm, ulf.hansson, paulmck, tglx, naresh.kamboju
On Tue, Sep 15, 2020 at 12:31:57PM +0200, Peter Zijlstra wrote:
> Boris tested an earlier version of these patches and they worked for his
> 32bit Atom board that was triggering complaints.
Want me to test them again?
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
^ permalink raw reply [flat|nested] 22+ messages in thread