lkml.org 
[lkml]   [2016]   [Jun]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 02/10] proc, oom: drop bogus sighand lock
    Date
    From: Michal Hocko <mhocko@suse.com>

    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 <oleg@redhat.com>
    Signed-off-by: Michal Hocko <mhocko@suse.com>
    ---
    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 a6014e45c516..968d5ea06e62 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;
    --
    2.8.1
    \
     
     \ /
      Last update: 2016-06-09 14:41    [W:4.483 / U:0.736 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site