Message-ID: <PH0PR11MB4824E26602680EED90B7FE2ACD919@PH0PR11MB4824.namprd11.prod.outlook.com>
Date:   Thu, 21 Jul 2022 03:09:20 +0000
From:   "Mi, Dapeng1" <dapeng1.mi@...el.com>
To:     "Rafael J. Wysocki" <rafael@...nel.org>
CC:     "Michael S. Tsirkin" <mst@...hat.com>,
        Arnd Bergmann <arnd@...db.de>,
        "Bart Van Assche" <bvanassche@....org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux PM <linux-pm@...r.kernel.org>,
        "Zhenyu Wang" <zhenyuw@...ux.intel.com>
Subject: RE: [PATCH] cpuidle: Move cpuidle driver forward before acpi driver
 in Makefile

> From: Rafael J. Wysocki <rafael@...nel.org>
> Sent: Wednesday, July 20, 2022 6:24 PM
> To: Mi, Dapeng1 <dapeng1.mi@...el.com>
> Cc: Rafael J. Wysocki <rafael@...nel.org>; Michael S. Tsirkin
> <mst@...hat.com>; Arnd Bergmann <arnd@...db.de>; Bart Van Assche
> <bvanassche@....org>; Linux Kernel Mailing List <linux-
> kernel@...r.kernel.org>; Linux PM <linux-pm@...r.kernel.org>; Zhenyu
> Wang <zhenyuw@...ux.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@...el.com>
> wrote:
> >
> > > > From: Rafael J. Wysocki <rafael@...nel.org>
> > > > Sent: Thursday, July 14, 2022 1:53 AM
> > > > To: Mi, Dapeng1 <dapeng1.mi@...el.com>
> > > > Cc: Rafael J. Wysocki <rafael@...nel.org>; Michael S. Tsirkin
> > > > <mst@...hat.com>; Arnd Bergmann <arnd@...db.de>; Bart Van
> Assche
> > > > <bvanassche@....org>; Linux Kernel Mailing List <linux-
> > > > kernel@...r.kernel.org>; Linux PM <linux-pm@...r.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@...el.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@...el.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 .

Powered by blists - more mailing lists