2022-12-07 12:46:55

by Li RongQing

[permalink] [raw]
Subject: [PATCH 2/2][v2] cpuidle-haltpoll: Build as module by default

From: Li RongQing <[email protected]>

Allow user to unload it in running

Signed-off-by: Li RongQing <[email protected]>
---
drivers/cpuidle/Kconfig | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
index ff71dd6..43ddb84 100644
--- a/drivers/cpuidle/Kconfig
+++ b/drivers/cpuidle/Kconfig
@@ -74,7 +74,7 @@ endmenu
config HALTPOLL_CPUIDLE
tristate "Halt poll cpuidle driver"
depends on X86 && KVM_GUEST
- default y
+ default m
help
This option enables halt poll cpuidle driver, which allows to poll
before halting in the guest (more efficient than polling in the
--
2.9.4


2022-12-07 15:17:32

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 2/2][v2] cpuidle-haltpoll: Build as module by default

On Wed, Dec 7, 2022 at 1:41 PM <[email protected]> wrote:
>
> From: Li RongQing <[email protected]>
>
> Allow user to unload it in running

Just like that? And corrupt things left and right while at it?

No way.

And why do you need this?

> Signed-off-by: Li RongQing <[email protected]>
> ---
> drivers/cpuidle/Kconfig | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/cpuidle/Kconfig b/drivers/cpuidle/Kconfig
> index ff71dd6..43ddb84 100644
> --- a/drivers/cpuidle/Kconfig
> +++ b/drivers/cpuidle/Kconfig
> @@ -74,7 +74,7 @@ endmenu
> config HALTPOLL_CPUIDLE
> tristate "Halt poll cpuidle driver"
> depends on X86 && KVM_GUEST
> - default y
> + default m
> help
> This option enables halt poll cpuidle driver, which allows to poll
> before halting in the guest (more efficient than polling in the
> --

2022-12-08 02:41:44

by Li RongQing

[permalink] [raw]
Subject: RE: [PATCH 2/2][v2] cpuidle-haltpoll: Build as module by default


> > Allow user to unload it in running
>
> Just like that? And corrupt things left and right while at it?
>
> No way.
>
> And why do you need this?

Cpuidle-haltpoll can not improve performance for all cases, like when guest has mwait, unixbench shows a small performance drop;
So change it as module, user can insmod this drivers and rmmod this driver at run time

And some downstream os, centos and ubuntu build it module

Of cause, it will cause performance drop since it is not installed by default, but user insmod this module, this performance can restore, so I think this is acceptable
If this reasom is acceptable, I will add v3; or drop this patch.

Thanks

-Li

2022-12-08 12:59:45

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 2/2][v2] cpuidle-haltpoll: Build as module by default

On Thu, Dec 8, 2022 at 3:32 AM Li,Rongqing <[email protected]> wrote:
>
>
> > > Allow user to unload it in running
> >
> > Just like that? And corrupt things left and right while at it?
> >
> > No way.
> >
> > And why do you need this?
>
> Cpuidle-haltpoll can not improve performance for all cases, like when guest has mwait, unixbench shows a small performance drop;
> So change it as module, user can insmod this drivers and rmmod this driver at run time

That is problematic, because in the mainline Linux kernel (which is
what we are talking about here) there is no support for modular
cpuidle governors.

Also, there is an interface for switching cpuidle governors at run
time already, so why can 't it be used to address this case?

> And some downstream os, centos and ubuntu build it module

Well, it's their problem.

2022-12-08 13:46:09

by Li RongQing

[permalink] [raw]
Subject: RE: [PATCH 2/2][v2] cpuidle-haltpoll: Build as module by default

> Also, there is an interface for switching cpuidle governors at run time already, so
> why can 't it be used to address this case?


I will study this interface, thanks

-Li

2022-12-08 14:21:14

by Rafael J. Wysocki

[permalink] [raw]
Subject: Re: [PATCH 2/2][v2] cpuidle-haltpoll: Build as module by default

On Thu, Dec 8, 2022 at 1:45 PM Li,Rongqing <[email protected]> wrote:
>
> > Also, there is an interface for switching cpuidle governors at run time already, so
> > why can 't it be used to address this case?
>
>
> I will study this interface, thanks

Sorry, this patch series is about the haltpoll driver, not the
haltpoll governor (which is there too), so you are right, it can be
modular, but it is not modular by default.

I guess it would be fine to make it modular by default, unless there
are expectations regarding it being present on system startup in the
field and that part is unclear. I think it would be better to defer
this change until it can be clarified.

2022-12-16 16:07:00

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH 2/2][v2] cpuidle-haltpoll: Build as module by default

On Thu, Dec 08, 2022 at 02:32:15AM +0000, Li,Rongqing wrote:
>
> > > Allow user to unload it in running
> >
> > Just like that? And corrupt things left and right while at it?
> >
> > No way.
> >
> > And why do you need this?
>
> Cpuidle-haltpoll can not improve performance for all cases, like when
> guest has mwait, unixbench shows a small performance drop; So change
> it as module, user can insmod this drivers and rmmod this driver at
> run time

I'm thinking all this can be achieved by a small change to
haltpoll_want().