[PATCH] cpuidle: Move cpuidle driver forward before acpi driver in Makefile

[PATCH] cpuidle: Move cpuidle driver forward before acpi driver in Makefile

[Dapeng Mi]

As long as Kconfig ACPI_PROCESSOR is enabled, ACPI_PROCESSOR would
select ACPI_PROCESSOR_IDLE and acpi_idle driver is enabled. But
in current driver loading order acpi_idle driver is always loaded
before cpuidle_haltpoll driver. This leads to cpuidle_hatpoll driver
has no chance to be loaded when it's enabled.

Thus, move cpuidle driver forward before acpi driver and make
cpuidle-hatpoll driver has a chance to be run when it's enabled.

Signed-off-by: Dapeng Mi <dapeng1.mi@intel.com>
---
 drivers/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/Makefile b/drivers/Makefile
index 9a30842b22c5..921ed481b520 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -26,6 +26,7 @@ obj-y				+= idle/
 # IPMI must come before ACPI in order to provide IPMI opregion support
 obj-y				+= char/ipmi/
 
+obj-$(CONFIG_CPU_IDLE)		+= cpuidle/
 obj-$(CONFIG_ACPI)		+= acpi/
 
 # PnP must come after ACPI since it will eventually need to check if acpi
@@ -126,7 +127,6 @@ obj-$(CONFIG_EDAC)		+= edac/
 obj-$(CONFIG_EISA)		+= eisa/
 obj-$(CONFIG_PM_OPP)		+= opp/
 obj-$(CONFIG_CPU_FREQ)		+= cpufreq/
-obj-$(CONFIG_CPU_IDLE)		+= cpuidle/
 obj-y				+= mmc/
 obj-y				+= ufs/
 obj-$(CONFIG_MEMSTICK)		+= memstick/
-- 
2.30.2

Re: [PATCH] cpuidle: Move cpuidle driver forward before acpi driver in Makefile

[Rafael J. Wysocki]

On Wed, Jul 13, 2022 at 10:21 AM Dapeng Mi <dapeng1.mi@intel.com> wrote:
>
> As long as Kconfig ACPI_PROCESSOR is enabled, ACPI_PROCESSOR would
> select ACPI_PROCESSOR_IDLE and acpi_idle driver is enabled. But
> in current driver loading order acpi_idle driver is always loaded
> before cpuidle_haltpoll driver. This leads to cpuidle_hatpoll driver
> has no chance to be loaded when it's enabled.
>
> Thus, move cpuidle driver forward before acpi driver and make
> cpuidle-hatpoll driver has a chance to be run when it's enabled.
>
> Signed-off-by: Dapeng Mi <dapeng1.mi@intel.com>
> ---
>  drivers/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/Makefile b/drivers/Makefile
> index 9a30842b22c5..921ed481b520 100644
> --- a/drivers/Makefile
> +++ b/drivers/Makefile
> @@ -26,6 +26,7 @@ obj-y                         += idle/
>  # IPMI must come before ACPI in order to provide IPMI opregion support
>  obj-y                          += char/ipmi/
>
> +obj-$(CONFIG_CPU_IDLE)         += cpuidle/
>  obj-$(CONFIG_ACPI)             += acpi/
>
>  # PnP must come after ACPI since it will eventually need to check if acpi
> @@ -126,7 +127,6 @@ obj-$(CONFIG_EDAC)          += edac/
>  obj-$(CONFIG_EISA)             += eisa/
>  obj-$(CONFIG_PM_OPP)           += opp/
>  obj-$(CONFIG_CPU_FREQ)         += cpufreq/
> -obj-$(CONFIG_CPU_IDLE)         += cpuidle/
>  obj-y                          += mmc/
>  obj-y                          += ufs/
>  obj-$(CONFIG_MEMSTICK)         += memstick/
> --

Well, this change doesn't guarantee loading haltpoll before ACPI idle.

Also what if haltpoll is enabled, but the user wants ACPI idle?

Re: [PATCH] cpuidle: Move cpuidle driver forward before acpi driver in Makefile

[Michael S. Tsirkin]

