| Message ID | 20231203193307.542794-12-yury.norov@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | bitops: add atomic find_bit() operations | expand |
Yury Norov <yury.norov@gmail.com> writes: > The function traverses stimer_pending_bitmap in 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. > > CC: Vitaly Kuznetsov <vkuznets@redhat.com> > 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..a0e45d20d451 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->config.enable && stimer->count) { > + if (!stimer->msg_pending) > + stimer_start(stimer); > + } else > + stimer_cleanup(stimer); Minor nitpick: it's better (and afair required by coding style) to use '{}' for both branches here: if (stimer->config.enable && stimer->count) { if (!stimer->msg_pending) stimer_start(stimer); } else { stimer_cleanup(stimer); } > + } > } > > void kvm_hv_vcpu_uninit(struct kvm_vcpu *vcpu) Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com>
On Mon, Dec 04, 2023, Vitaly Kuznetsov wrote: > > - 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)) { Another nit, please align the indendation: 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->config.enable && stimer->count) { > > + if (!stimer->msg_pending) > > + stimer_start(stimer); > > + } else > > + stimer_cleanup(stimer); > > Minor nitpick: it's better (and afair required by coding style) to use > '{}' for both branches here: Yeah, it's a hard requirement in KVM x86. > > if (stimer->config.enable && stimer->count) { > if (!stimer->msg_pending) > stimer_start(stimer); > } else { > stimer_cleanup(stimer); > } > > > + } > > } > > > > void kvm_hv_vcpu_uninit(struct kvm_vcpu *vcpu) > > Reviewed-by: Vitaly Kuznetsov <vkuznets@redhat.com> > > -- > Vitaly >
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c index 238afd7335e4..a0e45d20d451 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->config.enable && 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 in 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. CC: Vitaly Kuznetsov <vkuznets@redhat.com> Signed-off-by: Yury Norov <yury.norov@gmail.com> --- arch/x86/kvm/hyperv.c | 39 +++++++++++++++++++-------------------- 1 file changed, 19 insertions(+), 20 deletions(-)