[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAJZ5v0hdV=NRwiWj9-=t8jQjBA8f5S6n1pnQoGDDFNrq23p4oA@mail.gmail.com>
Date: Tue, 26 Jul 2022 17:47:42 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
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 Tue, Jul 26, 2022 at 3:46 AM Mi, Dapeng1 <dapeng1.mi@...el.com> wrote:
>
> > From: Mi, Dapeng1
> > Sent: Thursday, July 21, 2022 11:09 AM
> > 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 .
> >
>
> 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.
Powered by blists - more mailing lists