On Wed, Jul 13, 2022 at 07:53:13PM +0200, Rafael J. Wysocki wrote:
> On Wed, Jul 13, 2022 at 10:21 AM Dapeng Mi <dapeng1.mi@intel.com> wrote:
> >
> > As long as Kconfig ACPI_PROCESSOR is enabled, ACPI_PROCESSOR would
> > select ACPI_PROCESSOR_IDLE and acpi_idle driver is enabled. But
> > in current driver loading order acpi_idle driver is always loaded
> > before cpuidle_haltpoll driver. This leads to cpuidle_hatpoll driver
> > has no chance to be loaded when it's enabled.
> >
> > Thus, move cpuidle driver forward before acpi driver and make
> > cpuidle-hatpoll driver has a chance to be run when it's enabled.
> >
> > Signed-off-by: Dapeng Mi <dapeng1.mi@intel.com>
> > ---
> >  drivers/Makefile | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/Makefile b/drivers/Makefile
> > index 9a30842b22c5..921ed481b520 100644
> > --- a/drivers/Makefile
> > +++ b/drivers/Makefile
> > @@ -26,6 +26,7 @@ obj-y                         += idle/
> >  # IPMI must come before ACPI in order to provide IPMI opregion support
> >  obj-y                          += char/ipmi/
> >
> > +obj-$(CONFIG_CPU_IDLE)         += cpuidle/
> >  obj-$(CONFIG_ACPI)             += acpi/
> >
> >  # PnP must come after ACPI since it will eventually need to check if acpi
> > @@ -126,7 +127,6 @@ obj-$(CONFIG_EDAC)          += edac/
> >  obj-$(CONFIG_EISA)             += eisa/
> >  obj-$(CONFIG_PM_OPP)           += opp/
> >  obj-$(CONFIG_CPU_FREQ)         += cpufreq/
> > -obj-$(CONFIG_CPU_IDLE)         += cpuidle/
> >  obj-y                          += mmc/
> >  obj-y                          += ufs/
> >  obj-$(CONFIG_MEMSTICK)         += memstick/
> > --
> 
> Well, this change doesn't guarantee loading haltpoll before ACPI idle.
> 
> Also what if haltpoll is enabled, but the user wants ACPI idle?

Exactly. For example while on real boxes BIOS might often present broken
ACPI idle, on VMs I am guessing if ACPI is present one can assume it's
actually correct.

-- 
MST

RE: [PATCH] cpuidle: Move cpuidle driver forward before acpi driver in Makefile

[Mi, Dapeng1]

> From: Rafael J. Wysocki <rafael@kernel.org>
> Sent: Thursday, July 14, 2022 1:53 AM
> To: Mi, Dapeng1 <dapeng1.mi@intel.com>
> Cc: Rafael J. Wysocki <rafael@kernel.org>; Michael S. Tsirkin
> <mst@redhat.com>; Arnd Bergmann <arnd@arndb.de>; Bart Van Assche
> <bvanassche@acm.org>; Linux Kernel Mailing List <linux-
> kernel@vger.kernel.org>; Linux PM <linux-pm@vger.kernel.org>
> Subject: Re: [PATCH] cpuidle: Move cpuidle driver forward before acpi driver
> in Makefile
> 
> On Wed, Jul 13, 2022 at 10:21 AM Dapeng Mi <dapeng1.mi@intel.com> wrote:
> >
> > As long as Kconfig ACPI_PROCESSOR is enabled, ACPI_PROCESSOR would
> > select ACPI_PROCESSOR_IDLE and acpi_idle driver is enabled. But in
> > current driver loading order acpi_idle driver is always loaded before
> > cpuidle_haltpoll driver. This leads to cpuidle_hatpoll driver has no
> > chance to be loaded when it's enabled.
> >
> > Thus, move cpuidle driver forward before acpi driver and make
> > cpuidle-hatpoll driver has a chance to be run when it's enabled.
> >
> > Signed-off-by: Dapeng Mi <dapeng1.mi@intel.com>
> > ---
> >  drivers/Makefile | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/Makefile b/drivers/Makefile index
> > 9a30842b22c5..921ed481b520 100644
> > --- a/drivers/Makefile
> > +++ b/drivers/Makefile
> > @@ -26,6 +26,7 @@ obj-y                         += idle/
> >  # IPMI must come before ACPI in order to provide IPMI opregion support
> >  obj-y                          += char/ipmi/
> >
> > +obj-$(CONFIG_CPU_IDLE)         += cpuidle/
> >  obj-$(CONFIG_ACPI)             += acpi/
> >
> >  # PnP must come after ACPI since it will eventually need to check if acpi
> > @@ -126,7 +127,6 @@ obj-$(CONFIG_EDAC)          += edac/
> >  obj-$(CONFIG_EISA)             += eisa/
> >  obj-$(CONFIG_PM_OPP)           += opp/
> >  obj-$(CONFIG_CPU_FREQ)         += cpufreq/
> > -obj-$(CONFIG_CPU_IDLE)         += cpuidle/
> >  obj-y                          += mmc/
> >  obj-y                          += ufs/
> >  obj-$(CONFIG_MEMSTICK)         += memstick/
> > --
> 
> Well, this change doesn't guarantee loading haltpoll before ACPI idle.
> 
> Also what if haltpoll is enabled, but the user wants ACPI idle?

Thanks Rafael for reviewing this patch.

acpi_idle driver and cpuidle_haltpoll driver have same initialization level and both are initialized on the level device_initcall. So the building order would decide the loading sequence. Just like the intel_idle driver which also has same initialization level (device_initcall), but as it's built before acpi_idle driver, it would be loaded first before acpi_driver if intel_idle driver is enabled.

There is another method to make cpuidle_haltpoll driver loaded first before acpi_driver, it's change the initialization level to postcore_initcall. I'm not sure which one is better, but it seems current patch is more reasonable.

