[PATCH] mm/oom: Android oomhelper

[Peter Enderborg]

This is proff of concept of a pre-oom-killer that kill task
strictly on oom-score-adj order if the score is positive.

It act as lifeline when userspace does not have optimal performance.
---
 drivers/staging/Makefile              |  1 +
 drivers/staging/oomhelper/Makefile    |  2 +
 drivers/staging/oomhelper/oomhelper.c | 65 +++++++++++++++++++++++++++
 mm/oom_kill.c                         |  4 +-
 4 files changed, 70 insertions(+), 2 deletions(-)
 create mode 100644 drivers/staging/oomhelper/Makefile
 create mode 100644 drivers/staging/oomhelper/oomhelper.c

diff --git a/drivers/staging/Makefile b/drivers/staging/Makefile
index 2245059e69c7..4a5449b42568 100644
--- a/drivers/staging/Makefile
+++ b/drivers/staging/Makefile
@@ -47,3 +47,4 @@ obj-$(CONFIG_QLGE)Â Â Â  Â Â Â  += qlge/
 obj-$(CONFIG_WIMAX)        += wimax/
 obj-$(CONFIG_WFX)        += wfx/
 obj-y                += hikey9xx/
+obj-y                += oomhelper/
diff --git a/drivers/staging/oomhelper/Makefile b/drivers/staging/oomhelper/Makefile
new file mode 100644
index 000000000000..ee9b361957f8
--- /dev/null
+++ b/drivers/staging/oomhelper/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-y    += oomhelper.o
diff --git a/drivers/staging/oomhelper/oomhelper.c b/drivers/staging/oomhelper/oomhelper.c
new file mode 100644
index 000000000000..5a3fe0270cb8
--- /dev/null
+++ b/drivers/staging/oomhelper/oomhelper.c
@@ -0,0 +1,65 @@
+// SPDX-License-Identifier: GPL-2.0
+/* prof of concept of android aware oom killer */
+/* Author: peter.enderborg@xxxxxxxx */
+
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/slab.h>
+#include <linux/oom.h>
+void wake_oom_reaper(struct task_struct *tsk); /* need to public ... */
+void __oom_kill_process(struct task_struct *victim, const char *message);
+
+static int oomhelper_oom_notify(struct notifier_block *self,
+Â Â Â  Â Â Â  Â Â Â  Â Â Â  Â Â Â Â Â  unsigned long notused, void *param)
+{
+Â  struct task_struct *tsk;
+Â  struct task_struct *selected = NULL;
+Â  int highest = 0;
+
+Â  pr_info("invited");
+Â  rcu_read_lock();
+Â  for_each_process(tsk) {
+Â Â Â  Â  struct task_struct *candidate;
+Â Â Â  Â  if (tsk->flags & PF_KTHREAD)
+Â Â Â  Â Â Â  Â  continue;
+
+Â Â Â  Â  /* Ignore task if coredump in progress */
+Â Â Â  Â  if (tsk->mm && tsk->mm->core_state)
+Â Â Â  Â Â Â  Â  continue;
+Â Â Â  Â  candidate = find_lock_task_mm(tsk);
+Â Â Â  Â  if (!candidate)
+Â Â Â  Â Â Â  Â  continue;
+
+Â Â Â  Â  if (highest < candidate->signal->oom_score_adj) {
+Â Â Â  Â Â Â  Â  /* for test dont kill level 0 */
+Â Â Â  Â Â Â  Â  highest = candidate->signal->oom_score_adj;
+Â Â Â  Â Â Â  Â  selected = candidate;
+Â Â Â  Â Â Â  Â  pr_info("new selected %d %d", selected->pid,
+Â Â Â  Â Â Â  Â Â Â  Â  selected->signal->oom_score_adj);
+Â Â Â  Â  }
+Â Â Â  Â  task_unlock(candidate);
+Â  }
+Â  if (selected) {
+Â Â Â  Â  get_task_struct(selected);
+Â  }
+Â  rcu_read_unlock();
+Â  if (selected) {
+Â Â Â  Â  pr_info("oomhelper killing: %d", selected->pid);
+Â Â Â  Â  __oom_kill_process(selected, "oomhelper");
+Â  }
+
+Â  return NOTIFY_OK;
+}
+
+static struct notifier_block oomhelper_oom_nb = {
+Â Â Â  .notifier_call = oomhelper_oom_notify
+};
+
+int __init oomhelper_register_oom_notifier(void)
+{
+Â Â Â  register_oom_notifier(&oomhelper_oom_nb);
+Â Â Â  pr_info("oomhelper installed");
+Â Â Â  return 0;
+}
+
+subsys_initcall(oomhelper_register_oom_notifier);
diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index fa1cf18bac97..a5f7299af9a3 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -658,7 +658,7 @@ static int oom_reaper(void *unused)
     return 0;
 }
 
-static void wake_oom_reaper(struct task_struct *tsk)
+void wake_oom_reaper(struct task_struct *tsk)
 {
     /* mm is already queued? */
     if (test_and_set_bit(MMF_OOM_REAP_QUEUED, &tsk->signal->oom_mm->flags))
@@ -856,7 +856,7 @@ static bool task_will_free_mem(struct task_struct *task)
     return ret;
 }
 
-static void __oom_kill_process(struct task_struct *victim, const char *message)
+void __oom_kill_process(struct task_struct *victim, const char *message)
 {
     struct task_struct *p;
     struct mm_struct *mm;
-- 
2.17.1

Is that something that might be accepted?

It uses the notifications and that is no problem a guess.

But it also calls some oom-kill functions that is not exported.


>  
>> [...]
>>>> I was thinking of simply prctl(SET_MEMPOOL, bytes) to assign mempool
>>>> to a thread (not shared between threads) and prctl(RESET_MEMPOOL) to
>>>> free the mempool.
>>> I am not a great fan of prctl. It has become a dumping ground for all
>>> mix of unrelated functionality. But let's say this is a minor detail at
>>> this stage.
>> I agree this does not have to be prctl().
>>
>>> So you are proposing to have a per mm mem pool that would be
>> I was thinking of per-task_struct instead of per-mm_struct just for simplicity.
>>
>>> used as a fallback for an allocation which cannot make a forward
>>> progress, right?
>> Correct
>>
>>> Would that pool be preallocated and sitting idle?
>> Correct
>>
>>> What kind of allocations would be allowed to use the pool?
>> I was thinking of any type of allocation from the oom-killer (or
>> specific threads). Please note that the mempool is the backup and only
>> used in the slowpath.
>>
>>> What if the pool is depleted?
>> This would mean that either the estimate of mempool size is bad or
>> oom-killer is buggy and leaking memory.
>>
>> I am open to any design directions for mempool or some other way where
>> we can provide a notion of memory guarantee to oom-killer.
> OK, thanks for clarification. There will certainly be hard problems to
> sort out[1] but the overall idea makes sense to me and it sounds like a
> much better approach than a OOM specific solution.
>
>
> [1] - how the pool is going to be replenished without hitting all
> potential reclaim problems (thus dependencies on other all tasks
> directly/indirectly) yet to not rely on any background workers to do
> that on the task behalf without a proper accounting etc...