Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1310538 > unrolled thread

[PATCH 0/2] cpuidle optimizations (on top of linux-next)

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/2] cpuidle optimizations (on top of linux-next) "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-01-16 01:00 +0100
    [PATCH 2/2] cpuidle: menu: Avoid pointless checks in menu_select() "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-01-16 01:00 +0100
      [Resend][PATCH 2/2] cpuidle: menu: Avoid pointless checks in menu_select() "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-01-19 00:00 +0100
    Re: [PATCH 0/2] cpuidle optimizations (on top of linux-next) Sudeep Holla <sudeep.holla@arm.com> - 2016-01-18 14:50 +0100
      Re: [PATCH 0/2] cpuidle optimizations (on top of linux-next) Ingo Molnar <mingo@kernel.org> - 2016-01-19 08:30 +0100
        Re: [PATCH 0/2] cpuidle optimizations (on top of linux-next) "Rafael J. Wysocki" <rafael@kernel.org> - 2016-01-19 14:20 +0100
          Re: [PATCH 0/2] cpuidle optimizations (on top of linux-next) Ingo Molnar <mingo@kernel.org> - 2016-01-19 14:30 +0100
            Re: [PATCH 0/2] cpuidle optimizations (on top of linux-next) "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2016-01-19 15:00 +0100

#1310538 — [PATCH 0/2] cpuidle optimizations (on top of linux-next)

Hi,

When I was looking at the cpuidle code after the Sudeeps's problem report,
it occured to me that we had some pointless overhead there, so two
changes to reduce it follow.

[1/2] Make the fallback to to default_idle_call() in call_cpuidle()
      unnecessary and drop it.
[2/2] Make menu_select() avoid checking states that don't need to
      (or even shouldn't) be checked when making the selection.

Thanks,
Rafael

[toc] | [next] | [standalone]

#1310541 — [PATCH 2/2] cpuidle: menu: Avoid pointless checks in menu_select()

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Subject: [PATCH] 

If menu_select() cannot find a suitable state to return, it will
return the state index stored in data->last_state_idx.  This
means that it is pointless to look at the states whose indices
are less than or equal to data->last_state_idx in the main loop,
so don't do that.

Given that those checks are done on every idle state selection, this
change can save quite a bit of completely unnecessary overhead.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/cpuidle/governors/menu.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-pm/drivers/cpuidle/governors/menu.c
===================================================================
--- linux-pm.orig/drivers/cpuidle/governors/menu.c
+++ linux-pm/drivers/cpuidle/governors/menu.c
@@ -342,7 +342,7 @@ static int menu_select(struct cpuidle_dr
 	 * Find the idle state with the lowest power while satisfying
 	 * our constraints.
 	 */
-	for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++) {
+	for (i = data->last_state_idx + 1; i < drv->state_count; i++) {
 		struct cpuidle_state *s = &drv->states[i];
 		struct cpuidle_state_usage *su = &dev->states_usage[i];
 

[toc] | [prev] | [next] | [standalone]

#1311840 — [Resend][PATCH 2/2] cpuidle: menu: Avoid pointless checks in menu_select()

From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

If menu_select() cannot find a suitable state to return, it will
return the state index stored in data->last_state_idx.  This
means that it is pointless to look at the states whose indices
are less than or equal to data->last_state_idx in the main loop,
so don't do that.

Given that those checks are done on every idle state selection, this
change can save quite a bit of completely unnecessary overhead.

Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Tested-by: Sudeep Holla <sudeep.holla@arm.com>
---

I left a pointless Subject: tag in the patch body when I sent it last time,
so resend.

Note that this also is based on commit 9c4b2867ed7c (cpuidle: menu: Fix
menu_select() for CPUIDLE_DRIVER_STATE_START == 0) only present in linux-next ATM.

---
 drivers/cpuidle/governors/menu.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux-pm/drivers/cpuidle/governors/menu.c
===================================================================
--- linux-pm.orig/drivers/cpuidle/governors/menu.c
+++ linux-pm/drivers/cpuidle/governors/menu.c
@@ -342,7 +342,7 @@ static int menu_select(struct cpuidle_dr
 	 * Find the idle state with the lowest power while satisfying
 	 * our constraints.
 	 */
-	for (i = CPUIDLE_DRIVER_STATE_START; i < drv->state_count; i++) {
+	for (i = data->last_state_idx + 1; i < drv->state_count; i++) {
 		struct cpuidle_state *s = &drv->states[i];
 		struct cpuidle_state_usage *su = &dev->states_usage[i];
 

[toc] | [prev] | [next] | [standalone]

#1311561

On 15/01/16 23:53, Rafael J. Wysocki wrote:
> Hi,
>
> When I was looking at the cpuidle code after the Sudeeps's problem report,
> it occured to me that we had some pointless overhead there, so two
> changes to reduce it follow.
>
> [1/2] Make the fallback to to default_idle_call() in call_cpuidle()
>        unnecessary and drop it.
> [2/2] Make menu_select() avoid checking states that don't need to
>        (or even shouldn't) be checked when making the selection.
>

Tested-by: Sudeep Holla <sudeep.holla@arm.com>

-- 
Regards,
Sudeep

[toc] | [prev] | [next] | [standalone]

#1311974

* Sudeep Holla <sudeep.holla@arm.com> wrote:

> On 15/01/16 23:53, Rafael J. Wysocki wrote:
> >Hi,
> >
> >When I was looking at the cpuidle code after the Sudeeps's problem report,
> >it occured to me that we had some pointless overhead there, so two
> >changes to reduce it follow.
> >
> >[1/2] Make the fallback to to default_idle_call() in call_cpuidle()
> >       unnecessary and drop it.
> >[2/2] Make menu_select() avoid checking states that don't need to
> >       (or even shouldn't) be checked when making the selection.
> >
> 
> Tested-by: Sudeep Holla <sudeep.holla@arm.com>

Rafael, can I pick these up into the scheduler tree?

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1312218

On Tue, Jan 19, 2016 at 8:28 AM, Ingo Molnar <mingo@kernel.org> wrote:
>
> * Sudeep Holla <sudeep.holla@arm.com> wrote:
>
>> On 15/01/16 23:53, Rafael J. Wysocki wrote:
>> >Hi,
>> >
>> >When I was looking at the cpuidle code after the Sudeeps's problem report,
>> >it occured to me that we had some pointless overhead there, so two
>> >changes to reduce it follow.
>> >
>> >[1/2] Make the fallback to to default_idle_call() in call_cpuidle()
>> >       unnecessary and drop it.
>> >[2/2] Make menu_select() avoid checking states that don't need to
>> >       (or even shouldn't) be checked when making the selection.
>> >
>>
>> Tested-by: Sudeep Holla <sudeep.holla@arm.com>
>
> Rafael, can I pick these up into the scheduler tree?

They won't apply at this point as one commit they depend on is in my
linux-next branch waiting for the next push.

Would it be a problem if they went in through the PM tree instead?

Thanks,
Rafael

[toc] | [prev] | [next] | [standalone]

#1312224

* Rafael J. Wysocki <rafael@kernel.org> wrote:

> On Tue, Jan 19, 2016 at 8:28 AM, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > * Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> >> On 15/01/16 23:53, Rafael J. Wysocki wrote:
> >> >Hi,
> >> >
> >> >When I was looking at the cpuidle code after the Sudeeps's problem report,
> >> >it occured to me that we had some pointless overhead there, so two
> >> >changes to reduce it follow.
> >> >
> >> >[1/2] Make the fallback to to default_idle_call() in call_cpuidle()
> >> >       unnecessary and drop it.
> >> >[2/2] Make menu_select() avoid checking states that don't need to
> >> >       (or even shouldn't) be checked when making the selection.
> >> >
> >>
> >> Tested-by: Sudeep Holla <sudeep.holla@arm.com>
> >
> > Rafael, can I pick these up into the scheduler tree?
> 
> They won't apply at this point as one commit they depend on is in my
> linux-next branch waiting for the next push.
> 
> Would it be a problem if they went in through the PM tree instead?

Absolutely no problem:

Acked-by: Ingo Molnar <mingo@kernel.org>

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1312263

On Tuesday, January 19, 2016 02:28:58 PM Ingo Molnar wrote:
> 
> * Rafael J. Wysocki <rafael@kernel.org> wrote:
> 
> > On Tue, Jan 19, 2016 at 8:28 AM, Ingo Molnar <mingo@kernel.org> wrote:
> > >
> > > * Sudeep Holla <sudeep.holla@arm.com> wrote:
> > >
> > >> On 15/01/16 23:53, Rafael J. Wysocki wrote:
> > >> >Hi,
> > >> >
> > >> >When I was looking at the cpuidle code after the Sudeeps's problem report,
> > >> >it occured to me that we had some pointless overhead there, so two
> > >> >changes to reduce it follow.
> > >> >
> > >> >[1/2] Make the fallback to to default_idle_call() in call_cpuidle()
> > >> >       unnecessary and drop it.
> > >> >[2/2] Make menu_select() avoid checking states that don't need to
> > >> >       (or even shouldn't) be checked when making the selection.
> > >> >
> > >>
> > >> Tested-by: Sudeep Holla <sudeep.holla@arm.com>
> > >
> > > Rafael, can I pick these up into the scheduler tree?
> > 
> > They won't apply at this point as one commit they depend on is in my
> > linux-next branch waiting for the next push.
> > 
> > Would it be a problem if they went in through the PM tree instead?
> 
> Absolutely no problem:
> 
> Acked-by: Ingo Molnar <mingo@kernel.org>

OK, thanks!

Rafael

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web