There is an parameter "force" to manage the haltpoll enabling. If user want to use ACPI idle, it can change this parameter to disable haltpolll driver.

RE: [PATCH] cpuidle: Move cpuidle driver forward before acpi driver in Makefile

[Mi, Dapeng1]

> > From: Rafael J. Wysocki <rafael@kernel.org>
> > Sent: Thursday, July 14, 2022 1:53 AM
> > To: Mi, Dapeng1 <dapeng1.mi@intel.com>
> > Cc: Rafael J. Wysocki <rafael@kernel.org>; Michael S. Tsirkin
> > <mst@redhat.com>; Arnd Bergmann <arnd@arndb.de>; Bart Van Assche
> > <bvanassche@acm.org>; Linux Kernel Mailing List <linux-
> > kernel@vger.kernel.org>; Linux PM <linux-pm@vger.kernel.org>
> > Subject: Re: [PATCH] cpuidle: Move cpuidle driver forward before acpi
> > driver in Makefile
> >
> > On Wed, Jul 13, 2022 at 10:21 AM Dapeng Mi <dapeng1.mi@intel.com>
> wrote:
> > >
> > > As long as Kconfig ACPI_PROCESSOR is enabled, ACPI_PROCESSOR would
> > > select ACPI_PROCESSOR_IDLE and acpi_idle driver is enabled. But in
> > > current driver loading order acpi_idle driver is always loaded
> > > before cpuidle_haltpoll driver. This leads to cpuidle_hatpoll driver
> > > has no chance to be loaded when it's enabled.
> > >
> > > Thus, move cpuidle driver forward before acpi driver and make
> > > cpuidle-hatpoll driver has a chance to be run when it's enabled.
> > >
> > > Signed-off-by: Dapeng Mi <dapeng1.mi@intel.com>
> > > ---
> > >  drivers/Makefile | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/Makefile b/drivers/Makefile index
> > > 9a30842b22c5..921ed481b520 100644
> > > --- a/drivers/Makefile
> > > +++ b/drivers/Makefile
> > > @@ -26,6 +26,7 @@ obj-y                         += idle/
> > >  # IPMI must come before ACPI in order to provide IPMI opregion
> support
> > >  obj-y                          += char/ipmi/
> > >
> > > +obj-$(CONFIG_CPU_IDLE)         += cpuidle/
> > >  obj-$(CONFIG_ACPI)             += acpi/
> > >
> > >  # PnP must come after ACPI since it will eventually need to check if acpi
> > > @@ -126,7 +127,6 @@ obj-$(CONFIG_EDAC)          += edac/
> > >  obj-$(CONFIG_EISA)             += eisa/
> > >  obj-$(CONFIG_PM_OPP)           += opp/
> > >  obj-$(CONFIG_CPU_FREQ)         += cpufreq/
> > > -obj-$(CONFIG_CPU_IDLE)         += cpuidle/
> > >  obj-y                          += mmc/
> > >  obj-y                          += ufs/
> > >  obj-$(CONFIG_MEMSTICK)         += memstick/
> > > --
> >
> > Well, this change doesn't guarantee loading haltpoll before ACPI idle.
> >
> > Also what if haltpoll is enabled, but the user wants ACPI idle?
> 
> Thanks Rafael for reviewing this patch.
> 
> acpi_idle driver and cpuidle_haltpoll driver have same initialization level and
> both are initialized on the level device_initcall. So the building order would
> decide the loading sequence. Just like the intel_idle driver which also has
> same initialization level (device_initcall), but as it's built before acpi_idle
> driver, it would be loaded first before acpi_driver if intel_idle driver is
> enabled.
> 
> There is another method to make cpuidle_haltpoll driver loaded first before
> acpi_driver, it's change the initialization level to postcore_initcall. I'm not sure
> which one is better, but it seems current patch is more reasonable.
> 
> There is an parameter "force" to manage the haltpoll enabling. If user want
> to use ACPI idle, it can change this parameter to disable haltpolll driver.

Any feedback on this? Thanks.

Re: [PATCH] cpuidle: Move cpuidle driver forward before acpi driver in Makefile

[Rafael J. Wysocki]

