| Message ID | 20240905092645.2885200-3-christian.loehle@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cpufreq: cpuidle: Remove iowait behaviour | expand |
On Thu, Sep 5, 2024 at 11:27 AM Christian Loehle <christian.loehle@arm.com> wrote: > > Since menu no longer has the interactivity boost teo works better > overall, so make it the default. > > Signed-off-by: Christian Loehle <christian.loehle@arm.com> I know that this isn't strictly related to the use of iowait in menu, but I'd rather wait with this one until the previous change in menu settles down. Also it would be good to provide some numbers to support the "teo works better overall" claim above. > --- > drivers/cpuidle/Kconfig | 5 +---- > drivers/cpuidle/governors/menu.c | 2 +- > drivers/cpuidle/governors/teo.c | 2 +- > 3 files changed, 3 insertions(+), 6 deletions(-) > > diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig > index cac5997dca50..ae67a464025a 100644 > --- a/drivers/cpuidle/Kconfig > +++ b/drivers/cpuidle/Kconfig > @@ -5,7 +5,7 @@ config CPU_IDLE > bool "CPU idle PM support" > default y if ACPI || PPC_PSERIES > select CPU_IDLE_GOV_LADDER if (!NO_HZ && !NO_HZ_IDLE) > - select CPU_IDLE_GOV_MENU if (NO_HZ || NO_HZ_IDLE) && !CPU_IDLE_GOV_TEO > + select CPU_IDLE_GOV_TEO if (NO_HZ || NO_HZ_IDLE) && !CPU_IDLE_GOV_MENU > help > CPU idle is a generic framework for supporting software-controlled > idle processor power management. It includes modular cross-platform > @@ -30,9 +30,6 @@ config CPU_IDLE_GOV_TEO > This governor implements a simplified idle state selection method > focused on timer events and does not do any interactivity boosting. > > - Some workloads benefit from using it and it generally should be safe > - to use. Say Y here if you are not happy with the alternatives. > - > config CPU_IDLE_GOV_HALTPOLL > bool "Haltpoll governor (for virtualized systems)" > depends on KVM_GUEST > diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c > index 28363bfa3e4c..c0ae5e98d6f1 100644 > --- a/drivers/cpuidle/governors/menu.c > +++ b/drivers/cpuidle/governors/menu.c > @@ -508,7 +508,7 @@ static int menu_enable_device(struct cpuidle_driver *drv, > > static struct cpuidle_governor menu_governor = { > .name = "menu", > - .rating = 20, > + .rating = 19, > .enable = menu_enable_device, > .select = menu_select, > .reflect = menu_reflect, > diff --git a/drivers/cpuidle/governors/teo.c b/drivers/cpuidle/governors/teo.c > index f2992f92d8db..6c3cc39f285d 100644 > --- a/drivers/cpuidle/governors/teo.c > +++ b/drivers/cpuidle/governors/teo.c > @@ -537,7 +537,7 @@ static int teo_enable_device(struct cpuidle_driver *drv, > > static struct cpuidle_governor teo_governor = { > .name = "teo", > - .rating = 19, > + .rating = 20, > .enable = teo_enable_device, > .select = teo_select, > .reflect = teo_reflect, > -- > 2.34.1 >
On 9/30/24 16:06, Rafael J. Wysocki wrote: > On Thu, Sep 5, 2024 at 11:27 AM Christian Loehle > <christian.loehle@arm.com> wrote: >> >> Since menu no longer has the interactivity boost teo works better >> overall, so make it the default. >> >> Signed-off-by: Christian Loehle <christian.loehle@arm.com> First of all thank you for taking a look. > > I know that this isn't strictly related to the use of iowait in menu, > but I'd rather wait with this one until the previous change in menu > settles down. Sure, I will look at any regressions that are reported, although "teo is performing better/worse/eqyal" would already be a pretty helpful hint and for me personally, if they do both perform badly I find debugging teo way easier. > > Also it would be good to provide some numbers to support the "teo > works better overall" claim above. Definitely, there are some in the overall cover-letter if you just compare equivalent menu/teo columns, but with the very fragmented cpuidle world this isn't anywhere near enough to back up that claim. We have found it to provide better results in both mobile and infra/ server workloads on common arm64 platforms. That being said, I don't mind menu being around or even the default per-se, but would encourage anyone to give teo a try.
On Mon, Sep 30, 2024 at 6:12 PM Christian Loehle <christian.loehle@arm.com> wrote: > > On 9/30/24 16:06, Rafael J. Wysocki wrote: > > On Thu, Sep 5, 2024 at 11:27 AM Christian Loehle > > <christian.loehle@arm.com> wrote: > >> > >> Since menu no longer has the interactivity boost teo works better > >> overall, so make it the default. > >> > >> Signed-off-by: Christian Loehle <christian.loehle@arm.com> > > First of all thank you for taking a look. > > > > > I know that this isn't strictly related to the use of iowait in menu, > > but I'd rather wait with this one until the previous change in menu > > settles down. > > Sure, I will look at any regressions that are reported, although "teo > is performing better/worse/eqyal" would already be a pretty helpful hint > and for me personally, if they do both perform badly I find debugging > teo way easier. > > > > > Also it would be good to provide some numbers to support the "teo > > works better overall" claim above. > > Definitely, there are some in the overall cover-letter if you just > compare equivalent menu/teo columns, but with the very fragmented > cpuidle world this isn't anywhere near enough to back up that claim. > We have found it to provide better results in both mobile and infra/ > server workloads on common arm64 platforms. So why don't you add some numbers to the patch changelog? If you can at least demonstrate that they are on par with each other in some relevant benchmarks, then you can use the argument of teo being more straightforward and so easier to reason about. > That being said, I don't mind menu being around or even the default > per-se, but would encourage anyone to give teo a try. Fair enough.
diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig index cac5997dca50..ae67a464025a 100644 --- a/drivers/cpuidle/Kconfig +++ b/drivers/cpuidle/Kconfig @@ -5,7 +5,7 @@ config CPU_IDLE bool "CPU idle PM support" default y if ACPI || PPC_PSERIES select CPU_IDLE_GOV_LADDER if (!NO_HZ && !NO_HZ_IDLE) - select CPU_IDLE_GOV_MENU if (NO_HZ || NO_HZ_IDLE) && !CPU_IDLE_GOV_TEO + select CPU_IDLE_GOV_TEO if (NO_HZ || NO_HZ_IDLE) && !CPU_IDLE_GOV_MENU help CPU idle is a generic framework for supporting software-controlled idle processor power management. It includes modular cross-platform @@ -30,9 +30,6 @@ config CPU_IDLE_GOV_TEO This governor implements a simplified idle state selection method focused on timer events and does not do any interactivity boosting. - Some workloads benefit from using it and it generally should be safe - to use. Say Y here if you are not happy with the alternatives. - config CPU_IDLE_GOV_HALTPOLL bool "Haltpoll governor (for virtualized systems)" depends on KVM_GUEST diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c index 28363bfa3e4c..c0ae5e98d6f1 100644 --- a/drivers/cpuidle/governors/menu.c +++ b/drivers/cpuidle/governors/menu.c @@ -508,7 +508,7 @@ static int menu_enable_device(struct cpuidle_driver *drv, static struct cpuidle_governor menu_governor = { .name = "menu", - .rating = 20, + .rating = 19, .enable = menu_enable_device, .select = menu_select, .reflect = menu_reflect, diff --git a/drivers/cpuidle/governors/teo.c b/drivers/cpuidle/governors/teo.c index f2992f92d8db..6c3cc39f285d 100644 --- a/drivers/cpuidle/governors/teo.c +++ b/drivers/cpuidle/governors/teo.c @@ -537,7 +537,7 @@ static int teo_enable_device(struct cpuidle_driver *drv, static struct cpuidle_governor teo_governor = { .name = "teo", - .rating = 19, + .rating = 20, .enable = teo_enable_device, .select = teo_select, .reflect = teo_reflect,
Since menu no longer has the interactivity boost teo works better overall, so make it the default. Signed-off-by: Christian Loehle <christian.loehle@arm.com> --- drivers/cpuidle/Kconfig | 5 +---- drivers/cpuidle/governors/menu.c | 2 +- drivers/cpuidle/governors/teo.c | 2 +- 3 files changed, 3 insertions(+), 6 deletions(-)