[Devel] [PATCH rh7] ms/mm, oom: remove task_lock protecting comm printing

Andrey Ryabinin aryabinin at virtuozzo.com
Mon Dec 19 07:54:23 PST 2016


From: David Rientjes <rientjes at google.com>

ML: da39da3a54fed88e29024f2f1f6cd7357cd03a44

The oom killer takes task_lock() in a couple of places solely to protect
printing the task's comm.

A process's comm, including current's comm, may change due to
/proc/pid/comm or PR_SET_NAME.

The comm will always be NULL-terminated, so the worst race scenario would
only be during update.  We can tolerate a comm being printed that is in
the middle of an update to avoid taking the lock.

Other locations in the kernel have already dropped task_lock() when
printing comm, so this is consistent.

Signed-off-by: David Rientjes <rientjes at google.com>
Suggested-by: Oleg Nesterov <oleg at redhat.com>
Cc: Michal Hocko <mhocko at kernel.org>
Cc: Vladimir Davydov <vdavydov at parallels.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work at gmail.com>
Acked-by: Johannes Weiner <hannes at cmpxchg.org>
Signed-off-by: Andrew Morton <akpm at linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds at linux-foundation.org>

https://jira.sw.ru/browse/PSBM-56178

[Partial backport - this patch doesn't contain changes in kernel/cpuset.c
from the upstream patch. Our kernel still needs task_lock() task lock there
to protect task_cs(). Upstream switched to RCU protection thus it doesn't
need task_lock() to protect task_cs()]

Signed-off-by: Andrey Ryabinin <aryabinin at virtuozzo.com>
---
 mm/oom_kill.c | 11 +----------
 1 file changed, 1 insertion(+), 10 deletions(-)

diff --git a/mm/oom_kill.c b/mm/oom_kill.c
index f9a8e62..59b109e 100644
--- a/mm/oom_kill.c
+++ b/mm/oom_kill.c
@@ -570,10 +570,8 @@ bool oom_trylock(struct mem_cgroup *memcg)
 			struct task_struct *p = ctx->victim;
 
 			if (p && ctx->marked) {
-				task_lock(p);
 				pr_err("OOM kill timeout: %d (%s)\n",
 				       task_pid_nr(p), p->comm);
-				task_unlock(p);
 				show_stack(p, NULL);
 			}
 
@@ -766,12 +764,9 @@ static void oom_berserker(unsigned long points, unsigned long overdraft,
 		    points * 100 / totalpages)
 			continue;
 
-		if (__ratelimit(&berserker_rs)) {
-			task_lock(p);
+		if (__ratelimit(&berserker_rs))
 			pr_err("Rage kill process %d (%s)\n",
 			       task_pid_nr(p), p->comm);
-			task_unlock(p);
-		}
 
 		do_send_sig_info(SIGKILL, SEND_SIG_FORCED, p, true);
 		mem_cgroup_note_oom_kill(memcg, p);
@@ -818,10 +813,8 @@ void oom_kill_process(struct task_struct *p, gfp_t gfp_mask, int order,
 	if (__ratelimit(&oom_rs))
 		dump_header(p, gfp_mask, order, memcg, nodemask);
 
-	task_lock(p);
 	pr_err("%s: Kill process %d (%s) score %lu or sacrifice child\n",
 		message, task_pid_nr(p), p->comm, points * 1000 / totalpages);
-	task_unlock(p);
 
 	/*
 	 * If any of p's children has a different mm and is eligible for kill,
@@ -891,10 +884,8 @@ void oom_kill_process(struct task_struct *p, gfp_t gfp_mask, int order,
 			if (p->signal->oom_score_adj == OOM_SCORE_ADJ_MIN)
 				continue;
 
-			task_lock(p);	/* Protect ->comm from prctl() */
 			pr_err("Kill process %d (%s) in VE \"%s\" sharing same memory\n",
 				task_pid_nr(p), p->comm, task_ve_name(p));
-			task_unlock(p);
 			do_send_sig_info(SIGKILL, SEND_SIG_FORCED, p, true);
 			mem_cgroup_note_oom_kill(memcg, p);
 		}
-- 
2.10.2



More information about the Devel mailing list