Re: [PATCH v1] softmmu/physmem: fix memory leak in dirty_memory_extend()

[David Hildenbrand]

On 27.08.24 19:57, Peter Xu wrote:
> On Tue, Aug 27, 2024 at 10:37:15AM +0200, David Hildenbrand wrote:
> >   /* Called with ram_list.mutex held */
> > -static void dirty_memory_extend(ram_addr_t old_ram_size,
> > -                                ram_addr_t new_ram_size)
> > +static void dirty_memory_extend(ram_addr_t new_ram_size)
> >   {
> > -    ram_addr_t old_num_blocks = DIV_ROUND_UP(old_ram_size,
> > -                                             DIRTY_MEMORY_BLOCK_SIZE);
> >       ram_addr_t new_num_blocks = DIV_ROUND_UP(new_ram_size,
> >                                                DIRTY_MEMORY_BLOCK_SIZE);
> >       int i;
> >   
> > -    /* Only need to extend if block count increased */
> > -    if (new_num_blocks <= old_num_blocks) {
> > -        return;
> > -    }
>
> One nitpick here: IMHO we could move the n_blocks cache in ram_list
> instead, then we keep the check here and avoid caching it three times with
> the same value.

yes, as written in the patch description: "We'll store the number of 
blocks along with the actual pointer to keep it simple."

It's cleaner to me to store it along the RCU-freed data structure that 
has this size.

--
Cheers,

David / dhildenb