Re: [PATCH] mm: introduce MADV_CLR_HUGEPAGE

[Pavel Emelyanov]

On 05/24/2017 02:18 PM, Michal Hocko wrote:
> On Wed 24-05-17 13:39:48, Mike Rapoport wrote:
>> On Wed, May 24, 2017 at 09:58:06AM +0200, Vlastimil Babka wrote:
>>> On 05/24/2017 09:50 AM, Mike Rapoport wrote:
>>>> On Mon, May 22, 2017 at 05:52:47PM +0200, Vlastimil Babka wrote:
>>>>> On 05/22/2017 04:29 PM, Mike Rapoport wrote:
>>>>>>
>>>>>> Probably I didn't explained it too well.
>>>>>>
>>>>>> The range is intentionally not populated. When we combine pre- and
>>>>>> post-copy for process migration, we create memory pre-dump without stopping
>>>>>> the process, then we freeze the process without dumping the pages it has
>>>>>> dirtied between pre-dump and freeze, and then, during restore, we populate
>>>>>> the dirtied pages using userfaultfd.
>>>>>>
>>>>>> When CRIU restores a process in such scenario, it does something like:
>>>>>>
>>>>>> * mmap() memory region
>>>>>> * fill in the pages that were collected during the pre-dump
>>>>>> * do some other stuff
>>>>>> * register memory region with userfaultfd
>>>>>> * populate the missing memory on demand
>>>>>>
>>>>>> khugepaged collapses the pages in the partially populated regions before we
>>>>>> have a chance to register these regions with userfaultfd, which would
>>>>>> prevent the collapse.
>>>>>>
>>>>>> We could have used MADV_NOHUGEPAGE right after the mmap() call, and then
>>>>>> there would be no race because there would be nothing for khugepaged to
>>>>>> collapse at that point. But the problem is that we have no way to reset
>>>>>> *HUGEPAGE flags after the memory restore is complete.
>>>>>
>>>>> Hmm, I wouldn't be that sure if this is indeed race-free. Check that
>>>>> this scenario is indeed impossible?
>>>>>
>>>>> - you do the mmap
>>>>> - khugepaged will choose the process' mm to scan
>>>>> - khugepaged will get to the vma in question, it doesn't have
>>>>> MADV_NOHUGEPAGE yet
>>>>> - you set MADV_NOHUGEPAGE on the vma
>>>>> - you start populating the vma
>>>>> - khugepaged sees the vma is non-empty, collapses
>>>>>
>>>>> unless I'm wrong, the racers will have mmap_sem for reading only when
>>>>> setting/checking the MADV_NOHUGEPAGE? Might be actually considered a bug.
>>>>>
>>>>> However, can't you use prctl(PR_SET_THP_DISABLE) instead? "If arg2 has a
>>>>> nonzero value, the flag is set, otherwise it is cleared." says the
>>>>> manpage. Do it before the mmap and you avoid the race as well?
>>>>
>>>> Unfortunately, prctl(PR_SET_THP_DISABLE) didn't help :(
>>>> When I've tried to use it, I've ended up with VM_NOHUGEPAGE set on all VMAs
>>>> created after prctl(). This returns me to the state when checkpoint-restore
>>>> alters the application vma->vm_flags although it shouldn't and I do not see
>>>> a way to fix it using existing interfaces.
>>>
>>> [CC linux-api, should have been done in the initial posting already]
>>
>> Sorry, missed that.
>>  
>>> Hm so the prctl does:
>>>
>>>                 if (arg2)
>>>                         me->mm->def_flags |= VM_NOHUGEPAGE;
>>>                 else
>>>                         me->mm->def_flags &= ~VM_NOHUGEPAGE;
>>>
>>> That's rather lazy implementation IMHO. Could we change it so the flag
>>> is stored elsewhere in the mm, and the code that decides to (not) use
>>> THP will check both the per-vma flag and the per-mm flag?
>>
>> I afraid I don't understand how that can help.
>> What we need is an ability to temporarily disable collapse of the pages in
>> VMAs that do not have VM_*HUGEPAGE flags set and that after we re-enable
>> THP, the vma->vm_flags for those VMAs will remain intact.
> 
> Why cannot khugepaged simply skip over all VMAs which have userfault
> regions registered? This would sound like a less error prone approach to
> me.

It already does so. The problem is that there's a race window. We first populate VMA
with pages, then register it in UFFD. Between these two actions khugepaged comes
and generates a huge page out of populated pages and holes. And the holes in question
are not, well, holes -- they should be populated later via the UFFD, while the
generated huge page prevents this from happening.

-- Pavel