On Wed, Jul 20, 2022 at 5:00 AM Mi, Dapeng1 <dapeng1.mi@intel.com> wrote:
>
> > > From: Rafael J. Wysocki <rafael@kernel.org>
> > > Sent: Thursday, July 14, 2022 1:53 AM
> > > To: Mi, Dapeng1 <dapeng1.mi@intel.com>
> > > Cc: Rafael J. Wysocki <rafael@kernel.org>; Michael S. Tsirkin
> > > <mst@redhat.com>; Arnd Bergmann <arnd@arndb.de>; Bart Van Assche
> > > <bvanassche@acm.org>; Linux Kernel Mailing List <linux-
> > > kernel@vger.kernel.org>; Linux PM <linux-pm@vger.kernel.org>
> > > Subject: Re: [PATCH] cpuidle: Move cpuidle driver forward before acpi
> > > driver in Makefile
> > >
> > > On Wed, Jul 13, 2022 at 10:21 AM Dapeng Mi <dapeng1.mi@intel.com>
> > wrote:
> > > >
> > > > As long as Kconfig ACPI_PROCESSOR is enabled, ACPI_PROCESSOR would
> > > > select ACPI_PROCESSOR_IDLE and acpi_idle driver is enabled. But in
> > > > current driver loading order acpi_idle driver is always loaded
> > > > before cpuidle_haltpoll driver. This leads to cpuidle_hatpoll driver
> > > > has no chance to be loaded when it's enabled.
> > > >
> > > > Thus, move cpuidle driver forward before acpi driver and make
> > > > cpuidle-hatpoll driver has a chance to be run when it's enabled.
> > > >
> > > > Signed-off-by: Dapeng Mi <dapeng1.mi@intel.com>
> > > > ---
> > > >  drivers/Makefile | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/Makefile b/drivers/Makefile index
> > > > 9a30842b22c5..921ed481b520 100644
> > > > --- a/drivers/Makefile
> > > > +++ b/drivers/Makefile
> > > > @@ -26,6 +26,7 @@ obj-y                         += idle/
> > > >  # IPMI must come before ACPI in order to provide IPMI opregion
> > support
> > > >  obj-y                          += char/ipmi/
> > > >
> > > > +obj-$(CONFIG_CPU_IDLE)         += cpuidle/
> > > >  obj-$(CONFIG_ACPI)             += acpi/
> > > >
> > > >  # PnP must come after ACPI since it will eventually need to check if acpi
> > > > @@ -126,7 +127,6 @@ obj-$(CONFIG_EDAC)          += edac/
> > > >  obj-$(CONFIG_EISA)             += eisa/
> > > >  obj-$(CONFIG_PM_OPP)           += opp/
> > > >  obj-$(CONFIG_CPU_FREQ)         += cpufreq/
> > > > -obj-$(CONFIG_CPU_IDLE)         += cpuidle/
> > > >  obj-y                          += mmc/
> > > >  obj-y                          += ufs/
> > > >  obj-$(CONFIG_MEMSTICK)         += memstick/
> > > > --
> > >
> > > Well, this change doesn't guarantee loading haltpoll before ACPI idle.
> > >
> > > Also what if haltpoll is enabled, but the user wants ACPI idle?
> >
> > Thanks Rafael for reviewing this patch.
> >
> > acpi_idle driver and cpuidle_haltpoll driver have same initialization level and
> > both are initialized on the level device_initcall. So the building order would
> > decide the loading sequence. Just like the intel_idle driver which also has
> > same initialization level (device_initcall), but as it's built before acpi_idle
> > driver, it would be loaded first before acpi_driver if intel_idle driver is
> > enabled.
> >
> > There is another method to make cpuidle_haltpoll driver loaded first before
> > acpi_driver, it's change the initialization level to postcore_initcall. I'm not sure
> > which one is better, but it seems current patch is more reasonable.
> >
> > There is an parameter "force" to manage the haltpoll enabling. If user want
> > to use ACPI idle, it can change this parameter to disable haltpolll driver.

That would require things to be appended to the kernel command line in
cases where that's not necessary today and that's not acceptable.

What you really seem to be wanting to do is to use haltpoll instead of
ACPI idle.  Is that correct?

RE: [PATCH] cpuidle: Move cpuidle driver forward before acpi driver in Makefile

[Mi, Dapeng1]

