| Message ID | 20180819001746.21586-2-richard.weiyang@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] kvm: leverage change to adjust slots->used_slots in update_memslots() | expand |
On 19/08/2018 02:17, Wei Yang wrote: > This is a trivial optimization of the first loop in update_memslots(). > > Since used_slots records the number of valid slots, it could be used as the > boundary of the loop instead of looping the whole array and check npages. > > Signed-off-by: Wei Yang <richard.weiyang@gmail.com> > --- > virt/kvm/kvm_main.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c > index 5c1911790f25..fac3225eff35 100644 > --- a/virt/kvm/kvm_main.c > +++ b/virt/kvm/kvm_main.c > @@ -806,15 +806,13 @@ static void update_memslots(struct kvm_memslots *slots, > { > int id = new->id; > int i = slots->id_to_index[id]; > + int used_slots = slots->used_slots; > struct kvm_memory_slot *mslots = slots->memslots; > > WARN_ON(mslots[i].id != id); > slots->used_slots += (char)change; > > - while (i < KVM_MEM_SLOTS_NUM - 1 && > - new->base_gfn <= mslots[i + 1].base_gfn) { > - if (!mslots[i + 1].npages) > - break; > + while (i < used_slots - 1 && new->base_gfn <= mslots[i + 1].base_gfn) { > mslots[i] = mslots[i + 1]; > slots->id_to_index[mslots[i].id] = i; > i++; > The loop is removing an element from the array, so the correct limit for the index would be the *old* length of the array, not the new one. Alternatively... just keep the current code. :) It is already loading mslots[i+1] to get the base_gfn, so the load is not expensive. This is also not really a fast path. Paolo
On Mon, Aug 20, 2018 at 03:54:02PM +0200, Paolo Bonzini wrote: >On 19/08/2018 02:17, Wei Yang wrote: >> This is a trivial optimization of the first loop in update_memslots(). >> >> Since used_slots records the number of valid slots, it could be used as the >> boundary of the loop instead of looping the whole array and check npages. >> >> Signed-off-by: Wei Yang <richard.weiyang@gmail.com> >> --- >> virt/kvm/kvm_main.c | 6 ++---- >> 1 file changed, 2 insertions(+), 4 deletions(-) >> >> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c >> index 5c1911790f25..fac3225eff35 100644 >> --- a/virt/kvm/kvm_main.c >> +++ b/virt/kvm/kvm_main.c >> @@ -806,15 +806,13 @@ static void update_memslots(struct kvm_memslots *slots, >> { >> int id = new->id; >> int i = slots->id_to_index[id]; >> + int used_slots = slots->used_slots; >> struct kvm_memory_slot *mslots = slots->memslots; >> >> WARN_ON(mslots[i].id != id); >> slots->used_slots += (char)change; >> >> - while (i < KVM_MEM_SLOTS_NUM - 1 && >> - new->base_gfn <= mslots[i + 1].base_gfn) { >> - if (!mslots[i + 1].npages) >> - break; >> + while (i < used_slots - 1 && new->base_gfn <= mslots[i + 1].base_gfn) { >> mslots[i] = mslots[i + 1]; >> slots->id_to_index[mslots[i].id] = i; >> i++; >> > >The loop is removing an element from the array, so the correct limit for >the index would be the *old* length of the array, not the new one. Yes, this code checks with the *old* length, which is saved at beginning of the function instead of retrieving it from slots->used_slots. >Alternatively... just keep the current code. :) It is already loading >mslots[i+1] to get the base_gfn, so the load is not expensive. This is >also not really a fast path. Ah, you are right, mslots[i+1].base_gfn already loaded to memory, so it won't be expensive to access mslots[i+1].npages. My intention is to make it *looks* better, by which the loops just use the number of valid slots as the boundary instead of checking both array size and npages. At last, I agree with you that it won't do much to help the performance :) > >Paolo
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 5c1911790f25..fac3225eff35 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -806,15 +806,13 @@ static void update_memslots(struct kvm_memslots *slots, { int id = new->id; int i = slots->id_to_index[id]; + int used_slots = slots->used_slots; struct kvm_memory_slot *mslots = slots->memslots; WARN_ON(mslots[i].id != id); slots->used_slots += (char)change; - while (i < KVM_MEM_SLOTS_NUM - 1 && - new->base_gfn <= mslots[i + 1].base_gfn) { - if (!mslots[i + 1].npages) - break; + while (i < used_slots - 1 && new->base_gfn <= mslots[i + 1].base_gfn) { mslots[i] = mslots[i + 1]; slots->id_to_index[mslots[i].id] = i; i++;
This is a trivial optimization of the first loop in update_memslots(). Since used_slots records the number of valid slots, it could be used as the boundary of the loop instead of looping the whole array and check npages. Signed-off-by: Wei Yang <richard.weiyang@gmail.com> --- virt/kvm/kvm_main.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)