* [PATCH v2] mm/oom_kill: wake futex waiters before annihilating victim shared mutex @ 2021-12-08 18:17 Joel Savitz 2021-12-15 15:47 ` Qian Cai 0 siblings, 1 reply; 3+ messages in thread From: Joel Savitz @ 2021-12-08 18:17 UTC (permalink / raw) To: linux-kernel Cc: Joel Savitz, Waiman Long, linux-mm, Nico Pache, Peter Zijlstra, Thomas Gleixner, Ingo Molnar, Darren Hart, Davidlohr Bueso, André Almeida, Andrew Morton, Michal Hocko In the case that two or more processes share a futex located within a shared mmaped region, such as a process that shares a lock between itself and a number of child processes, we have observed that when a process holding the lock is oom killed, at least one waiter is never alerted to this new development and simply continues to wait. This is visible via pthreads by checking the __owner field of the pthread_mutex_t structure within a waiting process, perhaps with gdb. We identify reproduction of this issue by checking a waiting process of a test program and viewing the contents of the pthread_mutex_t, taking note of the value in the owner field, and then checking dmesg to see if the owner has already been killed. This issue can be tricky to reproduce, but with the modifications of this small patch, I have found it to be impossible to reproduce. There may be additional considerations that I have not taken into account in this patch and I welcome any comments and criticism. Changes from v1: - add comments before calls to futex_exit_release() Co-developed-by: Nico Pache <npache@redhat.com> Signed-off-by: Nico Pache <npache@redhat.com> Signed-off-by: Joel Savitz <jsavitz@redhat.com> --- mm/oom_kill.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/mm/oom_kill.c b/mm/oom_kill.c index 1ddabefcfb5a..884a5f15fd06 100644 --- a/mm/oom_kill.c +++ b/mm/oom_kill.c @@ -44,6 +44,7 @@ #include <linux/kthread.h> #include <linux/init.h> #include <linux/mmu_notifier.h> +#include <linux/futex.h> #include <asm/tlb.h> #include "internal.h" @@ -885,6 +886,11 @@ static void __oom_kill_process(struct task_struct *victim, const char *message) count_vm_event(OOM_KILL); memcg_memory_event_mm(mm, MEMCG_OOM_KILL); + /* + * We call futex_exit_release() on the victim task to ensure any waiters on any + * process-shared futexes held by the victim task are woken up. + */ + futex_exit_release(victim); /* * We should send SIGKILL before granting access to memory reserves * in order to prevent the OOM victim from depleting the memory @@ -930,6 +936,12 @@ static void __oom_kill_process(struct task_struct *victim, const char *message) */ if (unlikely(p->flags & PF_KTHREAD)) continue; + /* + * We call futex_exit_release() on any task p sharing the + * victim->mm to ensure any waiters on any + * process-shared futexes held by task p are woken up. + */ + futex_exit_release(p); do_send_sig_info(SIGKILL, SEND_SIG_PRIV, p, PIDTYPE_TGID); } rcu_read_unlock(); -- 2.27.0 ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] mm/oom_kill: wake futex waiters before annihilating victim shared mutex 2021-12-08 18:17 [PATCH v2] mm/oom_kill: wake futex waiters before annihilating victim shared mutex Joel Savitz @ 2021-12-15 15:47 ` Qian Cai 2021-12-15 15:53 ` Joel Savitz 0 siblings, 1 reply; 3+ messages in thread From: Qian Cai @ 2021-12-15 15:47 UTC (permalink / raw) To: Joel Savitz Cc: linux-kernel, Waiman Long, linux-mm, Nico Pache, Peter Zijlstra, Thomas Gleixner, Ingo Molnar, Darren Hart, Davidlohr Bueso, André Almeida, Andrew Morton, Michal Hocko On Wed, Dec 08, 2021 at 01:17:14PM -0500, Joel Savitz wrote: > In the case that two or more processes share a futex located within > a shared mmaped region, such as a process that shares a lock between > itself and a number of child processes, we have observed that when > a process holding the lock is oom killed, at least one waiter is never > alerted to this new development and simply continues to wait. > > This is visible via pthreads by checking the __owner field of the > pthread_mutex_t structure within a waiting process, perhaps with gdb. > > We identify reproduction of this issue by checking a waiting process of > a test program and viewing the contents of the pthread_mutex_t, taking note > of the value in the owner field, and then checking dmesg to see if the > owner has already been killed. > > This issue can be tricky to reproduce, but with the modifications of > this small patch, I have found it to be impossible to reproduce. There > may be additional considerations that I have not taken into account in > this patch and I welcome any comments and criticism. > > Changes from v1: > - add comments before calls to futex_exit_release() > > Co-developed-by: Nico Pache <npache@redhat.com> > Signed-off-by: Nico Pache <npache@redhat.com> > Signed-off-by: Joel Savitz <jsavitz@redhat.com> I am afraid we can't call futex_exit_release() under rcu_read_lock() because it might sleep. BUG: sleeping function called from invalid context at kernel/locking/mutex.c:577 in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 1509, name: lsbug preempt_count: 1, expected: 0 3 locks held by lsbug/1509: #0: ffff00004de99c98 (&mm->mmap_lock){++++}-{3:3}, at: do_page_fault #1: ffff800010fd8308 (oom_lock){+.+.}-{3:3}, at: __alloc_pages_slowpath.constprop.0 __alloc_pages_may_oom at /usr/src/linux-next/mm/page_alloc.c:4278 (inlined by) __alloc_pages_slowpath at /usr/src/linux-next/mm/page_alloc.c:5058 #2: ffff000867b3b0c0 (&p->alloc_lock){+.+.}-{2:2}, at: find_lock_task_mm find_lock_task_mm at /usr/src/linux-next/mm/oom_kill.c:145 CPU: 5 PID: 1509 Comm: lsbug Not tainted 5.16.0-rc5-next-20211214+ #172 Hardware name: MiTAC RAPTOR EV-883832-X3-0001/RAPTOR, BIOS 1.6 06/28/2020 Call trace: dump_backtrace show_stack dump_stack_lvl dump_stack __might_resched __might_sleep __mutex_lock mutex_lock_nested futex_cleanup_begin futex_cleanup_begin at /usr/src/linux-next/kernel/futex/core.c:1071 futex_exit_release __oom_kill_process oom_kill_process out_of_memory __alloc_pages_slowpath.constprop.0 __alloc_pages alloc_pages_vma alloc_zeroed_user_highpage_movable do_anonymous_page __handle_mm_fault handle_mm_fault do_page_fault do_translation_fault do_mem_abort el0_da el0t_64_sync_handler el0t_64_sync ============================= [ BUG: Invalid wait context ] 5.16.0-rc5-next-20211214+ #172 Tainted: G W ----------------------------- lsbug/1509 is trying to lock: ffff000867b3ba98 (&tsk->futex_exit_mutex){+.+.}-{3:3}, at: futex_cleanup_begin other info that might help us debug this: context-{4:4} 3 locks held by lsbug/1509: #0: ffff00004de99c98 (&mm->mmap_lock){++++}-{3:3}, at: do_page_fault #1: ffff800010fd8308 (oom_lock){+.+.}-{3:3}, at: __alloc_pages_slowpath.constprop.0 #2: ffff000867b3b0c0 (&p->alloc_lock){+.+.}-{2:2}, at: find_lock_task_mm stack backtrace: CPU: 5 PID: 1509 Comm: lsbug Tainted: G W 5.16.0-rc5-next-20211214+ #172 Hardware name: MiTAC RAPTOR EV-883832-X3-0001/RAPTOR, BIOS 1.6 06/28/2020 Call trace: dump_backtrace show_stack dump_stack_lvl dump_stack __lock_acquire lock_acquire __mutex_lock mutex_lock_nested futex_cleanup_begin futex_exit_release __oom_kill_process oom_kill_process out_of_memory __alloc_pages_slowpath.constprop.0 __alloc_pages alloc_pages_vma alloc_zeroed_user_highpage_movable do_anonymous_page __handle_mm_fault handle_mm_fault do_page_fault do_translation_fault do_mem_abort el0_da el0t_64_sync_handler el0t_64_sync > --- > mm/oom_kill.c | 12 ++++++++++++ > 1 file changed, 12 insertions(+) > > diff --git a/mm/oom_kill.c b/mm/oom_kill.c > index 1ddabefcfb5a..884a5f15fd06 100644 > --- a/mm/oom_kill.c > +++ b/mm/oom_kill.c > @@ -44,6 +44,7 @@ > #include <linux/kthread.h> > #include <linux/init.h> > #include <linux/mmu_notifier.h> > +#include <linux/futex.h> > > #include <asm/tlb.h> > #include "internal.h" > @@ -885,6 +886,11 @@ static void __oom_kill_process(struct task_struct *victim, const char *message) > count_vm_event(OOM_KILL); > memcg_memory_event_mm(mm, MEMCG_OOM_KILL); > > + /* > + * We call futex_exit_release() on the victim task to ensure any waiters on any > + * process-shared futexes held by the victim task are woken up. > + */ > + futex_exit_release(victim); > /* > * We should send SIGKILL before granting access to memory reserves > * in order to prevent the OOM victim from depleting the memory > @@ -930,6 +936,12 @@ static void __oom_kill_process(struct task_struct *victim, const char *message) > */ > if (unlikely(p->flags & PF_KTHREAD)) > continue; > + /* > + * We call futex_exit_release() on any task p sharing the > + * victim->mm to ensure any waiters on any > + * process-shared futexes held by task p are woken up. > + */ > + futex_exit_release(p); > do_send_sig_info(SIGKILL, SEND_SIG_PRIV, p, PIDTYPE_TGID); > } > rcu_read_unlock(); > -- > 2.27.0 > ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] mm/oom_kill: wake futex waiters before annihilating victim shared mutex 2021-12-15 15:47 ` Qian Cai @ 2021-12-15 15:53 ` Joel Savitz 0 siblings, 0 replies; 3+ messages in thread From: Joel Savitz @ 2021-12-15 15:53 UTC (permalink / raw) To: Qian Cai Cc: linux-kernel, Waiman Long, linux-mm, Nico Pache, Peter Zijlstra, Thomas Gleixner, Ingo Molnar, Darren Hart, Davidlohr Bueso, André Almeida, Andrew Morton, Michal Hocko > I am afraid we can't call futex_exit_release() under rcu_read_lock() > because it might sleep. Ah that's too bad. Is there an equivalent atomic call suitable for this purpose? > > BUG: sleeping function called from invalid context at kernel/locking/mutex.c:577 > in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 1509, name: lsbug > preempt_count: 1, expected: 0 > 3 locks held by lsbug/1509: > #0: ffff00004de99c98 (&mm->mmap_lock){++++}-{3:3}, at: do_page_fault > #1: ffff800010fd8308 (oom_lock){+.+.}-{3:3}, at: __alloc_pages_slowpath.constprop.0 > __alloc_pages_may_oom at /usr/src/linux-next/mm/page_alloc.c:4278 > (inlined by) __alloc_pages_slowpath at /usr/src/linux-next/mm/page_alloc.c:5058 > #2: ffff000867b3b0c0 (&p->alloc_lock){+.+.}-{2:2}, at: find_lock_task_mm > find_lock_task_mm at /usr/src/linux-next/mm/oom_kill.c:145 > CPU: 5 PID: 1509 Comm: lsbug Not tainted 5.16.0-rc5-next-20211214+ #172 > Hardware name: MiTAC RAPTOR EV-883832-X3-0001/RAPTOR, BIOS 1.6 06/28/2020 > Call trace: > dump_backtrace > show_stack > dump_stack_lvl > dump_stack > __might_resched > __might_sleep > __mutex_lock > mutex_lock_nested > futex_cleanup_begin > futex_cleanup_begin at /usr/src/linux-next/kernel/futex/core.c:1071 > futex_exit_release > __oom_kill_process > oom_kill_process > out_of_memory > __alloc_pages_slowpath.constprop.0 > __alloc_pages > alloc_pages_vma > alloc_zeroed_user_highpage_movable > do_anonymous_page > __handle_mm_fault > handle_mm_fault > do_page_fault > do_translation_fault > do_mem_abort > el0_da > el0t_64_sync_handler > el0t_64_sync > ============================= > [ BUG: Invalid wait context ] > 5.16.0-rc5-next-20211214+ #172 Tainted: G W > ----------------------------- > lsbug/1509 is trying to lock: > ffff000867b3ba98 (&tsk->futex_exit_mutex){+.+.}-{3:3}, at: futex_cleanup_begin > other info that might help us debug this: > context-{4:4} > 3 locks held by lsbug/1509: > #0: ffff00004de99c98 (&mm->mmap_lock){++++}-{3:3}, at: do_page_fault > #1: ffff800010fd8308 (oom_lock){+.+.}-{3:3}, at: __alloc_pages_slowpath.constprop.0 > #2: ffff000867b3b0c0 (&p->alloc_lock){+.+.}-{2:2}, at: find_lock_task_mm > stack backtrace: > CPU: 5 PID: 1509 Comm: lsbug Tainted: G W 5.16.0-rc5-next-20211214+ #172 > Hardware name: MiTAC RAPTOR EV-883832-X3-0001/RAPTOR, BIOS 1.6 06/28/2020 > Call trace: > dump_backtrace > show_stack > dump_stack_lvl > dump_stack > __lock_acquire > lock_acquire > __mutex_lock > mutex_lock_nested > futex_cleanup_begin > futex_exit_release > __oom_kill_process > oom_kill_process > out_of_memory > __alloc_pages_slowpath.constprop.0 > __alloc_pages > alloc_pages_vma > alloc_zeroed_user_highpage_movable > do_anonymous_page > __handle_mm_fault > handle_mm_fault > do_page_fault > do_translation_fault > do_mem_abort > el0_da > el0t_64_sync_handler > el0t_64_sync > > > --- > > mm/oom_kill.c | 12 ++++++++++++ > > 1 file changed, 12 insertions(+) > > > > diff --git a/mm/oom_kill.c b/mm/oom_kill.c > > index 1ddabefcfb5a..884a5f15fd06 100644 > > --- a/mm/oom_kill.c > > +++ b/mm/oom_kill.c > > @@ -44,6 +44,7 @@ > > #include <linux/kthread.h> > > #include <linux/init.h> > > #include <linux/mmu_notifier.h> > > +#include <linux/futex.h> > > > > #include <asm/tlb.h> > > #include "internal.h" > > @@ -885,6 +886,11 @@ static void __oom_kill_process(struct task_struct *victim, const char *message) > > count_vm_event(OOM_KILL); > > memcg_memory_event_mm(mm, MEMCG_OOM_KILL); > > > > + /* > > + * We call futex_exit_release() on the victim task to ensure any waiters on any > > + * process-shared futexes held by the victim task are woken up. > > + */ > > + futex_exit_release(victim); > > /* > > * We should send SIGKILL before granting access to memory reserves > > * in order to prevent the OOM victim from depleting the memory > > @@ -930,6 +936,12 @@ static void __oom_kill_process(struct task_struct *victim, const char *message) > > */ > > if (unlikely(p->flags & PF_KTHREAD)) > > continue; > > + /* > > + * We call futex_exit_release() on any task p sharing the > > + * victim->mm to ensure any waiters on any > > + * process-shared futexes held by task p are woken up. > > + */ > > + futex_exit_release(p); > > do_send_sig_info(SIGKILL, SEND_SIG_PRIV, p, PIDTYPE_TGID); > > } > > rcu_read_unlock(); > > -- > > 2.27.0 > > > Best, Joel Savitz ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-12-15 16:18 UTC | newest] Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-08 18:17 [PATCH v2] mm/oom_kill: wake futex waiters before annihilating victim shared mutex Joel Savitz 2021-12-15 15:47 ` Qian Cai 2021-12-15 15:53 ` Joel Savitz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox