Re: [PATCH v2 10/13] migration/ram: Handle RAM block resizes during postcopy

[David Hildenbrand <david@redhat.com>]

On 24.02.20 23:26, Peter Xu wrote:
> On Fri, Feb 21, 2020 at 05:42:01PM +0100, David Hildenbrand wrote:
> 
> [...]
> 
>> @@ -3160,7 +3160,13 @@ static int ram_load_postcopy(QEMUFile *f)
>>                  break;
>>              }
>>  
>> -            if (!offset_in_ramblock(block, addr)) {
>> +            /*
>> +             * Relying on used_length is racy and can result in false positives.
>> +             * We might place pages beyond used_length in case RAM was shrunk
>> +             * while in postcopy, which is fine - trying to place via
>> +             * UFFDIO_COPY/UFFDIO_ZEROPAGE will never segfault.
>> +             */
>> +            if (!block->host || addr >= block->postcopy_length) {
> 
> I'm thinking whether we can even avoid the -ENOENT failure of
> UFFDIO_COPY.  With the postcopy_length you introduced, I think it's
> the case when addr >= used_length && addr < postcopy_length, right?
> Can we skip those?

1. Recall that any check against used_length is completely racy. So no,
it's not that easy. There is no trusting on used_length at all. It
should never be access from asynchronous postcopy code.

2. There is one theoretical case with resizable allocations: Assume you
first shrink and then grow again. You would have some addr < used_length
where you cannot (and don't want to) place.


Note: Before discovering the nice -ENOENT handling, I had a second
variable postcopy_place_length stored in RAM blocks that would be

- Initialized to postcopy_length
- Synchronized by a mutex
- Changed inside the resize callback on any resizes to
-- postcopy_place_length = min(postcopy_place_length, newsize)

But TBH, I find using -ENOENT much more elegant. It was designed to
handle mmap changes like this.

-- 
Thanks,

David / dhildenb