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

Konstantin Khorenko khorenko at virtuozzo.com
Mon Feb 6 05:18:16 PST 2017


The commit is pushed to "branch-rh7-3.10.0-514.6.1.vz7.28.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-514.6.1.vz7.28.4
------>
commit d925321ba9dc31b3de024d03053e92231a61051f
Author: David Rientjes <rientjes at google.com>
Date:   Mon Feb 6 17:18:16 2017 +0400

    ms/mm, oom: remove task_lock protecting comm printing
    
    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>
    Reviewed-by: Dmitry Safonov <dsafonov 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 a82433f..c41c675 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,
@@ -892,10 +885,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);
 		}


More information about the Devel mailing list