On Tue 25-09-18 15:43:22, Rong Chen wrote:
>
>
> On 09/25/2018 03:31 PM, Michal Hocko wrote:
> > On Tue 25-09-18 15:00:15, Rong Chen wrote:
> > >
> > > On 09/25/2018 02:06 PM, Michal Hocko wrote:
> > > > On Tue 25-09-18 13:48:20, kernel test robot wrote:
> > > > > FYI, we noticed the following commit (built with gcc-7):
> > > > >
> > > > > commit: 6209f6fc62835d84c2a92d237588a114e39436ce ("mm, oom: rework
> > > > > mmap_exit vs. oom_reaper synchronization")
> > > > > https://github.com/0day-ci/linux
> > > > > UPDATE-20180911-024633/Tetsuo-Handa/mm-oom-Fix-unnecessary-killing-of-additional-processes/20180910-163452
> > > > Do you have a msg-id to the patch that has been tested?
> > > message_id: 20180910125513.311-2-mho...@kernel.org
> > Thanks! It woudl be really great if this was a part of the report when
> > testing patches which are not mainline yet.
> >
> > This patch resulting in a crash is quite surprising. The patch is RFC
> > and not tested yet but I will definitely have a look. Could you help me
> > some more and give faddr2line __oom_reap_task_mm+0x40 please?
> $ faddr2line ./vmlinux __oom_reap_task_mm+0x40
> __oom_reap_task_mm+0x40/0x175:
> can_madv_dontneed_vma at mm/internal.h:48
> (inlined by) __oom_reap_task_mm at mm/oom_kill.c:505
OK, this suggests the vma is gone. The problem is that I have removed
MMF_OOM_SKIP from the exit path and mm->mmap still points to the freed
memory. Later patch in the series does the later so the resulting code
is safe but not bisect safe. I will return MMF_OOM_SKIP setting to the
exit_mmap path and that should cure the issue.
diff --git a/mm/mmap.c b/mm/mmap.c
index 3481424717ac..a02b314c0546 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -3083,8 +3083,10 @@ void exit_mmap(struct mm_struct *mm)
unmap_vmas(&tlb, vma, 0, -1);
/* oom_reaper cannot race with the page tables teardown */
- if (oom)
+ if (oom) {
down_write(&mm->mmap_sem);
+ set_bit(MMF_OOM_SKIP, &mm->flags);
+ }
free_pgtables(&tlb, vma, FIRST_USER_ADDRESS, USER_PGTABLES_CEILING);
tlb_finish_mmu(&tlb, 0, -1);
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index f10aa5360616..b7137ca9c7a2 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -554,7 +554,7 @@ static bool oom_reap_task_mm(struct task_struct *tsk,
struct mm_struct *mm)
* MMF_OOM_SKIP is set by exit_mmap when the OOM reaper can't
* work on the mm anymore. The check for MMF_OOM_SKIP must run
* under mmap_sem for reading because it serializes against the
- * down_write();up_write() cycle in exit_mmap().
+ * down_write() in exit_mmap().
*/
if (test_bit(MMF_OOM_SKIP, &mm->flags)) {
trace_skip_task_reaping(tsk->pid);
--
Michal Hocko
SUSE Labs
