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@xxxxxxxxxx>
> Signed-off-by: Nico Pache <npache@xxxxxxxxxx>
> Signed-off-by: Joel Savitz <jsavitz@xxxxxxxxxx>
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
>
