On Wed, Nov 06, 2019 at 07:55:00PM +0800, Zhenzhong Duan wrote: > 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. This doesn't explain why allowing an admin to disable halt polling by writing halt_poll_grow_start=0 is a bad thing. Paolo had the same question in v1, here[1] and in the guest driver[2]. [1] https://lkml.kernel.org/r/57679389-6e4a-b7ad-559f-3128a608c28a@xxxxxxxxxx [2] https://lkml.kernel.org/r/391dd11b-ebbb-28ff-5e57-4a795cd16a1b@xxxxxxxxxx > > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@xxxxxxxxxx> > --- > 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 >
