Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755019AbdGJXz0 (ORCPT ); Mon, 10 Jul 2017 19:55:26 -0400 Received: from mail-pg0-f53.google.com ([74.125.83.53]:34025 "EHLO mail-pg0-f53.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754956AbdGJXzY (ORCPT ); Mon, 10 Jul 2017 19:55:24 -0400 Date: Mon, 10 Jul 2017 16:55:22 -0700 (PDT) From: David Rientjes X-X-Sender: rientjes@chino.kir.corp.google.com To: Michal Hocko cc: linux-mm@kvack.org, Tetsuo Handa , Oleg Nesterov , Andrea Argangeli , Andrew Morton , LKML , Michal Hocko Subject: Re: [RFC PATCH] mm, oom: allow oom reaper to race with exit_mmap In-Reply-To: <20170626130346.26314-1-mhocko@kernel.org> Message-ID: References: <20170626130346.26314-1-mhocko@kernel.org> User-Agent: Alpine 2.10 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2163 Lines: 70 On Mon, 26 Jun 2017, Michal Hocko wrote: > diff --git a/mm/mmap.c b/mm/mmap.c > index 3bd5ecd20d4d..253808e716dc 100644 > --- a/mm/mmap.c > +++ b/mm/mmap.c > @@ -2962,6 +2962,11 @@ void exit_mmap(struct mm_struct *mm) > /* Use -1 here to ensure all VMAs in the mm are unmapped */ > unmap_vmas(&tlb, vma, 0, -1); > > + /* > + * oom reaper might race with exit_mmap so make sure we won't free > + * page tables or unmap VMAs under its feet > + */ > + down_write(&mm->mmap_sem); > free_pgtables(&tlb, vma, FIRST_USER_ADDRESS, USER_PGTABLES_CEILING); > tlb_finish_mmu(&tlb, 0, -1); > > @@ -2974,7 +2979,9 @@ void exit_mmap(struct mm_struct *mm) > nr_accounted += vma_pages(vma); > vma = remove_vma(vma); > } > + mm->mmap = NULL; > vm_unacct_memory(nr_accounted); > + up_write(&mm->mmap_sem); > } > > /* Insert vm structure into process list sorted by address > diff --git a/mm/oom_kill.c b/mm/oom_kill.c > index 0e2c925e7826..5dc0ff22d567 100644 > --- a/mm/oom_kill.c > +++ b/mm/oom_kill.c > @@ -472,36 +472,8 @@ static bool __oom_reap_task_mm(struct task_struct *tsk, struct mm_struct *mm) > struct vm_area_struct *vma; > bool ret = true; > > - /* > - * We have to make sure to not race with the victim exit path > - * and cause premature new oom victim selection: > - * __oom_reap_task_mm exit_mm > - * mmget_not_zero > - * mmput > - * atomic_dec_and_test > - * exit_oom_victim > - * [...] > - * out_of_memory > - * select_bad_process > - * # no TIF_MEMDIE task selects new victim > - * unmap_page_range # frees some memory > - */ > - mutex_lock(&oom_lock); > - > - if (!down_read_trylock(&mm->mmap_sem)) { > - ret = false; > - goto unlock_oom; > - } > - > - /* > - * increase mm_users only after we know we will reap something so > - * that the mmput_async is called only when we have reaped something > - * and delayed __mmput doesn't matter that much > - */ > - if (!mmget_not_zero(mm)) { > - up_read(&mm->mmap_sem); > - goto unlock_oom; > - } > + if (!down_read_trylock(&mm->mmap_sem)) > + return false; I think this should return true if mm->mmap == NULL here.