Re: [PATCH 3/4] KVM: Dynamic sized kvm memslots array

[Peter Xu <peterx@redhat.com>]

On Wed, Sep 04, 2024 at 11:23:33PM +0200, David Hildenbrand wrote:
> 
> > > 
> > > Then, you can remove the parameter from kvm_slots_grow() completely and just call it
> > > kvm_slots_double() and simplify a bit:
> > > 
> > > static bool kvm_slots_double(KVMMemoryListener *kml)
> > > {
> > >      unsigned int i, nr_slots_new, cur = kml->nr_slots_allocated;
> > >      KVMSlot *slots;
> > > 
> > >      nr_slots_new = MIN(cur * 2, kvm_state->nr_slots_max);
> > >      if (nr_slots_new == kvm_state->nr_slots_max) {
> > >          /* We reached the maximum */
> > > 	return false;
> > >      }
> > > 
> > >      assert(kml->slots);
> > >      slots = g_renew(KVMSlot, kml->slots, nr_slots_new);
> > >      /*
> > >       * g_renew() doesn't initialize extended buffers, however kvm
> > >       * memslots require fields to be zero-initialized. E.g. pointers,
> > >       * memory_size field, etc.
> > >       */
> > >      memset(&slots[cur], 0x0, sizeof(slots[0]) * (nr_slots_new - cur));
> > > 
> > >      for (i = cur; i < nr_slots_new; i++) {
> > >          slots[i].slot = i;
> > >      }
> > > 
> > >      kml->slots = slots;
> > >      kml->nr_slots_allocated = nr_slots_new;
> > >      trace_kvm_slots_grow(cur, nr_slots_new);
> > > 
> > >      return true;
> > > }
> > 
> > Personally I still think it cleaner to allow setting whatever size.
> 
> Why would one need that? If any, at some point we would want to shrink or
> rather "compact".
> 
> > 
> > We only have one place growing so far, which is pretty trivial to double
> > there, IMO.  I'll wait for a second opinion, or let me know if you have
> > strong feelings..
> 
> I think the simplicity of kvm_slots_double() speaks for itself, but I won't
> fight for it.

Using kvm_slots_double() won't be able to share the same code when
initialize (to e.g. avoid hard-coded initialize of "slots[i].slot").

Thanks,

-- 
Peter Xu