--
Best regards,
Konstantin Khorenko,
Virtuozzo Linux Kernel Team
On 26.11.2022 23:15, nb wrote:
On 23.11.22 г. 20:53 ч., Konstantin Khorenko wrote:
Currently it's possible to mark a task as a victim even in case it has
already cleared its ->mm.
This might lead (and leads) to a situation when oom_unlock() believes
the OOM context will be released by the "victim" do_exit() ->
exit_oom_victim(), but our "victim" already passed the point of calling
exit_oom_victim() and thus OOM context is not released.
Add additional checks for task->mm in appropriate places, similar checks
are applied in mainstream code in the scope of:
1af8bb432695 ("mm, oom: fortify task_will_free_mem()")
091f362c53c2 ("mm, oom: tighten task_will_free_mem() locking")
https://jira.sw.ru/browse/PSBM-143283
Signed-off-by: Denis Lunev <d...@virtuozzo.com>
Signed-off-by: Konstantin Khorenko <khore...@virtuozzo.com>
---
include/linux/oom.h | 14 ++++++++++++++
mm/memcontrol.c | 7 ++++++-
2 files changed, 20 insertions(+), 1 deletion(-)
diff --git a/include/linux/oom.h b/include/linux/oom.h
index 3a6e073a5dd4..ef0096799ee3 100644
--- a/include/linux/oom.h
+++ b/include/linux/oom.h
@@ -125,8 +125,22 @@ static inline void oom_killer_enable(void)
extern struct task_struct *find_lock_task_mm(struct task_struct *p);
+/*
+ * Caller has to make sure that task->mm is stable (hold task_lock or
+ * it operates on the current).
nit:
Instead of spelling this in comment I find it more robust if such
conditions are enforced via lockdep. For example :
lockdep_assert(lockdep_is_held(l) != LOCK_STATE_NOT_HELD || task == current)
Thank you for the nit, addressed it in the
[PATCH rh7 4/4] oom: Cast the lockdep spell in the code
Of course that's only helpful if we run tests with lockdep enabled
(which I don't know if we do ? )
Sure we do build debug kernels (with lockdep enabled) and test them.
+ */
static inline bool task_will_free_mem(struct task_struct *task)
{
+ struct mm_struct *mm = task->mm;
+
+ /*
+ * Skip tasks without mm because it might have passed its exit_mm and
+ * exit_oom_victim. oom_reaper could have rescued that but do not rely
+ * on that for now. We can consider find_lock_task_mm in future.
+ */
+ if (!mm)
+ return false;
+
/*
* A coredumping process may sleep for an extended period in exit_mm(),
* so the oom killer cannot assume that the process will promptly exit
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index fdc5245e48a9..7135306c6ac0 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2492,8 +2492,13 @@ static void mem_cgroup_out_of_memory(struct mem_cgroup
*memcg, gfp_t gfp_mask,
* If current has a pending SIGKILL or is exiting, then automatically
* select it. The goal is to allow it to allocate so that it may
* quickly exit and free its memory.
+ *
+ * But don't select if current has already released its mm at
+ * exit_mm(), otherwise we might skip exit_oom_victim() and
+ * thus OOM context won't be released.
*/
- if (fatal_signal_pending(current) || task_will_free_mem(current)) {
+ if (current->mm &&
+ (fatal_signal_pending(current) || task_will_free_mem(current))) {
mark_oom_victim(current);
return;
}
_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel
