Re: [PATCH v1 3/3] mm: process_mrelease: introduce PROCESS_MRELEASE_REAP_KILL flag

[Minchan Kim <minchan@kernel.org>]

On Mon, Apr 27, 2026 at 01:34:37PM -0700, Suren Baghdasaryan wrote:
> On Tue, Apr 21, 2026 at 4:03 PM Minchan Kim <minchan@kernel.org> wrote:
> >
> > Currently, process_mrelease() requires userspace to send a SIGKILL signal
> > prior to the call. This separation introduces a scheduling race window
> > where the victim task may receive the signal and enter the exit path
> > before the reaper can invoke process_mrelease().
> >
> > When the victim enters the exit path (do_exit -> exit_mm), it clears its
> > task->mm immediately. This causes process_mrelease() to fail with -ESRCH,
> > leaving the actual address space teardown (exit_mmap) to be deferred until
> > the mm's reference count drops to zero. In Android, arbitrary reference counts
> > (e.g., async I/O, reading /proc/<pid>/cmdline, or various other remote
> > VM accesses) frequently delay this teardown indefinitely, defeating the
> > purpose of expedited reclamation.
> >
> > This delay keeps memory pressure high, forcing the system to unnecessarily
> > kill additional innocent background apps before the memory from the first
> > victim is recovered.
> >
> > This patch introduces the PROCESS_MRELEASE_REAP_KILL UAPI flag to support
> > an integrated auto-kill mode. When specified, process_mrelease() directly
> > injects a SIGKILL into the target task.
> >
> > To solve the race condition deterministically, we grab the mm reference
> > via mmget() and set the MMF_UNSTABLE flag *before* sending the SIGKILL.
> > Using mmget() instead of mmgrab() keeps mm_users > 0, preventing the
> > victim from calling exit_mmap() in its own exit path. This ensures that
> > the memory is reclaimed synchronously and deterministically by the reaper
> > in the context of process_mrelease(), avoiding delays caused by
> > non-deterministic scheduling of the victim task.
> >
> > Signed-off-by: Minchan Kim <minchan@kernel.org>
> > ---
> >  include/uapi/linux/mman.h |  4 +++
> >  mm/oom_kill.c             | 56 +++++++++++++++++++++++++++------------
> >  2 files changed, 43 insertions(+), 17 deletions(-)
> >
> > diff --git a/include/uapi/linux/mman.h b/include/uapi/linux/mman.h
> > index e89d00528f2f..4266976b45ad 100644
> > --- a/include/uapi/linux/mman.h
> > +++ b/include/uapi/linux/mman.h
> > @@ -56,4 +56,8 @@ struct cachestat {
> >         __u64 nr_recently_evicted;
> >  };
> >
> > +/* Flags for process_mrelease */
> > +#define PROCESS_MRELEASE_REAP_KILL     (1 << 0)
> > +#define PROCESS_MRELEASE_VALID_FLAGS   (PROCESS_MRELEASE_REAP_KILL)
> > +
> >  #endif /* _UAPI_LINUX_MMAN_H */
> > diff --git a/mm/oom_kill.c b/mm/oom_kill.c
> > index 5c6c95c169ee..730ba0d19b53 100644
> > --- a/mm/oom_kill.c
> > +++ b/mm/oom_kill.c
> > @@ -20,6 +20,7 @@
> >
> >  #include <linux/oom.h>
> >  #include <linux/mm.h>
> > +#include <uapi/linux/mman.h>
> >  #include <linux/err.h>
> >  #include <linux/gfp.h>
> >  #include <linux/sched.h>
> > @@ -850,7 +851,7 @@ bool oom_killer_disable(signed long timeout)
> >         return true;
> >  }
> >
> > -static inline bool __task_will_free_mem(struct task_struct *task)
> > +static inline bool __task_will_free_mem(struct task_struct *task, bool ignore_exit)
> >  {
> >         struct signal_struct *sig = task->signal;
> >
> > @@ -862,6 +863,9 @@ static inline bool __task_will_free_mem(struct task_struct *task)
> >         if (sig->core_state)
> >                 return false;
> >
> > +       if (ignore_exit)
> > +               return true;
> > +
> >         if (sig->flags & SIGNAL_GROUP_EXIT)
> >                 return true;
> >
> > @@ -878,7 +882,7 @@ static inline bool __task_will_free_mem(struct task_struct *task)
> >   * Caller has to make sure that task->mm is stable (hold task_lock or
> >   * it operates on the current).
> >   */
> > -static bool task_will_free_mem(struct task_struct *task)
> > +static bool task_will_free_mem(struct task_struct *task, bool ignore_exit)
> >  {
> >         struct mm_struct *mm = task->mm;
> >         struct task_struct *p;
> > @@ -892,7 +896,7 @@ static bool task_will_free_mem(struct task_struct *task)
> >         if (!mm)
> >                 return false;
> >
> > -       if (!__task_will_free_mem(task))
> > +       if (!__task_will_free_mem(task, ignore_exit))
> >                 return false;
> >
> >         /*
> > @@ -916,7 +920,7 @@ static bool task_will_free_mem(struct task_struct *task)
> >                         continue;
> >                 if (same_thread_group(task, p))
> >                         continue;
> > -               ret = __task_will_free_mem(p);
> > +               ret = __task_will_free_mem(p, false);
> >                 if (!ret)
> >                         break;
> >         }
> > @@ -1034,7 +1038,7 @@ static void oom_kill_process(struct oom_control *oc, const char *message)
> >          * so it can die quickly
> >          */
> >         task_lock(victim);
> > -       if (task_will_free_mem(victim)) {
> > +       if (task_will_free_mem(victim, false)) {
> >                 mark_oom_victim(victim);
> >                 queue_oom_reaper(victim);
> >                 task_unlock(victim);
> > @@ -1135,7 +1139,7 @@ bool out_of_memory(struct oom_control *oc)
> >          * select it.  The goal is to allow it to allocate so that it may
> >          * quickly exit and free its memory.
> >          */
> > -       if (task_will_free_mem(current)) {
> > +       if (task_will_free_mem(current, false)) {
> >                 mark_oom_victim(current);
> >                 queue_oom_reaper(current);
> >                 return true;
> > @@ -1217,8 +1221,9 @@ SYSCALL_DEFINE2(process_mrelease, int, pidfd, unsigned int, flags)
> >         unsigned int f_flags;
> >         bool reap = false;
> >         long ret = 0;
> > +       bool reap_kill;
> >
> > -       if (flags)
> > +       if (flags & ~PROCESS_MRELEASE_VALID_FLAGS)
> >                 return -EINVAL;
> >
> >         task = pidfd_get_task(pidfd, &f_flags);
> > @@ -1236,19 +1241,33 @@ SYSCALL_DEFINE2(process_mrelease, int, pidfd, unsigned int, flags)
> >         }
> >
> >         mm = p->mm;
> > -       mmgrab(mm);
> >
> > -       if (task_will_free_mem(p))
> > -               reap = true;
> > -       else {
> > -               /* Error only if the work has not been done already */
> > -               if (!mm_flags_test(MMF_OOM_SKIP, mm))
> > +       reap_kill = !!(flags & PROCESS_MRELEASE_REAP_KILL);
> > +       reap = task_will_free_mem(p, reap_kill);
> > +       if (!reap) {
> > +               if (reap_kill || !mm_flags_test(MMF_OOM_SKIP, mm))
> >                         ret = -EINVAL;
> > +
> > +               task_unlock(p);
> > +               goto put_task;
> >         }
> > -       task_unlock(p);
> >
> > -       if (!reap)
> > -               goto drop_mm;
> > +       if (reap_kill) {
> > +               /*
> > +                * We use mmget() instead of mmgrab() to keep mm_users > 0,
> > +                * preventing the victim from calling exit_mmap() in its
> > +                * own exit path. This ensures that the memory is reclaimed
> > +                * synchronously and deterministically by the reaper.
> > +                */
> > +               mmget(mm);
> 
> I don't quite understand why you need to mmget() and prevent the
> victim from calling exit_mmap() here. As long as we successfully
> mmgrab'ed the mm, we can safely proceed with mmap locking and doing
> __oom_reap_task_mm(). Victim can execute exit_mmap() in parallel with
> __oom_reap_task_mm(). In fact that's even desirable! I remember a
> recent patch that used mas_for_each_rev() in __oom_reap_task_mm() to
> reclaim memory in reverse order so that exit_mmap() and
> __oom_reap_task_mm() can start from opposite ends of the address space
> and converge in the middle, working concurrently.

Good information. Will change it in next respin.