Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-08 Thread Michal Hocko
On Fri 08-12-17 01:26:46, David Rientjes wrote: > On Thu, 7 Dec 2017, David Rientjes wrote: > > > I'm backporting and testing the following patch against Linus's tree. To > > clarify an earlier point, we don't actually have any change from upstream > > code that allows for free_pgtables()

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-08 Thread Michal Hocko
On Fri 08-12-17 01:26:46, David Rientjes wrote: > On Thu, 7 Dec 2017, David Rientjes wrote: > > > I'm backporting and testing the following patch against Linus's tree. To > > clarify an earlier point, we don't actually have any change from upstream > > code that allows for free_pgtables()

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-08 Thread Tetsuo Handa
Michal Hocko wrote: > On Fri 08-12-17 00:44:11, Tetsuo Handa wrote: > > Michal Hocko wrote: > > > David, could you test with this patch please? > > > > Even if this patch solved David's case, you need to update > > > > * tsk_is_oom_victim() cannot be set from under us > > * either

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-08 Thread Tetsuo Handa
Michal Hocko wrote: > On Fri 08-12-17 00:44:11, Tetsuo Handa wrote: > > Michal Hocko wrote: > > > David, could you test with this patch please? > > > > Even if this patch solved David's case, you need to update > > > > * tsk_is_oom_victim() cannot be set from under us > > * either

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-08 Thread David Rientjes
On Thu, 7 Dec 2017, David Rientjes wrote: > I'm backporting and testing the following patch against Linus's tree. To > clarify an earlier point, we don't actually have any change from upstream > code that allows for free_pgtables() before the > set_bit(MMF_OOM_SKIP);down_write();up_write()

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-08 Thread David Rientjes
On Thu, 7 Dec 2017, David Rientjes wrote: > I'm backporting and testing the following patch against Linus's tree. To > clarify an earlier point, we don't actually have any change from upstream > code that allows for free_pgtables() before the > set_bit(MMF_OOM_SKIP);down_write();up_write()

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-07 Thread Michal Hocko
On Thu 07-12-17 13:22:30, David Rientjes wrote: [...] > > diff --git a/include/linux/sched/coredump.h b/include/linux/sched/coredump.h > > index 9c8847395b5e..da673ca66e7a 100644 > > --- a/include/linux/sched/coredump.h > > +++ b/include/linux/sched/coredump.h > > @@ -68,8 +68,9 @@ static inline

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-07 Thread Michal Hocko
On Thu 07-12-17 13:22:30, David Rientjes wrote: [...] > > diff --git a/include/linux/sched/coredump.h b/include/linux/sched/coredump.h > > index 9c8847395b5e..da673ca66e7a 100644 > > --- a/include/linux/sched/coredump.h > > +++ b/include/linux/sched/coredump.h > > @@ -68,8 +68,9 @@ static inline

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-07 Thread David Rientjes
On Thu, 7 Dec 2017, Michal Hocko wrote: > yes. I will fold the following in if this turned out to really address > David's issue. But I suspect this will be the case considering the NULL > pmd in the report which would suggest racing with free_pgtable... > I'm backporting and testing the

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-07 Thread David Rientjes
On Thu, 7 Dec 2017, Michal Hocko wrote: > yes. I will fold the following in if this turned out to really address > David's issue. But I suspect this will be the case considering the NULL > pmd in the report which would suggest racing with free_pgtable... > I'm backporting and testing the

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-07 Thread David Rientjes
On Thu, 7 Dec 2017, Michal Hocko wrote: > Very well spotted! It could be any task in fact (e.g. somebody reading > from /proc/ file which requires mm_struct). > > oom_reaperoom_victim task > mmget_not_zero >

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-07 Thread David Rientjes
On Thu, 7 Dec 2017, Michal Hocko wrote: > Very well spotted! It could be any task in fact (e.g. somebody reading > from /proc/ file which requires mm_struct). > > oom_reaperoom_victim task > mmget_not_zero >

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-07 Thread Michal Hocko
On Fri 08-12-17 00:44:11, Tetsuo Handa wrote: > Michal Hocko wrote: > > David, could you test with this patch please? > > Even if this patch solved David's case, you need to update > >* tsk_is_oom_victim() cannot be set from under us >* either because current->mm is already set

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-07 Thread Michal Hocko
On Fri 08-12-17 00:44:11, Tetsuo Handa wrote: > Michal Hocko wrote: > > David, could you test with this patch please? > > Even if this patch solved David's case, you need to update > >* tsk_is_oom_victim() cannot be set from under us >* either because current->mm is already set

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-07 Thread Tetsuo Handa
Michal Hocko wrote: > David, could you test with this patch please? Even if this patch solved David's case, you need to update * tsk_is_oom_victim() cannot be set from under us * either because current->mm is already set to NULL * under task_lock before calling mmput

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-07 Thread Tetsuo Handa
Michal Hocko wrote: > David, could you test with this patch please? Even if this patch solved David's case, you need to update * tsk_is_oom_victim() cannot be set from under us * either because current->mm is already set to NULL * under task_lock before calling mmput

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-07 Thread Michal Hocko
David, could you test with this patch please? --- >From c4a2a58d3856bc1e528f7c1622a59cef58033cb3 Mon Sep 17 00:00:00 2001 From: Michal Hocko Date: Thu, 7 Dec 2017 09:39:03 +0100 Subject: [PATCH] mm, oom_reaper: fix memory corruption David Rientjes has reported the following

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-07 Thread Michal Hocko
David, could you test with this patch please? --- >From c4a2a58d3856bc1e528f7c1622a59cef58033cb3 Mon Sep 17 00:00:00 2001 From: Michal Hocko Date: Thu, 7 Dec 2017 09:39:03 +0100 Subject: [PATCH] mm, oom_reaper: fix memory corruption David Rientjes has reported the following memory corruption

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-07 Thread Tetsuo Handa
Michal Hocko wrote: > Hmm, so you are creating a separate process (from the signal point of > view) and I suspect it is one of those that holds the last reference to > the mm_struct which is released here and it has tsk_oom_victim = F Right. > So we need a more robust test for the oom victim.

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-07 Thread Tetsuo Handa
Michal Hocko wrote: > Hmm, so you are creating a separate process (from the signal point of > view) and I suspect it is one of those that holds the last reference to > the mm_struct which is released here and it has tsk_oom_victim = F Right. > So we need a more robust test for the oom victim.

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-07 Thread Michal Hocko
On Thu 07-12-17 09:28:01, Michal Hocko wrote: [...] > oom_reaperoom_victim task > mmget_not_zero > exit_mmap bleh, this should have been do_exit > mmput > __oom_reap_task_mm

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-07 Thread Michal Hocko
On Thu 07-12-17 09:28:01, Michal Hocko wrote: [...] > oom_reaperoom_victim task > mmget_not_zero > exit_mmap bleh, this should have been do_exit > mmput > __oom_reap_task_mm

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-07 Thread Michal Hocko
On Thu 07-12-17 16:20:47, Tetsuo Handa wrote: [...] > int main(int argc, char *argv[]) > { > int i; > char *stack; > if (fork() || fork() || setsid() == EOF || pipe(pipe_fd)) > _exit(0); > stack = mmap(NULL, STACKSIZE * NUMTHREADS, PROT_WRITE | PROT_READ, >

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-07 Thread Michal Hocko
On Thu 07-12-17 16:20:47, Tetsuo Handa wrote: [...] > int main(int argc, char *argv[]) > { > int i; > char *stack; > if (fork() || fork() || setsid() == EOF || pipe(pipe_fd)) > _exit(0); > stack = mmap(NULL, STACKSIZE * NUMTHREADS, PROT_WRITE | PROT_READ, >

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-06 Thread Tetsuo Handa
David Rientjes wrote: > On Wed, 6 Dec 2017, Tetsuo Handa wrote: > > Also, I don't know what exit_mmap() is doing but I think that there is a > > possibility that the OOM reaper tries to reclaim mlocked pages as soon as > > exit_mmap() cleared VM_LOCKED flag by calling munlock_vma_pages_all(). > >

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-06 Thread Tetsuo Handa
David Rientjes wrote: > On Wed, 6 Dec 2017, Tetsuo Handa wrote: > > Also, I don't know what exit_mmap() is doing but I think that there is a > > possibility that the OOM reaper tries to reclaim mlocked pages as soon as > > exit_mmap() cleared VM_LOCKED flag by calling munlock_vma_pages_all(). > >

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-06 Thread Michal Hocko
On Tue 05-12-17 23:48:21, David Rientjes wrote: [...] > I think this argues to do MMF_REAPING-style behavior at the beginning of > exit_mmap() and avoid reaping all together once we have reached that > point. There are no more users of the mm and we are in the process of > tearing it down, I'm

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-06 Thread Michal Hocko
On Tue 05-12-17 23:48:21, David Rientjes wrote: [...] > I think this argues to do MMF_REAPING-style behavior at the beginning of > exit_mmap() and avoid reaping all together once we have reached that > point. There are no more users of the mm and we are in the process of > tearing it down, I'm

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-06 Thread Michal Hocko
On Wed 06-12-17 12:28:53, Tetsuo Handa wrote: > David Rientjes wrote: > > On Tue, 5 Dec 2017, David Rientjes wrote: > > > > > One way to solve the issue is to have two mm flags: one to indicate the > > > mm > > > is entering unmap_vmas(): set the flag, do down_write(>mmap_sem); > > >

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-06 Thread Michal Hocko
On Wed 06-12-17 12:28:53, Tetsuo Handa wrote: > David Rientjes wrote: > > On Tue, 5 Dec 2017, David Rientjes wrote: > > > > > One way to solve the issue is to have two mm flags: one to indicate the > > > mm > > > is entering unmap_vmas(): set the flag, do down_write(>mmap_sem); > > >

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-06 Thread Michal Hocko
On Tue 05-12-17 18:43:48, David Rientjes wrote: > Hi, > > I'd like to understand the synchronization between the oom_reaper's > unmap_page_range() and exit_mmap(). The latter does not hold > mm->mmap_sem: it's supposed to be the last thread operating on the mm > before it is destroyed. > >

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-06 Thread Michal Hocko
On Tue 05-12-17 18:43:48, David Rientjes wrote: > Hi, > > I'd like to understand the synchronization between the oom_reaper's > unmap_page_range() and exit_mmap(). The latter does not hold > mm->mmap_sem: it's supposed to be the last thread operating on the mm > before it is destroyed. > >

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-05 Thread David Rientjes
On Wed, 6 Dec 2017, Tetsuo Handa wrote: > > > One way to solve the issue is to have two mm flags: one to indicate the > > > mm > > > is entering unmap_vmas(): set the flag, do down_write(>mmap_sem); > > > up_write(>mmap_sem), then unmap_vmas(). The oom reaper needs this > > > flag clear, not

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-05 Thread David Rientjes
On Wed, 6 Dec 2017, Tetsuo Handa wrote: > > > One way to solve the issue is to have two mm flags: one to indicate the > > > mm > > > is entering unmap_vmas(): set the flag, do down_write(>mmap_sem); > > > up_write(>mmap_sem), then unmap_vmas(). The oom reaper needs this > > > flag clear, not

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-05 Thread David Rientjes
On Tue, 5 Dec 2017, David Rientjes wrote: > One way to solve the issue is to have two mm flags: one to indicate the mm > is entering unmap_vmas(): set the flag, do down_write(>mmap_sem); > up_write(>mmap_sem), then unmap_vmas(). The oom reaper needs this > flag clear, not MMF_OOM_SKIP, while

Re: Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-05 Thread David Rientjes
On Tue, 5 Dec 2017, David Rientjes wrote: > One way to solve the issue is to have two mm flags: one to indicate the mm > is entering unmap_vmas(): set the flag, do down_write(>mmap_sem); > up_write(>mmap_sem), then unmap_vmas(). The oom reaper needs this > flag clear, not MMF_OOM_SKIP, while

Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-05 Thread David Rientjes
Hi, I'd like to understand the synchronization between the oom_reaper's unmap_page_range() and exit_mmap(). The latter does not hold mm->mmap_sem: it's supposed to be the last thread operating on the mm before it is destroyed. If unmap_page_range() races with unmap_vmas(), we trivially call

Multiple oom_reaper BUGs: unmap_page_range racing with exit_mmap

2017-12-05 Thread David Rientjes
Hi, I'd like to understand the synchronization between the oom_reaper's unmap_page_range() and exit_mmap(). The latter does not hold mm->mmap_sem: it's supposed to be the last thread operating on the mm before it is destroyed. If unmap_page_range() races with unmap_vmas(), we trivially call