diff mbox series

[RFC,1/4] acpi: Use CPUIDLE_FLAG_TIMER_STOP

Message ID 20200915103806.280265587@infradead.org
State New
Headers show
Series [RFC,1/4] acpi: Use CPUIDLE_FLAG_TIMER_STOP | expand

Commit Message

Peter Zijlstra Sept. 15, 2020, 10:31 a.m. UTC
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(-)

Comments

Rafael J. Wysocki Sept. 15, 2020, 4:26 p.m. UTC | #1
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;

>

>
Peter Zijlstra Sept. 16, 2020, 3:42 p.m. UTC | #2
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.
Borislav Petkov Sept. 16, 2020, 4:01 p.m. UTC | #3
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.
Rafael J. Wysocki Sept. 16, 2020, 5:38 p.m. UTC | #4
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!
Rafael J. Wysocki Sept. 16, 2020, 5:38 p.m. UTC | #5
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.
diff mbox series

Patch

--- 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;