On Wed, 18 Apr 2018, Tetsuo Handa wrote: > > Since exit_mmap() is done without the protection of mm->mmap_sem, it is > > possible for the oom reaper to concurrently operate on an mm until > > MMF_OOM_SKIP is set. > > > > This allows munlock_vma_pages_all() to concurrently run while the oom > > reaper is operating on a vma. Since munlock_vma_pages_range() depends on > > clearing VM_LOCKED from vm_flags before actually doing the munlock to > > determine if any other vmas are locking the same memory, the check for > > VM_LOCKED in the oom reaper is racy. > > > > This is especially noticeable on architectures such as powerpc where > > clearing a huge pmd requires kick_all_cpus_sync(). If the pmd is zapped > > by the oom reaper during follow_page_mask() after the check for pmd_none() > > is bypassed, this ends up deferencing a NULL ptl. > > I don't know whether the explanation above is correct. > Did you actually see a crash caused by this race? > Yes, it's trivially reproducible on power by simply mlocking a ton of memory and triggering oom kill. > > Fix this by reusing MMF_UNSTABLE to specify that an mm should not be > > reaped. This prevents the concurrent munlock_vma_pages_range() and > > unmap_page_range(). The oom reaper will simply not operate on an mm that > > has the bit set and leave the unmapping to exit_mmap(). > > But this patch is setting MMF_OOM_SKIP without reaping any memory as soon as > MMF_UNSTABLE is set, which is the situation described in 212925802454: > Oh, you're referring to __oom_reap_task_mm() returning true because of MMF_UNSTABLE and then setting MMF_OOM_SKIP itself? Yes, that is dumb. We could change __oom_reap_task_mm() to only set MMF_OOM_SKIP if MMF_UNSTABLE hasn't been set. I'll send a v2, which I needed to do anyway to do s/kick_all_cpus_sync/serialize_against_pte_lookup/ in the changelog (power only does it for the needed cpus).