> From: Rafael J. Wysocki <rafael@kernel.org>
> Sent: Wednesday, July 20, 2022 6:24 PM
> To: Mi, Dapeng1 <dapeng1.mi@intel.com>
> Cc: Rafael J. Wysocki <rafael@kernel.org>; Michael S. Tsirkin
> <mst@redhat.com>; Arnd Bergmann <arnd@arndb.de>; Bart Van Assche
> <bvanassche@acm.org>; Linux Kernel Mailing List <linux-
> kernel@vger.kernel.org>; Linux PM <linux-pm@vger.kernel.org>; Zhenyu
> Wang <zhenyuw@linux.intel.com>
> Subject: Re: [PATCH] cpuidle: Move cpuidle driver forward before acpi driver
> in Makefile
> 
> On Wed, Jul 20, 2022 at 5:00 AM Mi, Dapeng1 <dapeng1.mi@intel.com>
> wrote:
> >
> > > > From: Rafael J. Wysocki <rafael@kernel.org>
> > > > Sent: Thursday, July 14, 2022 1:53 AM
> > > > To: Mi, Dapeng1 <dapeng1.mi@intel.com>
> > > > Cc: Rafael J. Wysocki <rafael@kernel.org>; Michael S. Tsirkin
> > > > <mst@redhat.com>; Arnd Bergmann <arnd@arndb.de>; Bart Van
> Assche
> > > > <bvanassche@acm.org>; Linux Kernel Mailing List <linux-
> > > > kernel@vger.kernel.org>; Linux PM <linux-pm@vger.kernel.org>
> > > > Subject: Re: [PATCH] cpuidle: Move cpuidle driver forward before
> > > > acpi driver in Makefile
> > > >
> > > > On Wed, Jul 13, 2022 at 10:21 AM Dapeng Mi <dapeng1.mi@intel.com>
> > > wrote:
> > > > >
> > > > > As long as Kconfig ACPI_PROCESSOR is enabled, ACPI_PROCESSOR
> > > > > would select ACPI_PROCESSOR_IDLE and acpi_idle driver is
> > > > > enabled. But in current driver loading order acpi_idle driver is
> > > > > always loaded before cpuidle_haltpoll driver. This leads to
> > > > > cpuidle_hatpoll driver has no chance to be loaded when it's enabled.
> > > > >
> > > > > Thus, move cpuidle driver forward before acpi driver and make
> > > > > cpuidle-hatpoll driver has a chance to be run when it's enabled.
> > > > >
> > > > > Signed-off-by: Dapeng Mi <dapeng1.mi@intel.com>
> > > > > ---
> > > > >  drivers/Makefile | 2 +-
> > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/Makefile b/drivers/Makefile index
> > > > > 9a30842b22c5..921ed481b520 100644
> > > > > --- a/drivers/Makefile
> > > > > +++ b/drivers/Makefile
> > > > > @@ -26,6 +26,7 @@ obj-y                         += idle/
> > > > >  # IPMI must come before ACPI in order to provide IPMI opregion
> > > support
> > > > >  obj-y                          += char/ipmi/
> > > > >
> > > > > +obj-$(CONFIG_CPU_IDLE)         += cpuidle/
> > > > >  obj-$(CONFIG_ACPI)             += acpi/
> > > > >
> > > > >  # PnP must come after ACPI since it will eventually need to check if
> acpi
> > > > > @@ -126,7 +127,6 @@ obj-$(CONFIG_EDAC)          += edac/
> > > > >  obj-$(CONFIG_EISA)             += eisa/
> > > > >  obj-$(CONFIG_PM_OPP)           += opp/
> > > > >  obj-$(CONFIG_CPU_FREQ)         += cpufreq/
> > > > > -obj-$(CONFIG_CPU_IDLE)         += cpuidle/
> > > > >  obj-y                          += mmc/
> > > > >  obj-y                          += ufs/
> > > > >  obj-$(CONFIG_MEMSTICK)         += memstick/
> > > > > --
> > > >
> > > > Well, this change doesn't guarantee loading haltpoll before ACPI idle.
> > > >
> > > > Also what if haltpoll is enabled, but the user wants ACPI idle?
> > >
> > > Thanks Rafael for reviewing this patch.
> > >
> > > acpi_idle driver and cpuidle_haltpoll driver have same
> > > initialization level and both are initialized on the level
> > > device_initcall. So the building order would decide the loading
> > > sequence. Just like the intel_idle driver which also has same
> > > initialization level (device_initcall), but as it's built before
> > > acpi_idle driver, it would be loaded first before acpi_driver if intel_idle
> driver is enabled.
> > >
> > > There is another method to make cpuidle_haltpoll driver loaded first
> > > before acpi_driver, it's change the initialization level to
> > > postcore_initcall. I'm not sure which one is better, but it seems current
> patch is more reasonable.
> > >
> > > There is an parameter "force" to manage the haltpoll enabling. If
> > > user want to use ACPI idle, it can change this parameter to disable
> haltpolll driver.
> 
> That would require things to be appended to the kernel command line in
> cases where that's not necessary today and that's not acceptable.
> 
The haltpoll driver's "force" parameter is false by default, we don't need to add extra command line options in most cases except we want to enable the haltpolling driver. 

> What you really seem to be wanting to do is to use haltpoll instead of ACPI
> idle.  Is that correct?

Yes, I'm trying to enable guest halt polling to improve the performance of some Virtual Machine. But I found the halt poll driver can't be enabled as long as acpi idle driver is enabled. I tried to disable the acpi idle first, but I found there is no parameter to enable/disable acpi idle driver except disabling the Kconfig "CONFIG_ACPI_PROCESSOR_IDLE", and but unfortunately Kconfig "ACPI_PROCESSOR" would enable "ACPI_PROCESSOR_IDLE" by default. If I want to disable acpi_idle driver, I have to disable the "ACPI_PROCESSOR", but this would cause acpi throttling and acpi thermal are also disabled. That's not what I want. That's why I do this change to make halt poll driver has a  chance to run without disable the entire acpi processor functionality .

RE: [PATCH] cpuidle: Move cpuidle driver forward before acpi driver in Makefile

[Mi, Dapeng1]

