On Thu 13-09-18 20:20:12, Tetsuo Handa wrote:
> On 2018/09/13 18:09, Michal Hocko wrote:
> >> This is bad because architectures where hugetlb_free_pgd_range() does
> >> more than free_pgd_range() need to check VM_HUGETLB flag for each "vma".
> >> Thus, I think we need to keep the iteration.
> >
> > Fair point. I have looked more closely and most of them simply redirect
> > to free_pgd_range but ppc and sparc are doing some pretty involved
> > tricks which we cannot really skip. So I will go and split
> > free_pgtables into two phases and keep per vma loops. So this
> > incremental update on top
>
> 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).
> By the way, there is a potential bug in hugepd_free() in arch/powerpc/mm/hugetlbpage.c
>
> if (*batchp == NULL) {
> *batchp = (struct hugepd_freelist *)__get_free_page(GFP_ATOMIC);
> (*batchp)->index = 0;
> }
>
> because GFP_ATOMIC allocation might fail if ALLOC_OOM allocations are in progress?
I am not familiar with that code so I would recommend to ask
maintainers.
--
Michal Hocko
SUSE Labs
