Re: [PATCH v1 09/13] util/mmap-alloc: Implement resizable mmaps

[David Hildenbrand]

On 06.02.20 13:08, Richard Henderson wrote:
> On 2/3/20 6:31 PM, David Hildenbrand wrote:
>> +void *qemu_ram_mmap_resize(void *ptr, int fd, size_t old_size, size_t 
>> new_size,
>> +                           bool shared, bool is_pmem)
>>  {
>>      const size_t pagesize = mmap_pagesize(fd);
>>  
>>      /* we can only map whole pages */
>> -    size = QEMU_ALIGN_UP(size, pagesize);
>> +    old_size = QEMU_ALIGN_UP(old_size, pagesize);
>> +    new_size = QEMU_ALIGN_UP(new_size, pagesize);
>> +
>> +    /* we support actually resizable memory regions only on Linux */
>> +    if (old_size < new_size) {
>> +        /* populate the missing piece into the reserved area */
>> +        ptr = mmap_populate(ptr + old_size, new_size - old_size, fd, 
>> old_size,
>> +                            shared, is_pmem);
>> +    } else if (old_size > new_size) {
>> +        /* discard this piece, keeping the area reserved (should never 
>> fail) */
>> +        ptr = mmap_reserve(ptr + new_size, old_size - new_size, fd);
>> +    }
>> +    return ptr;
>> +}
> 
> What does the return value indicate?
> Is it just for != MAP_FAILED?

It indicates if resizing succeeded. In a previous version I returned an
int via

ptr == MAP_FAILED ? -errno : 0;


Populating will usually only fail because we're out of memory.

Populating and reserving *might* fail if we are out of VMAs in the
kernel. VMA merging will make sure that the number of VMAs will not
explode (usually 2-3 VMAs for one resizable region: populated VMA +
Reserved VMA + Guard page VMA). But once we would be close to the VMA
limit, it could happen - but it's highly unlikely.

> Assuming an assert isn't viable, are we better off with a boolean return?  
> With
> an Error **ptr?

either that or an int. What do you prefer?

Thanks!

-- 
Thanks,

David / dhildenb