cpufreq: cppc: Mark frequency invariance broken

Message ID 28308fc0d38f252baf90e6ffb31fd2f8660be273.1623311808.git.viresh.kumar@linaro.org
State New
Headers show
Series
  • cpufreq: cppc: Mark frequency invariance broken
Related show

Commit Message

Viresh Kumar June 10, 2021, 7:58 a.m.
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 <quic_qiancai@quicinc.com>
Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

---
Rafael, please apply this for v5.13-rc if it looks fine to you.

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

-- 
2.31.1.272.g89b43f80a514

Comments

Rafael J. Wysocki June 10, 2021, 11:19 a.m. | #1
On Thu, Jun 10, 2021 at 9:58 AM Viresh Kumar <viresh.kumar@linaro.org> 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 <quic_qiancai@quicinc.com>

> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>


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

>
Viresh Kumar June 10, 2021, 11:33 a.m. | #2
On 10-06-21, 13:19, Rafael J. Wysocki wrote:
> On Thu, Jun 10, 2021 at 9:58 AM Viresh Kumar <viresh.kumar@linaro.org> 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 <quic_qiancai@quicinc.com>

> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

> 

> 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
Rafael J. Wysocki June 10, 2021, 12:30 p.m. | #3
On Thu, Jun 10, 2021 at 1:34 PM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>

> On 10-06-21, 13:19, Rafael J. Wysocki wrote:

> > On Thu, Jun 10, 2021 at 9:58 AM Viresh Kumar <viresh.kumar@linaro.org> 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 <quic_qiancai@quicinc.com>

> > > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>

> >

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

Patch

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,