From f913da596a407d3be7c8c220697beacd8fe7312a Mon Sep 17 00:00:00 2001 From: Michal Hocko Date: Thu, 28 Jul 2016 15:44:37 -0700 Subject: [PATCH] proc, oom: drop bogus sighand lock Oleg has pointed out that can simplify both oom_adj_{read,write} and oom_score_adj_{read,write} even further and drop the sighand lock. The main purpose of the lock was to protect p->signal from going away but this will not happen since ea6d290ca34c ("signals: make task_struct->signal immutable/refcountable"). The other role of the lock was to synchronize different writers, especially those with CAP_SYS_RESOURCE. Introduce a mutex for this purpose. Later patches will need this lock anyway. Suggested-by: Oleg Nesterov Link: http://lkml.kernel.org/r/1466426628-15074-3-git-send-email-mhocko@kernel.org Signed-off-by: Michal Hocko Acked-by: Oleg Nesterov Cc: Vladimir Davydov Cc: David Rientjes Cc: Tetsuo Handa Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/proc/base.c | 51 +++++++++++++++++--------------------------------- 1 file changed, 17 insertions(+), 34 deletions(-) diff --git a/fs/proc/base.c b/fs/proc/base.c index 2a0f5ee9b623..f7dc1050387f 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -1024,23 +1024,21 @@ static ssize_t oom_adj_read(struct file *file, char __user *buf, size_t count, char buffer[PROC_NUMBUF]; int oom_adj = OOM_ADJUST_MIN; size_t len; - unsigned long flags; if (!task) return -ESRCH; - if (lock_task_sighand(task, &flags)) { - if (task->signal->oom_score_adj == OOM_SCORE_ADJ_MAX) - oom_adj = OOM_ADJUST_MAX; - else - oom_adj = (task->signal->oom_score_adj * -OOM_DISABLE) / - OOM_SCORE_ADJ_MAX; - unlock_task_sighand(task, &flags); - } + if (task->signal->oom_score_adj == OOM_SCORE_ADJ_MAX) + oom_adj = OOM_ADJUST_MAX; + else + oom_adj = (task->signal->oom_score_adj * -OOM_DISABLE) / + OOM_SCORE_ADJ_MAX; put_task_struct(task); len = snprintf(buffer, sizeof(buffer), "%d\n", oom_adj); return simple_read_from_buffer(buf, count, ppos, buffer, len); } +static DEFINE_MUTEX(oom_adj_mutex); + /* * /proc/pid/oom_adj exists solely for backwards compatibility with previous * kernels. The effective policy is defined by oom_score_adj, which has a @@ -1057,7 +1055,6 @@ static ssize_t oom_adj_write(struct file *file, const char __user *buf, struct task_struct *task; char buffer[PROC_NUMBUF]; int oom_adj; - unsigned long flags; int err; memset(buffer, 0, sizeof(buffer)); @@ -1083,11 +1080,6 @@ static ssize_t oom_adj_write(struct file *file, const char __user *buf, goto out; } - if (!lock_task_sighand(task, &flags)) { - err = -ESRCH; - goto err_put_task; - } - /* * Scale /proc/pid/oom_score_adj appropriately ensuring that a maximum * value is always attainable. @@ -1097,10 +1089,11 @@ static ssize_t oom_adj_write(struct file *file, const char __user *buf, else oom_adj = (oom_adj * OOM_SCORE_ADJ_MAX) / -OOM_DISABLE; + mutex_lock(&oom_adj_mutex); if (oom_adj < task->signal->oom_score_adj && !capable(CAP_SYS_RESOURCE)) { err = -EACCES; - goto err_sighand; + goto err_unlock; } /* @@ -1113,9 +1106,8 @@ static ssize_t oom_adj_write(struct file *file, const char __user *buf, task->signal->oom_score_adj = oom_adj; trace_oom_score_adj_update(task); -err_sighand: - unlock_task_sighand(task, &flags); -err_put_task: +err_unlock: + mutex_unlock(&oom_adj_mutex); put_task_struct(task); out: return err < 0 ? err : count; @@ -1133,15 +1125,11 @@ static ssize_t oom_score_adj_read(struct file *file, char __user *buf, struct task_struct *task = get_proc_task(file_inode(file)); char buffer[PROC_NUMBUF]; short oom_score_adj = OOM_SCORE_ADJ_MIN; - unsigned long flags; size_t len; if (!task) return -ESRCH; - if (lock_task_sighand(task, &flags)) { - oom_score_adj = task->signal->oom_score_adj; - unlock_task_sighand(task, &flags); - } + oom_score_adj = task->signal->oom_score_adj; put_task_struct(task); len = snprintf(buffer, sizeof(buffer), "%hd\n", oom_score_adj); return simple_read_from_buffer(buf, count, ppos, buffer, len); @@ -1152,7 +1140,6 @@ static ssize_t oom_score_adj_write(struct file *file, const char __user *buf, { struct task_struct *task; char buffer[PROC_NUMBUF]; - unsigned long flags; int oom_score_adj; int err; @@ -1179,25 +1166,21 @@ static ssize_t oom_score_adj_write(struct file *file, const char __user *buf, goto out; } - if (!lock_task_sighand(task, &flags)) { - err = -ESRCH; - goto err_put_task; - } - + mutex_lock(&oom_adj_mutex); if ((short)oom_score_adj < task->signal->oom_score_adj_min && !capable(CAP_SYS_RESOURCE)) { err = -EACCES; - goto err_sighand; + goto err_unlock; } task->signal->oom_score_adj = (short)oom_score_adj; if (has_capability_noaudit(current, CAP_SYS_RESOURCE)) task->signal->oom_score_adj_min = (short)oom_score_adj; + trace_oom_score_adj_update(task); -err_sighand: - unlock_task_sighand(task, &flags); -err_put_task: +err_unlock: + mutex_unlock(&oom_adj_mutex); put_task_struct(task); out: return err < 0 ? err : count; -- GitLab