> From: Mi, Dapeng1
> Sent: Thursday, July 21, 2022 11:09 AM
> To: Rafael J. Wysocki <rafael@kernel.org>
> Cc: Michael S. Tsirkin <mst@redhat.com>; Arnd Bergmann <arnd@arndb.de>;
> Bart Van Assche <bvanassche@acm.org>; Linux Kernel Mailing List <linux-
> kernel@vger.kernel.org>; Linux PM <linux-pm@vger.kernel.org>; Zhenyu
> Wang <zhenyuw@linux.intel.com>
> Subject: RE: [PATCH] cpuidle: Move cpuidle driver forward before acpi driver
> in Makefile
> 
> > From: Rafael J. Wysocki <rafael@kernel.org>
> > Sent: Wednesday, July 20, 2022 6:24 PM
> > To: Mi, Dapeng1 <dapeng1.mi@intel.com>
> > Cc: Rafael J. Wysocki <rafael@kernel.org>; Michael S. Tsirkin
> > <mst@redhat.com>; Arnd Bergmann <arnd@arndb.de>; Bart Van Assche
> > <bvanassche@acm.org>; Linux Kernel Mailing List <linux-
> > kernel@vger.kernel.org>; Linux PM <linux-pm@vger.kernel.org>; Zhenyu
> > Wang <zhenyuw@linux.intel.com>
> > Subject: Re: [PATCH] cpuidle: Move cpuidle driver forward before acpi
> > driver in Makefile
> >
> > On Wed, Jul 20, 2022 at 5:00 AM Mi, Dapeng1 <dapeng1.mi@intel.com>
> > wrote:
> > >
> > > > > From: Rafael J. Wysocki <rafael@kernel.org>
> > > > > Sent: Thursday, July 14, 2022 1:53 AM
> > > > > To: Mi, Dapeng1 <dapeng1.mi@intel.com>
> > > > > Cc: Rafael J. Wysocki <rafael@kernel.org>; Michael S. Tsirkin
> > > > > <mst@redhat.com>; Arnd Bergmann <arnd@arndb.de>; Bart Van
> > Assche
> > > > > <bvanassche@acm.org>; Linux Kernel Mailing List <linux-
> > > > > kernel@vger.kernel.org>; Linux PM <linux-pm@vger.kernel.org>
> > > > > Subject: Re: [PATCH] cpuidle: Move cpuidle driver forward before
> > > > > acpi driver in Makefile
> > > > >
> > > > > On Wed, Jul 13, 2022 at 10:21 AM Dapeng Mi
> > > > > <dapeng1.mi@intel.com>
> > > > wrote:
> > > > > >
> > > > > > As long as Kconfig ACPI_PROCESSOR is enabled, ACPI_PROCESSOR
> > > > > > would select ACPI_PROCESSOR_IDLE and acpi_idle driver is
> > > > > > enabled. But in current driver loading order acpi_idle driver
> > > > > > is always loaded before cpuidle_haltpoll driver. This leads to
> > > > > > cpuidle_hatpoll driver has no chance to be loaded when it's enabled.
> > > > > >
> > > > > > Thus, move cpuidle driver forward before acpi driver and make
> > > > > > cpuidle-hatpoll driver has a chance to be run when it's enabled.
> > > > > >
> > > > > > Signed-off-by: Dapeng Mi <dapeng1.mi@intel.com>
> > > > > > ---
> > > > > >  drivers/Makefile | 2 +-
> > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > >
> > > > > > diff --git a/drivers/Makefile b/drivers/Makefile index
> > > > > > 9a30842b22c5..921ed481b520 100644
> > > > > > --- a/drivers/Makefile
> > > > > > +++ b/drivers/Makefile
> > > > > > @@ -26,6 +26,7 @@ obj-y                         += idle/
> > > > > >  # IPMI must come before ACPI in order to provide IPMI
> > > > > > opregion
> > > > support
> > > > > >  obj-y                          += char/ipmi/
> > > > > >
> > > > > > +obj-$(CONFIG_CPU_IDLE)         += cpuidle/
> > > > > >  obj-$(CONFIG_ACPI)             += acpi/
> > > > > >
> > > > > >  # PnP must come after ACPI since it will eventually need to
> > > > > > check if
> > acpi
> > > > > > @@ -126,7 +127,6 @@ obj-$(CONFIG_EDAC)          += edac/
> > > > > >  obj-$(CONFIG_EISA)             += eisa/
> > > > > >  obj-$(CONFIG_PM_OPP)           += opp/
> > > > > >  obj-$(CONFIG_CPU_FREQ)         += cpufreq/
> > > > > > -obj-$(CONFIG_CPU_IDLE)         += cpuidle/
> > > > > >  obj-y                          += mmc/
> > > > > >  obj-y                          += ufs/
> > > > > >  obj-$(CONFIG_MEMSTICK)         += memstick/
> > > > > > --
> > > > >
> > > > > Well, this change doesn't guarantee loading haltpoll before ACPI idle.
> > > > >
> > > > > Also what if haltpoll is enabled, but the user wants ACPI idle?
> > > >
> > > > Thanks Rafael for reviewing this patch.
> > > >
> > > > acpi_idle driver and cpuidle_haltpoll driver have same
> > > > initialization level and both are initialized on the level
> > > > device_initcall. So the building order would decide the loading
> > > > sequence. Just like the intel_idle driver which also has same
> > > > initialization level (device_initcall), but as it's built before
> > > > acpi_idle driver, it would be loaded first before acpi_driver if
> > > > intel_idle
> > driver is enabled.
> > > >
> > > > There is another method to make cpuidle_haltpoll driver loaded
> > > > first before acpi_driver, it's change the initialization level to
> > > > postcore_initcall. I'm not sure which one is better, but it seems
> > > > current
> > patch is more reasonable.
> > > >
> > > > There is an parameter "force" to manage the haltpoll enabling. If
> > > > user want to use ACPI idle, it can change this parameter to
> > > > disable
> > haltpolll driver.
> >
> > That would require things to be appended to the kernel command line in
> > cases where that's not necessary today and that's not acceptable.
> >
> The haltpoll driver's "force" parameter is false by default, we don't need to
> add extra command line options in most cases except we want to enable the
> haltpolling driver.
> 
> > What you really seem to be wanting to do is to use haltpoll instead of
> > ACPI idle.  Is that correct?
> 
> Yes, I'm trying to enable guest halt polling to improve the performance of
> some Virtual Machine. But I found the halt poll driver can't be enabled as
> long as acpi idle driver is enabled. I tried to disable the acpi idle first, but I
> found there is no parameter to enable/disable acpi idle driver except
> disabling the Kconfig "CONFIG_ACPI_PROCESSOR_IDLE", and but
> unfortunately Kconfig "ACPI_PROCESSOR" would enable
> "ACPI_PROCESSOR_IDLE" by default. If I want to disable acpi_idle driver, I
> have to disable the "ACPI_PROCESSOR", but this would cause acpi throttling
> and acpi thermal are also disabled. That's not what I want. That's why I do this
> change to make halt poll driver has a  chance to run without disable the
> entire acpi processor functionality .
> 

