On 04.09.24 23:34, Peter Xu wrote:
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").
I don't see that as any problem and if you really care you could factor
exactly that part out in a helper. Anyhow, I learned that I am not good
at convincing you, so do what you think is best. The code itself should
get the job done.
Acked-by: David Hildenbrand <da...@redhat.com>
--
Cheers,
David / dhildenb
