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

[David Hildenbrand <david@redhat.com>]

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