Any feedback? Thanks.

Re: [PATCH] cpuidle: Move cpuidle driver forward before acpi driver in Makefile

[Rafael J. Wysocki]

On Tue, Jul 26, 2022 at 3:46 AM Mi, Dapeng1 <dapeng1.mi@intel.com> wrote:
>
> > From: Mi, Dapeng1
> > Sent: Thursday, July 21, 2022 11:09 AM
> > To: Rafael J. Wysocki <rafael@kernel.org>
> > Cc: Michael S. Tsirkin <mst@redhat.com>; Arnd Bergmann <arnd@arndb.de>;
> > Bart Van Assche <bvanassche@acm.org>; Linux Kernel Mailing List <linux-
> > kernel@vger.kernel.org>; Linux PM <linux-pm@vger.kernel.org>; Zhenyu
> > Wang <zhenyuw@linux.intel.com>
> > Subject: RE: [PATCH] cpuidle: Move cpuidle driver forward before acpi driver
> > in Makefile
> >
> > > From: Rafael J. Wysocki <rafael@kernel.org>
> > > Sent: Wednesday, July 20, 2022 6:24 PM
> > > To: Mi, Dapeng1 <dapeng1.mi@intel.com>
> > > Cc: Rafael J. Wysocki <rafael@kernel.org>; Michael S. Tsirkin
> > > <mst@redhat.com>; Arnd Bergmann <arnd@arndb.de>; Bart Van Assche
> > > <bvanassche@acm.org>; Linux Kernel Mailing List <linux-
> > > kernel@vger.kernel.org>; Linux PM <linux-pm@vger.kernel.org>; Zhenyu
> > > Wang <zhenyuw@linux.intel.com>
> > > Subject: Re: [PATCH] cpuidle: Move cpuidle driver forward before acpi
> > > driver in Makefile
> > >
> > > On Wed, Jul 20, 2022 at 5:00 AM Mi, Dapeng1 <dapeng1.mi@intel.com>
> > > wrote:
> > > >
> > > > > > From: Rafael J. Wysocki <rafael@kernel.org>
> > > > > > Sent: Thursday, July 14, 2022 1:53 AM
> > > > > > To: Mi, Dapeng1 <dapeng1.mi@intel.com>
> > > > > > Cc: Rafael J. Wysocki <rafael@kernel.org>; Michael S. Tsirkin
> > > > > > <mst@redhat.com>; Arnd Bergmann <arnd@arndb.de>; Bart Van
> > > Assche
> > > > > > <bvanassche@acm.org>; Linux Kernel Mailing List <linux-
> > > > > > kernel@vger.kernel.org>; Linux PM <linux-pm@vger.kernel.org>
> > > > > > Subject: Re: [PATCH] cpuidle: Move cpuidle driver forward before
> > > > > > acpi driver in Makefile
> > > > > >
> > > > > > On Wed, Jul 13, 2022 at 10:21 AM Dapeng Mi
> > > > > > <dapeng1.mi@intel.com>
> > > > > wrote:
> > > > > > >
> > > > > > > As long as Kconfig ACPI_PROCESSOR is enabled, ACPI_PROCESSOR
> > > > > > > would select ACPI_PROCESSOR_IDLE and acpi_idle driver is
> > > > > > > enabled. But in current driver loading order acpi_idle driver
> > > > > > > is always loaded before cpuidle_haltpoll driver. This leads to
> > > > > > > cpuidle_hatpoll driver has no chance to be loaded when it's enabled.
> > > > > > >
> > > > > > > Thus, move cpuidle driver forward before acpi driver and make
> > > > > > > cpuidle-hatpoll driver has a chance to be run when it's enabled.
> > > > > > >
> > > > > > > Signed-off-by: Dapeng Mi <dapeng1.mi@intel.com>
> > > > > > > ---
> > > > > > >  drivers/Makefile | 2 +-
> > > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/drivers/Makefile b/drivers/Makefile index
> > > > > > > 9a30842b22c5..921ed481b520 100644
> > > > > > > --- a/drivers/Makefile
> > > > > > > +++ b/drivers/Makefile
> > > > > > > @@ -26,6 +26,7 @@ obj-y                         += idle/
> > > > > > >  # IPMI must come before ACPI in order to provide IPMI
> > > > > > > opregion
> > > > > support
> > > > > > >  obj-y                          += char/ipmi/
> > > > > > >
> > > > > > > +obj-$(CONFIG_CPU_IDLE)         += cpuidle/
> > > > > > >  obj-$(CONFIG_ACPI)             += acpi/
> > > > > > >
> > > > > > >  # PnP must come after ACPI since it will eventually need to
> > > > > > > check if
> > > acpi
> > > > > > > @@ -126,7 +127,6 @@ obj-$(CONFIG_EDAC)          += edac/
> > > > > > >  obj-$(CONFIG_EISA)             += eisa/
> > > > > > >  obj-$(CONFIG_PM_OPP)           += opp/
> > > > > > >  obj-$(CONFIG_CPU_FREQ)         += cpufreq/
> > > > > > > -obj-$(CONFIG_CPU_IDLE)         += cpuidle/
> > > > > > >  obj-y                          += mmc/
> > > > > > >  obj-y                          += ufs/
> > > > > > >  obj-$(CONFIG_MEMSTICK)         += memstick/
> > > > > > > --
> > > > > >
> > > > > > Well, this change doesn't guarantee loading haltpoll before ACPI idle.
> > > > > >
> > > > > > Also what if haltpoll is enabled, but the user wants ACPI idle?
> > > > >
> > > > > Thanks Rafael for reviewing this patch.
> > > > >
> > > > > acpi_idle driver and cpuidle_haltpoll driver have same
> > > > > initialization level and both are initialized on the level
> > > > > device_initcall. So the building order would decide the loading
> > > > > sequence. Just like the intel_idle driver which also has same
> > > > > initialization level (device_initcall), but as it's built before
> > > > > acpi_idle driver, it would be loaded first before acpi_driver if
> > > > > intel_idle
> > > driver is enabled.
> > > > >
> > > > > There is another method to make cpuidle_haltpoll driver loaded
> > > > > first before acpi_driver, it's change the initialization level to
> > > > > postcore_initcall. I'm not sure which one is better, but it seems
> > > > > current
> > > patch is more reasonable.
> > > > >
> > > > > There is an parameter "force" to manage the haltpoll enabling. If
> > > > > user want to use ACPI idle, it can change this parameter to
> > > > > disable
> > > haltpolll driver.
> > >
> > > That would require things to be appended to the kernel command line in
> > > cases where that's not necessary today and that's not acceptable.
> > >
> > The haltpoll driver's "force" parameter is false by default, we don't need to
> > add extra command line options in most cases except we want to enable the
> > haltpolling driver.
> >
> > > What you really seem to be wanting to do is to use haltpoll instead of
> > > ACPI idle.  Is that correct?
> >
> > Yes, I'm trying to enable guest halt polling to improve the performance of
> > some Virtual Machine. But I found the halt poll driver can't be enabled as
> > long as acpi idle driver is enabled. I tried to disable the acpi idle first, but I
> > found there is no parameter to enable/disable acpi idle driver except
> > disabling the Kconfig "CONFIG_ACPI_PROCESSOR_IDLE", and but
> > unfortunately Kconfig "ACPI_PROCESSOR" would enable
> > "ACPI_PROCESSOR_IDLE" by default. If I want to disable acpi_idle driver, I
> > have to disable the "ACPI_PROCESSOR", but this would cause acpi throttling
> > and acpi thermal are also disabled. That's not what I want. That's why I do this
> > change to make halt poll driver has a  chance to run without disable the
> > entire acpi processor functionality .
> >
>
> Any feedback? Thanks.

I've already said that messing up with Makefiles is not the way to go here.

You have to enforce a specific ordering of initialization in the code.