| Message ID | 1573047398-7665-2-git-send-email-nitesh@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: deliver IOAPIC scan request only to the target vCPUs | expand |
On Wed, Nov 06, 2019 at 08:36:37AM -0500, Nitesh Narayan Lal wrote: > From: Radim Krčmář <rkrcmar@redhat.com> > > Fetching an index for any vcpu in kvm->vcpus array by traversing > the entire array everytime is costly. > This patch remembers the position of each vcpu in kvm->vcpus array > by storing it in vcpus_idx under kvm_vcpu structure. > > Signed-off-by: Radim Krčmář <rkrcmar@redhat.com> > Signed-off-by: Nitesh Narayan Lal <nitesh@redhat.com> > --- > include/linux/kvm_host.h | 11 +++-------- > virt/kvm/kvm_main.c | 5 ++++- > 2 files changed, 7 insertions(+), 9 deletions(-) > > diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h > index 719fc3e..31c4fde 100644 > --- a/include/linux/kvm_host.h > +++ b/include/linux/kvm_host.h > @@ -266,7 +266,8 @@ struct kvm_vcpu { > struct preempt_notifier preempt_notifier; > #endif > int cpu; > - int vcpu_id; > + int vcpu_id; /* id given by userspace at creation */ > + int vcpus_idx; /* index in kvm->vcpus array */ I'd probably prefer vcpu_idx or vcpu_index, but it's not a strong preference by any means. > int srcu_idx; > int mode; > u64 requests; > @@ -571,13 +572,7 @@ static inline struct kvm_vcpu *kvm_get_vcpu_by_id(struct kvm *kvm, int id) > > static inline int kvm_vcpu_get_idx(struct kvm_vcpu *vcpu) > { > - struct kvm_vcpu *tmp; > - int idx; > - > - kvm_for_each_vcpu(idx, tmp, vcpu->kvm) > - if (tmp == vcpu) > - return idx; > - BUG(); > + return vcpu->vcpus_idx; > } > > #define kvm_for_each_memslot(memslot, slots) \ > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index 67ef3f2..24ab711 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -2673,7 +2673,10 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id) > goto unlock_vcpu_destroy; > } > > - BUG_ON(kvm->vcpus[atomic_read(&kvm->online_vcpus)]); > + vcpu->vcpus_idx = atomic_read(&kvm->online_vcpus); > + Nit: I'd omit this newline since the assignment and BUG_ON() are directly related. > + BUG_ON(kvm->vcpus[vcpu->vcpus_idx]); The assignment to kvm->vcpus a few lines below should be updated to use the new index. > + > > /* Now it's all set up, let userspace reach it */ > kvm_get_kvm(kvm); > -- > 1.8.3.1 >
On 11/6/19 9:43 AM, Sean Christopherson wrote: > On Wed, Nov 06, 2019 at 08:36:37AM -0500, Nitesh Narayan Lal wrote: >> From: Radim Krčmář <rkrcmar@redhat.com> >> >> Fetching an index for any vcpu in kvm->vcpus array by traversing >> the entire array everytime is costly. >> This patch remembers the position of each vcpu in kvm->vcpus array >> by storing it in vcpus_idx under kvm_vcpu structure. >> >> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com> >> Signed-off-by: Nitesh Narayan Lal <nitesh@redhat.com> >> --- >> include/linux/kvm_host.h | 11 +++-------- >> virt/kvm/kvm_main.c | 5 ++++- >> 2 files changed, 7 insertions(+), 9 deletions(-) >> >> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h >> index 719fc3e..31c4fde 100644 >> --- a/include/linux/kvm_host.h >> +++ b/include/linux/kvm_host.h >> @@ -266,7 +266,8 @@ struct kvm_vcpu { >> struct preempt_notifier preempt_notifier; >> #endif >> int cpu; >> - int vcpu_id; >> + int vcpu_id; /* id given by userspace at creation */ >> + int vcpus_idx; /* index in kvm->vcpus array */ > I'd probably prefer vcpu_idx or vcpu_index, but it's not a strong > preference by any means. Sure, I will probably replace it with vcpu_idx. > >> int srcu_idx; >> int mode; >> u64 requests; >> @@ -571,13 +572,7 @@ static inline struct kvm_vcpu *kvm_get_vcpu_by_id(struct kvm *kvm, int id) >> >> static inline int kvm_vcpu_get_idx(struct kvm_vcpu *vcpu) >> { >> - struct kvm_vcpu *tmp; >> - int idx; >> - >> - kvm_for_each_vcpu(idx, tmp, vcpu->kvm) >> - if (tmp == vcpu) >> - return idx; >> - BUG(); >> + return vcpu->vcpus_idx; >> } >> >> #define kvm_for_each_memslot(memslot, slots) \ >> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c >> index 67ef3f2..24ab711 100644 >> --- a/virt/kvm/kvm_main.c >> +++ b/virt/kvm/kvm_main.c >> @@ -2673,7 +2673,10 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id) >> goto unlock_vcpu_destroy; >> } >> >> - BUG_ON(kvm->vcpus[atomic_read(&kvm->online_vcpus)]); >> + vcpu->vcpus_idx = atomic_read(&kvm->online_vcpus); >> + > Nit: I'd omit this newline since the assignment and BUG_ON() are directly > related. Makes sense to me. > >> + BUG_ON(kvm->vcpus[vcpu->vcpus_idx]); > The assignment to kvm->vcpus a few lines below should be updated to use > the new index. Ah yes. Thanks for pointing this out. > >> + >> >> /* Now it's all set up, let userspace reach it */ >> kvm_get_kvm(kvm); >> -- >> 1.8.3.1 >>
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h index 719fc3e..31c4fde 100644 --- a/include/linux/kvm_host.h +++ b/include/linux/kvm_host.h @@ -266,7 +266,8 @@ struct kvm_vcpu { struct preempt_notifier preempt_notifier; #endif int cpu; - int vcpu_id; + int vcpu_id; /* id given by userspace at creation */ + int vcpus_idx; /* index in kvm->vcpus array */ int srcu_idx; int mode; u64 requests; @@ -571,13 +572,7 @@ static inline struct kvm_vcpu *kvm_get_vcpu_by_id(struct kvm *kvm, int id) static inline int kvm_vcpu_get_idx(struct kvm_vcpu *vcpu) { - struct kvm_vcpu *tmp; - int idx; - - kvm_for_each_vcpu(idx, tmp, vcpu->kvm) - if (tmp == vcpu) - return idx; - BUG(); + return vcpu->vcpus_idx; } #define kvm_for_each_memslot(memslot, slots) \ diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 67ef3f2..24ab711 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -2673,7 +2673,10 @@ static int kvm_vm_ioctl_create_vcpu(struct kvm *kvm, u32 id) goto unlock_vcpu_destroy; } - BUG_ON(kvm->vcpus[atomic_read(&kvm->online_vcpus)]); + vcpu->vcpus_idx = atomic_read(&kvm->online_vcpus); + + BUG_ON(kvm->vcpus[vcpu->vcpus_idx]); + /* Now it's all set up, let userspace reach it */ kvm_get_kvm(kvm);