| Message ID | 20231118155105.25678-14-yury.norov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | biops: add atomig find_bit() operations | expand |
Yury Norov <yury.norov@gmail.com> writes: > The function traverses stimer_pending_bitmap n a for-loop bit by bit. > We can do it faster by using atomic find_and_set_bit(). > > While here, refactor the logic by decreasing indentation level > and dropping 2nd check for stimer->config.enable. > > Signed-off-by: Yury Norov <yury.norov@gmail.com> > --- > arch/x86/kvm/hyperv.c | 39 +++++++++++++++++++-------------------- > 1 file changed, 19 insertions(+), 20 deletions(-) > > diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c > index 238afd7335e4..460e300b558b 100644 > --- a/arch/x86/kvm/hyperv.c > +++ b/arch/x86/kvm/hyperv.c > @@ -870,27 +870,26 @@ void kvm_hv_process_stimers(struct kvm_vcpu *vcpu) > if (!hv_vcpu) > return; > > - for (i = 0; i < ARRAY_SIZE(hv_vcpu->stimer); i++) > - if (test_and_clear_bit(i, hv_vcpu->stimer_pending_bitmap)) { > - stimer = &hv_vcpu->stimer[i]; > - if (stimer->config.enable) { > - exp_time = stimer->exp_time; > - > - if (exp_time) { > - time_now = > - get_time_ref_counter(vcpu->kvm); > - if (time_now >= exp_time) > - stimer_expiration(stimer); > - } > - > - if ((stimer->config.enable) && > - stimer->count) { > - if (!stimer->msg_pending) > - stimer_start(stimer); > - } else > - stimer_cleanup(stimer); > - } > + for_each_test_and_clear_bit(i, hv_vcpu->stimer_pending_bitmap, > + ARRAY_SIZE(hv_vcpu->stimer)) { > + stimer = &hv_vcpu->stimer[i]; > + if (!stimer->config.enable) > + continue; > + > + exp_time = stimer->exp_time; > + > + if (exp_time) { > + time_now = get_time_ref_counter(vcpu->kvm); > + if (time_now >= exp_time) > + stimer_expiration(stimer); > } > + > + if (stimer->count) { You can't drop 'stimer->config.enable' check here as stimer_expiration() call above actually changes it. This is done on purpose: oneshot timers fire only once so 'config.enable' is reset to 0. > + if (!stimer->msg_pending) > + stimer_start(stimer); > + } else > + stimer_cleanup(stimer); > + } > } > > void kvm_hv_vcpu_uninit(struct kvm_vcpu *vcpu)
On Mon, Nov 20, 2023 at 03:26:08PM +0100, Vitaly Kuznetsov wrote: > Yury Norov <yury.norov@gmail.com> writes: > > > The function traverses stimer_pending_bitmap n a for-loop bit by bit. > > We can do it faster by using atomic find_and_set_bit(). > > > > While here, refactor the logic by decreasing indentation level > > and dropping 2nd check for stimer->config.enable. > > > > Signed-off-by: Yury Norov <yury.norov@gmail.com> > > --- > > arch/x86/kvm/hyperv.c | 39 +++++++++++++++++++-------------------- > > 1 file changed, 19 insertions(+), 20 deletions(-) > > > > diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c > > index 238afd7335e4..460e300b558b 100644 > > --- a/arch/x86/kvm/hyperv.c > > +++ b/arch/x86/kvm/hyperv.c > > @@ -870,27 +870,26 @@ void kvm_hv_process_stimers(struct kvm_vcpu *vcpu) > > if (!hv_vcpu) > > return; > > > > - for (i = 0; i < ARRAY_SIZE(hv_vcpu->stimer); i++) > > - if (test_and_clear_bit(i, hv_vcpu->stimer_pending_bitmap)) { > > - stimer = &hv_vcpu->stimer[i]; > > - if (stimer->config.enable) { > > - exp_time = stimer->exp_time; > > - > > - if (exp_time) { > > - time_now = > > - get_time_ref_counter(vcpu->kvm); > > - if (time_now >= exp_time) > > - stimer_expiration(stimer); > > - } > > - > > - if ((stimer->config.enable) && > > - stimer->count) { > > - if (!stimer->msg_pending) > > - stimer_start(stimer); > > - } else > > - stimer_cleanup(stimer); > > - } > > + for_each_test_and_clear_bit(i, hv_vcpu->stimer_pending_bitmap, > > + ARRAY_SIZE(hv_vcpu->stimer)) { > > + stimer = &hv_vcpu->stimer[i]; > > + if (!stimer->config.enable) > > + continue; > > + > > + exp_time = stimer->exp_time; > > + > > + if (exp_time) { > > + time_now = get_time_ref_counter(vcpu->kvm); > > + if (time_now >= exp_time) > > + stimer_expiration(stimer); > > } > > + > > + if (stimer->count) { > > You can't drop 'stimer->config.enable' check here as stimer_expiration() > call above actually changes it. This is done on purpose: oneshot timers > fire only once so 'config.enable' is reset to 0. Ok, I see. Will fix in v2
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c index 238afd7335e4..460e300b558b 100644 --- a/arch/x86/kvm/hyperv.c +++ b/arch/x86/kvm/hyperv.c @@ -870,27 +870,26 @@ void kvm_hv_process_stimers(struct kvm_vcpu *vcpu) if (!hv_vcpu) return; - for (i = 0; i < ARRAY_SIZE(hv_vcpu->stimer); i++) - if (test_and_clear_bit(i, hv_vcpu->stimer_pending_bitmap)) { - stimer = &hv_vcpu->stimer[i]; - if (stimer->config.enable) { - exp_time = stimer->exp_time; - - if (exp_time) { - time_now = - get_time_ref_counter(vcpu->kvm); - if (time_now >= exp_time) - stimer_expiration(stimer); - } - - if ((stimer->config.enable) && - stimer->count) { - if (!stimer->msg_pending) - stimer_start(stimer); - } else - stimer_cleanup(stimer); - } + for_each_test_and_clear_bit(i, hv_vcpu->stimer_pending_bitmap, + ARRAY_SIZE(hv_vcpu->stimer)) { + stimer = &hv_vcpu->stimer[i]; + if (!stimer->config.enable) + continue; + + exp_time = stimer->exp_time; + + if (exp_time) { + time_now = get_time_ref_counter(vcpu->kvm); + if (time_now >= exp_time) + stimer_expiration(stimer); } + + if (stimer->count) { + if (!stimer->msg_pending) + stimer_start(stimer); + } else + stimer_cleanup(stimer); + } } void kvm_hv_vcpu_uninit(struct kvm_vcpu *vcpu)
The function traverses stimer_pending_bitmap n a for-loop bit by bit. We can do it faster by using atomic find_and_set_bit(). While here, refactor the logic by decreasing indentation level and dropping 2nd check for stimer->config.enable. Signed-off-by: Yury Norov <yury.norov@gmail.com> --- arch/x86/kvm/hyperv.c | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-)