lkml.org 
[lkml]   [2011]   [Oct]   [31]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 08/17] freezer: use dedicated lock instead of task_lock() + memory barrier
    Date
    Freezer synchronization is needlessly complicated - it's by no means a
    hot path and the priority is staying unintrusive and safe. This patch
    makes it simply use a dedicated lock instead of piggy-backing on
    task_lock() and playing with memory barriers.

    On the failure path of try_to_freeze_tasks(), locking is moved from it
    to cancel_freezing(). This makes the frozen() test racy but the race
    here is a non-issue as the warning is printed for tasks which failed
    to enter frozen for 20 seconds and race on PF_FROZEN at the last
    moment doesn't change anything.

    This simplifies freezer implementation and eases further changes
    including some race fixes.

    Signed-off-by: Tejun Heo <tj@kernel.org>
    ---
    kernel/freezer.c | 84 +++++++++++++++++++++---------------------------
    kernel/power/process.c | 2 -
    2 files changed, 37 insertions(+), 49 deletions(-)

    diff --git a/kernel/freezer.c b/kernel/freezer.c
    index 8faa0e0..10dd2e0 100644
    --- a/kernel/freezer.c
    +++ b/kernel/freezer.c
    @@ -11,17 +11,8 @@
    #include <linux/freezer.h>
    #include <linux/kthread.h>

    -/*
    - * freezing is complete, mark current process as frozen
    - */
    -static inline void frozen_process(void)
    -{
    - if (!unlikely(current->flags & PF_NOFREEZE)) {
    - current->flags |= PF_FROZEN;
    - smp_wmb();
    - }
    - clear_freeze_flag(current);
    -}
    +/* protects freezing and frozen transitions */
    +static DEFINE_SPINLOCK(freezer_lock);

    /* Refrigerator is place where frozen processes are stored :-). */
    bool __refrigerator(bool check_kthr_stop)
    @@ -31,14 +22,16 @@ bool __refrigerator(bool check_kthr_stop)
    bool was_frozen = false;
    long save;

    - task_lock(current);
    - if (freezing(current)) {
    - frozen_process();
    - task_unlock(current);
    - } else {
    - task_unlock(current);
    + spin_lock_irq(&freezer_lock);
    + if (!freezing(current)) {
    + spin_unlock_irq(&freezer_lock);
    return was_frozen;
    }
    + if (!(current->flags & PF_NOFREEZE))
    + current->flags |= PF_FROZEN;
    + clear_freeze_flag(current);
    + spin_unlock_irq(&freezer_lock);
    +
    save = current->state;
    pr_debug("%s entered refrigerator\n", current->comm);

    @@ -99,21 +92,18 @@ static void fake_signal_wake_up(struct task_struct *p)
    */
    bool freeze_task(struct task_struct *p, bool sig_only)
    {
    - /*
    - * We first check if the task is freezing and next if it has already
    - * been frozen to avoid the race with frozen_process() which first marks
    - * the task as frozen and next clears its TIF_FREEZE.
    - */
    - if (!freezing(p)) {
    - smp_rmb();
    - if (frozen(p))
    - return false;
    -
    - if (!sig_only || should_send_signal(p))
    - set_freeze_flag(p);
    - else
    - return false;
    - }
    + unsigned long flags;
    + bool ret = false;
    +
    + spin_lock_irqsave(&freezer_lock, flags);
    +
    + if (sig_only && !should_send_signal(p))
    + goto out_unlock;
    +
    + if (frozen(p))
    + goto out_unlock;
    +
    + set_freeze_flag(p);

    if (should_send_signal(p)) {
    fake_signal_wake_up(p);
    @@ -123,26 +113,28 @@ bool freeze_task(struct task_struct *p, bool sig_only)
    * TASK_RUNNING transition can't race with task state
    * testing in try_to_freeze_tasks().
    */
    - } else if (sig_only) {
    - return false;
    } else {
    wake_up_state(p, TASK_INTERRUPTIBLE);
    }
    -
    - return true;
    + ret = true;
    +out_unlock:
    + spin_unlock_irqrestore(&freezer_lock, flags);
    + return ret;
    }

    void cancel_freezing(struct task_struct *p)
    {
    unsigned long flags;

    + spin_lock_irqsave(&freezer_lock, flags);
    if (freezing(p)) {
    pr_debug(" clean up: %s\n", p->comm);
    clear_freeze_flag(p);
    - spin_lock_irqsave(&p->sighand->siglock, flags);
    + spin_lock(&p->sighand->siglock);
    recalc_sigpending_and_wake(p);
    - spin_unlock_irqrestore(&p->sighand->siglock, flags);
    + spin_unlock(&p->sighand->siglock);
    }
    + spin_unlock_irqrestore(&freezer_lock, flags);
    }

    /*
    @@ -156,16 +148,14 @@ void cancel_freezing(struct task_struct *p)
    */
    void __thaw_task(struct task_struct *p)
    {
    - bool was_frozen;
    + unsigned long flags;

    - task_lock(p);
    - was_frozen = frozen(p);
    - if (was_frozen)
    + spin_lock_irqsave(&freezer_lock, flags);
    + if (frozen(p)) {
    p->flags &= ~PF_FROZEN;
    - else
    - clear_freeze_flag(p);
    - task_unlock(p);
    -
    - if (was_frozen)
    wake_up_process(p);
    + } else {
    + clear_freeze_flag(p);
    + }
    + spin_unlock_irqrestore(&freezer_lock, flags);
    }
    diff --git a/kernel/power/process.c b/kernel/power/process.c
    index 9db048f..bd420ca 100644
    --- a/kernel/power/process.c
    +++ b/kernel/power/process.c
    @@ -118,11 +118,9 @@ static int try_to_freeze_tasks(bool sig_only)

    read_lock(&tasklist_lock);
    do_each_thread(g, p) {
    - task_lock(p);
    if (!wakeup && freezing(p) && !freezer_should_skip(p))
    sched_show_task(p);
    cancel_freezing(p);
    - task_unlock(p);
    } while_each_thread(g, p);
    read_unlock(&tasklist_lock);
    } else {
    --
    1.7.3.1


    \
     
     \ /
      Last update: 2011-10-31 20:11    [W:0.032 / U:0.048 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site