Re: [PATCH 0/6] mm: split underutilized THPs

[David Hildenbrand <david@redhat.com>]

On 06.08.24 19:28, Johannes Weiner wrote:
> On Thu, Aug 01, 2024 at 08:36:32AM +0200, David Hildenbrand wrote:
>> I just added another printf to postcopy_ram_supported_by_host(), where
>> we temporarily do a UFFDIO_REGISTER on some test area.
>>
>> Sensing UFFD support # postcopy_ram_supported_by_host()
>> Sensing UFFD support
>> Writing received pages during precopy # ram_load_precopy()
>> Writing received pages during precopy
>> Writing received pages during precopy
>> Writing received pages during precopy
>> Writing received pages during precopy
>> Writing received pages during precopy
>> Writing received pages during precopy
>> Writing received pages during precopy
>> Writing received pages during precopy
>> Writing received pages during precopy
>> Writing received pages during precopy
>> Writing received pages during precopy
>> Writing received pages during precopy
>> Writing received pages during precopy
>> Writing received pages during precopy
>> Writing received pages during precopy
>> Writing received pages during precopy
>> Writing received pages during precopy
>> Writing received pages during precopy
>> Writing received pages during precopy
>> Writing received pages during precopy
>> Writing received pages during precopy
>> Disabling THP: MADV_NOHUGEPAGE # postcopy_ram_prepare_discard()
>> Discarding pages # loadvm_postcopy_ram_handle_discard()
>> Discarding pages
>> Discarding pages
>> Discarding pages
>> Discarding pages
>> Discarding pages
>> Discarding pages
>> Discarding pages
>> Discarding pages
>> Discarding pages
>> Discarding pages
>> Discarding pages
>> Discarding pages
>> Discarding pages
>> Discarding pages
>> Discarding pages
>> Registering UFFD # postcopy_ram_incoming_setup()
>>
>> We could think about using this "ever user uffd" to avoid the shared
>> zeropage in most processes.
>>
>> Of course, there might be other applications where that wouldn't work,
>> but I think this behavior (write to area before enabling uffd) might be
>> fairly QEMU specific already.
> 
> It makes me a bit uneasy to hardcode this into the kernel. It's fairly
> specific to qemu/criu, and won't protect usecases that behave slightly
> differently.
> 
> It would also give userfaultfd users that aren't susceptible to this
> particular scenario a different code path.

True, but let's be honest, it's not like there are a million userfaultfd 
users out there :)

Anyhow ...

> 
>> Avoiding the shared zeropage has the benefit that a later write fault
>> won't have to do a TLB flush and can simply install a fresh anon page.
> 
> That's true - although if that happens frequently, it's something we
> might want to tune the shrinker for anyway. If subpages do get used
> later, we probably shouldn't have split the THP to begin with.
> 
> IMO the safest bet would be to use the zero page unconditionally.
> 

... I don't disagree. It also smells more like an optimization on top, 
if ever.

>>> 		return false;
>>>
>>> 	newpte = pte_mkspecial(pfn_pte(page_to_pfn(ZERO_PAGE(pvmw->address)),
>>> 					pvmw->vma->vm_page_prot));
>>>
>>> 	set_pte_at(pvmw->vma->vm_mm, pvmw->address, pvmw->pte, newpte);
>>
>> We're replacing a present page by another present page without doing a
>> TLB flush in between. I *think* this should be fine because the new
>> present page is R/O and cannot possibly be written to.
> 
> It's safe because it's replacing a migration entry. The TLB was
> flushed when that was installed, and since the migration pte is not
> marked present it couldn't have re-established a TLB entry.

Oh, right, we're dealing with a migration entry.

-- 
Cheers,

David / dhildenb