2021-06-10 08:03:25

by Viresh Kumar

[permalink] [raw]
Subject: [PATCH] cpufreq: cppc: Mark frequency invariance broken

There are few races in the frequency invariance support for CPPC driver,
namely the driver doesn't stop the kthread_work and irq_work on policy
exit during suspend/resume or CPU hotplug.

A proper fix won't be possible for the 5.13-rc, as it requires a lot of
changes. Instead of reverting the patch, mark this feature BROKEN for
now.

Fixes: 4c38f2df71c8 ("cpufreq: CPPC: Add support for frequency invariance")
Reported-by: Qian Cai <[email protected]>
Signed-off-by: Viresh Kumar <[email protected]>
---
Rafael, please apply this for v5.13-rc if it looks fine to you.

drivers/cpufreq/Kconfig.arm | 1 +
1 file changed, 1 insertion(+)

diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
index a5c5f70acfc9..614c34350f41 100644
--- a/drivers/cpufreq/Kconfig.arm
+++ b/drivers/cpufreq/Kconfig.arm
@@ -22,6 +22,7 @@ config ACPI_CPPC_CPUFREQ
config ACPI_CPPC_CPUFREQ_FIE
bool "Frequency Invariance support for CPPC cpufreq driver"
depends on ACPI_CPPC_CPUFREQ && GENERIC_ARCH_TOPOLOGY
+ depends on BROKEN
default y
help
This extends frequency invariance support in the CPPC cpufreq driver,
--
2.31.1.272.g89b43f80a514


2021-06-10 11:20:51

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] cpufreq: cppc: Mark frequency invariance broken

On Thu, Jun 10, 2021 at 9:58 AM Viresh Kumar <[email protected]> wrote:
>
> There are few races in the frequency invariance support for CPPC driver,
> namely the driver doesn't stop the kthread_work and irq_work on policy
> exit during suspend/resume or CPU hotplug.
>
> A proper fix won't be possible for the 5.13-rc, as it requires a lot of
> changes. Instead of reverting the patch, mark this feature BROKEN for
> now.
>
> Fixes: 4c38f2df71c8 ("cpufreq: CPPC: Add support for frequency invariance")
> Reported-by: Qian Cai <[email protected]>
> Signed-off-by: Viresh Kumar <[email protected]>

Well, why don't we revert 4c38f2df71c8 instead?

Is there any particular reason for retaining it?

> ---
> Rafael, please apply this for v5.13-rc if it looks fine to you.
>
> drivers/cpufreq/Kconfig.arm | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/cpufreq/Kconfig.arm b/drivers/cpufreq/Kconfig.arm
> index a5c5f70acfc9..614c34350f41 100644
> --- a/drivers/cpufreq/Kconfig.arm
> +++ b/drivers/cpufreq/Kconfig.arm
> @@ -22,6 +22,7 @@ config ACPI_CPPC_CPUFREQ
> config ACPI_CPPC_CPUFREQ_FIE
> bool "Frequency Invariance support for CPPC cpufreq driver"
> depends on ACPI_CPPC_CPUFREQ && GENERIC_ARCH_TOPOLOGY
> + depends on BROKEN
> default y
> help
> This extends frequency invariance support in the CPPC cpufreq driver,
> --
> 2.31.1.272.g89b43f80a514
>

2021-06-10 11:37:12

by Viresh Kumar

[permalink] [raw]
Subject: Re: [PATCH] cpufreq: cppc: Mark frequency invariance broken

On 10-06-21, 13:19, Rafael J. Wysocki wrote:
> On Thu, Jun 10, 2021 at 9:58 AM Viresh Kumar <[email protected]> wrote:
> >
> > There are few races in the frequency invariance support for CPPC driver,
> > namely the driver doesn't stop the kthread_work and irq_work on policy
> > exit during suspend/resume or CPU hotplug.
> >
> > A proper fix won't be possible for the 5.13-rc, as it requires a lot of
> > changes. Instead of reverting the patch, mark this feature BROKEN for
> > now.
> >
> > Fixes: 4c38f2df71c8 ("cpufreq: CPPC: Add support for frequency invariance")
> > Reported-by: Qian Cai <[email protected]>
> > Signed-off-by: Viresh Kumar <[email protected]>
>
> Well, why don't we revert 4c38f2df71c8 instead?
>
> Is there any particular reason for retaining it?

I was just trying to reduce the diff size here, since this feature
(which broke) was controlled by a CONFIG option, it looked like a nice
way of doing it.

It was already reviewed and a diff over it should be easier to review.

I can do a full revert if that's what you want.

--
viresh

2021-06-10 12:33:16

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH] cpufreq: cppc: Mark frequency invariance broken

On Thu, Jun 10, 2021 at 1:34 PM Viresh Kumar <[email protected]> wrote:
>
> On 10-06-21, 13:19, Rafael J. Wysocki wrote:
> > On Thu, Jun 10, 2021 at 9:58 AM Viresh Kumar <[email protected]> wrote:
> > >
> > > There are few races in the frequency invariance support for CPPC driver,
> > > namely the driver doesn't stop the kthread_work and irq_work on policy
> > > exit during suspend/resume or CPU hotplug.
> > >
> > > A proper fix won't be possible for the 5.13-rc, as it requires a lot of
> > > changes. Instead of reverting the patch, mark this feature BROKEN for
> > > now.
> > >
> > > Fixes: 4c38f2df71c8 ("cpufreq: CPPC: Add support for frequency invariance")
> > > Reported-by: Qian Cai <[email protected]>
> > > Signed-off-by: Viresh Kumar <[email protected]>
> >
> > Well, why don't we revert 4c38f2df71c8 instead?
> >
> > Is there any particular reason for retaining it?
>
> I was just trying to reduce the diff size here, since this feature
> (which broke) was controlled by a CONFIG option, it looked like a nice
> way of doing it.
>
> It was already reviewed and a diff over it should be easier to review.
>
> I can do a full revert if that's what you want.

I would prefer a full revert TBH.

Making a new feature depend on BROKEN feels like it shouldn't have
been added at this point in the first place.