On Wed, Aug 28, 2024 at 11:07:43AM +0200, David Hildenbrand wrote: > As reported by Peter, we might be leaking memory when removing the > highest RAMBlock (in the weird ram_addr_t space), and adding a new one. > > We will fail to realize that we already allocated bitmaps for more > dirty memory blocks, and effectively discard the pointers to them. > > Fix it by getting rid of last_ram_page() and by remembering the number > of dirty memory blocks that have been allocated already. > > While at it, let's use "unsigned int" for the number of blocks, which > should be sufficient until we reach ~32 exabytes. > > Looks like this leak was introduced as we switched from using a single > bitmap_zero_extend() to allocating multiple bitmaps: > bitmap_zero_extend() relies on g_renew() which should have taken care of > this. > > Resolves: > https://lkml.kernel.org/r/CAFEAcA-k7a+VObGAfCFNygQNfCKL=AfX6A4kScq=vssk0pe...@mail.gmail.com > Reported-by: Peter Maydell <peter.mayd...@linaro.org> > Fixes: 5b82b703b69a ("memory: RCU ram_list.dirty_memory[] for safe RAM > hotplug") > Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com> > Reviewed-by: Peter Xu <pet...@redhat.com> > Tested-by: Peter Maydell <peter.mayd...@linaro.org> > Cc: qemu-sta...@nongnu.org > Cc: Stefan Hajnoczi <stefa...@redhat.com> > Cc: Paolo Bonzini <pbonz...@redhat.com> > Cc: Peter Xu <pet...@redhat.com> > Cc: "Philippe Mathieu-Daudé" <phi...@linaro.org> > Signed-off-by: David Hildenbrand <da...@redhat.com> > --- > > v1 -> v2: > * Move the counter to RAMList > * Use "unsigned int" instead of "ram_addr_t" as type for the number of > blocks
Thanks. I'll pick this in the next pull if nobody disagrees (or beat me to it). -- Peter Xu
