On 2018/09/13 20:35, Michal Hocko wrote:
>> Next question.
>>
>> /* Use -1 here to ensure all VMAs in the mm are unmapped */
>> unmap_vmas(&tlb, vma, 0, -1);
>>
>> in exit_mmap() will now race with the OOM reaper. And unmap_vmas() will handle
>> VM_HUGETLB or VM_PFNMAP or VM_SHARED or !vma_is_anonymous() vmas, won't it?
>> Then, is it guaranteed to be safe if the OOM reaper raced with unmap_vmas() ?
>
> I do not understand the question. unmap_vmas is basically MADV_DONTNEED
> and that doesn't require the exclusive mmap_sem lock so yes it should be
> safe those two to race (modulo bugs of course but I am not aware of any
> there).
>
You need to verify that races we observed with VM_LOCKED can't happen
for VM_HUGETLB / VM_PFNMAP / VM_SHARED / !vma_is_anonymous() cases.
for (vma = mm->mmap; vma; vma = vma->vm_next) {
if (!(vma->vm_flags & VM_LOCKED))
continue;
/*
* oom_reaper cannot handle mlocked vmas but we
* need to serialize it with munlock_vma_pages_all
* which clears VM_LOCKED, otherwise the oom reaper
* cannot reliably test it.
*/
if (oom)
down_write(&mm->mmap_sem);
munlock_vma_pages_all(vma);
if (oom)
up_write(&mm->mmap_sem);
}
Without enough comments, future changes might overlook the assumption.
