| Message ID | 20181105064503.36285-1-richard.weiyang@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] KVM: use same boundary check for async_pf | expand |
2018-11-05 14:45+0800, Wei Yang: > Commit af585b921e5d ("KVM: Halt vcpu if page it tries to access is > swapped out") introduces async_pf. > > The gfn hash table size is defined as: > > gfn_t gfns[roundup_pow_of_two(ASYNC_PF_PER_VCPU)]; > > And iterations in arch/x86/kvm/x86.c are checked with: > > i < roundup_pow_of_two(ASYNC_PF_PER_VCPU) > > While the check in kvm_setup_async_pf() is: > > if (vcpu->async_pf.queued >= ASYNC_PF_PER_VCPU) > > Generally this works good, while the check is not exact. > > This patch adjust the check in kvm_setup_async_pf() to use the same > boundary as others. > > Signed-off-by: Wei Yang <richard.weiyang@gmail.com> > --- > virt/kvm/async_pf.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/virt/kvm/async_pf.c b/virt/kvm/async_pf.c > index 23c2519c5b32..4f4f6eac88a4 100644 > --- a/virt/kvm/async_pf.c > +++ b/virt/kvm/async_pf.c > @@ -182,7 +182,7 @@ int kvm_setup_async_pf(struct kvm_vcpu *vcpu, gva_t gva, unsigned long hva, > { > struct kvm_async_pf *work; > > - if (vcpu->async_pf.queued >= ASYNC_PF_PER_VCPU) > + if (vcpu->async_pf.queued >= roundup_pow_of_two(ASYNC_PF_PER_VCPU)) This is actually correct. Unlike the others in arch/x86, this condition is in general code that should have no idea about architectural constraints, like ASYNC_PF_PER_VCPU being a power of two. It'd be better to change the x86 code to avoid the weird roundup_pow_of_two() everywhere. I'm thinking about #define ASYNC_PF_PER_VCPU_ORDER 6 #define ASYNC_PF_PER_VCPU (1 << ASYNC_PF_PER_VCPU) Thanks.
On Thu, Dec 20, 2018 at 06:56:08PM +0100, Radim Kr??m???? wrote: >2018-11-05 14:45+0800, Wei Yang: >> Commit af585b921e5d ("KVM: Halt vcpu if page it tries to access is >> swapped out") introduces async_pf. >> >> The gfn hash table size is defined as: >> >> gfn_t gfns[roundup_pow_of_two(ASYNC_PF_PER_VCPU)]; >> >> And iterations in arch/x86/kvm/x86.c are checked with: >> >> i < roundup_pow_of_two(ASYNC_PF_PER_VCPU) >> >> While the check in kvm_setup_async_pf() is: >> >> if (vcpu->async_pf.queued >= ASYNC_PF_PER_VCPU) >> >> Generally this works good, while the check is not exact. >> >> This patch adjust the check in kvm_setup_async_pf() to use the same >> boundary as others. >> >> Signed-off-by: Wei Yang <richard.weiyang@gmail.com> >> --- >> virt/kvm/async_pf.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/virt/kvm/async_pf.c b/virt/kvm/async_pf.c >> index 23c2519c5b32..4f4f6eac88a4 100644 >> --- a/virt/kvm/async_pf.c >> +++ b/virt/kvm/async_pf.c >> @@ -182,7 +182,7 @@ int kvm_setup_async_pf(struct kvm_vcpu *vcpu, gva_t gva, unsigned long hva, >> { >> struct kvm_async_pf *work; >> >> - if (vcpu->async_pf.queued >= ASYNC_PF_PER_VCPU) >> + if (vcpu->async_pf.queued >= roundup_pow_of_two(ASYNC_PF_PER_VCPU)) > >This is actually correct. > >Unlike the others in arch/x86, this condition is in general code that >should have no idea about architectural constraints, like >ASYNC_PF_PER_VCPU being a power of two. > >It'd be better to change the x86 code to avoid the weird >roundup_pow_of_two() everywhere. I'm thinking about > > #define ASYNC_PF_PER_VCPU_ORDER 6 > #define ASYNC_PF_PER_VCPU (1 << ASYNC_PF_PER_VCPU) > Thats reasonable, thanks. >Thanks.
diff --git a/virt/kvm/async_pf.c b/virt/kvm/async_pf.c index 23c2519c5b32..4f4f6eac88a4 100644 --- a/virt/kvm/async_pf.c +++ b/virt/kvm/async_pf.c @@ -182,7 +182,7 @@ int kvm_setup_async_pf(struct kvm_vcpu *vcpu, gva_t gva, unsigned long hva, { struct kvm_async_pf *work; - if (vcpu->async_pf.queued >= ASYNC_PF_PER_VCPU) + if (vcpu->async_pf.queued >= roundup_pow_of_two(ASYNC_PF_PER_VCPU)) return 0; /* setup delayed work */
Commit af585b921e5d ("KVM: Halt vcpu if page it tries to access is swapped out") introduces async_pf. The gfn hash table size is defined as: gfn_t gfns[roundup_pow_of_two(ASYNC_PF_PER_VCPU)]; And iterations in arch/x86/kvm/x86.c are checked with: i < roundup_pow_of_two(ASYNC_PF_PER_VCPU) While the check in kvm_setup_async_pf() is: if (vcpu->async_pf.queued >= ASYNC_PF_PER_VCPU) Generally this works good, while the check is not exact. This patch adjust the check in kvm_setup_async_pf() to use the same boundary as others. Signed-off-by: Wei Yang <richard.weiyang@gmail.com> --- virt/kvm/async_pf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)