From: Waiman Long <longman@redhat.com>
To: Joel Savitz <jsavitz@redhat.com>, Michal Hocko <mhocko@suse.com>
Cc: "Andrew Morton" <akpm@linux-foundation.org>,
linux-kernel <linux-kernel@vger.kernel.org>,
linux-mm@kvack.org, "Nico Pache" <npache@redhat.com>,
"Peter Zijlstra" <peterz@infradead.org>,
"Thomas Gleixner" <tglx@linutronix.de>,
"Ingo Molnar" <mingo@redhat.com>,
"Darren Hart" <dvhart@infradead.org>,
"Davidlohr Bueso" <dave@stgolabs.net>,
"André Almeida" <andrealmeid@collabora.com>
Subject: Re: [PATCH] mm/oom_kill: wake futex waiters before annihilating victim shared mutex
Date: Fri, 14 Jan 2022 09:55:10 -0500 [thread overview]
Message-ID: <eee96817-1814-5849-65b8-0038235f2617@redhat.com> (raw)
In-Reply-To: <CAL1p7m7mWxLE-7Qf_QjmREJ2AvfSexPvybPyHvxTUugxsPPxjQ@mail.gmail.com>
On 1/14/22 09:39, Joel Savitz wrote:
>> What has happened to the oom victim and why it has never exited?
> What appears to happen is that the oom victim is sent SIGKILL by the
> process that triggers the oom while also being marked as an oom
> victim.
>
> As you mention in your patchset introducing the oom reaper in commit
> aac4536355496 ("mm, oom: introduce oom reaper"), the purpose the the
> oom reaper is to try and free more memory more quickly than it
> otherwise would have been by assuming anonymous or swapped out pages
> won't be needed in the exit path as the owner is already dying.
> However, this assumption is violated by the futex_cleanup() path,
> which needs access to userspace in fetch_robust_entry() when it is
> called in exit_robust_list(). Trace_printk()s in this failure path
> reveal an apparent race between the oom reaper thread reaping the
> victim's mm and the futex_cleanup() path. There may be other ways that
> this race manifests but we have been most consistently able to trace
> that one.
>
> Since in the case of an oom victim using robust futexes the core
> assumption of the oom reaper is violated, we propose to solve this
> problem by either canceling or delaying the waking of the oom reaper
> thread by wake_oom_reaper in the case that tsk->robust_list is
> non-NULL.
>
> e.g. the bug does not reproduce with this patch (from npache@redhat.com):
>
> diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> index 989f35a2bbb1..b8c518fdcf4d 100644
> --- a/mm/oom_kill.c
> +++ b/mm/oom_kill.c
> @@ -665,6 +665,19 @@ static void wake_oom_reaper(struct task_struct *tsk)
> if (test_and_set_bit(MMF_OOM_REAP_QUEUED, &tsk->signal->oom_mm->flags))
> return;
>
> +#ifdef CONFIG_FUTEX
> + /*
> + * don't wake the oom_reaper thread if we still have a robust
> list to handle
> + * This will then rely on the sigkill to handle the cleanup of memory
> + */
> + if(tsk->robust_list)
> + return;
> +#ifdef CONFIG_COMPAT
> + if(tsk->compat_robust_list)
> + return;
> +#endif
> +#endif
> +
> get_task_struct(tsk);
>
> spin_lock(&oom_reaper_lock);
OK, that can explain why the robust futex is not properly cleaned up.
Could you post a more formal v2 patch with description about the
possible race condition?
Cheers,
Longman
next prev parent reply other threads:[~2022-01-14 14:55 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-12-07 21:49 [PATCH] mm/oom_kill: wake futex waiters before annihilating victim shared mutex Joel Savitz
2021-12-07 22:34 ` Joel Savitz
2021-12-07 23:47 ` Andrew Morton
2021-12-08 0:46 ` Nico Pache
2021-12-08 1:58 ` Andrew Morton
2021-12-08 3:38 ` Joel Savitz
2021-12-08 9:01 ` Michal Hocko
2021-12-08 16:05 ` Michal Hocko
2021-12-09 2:59 ` Joel Savitz
2021-12-09 7:51 ` Michal Hocko
2022-01-14 14:39 ` Joel Savitz
2022-01-14 14:55 ` Waiman Long [this message]
2022-01-14 14:58 ` Waiman Long
2022-01-17 11:33 ` Michal Hocko
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=eee96817-1814-5849-65b8-0038235f2617@redhat.com \
--to=longman@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=andrealmeid@collabora.com \
--cc=dave@stgolabs.net \
--cc=dvhart@infradead.org \
--cc=jsavitz@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@suse.com \
--cc=mingo@redhat.com \
--cc=npache@redhat.com \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).