This patchset tries to fix below issues:
1. Admin could set halt_poll_ns to 0 at runtime to disable poll and kernel
behave just like the generic halt driver. Then If guest_halt_poll_grow_start
is set to 0 and guest_halt_poll_ns set to nonzero later, cpu_halt_poll_us will
never grow beyond 0. The first two patches fix this issue from both kvm and
guest side.
2. guest_halt_poll_grow_start and guest_halt_poll_ns could be adjusted at
runtime by admin, this could make a window where cpu_halt_poll_us jump out
of the boundary. the window could be long in some cases(e.g. guest_halt_poll_grow_start
is bumped and cpu_halt_poll_us is shrinking) The last two patches fix this
issue from both kvm and guest side.
3. The 4th patch also simplifies branch check code.
v2:
Rewrite the patches and drop unnecessory changes
Zhenzhong Duan (4):
cpuidle-haltpoll: ensure grow start value is nonzero
KVM: ensure grow start value is nonzero
cpuidle-haltpoll: ensure cpu_halt_poll_us in right scope
KVM: ensure vCPU halt_poll_us in right scope
drivers/cpuidle/governors/haltpoll.c | 50 ++++++++++++++++++++++++-----------
virt/kvm/kvm_main.c | 51 ++++++++++++++++++++++++------------
2 files changed, 68 insertions(+), 33 deletions(-)
--
1.8.3.1
vcpu->halt_poll_ns could be zeroed in certain cases (e.g. by
halt_poll_ns = 0). If halt_poll_grow_start is zero,
vcpu->halt_poll_ns will never be bigger than zero.
Use param callback to avoid writing zero to halt_poll_grow_start.
Signed-off-by: Zhenzhong Duan <[email protected]>
---
virt/kvm/kvm_main.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index d6f0696..359516b 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -69,6 +69,26 @@
MODULE_AUTHOR("Qumranet");
MODULE_LICENSE("GPL");
+static int grow_start_set(const char *val, const struct kernel_param *kp)
+{
+ int ret;
+ unsigned int n;
+
+ if (!val)
+ return -EINVAL;
+
+ ret = kstrtouint(val, 0, &n);
+ if (ret || !n)
+ return -EINVAL;
+
+ return param_set_uint(val, kp);
+}
+
+static const struct kernel_param_ops grow_start_ops = {
+ .set = grow_start_set,
+ .get = param_get_uint,
+};
+
/* Architectures should define their poll value according to the halt latency */
unsigned int halt_poll_ns = KVM_HALT_POLL_NS_DEFAULT;
module_param(halt_poll_ns, uint, 0644);
@@ -81,7 +101,7 @@
/* The start value to grow halt_poll_ns from */
unsigned int halt_poll_ns_grow_start = 10000; /* 10us */
-module_param(halt_poll_ns_grow_start, uint, 0644);
+module_param_cb(halt_poll_ns_grow_start, &grow_start_ops, &halt_poll_ns_grow_start, 0644);
EXPORT_SYMBOL_GPL(halt_poll_ns_grow_start);
/* Default resets per-vcpu halt_poll_ns . */
--
1.8.3.1
As user can adjust halt_poll_ns_grow_start and halt_poll_ns which
leads to vcpu->halt_poll_ns beyond the two boundaries. This patch
ensures vcpu->halt_poll_ns in that scope after growing or shrinking.
If halt_poll_ns_shrink is 0, shrink vcpu->halt_poll_ns to
halt_poll_ns_grow_start instead of 0. To disable poll we can set
halt_poll_ns to 0.
In case user wrongly set halt_poll_ns_grow_start > halt_poll_ns > 0,
halt_poll_ns take precedency and poll time is a fixed value of
halt_poll_ns.
This patch also simplifies branch check based on the guest haltpoll
code.
Signed-off-by: Zhenzhong Duan <[email protected]>
---
virt/kvm/kvm_main.c | 29 +++++++++++++----------------
1 file changed, 13 insertions(+), 16 deletions(-)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 359516b..b4fca66 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2308,9 +2308,15 @@ static void shrink_halt_poll_ns(struct kvm_vcpu *vcpu)
old = val = vcpu->halt_poll_ns;
shrink = READ_ONCE(halt_poll_ns_shrink);
if (shrink == 0)
- val = 0;
- else
+ val = halt_poll_ns_grow_start;
+ else {
val /= shrink;
+ if (val < halt_poll_ns_grow_start)
+ val = halt_poll_ns_grow_start;
+ }
+
+ if (val > halt_poll_ns)
+ val = halt_poll_ns;
vcpu->halt_poll_ns = val;
trace_kvm_halt_poll_ns_shrink(vcpu->vcpu_id, val, old);
@@ -2385,21 +2391,12 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
block_ns = ktime_to_ns(cur) - ktime_to_ns(start);
if (!kvm_arch_no_poll(vcpu)) {
- if (!vcpu_valid_wakeup(vcpu)) {
+ /* we had a long block, shrink polling */
+ if (!vcpu_valid_wakeup(vcpu) || block_ns > halt_poll_ns)
shrink_halt_poll_ns(vcpu);
- } else if (halt_poll_ns) {
- if (block_ns <= vcpu->halt_poll_ns)
- ;
- /* we had a long block, shrink polling */
- else if (vcpu->halt_poll_ns && block_ns > halt_poll_ns)
- shrink_halt_poll_ns(vcpu);
- /* we had a short halt and our poll time is too small */
- else if (vcpu->halt_poll_ns < halt_poll_ns &&
- block_ns < halt_poll_ns)
- grow_halt_poll_ns(vcpu);
- } else {
- vcpu->halt_poll_ns = 0;
- }
+ /* we had a short block and our poll time is too small */
+ else if (block_ns > vcpu->halt_poll_ns)
+ grow_halt_poll_ns(vcpu);
}
trace_kvm_vcpu_wakeup(block_ns, waited, vcpu_valid_wakeup(vcpu));
--
1.8.3.1
As user can adjust guest_halt_poll_grow_start and guest_halt_poll_ns
which leads to cpu_halt_poll_us beyond the two boundaries. This patch
ensures cpu_halt_poll_us in that scope.
If guest_halt_poll_shrink is 0, shrink the cpu_halt_poll_us to
guest_halt_poll_grow_start instead of 0. To disable poll we can set
guest_halt_poll_ns to 0.
If user wrongly set guest_halt_poll_grow_start > guest_halt_poll_ns > 0,
guest_halt_poll_ns take precedency and poll time is a fixed value of
guest_halt_poll_ns.
Signed-off-by: Zhenzhong Duan <[email protected]>
---
drivers/cpuidle/governors/haltpoll.c | 28 +++++++++++++---------------
1 file changed, 13 insertions(+), 15 deletions(-)
diff --git a/drivers/cpuidle/governors/haltpoll.c b/drivers/cpuidle/governors/haltpoll.c
index 660859d..d0b38b53 100644
--- a/drivers/cpuidle/governors/haltpoll.c
+++ b/drivers/cpuidle/governors/haltpoll.c
@@ -97,32 +97,30 @@ static int haltpoll_select(struct cpuidle_driver *drv,
static void adjust_poll_limit(struct cpuidle_device *dev, unsigned int block_us)
{
- unsigned int val;
+ unsigned int val = dev->poll_limit_ns;
u64 block_ns = block_us*NSEC_PER_USEC;
/* Grow cpu_halt_poll_us if
- * cpu_halt_poll_us < block_ns < guest_halt_poll_us
+ * cpu_halt_poll_us < block_ns <= guest_halt_poll_us
*/
- if (block_ns > dev->poll_limit_ns && block_ns <= guest_halt_poll_ns) {
+ if (block_ns > dev->poll_limit_ns && block_ns <= guest_halt_poll_ns &&
+ guest_halt_poll_grow)
val = dev->poll_limit_ns * guest_halt_poll_grow;
-
- if (val < guest_halt_poll_grow_start)
- val = guest_halt_poll_grow_start;
- if (val > guest_halt_poll_ns)
- val = guest_halt_poll_ns;
-
- dev->poll_limit_ns = val;
- } else if (block_ns > guest_halt_poll_ns &&
- guest_halt_poll_allow_shrink) {
+ else if (block_ns > guest_halt_poll_ns &&
+ guest_halt_poll_allow_shrink) {
unsigned int shrink = guest_halt_poll_shrink;
- val = dev->poll_limit_ns;
if (shrink == 0)
- val = 0;
+ val = guest_halt_poll_grow_start;
else
val /= shrink;
- dev->poll_limit_ns = val;
}
+ if (val < guest_halt_poll_grow_start)
+ val = guest_halt_poll_grow_start;
+ if (val > guest_halt_poll_ns)
+ val = guest_halt_poll_ns;
+
+ dev->poll_limit_ns = val;
}
/**
--
1.8.3.1
dev->poll_limit_ns could be zeroed in certain cases (e.g. by
guest_halt_poll_ns = 0). If guest_halt_poll_grow_start is zero,
dev->poll_limit_ns will never be bigger than zero.
Use param callback to avoid writing zero to guest_halt_poll_grow_start.
Signed-off-by: Zhenzhong Duan <[email protected]>
---
drivers/cpuidle/governors/haltpoll.c | 22 +++++++++++++++++++++-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/cpuidle/governors/haltpoll.c b/drivers/cpuidle/governors/haltpoll.c
index 7a703d2..660859d 100644
--- a/drivers/cpuidle/governors/haltpoll.c
+++ b/drivers/cpuidle/governors/haltpoll.c
@@ -20,6 +20,26 @@
#include <linux/module.h>
#include <linux/kvm_para.h>
+static int grow_start_set(const char *val, const struct kernel_param *kp)
+{
+ int ret;
+ unsigned int n;
+
+ if (!val)
+ return -EINVAL;
+
+ ret = kstrtouint(val, 0, &n);
+ if (ret || !n)
+ return -EINVAL;
+
+ return param_set_uint(val, kp);
+}
+
+static const struct kernel_param_ops grow_start_ops = {
+ .set = grow_start_set,
+ .get = param_get_uint,
+};
+
static unsigned int guest_halt_poll_ns __read_mostly = 200000;
module_param(guest_halt_poll_ns, uint, 0644);
@@ -33,7 +53,7 @@
/* value in us to start growing per-cpu halt_poll_ns */
static unsigned int guest_halt_poll_grow_start __read_mostly = 50000;
-module_param(guest_halt_poll_grow_start, uint, 0644);
+module_param_cb(guest_halt_poll_grow_start, &grow_start_ops, &guest_halt_poll_grow_start, 0644);
/* allow shrinking guest halt poll */
static bool guest_halt_poll_allow_shrink __read_mostly = true;
--
1.8.3.1
On 11/6/2019 10:08 AM, Zhenzhong Duan wrote:
> This patchset tries to fix below issues:
>
> 1. Admin could set halt_poll_ns to 0 at runtime to disable poll and kernel
> behave just like the generic halt driver. Then If guest_halt_poll_grow_start
> is set to 0 and guest_halt_poll_ns set to nonzero later, cpu_halt_poll_us will
> never grow beyond 0. The first two patches fix this issue from both kvm and
> guest side.
>
> 2. guest_halt_poll_grow_start and guest_halt_poll_ns could be adjusted at
> runtime by admin, this could make a window where cpu_halt_poll_us jump out
> of the boundary. the window could be long in some cases(e.g. guest_halt_poll_grow_start
> is bumped and cpu_halt_poll_us is shrinking) The last two patches fix this
> issue from both kvm and guest side.
>
> 3. The 4th patch also simplifies branch check code.
>
> v2:
> Rewrite the patches and drop unnecessory changes
>
> Zhenzhong Duan (4):
> cpuidle-haltpoll: ensure grow start value is nonzero
> KVM: ensure grow start value is nonzero
> cpuidle-haltpoll: ensure cpu_halt_poll_us in right scope
> KVM: ensure vCPU halt_poll_us in right scope
>
> drivers/cpuidle/governors/haltpoll.c | 50 ++++++++++++++++++++++++-----------
> virt/kvm/kvm_main.c | 51 ++++++++++++++++++++++++------------
> 2 files changed, 68 insertions(+), 33 deletions(-)
>
Please resend the series with CCs to [email protected], thanks!