[PATCH 1/4] freezer_cg: fix improper BUG_ON() causing oops - Kernel

This is a discussion on [PATCH 1/4] freezer_cg: fix improper BUG_ON() causing oops - Kernel ; The BUG_ON() should be protected by freezer->lock, otherwise it can be triggered easily when a task has been unfreezed but the corresponding cgroup hasn't been changed to FROZEN state. Signed-off-by: Li Zefan --- kernel/cgroup_freezer.c | 3 ++- 1 files changed, ...

+ Reply to Thread
Results 1 to 13 of 13

Thread: [PATCH 1/4] freezer_cg: fix improper BUG_ON() causing oops

  1. [PATCH 1/4] freezer_cg: fix improper BUG_ON() causing oops

    The BUG_ON() should be protected by freezer->lock, otherwise it
    can be triggered easily when a task has been unfreezed but the
    corresponding cgroup hasn't been changed to FROZEN state.

    Signed-off-by: Li Zefan
    ---
    kernel/cgroup_freezer.c | 3 ++-
    1 files changed, 2 insertions(+), 1 deletions(-)

    diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
    index e950569..7f54d1c 100644
    --- a/kernel/cgroup_freezer.c
    +++ b/kernel/cgroup_freezer.c
    @@ -190,8 +190,9 @@ static void freezer_fork(struct cgroup_subsys *ss, struct task_struct *task)
    freezer = task_freezer(task);
    task_unlock(task);

    - BUG_ON(freezer->state == CGROUP_FROZEN);
    spin_lock_irq(&freezer->lock);
    + BUG_ON(freezer->state == CGROUP_FROZEN);
    +
    /* Locking avoids race with FREEZING -> THAWED transitions. */
    if (freezer->state == CGROUP_FREEZING)
    freeze_task(task, true);
    --
    1.5.4.rc3

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. [PATCH 3/4] freezer_cg: use thaw_process() in unfreeze_cgroup()

    Don't duplicate the implementation of thaw_process().

    Signed-off-by: Li Zefan
    ---
    kernel/cgroup_freezer.c | 15 ++++-----------
    1 files changed, 4 insertions(+), 11 deletions(-)

    diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
    index 6fadafe..3ea57e4 100644
    --- a/kernel/cgroup_freezer.c
    +++ b/kernel/cgroup_freezer.c
    @@ -275,25 +275,18 @@ static int try_to_freeze_cgroup(struct cgroup *cgroup, struct freezer *freezer)
    return num_cant_freeze_now ? -EBUSY : 0;
    }

    -static int unfreeze_cgroup(struct cgroup *cgroup, struct freezer *freezer)
    +static void unfreeze_cgroup(struct cgroup *cgroup, struct freezer *freezer)
    {
    struct cgroup_iter it;
    struct task_struct *task;

    cgroup_iter_start(cgroup, &it);
    while ((task = cgroup_iter_next(cgroup, &it))) {
    - int do_wake;
    -
    - task_lock(task);
    - do_wake = __thaw_process(task);
    - task_unlock(task);
    - if (do_wake)
    - wake_up_process(task);
    + thaw_process(task);
    }
    cgroup_iter_end(cgroup, &it);
    - freezer->state = CGROUP_THAWED;

    - return 0;
    + freezer->state = CGROUP_THAWED;
    }

    static int freezer_change_state(struct cgroup *cgroup,
    @@ -320,7 +313,7 @@ static int freezer_change_state(struct cgroup *cgroup,
    }
    /* state == FREEZING and goal_state == THAWED, so unfreeze */
    case CGROUP_FROZEN:
    - retval = unfreeze_cgroup(cgroup, freezer);
    + unfreeze_cgroup(cgroup, freezer);
    break;
    default:
    break;
    --
    1.5.4.rc3
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. [PATCH 2/4] freezer_cg: remove redundant check in freezer_can_attach()

    It is sufficient to check if @task is frozen, and no need to check if
    the original freezer is frozen.

    Signed-off-by: Li Zefan
    ---
    kernel/cgroup_freezer.c | 16 +++++++---------
    1 files changed, 7 insertions(+), 9 deletions(-)

    diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
    index 7f54d1c..6fadafe 100644
    --- a/kernel/cgroup_freezer.c
    +++ b/kernel/cgroup_freezer.c
    @@ -162,9 +162,13 @@ static int freezer_can_attach(struct cgroup_subsys *ss,
    struct task_struct *task)
    {
    struct freezer *freezer;
    - int retval;

    - /* Anything frozen can't move or be moved to/from */
    + /*
    + * Anything frozen can't move or be moved to/from.
    + *
    + * Since orig_freezer->state == FROZEN means that @task has been
    + * frozen, so it's sufficient to check the latter condition.
    + */

    if (is_task_frozen_enough(task))
    return -EBUSY;
    @@ -173,13 +177,7 @@ static int freezer_can_attach(struct cgroup_subsys *ss,
    if (freezer->state == CGROUP_FROZEN)
    return -EBUSY;

    - retval = 0;
    - task_lock(task);
    - freezer = task_freezer(task);
    - if (freezer->state == CGROUP_FROZEN)
    - retval = -EBUSY;
    - task_unlock(task);
    - return retval;
    + return 0;
    }

    static void freezer_fork(struct cgroup_subsys *ss, struct task_struct *task)
    --
    1.5.4.rc3

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. [PATCH 4/4] freezer_cg: simplify freezer_change_state()

    Just call unfreeze_cgroup() if goal_state == THAWED, and call
    try_to_freeze_cgroup() if goal_state == FROZEN.

    No behavior has been changed.

    Signed-off-by: Li Zefan
    ---
    kernel/cgroup_freezer.c | 19 +++++++------------
    1 files changed, 7 insertions(+), 12 deletions(-)

    diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
    index 3ea57e4..cdef2d8 100644
    --- a/kernel/cgroup_freezer.c
    +++ b/kernel/cgroup_freezer.c
    @@ -296,27 +296,22 @@ static int freezer_change_state(struct cgroup *cgroup,
    int retval = 0;

    freezer = cgroup_freezer(cgroup);
    +
    spin_lock_irq(&freezer->lock);
    +
    update_freezer_state(cgroup, freezer);
    if (goal_state == freezer->state)
    goto out;
    - switch (freezer->state) {
    +
    + switch (goal_state) {
    case CGROUP_THAWED:
    - retval = try_to_freeze_cgroup(cgroup, freezer);
    + unfreeze_cgroup(cgroup, freezer);
    break;
    - case CGROUP_FREEZING:
    - if (goal_state == CGROUP_FROZEN) {
    - /* Userspace is retrying after
    - * "/bin/echo FROZEN > freezer.state" returned -EBUSY */
    - retval = try_to_freeze_cgroup(cgroup, freezer);
    - break;
    - }
    - /* state == FREEZING and goal_state == THAWED, so unfreeze */
    case CGROUP_FROZEN:
    - unfreeze_cgroup(cgroup, freezer);
    + retval = try_to_freeze_cgroup(cgroup, freezer);
    break;
    default:
    - break;
    + BUG();
    }
    out:
    spin_unlock_irq(&freezer->lock);
    --
    1.5.4.rc3

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [PATCH 1/4] freezer_cg: fix improper BUG_ON() causing oops

    Li Zefan wrote:
    > The BUG_ON() should be protected by freezer->lock, otherwise it
    > can be triggered easily when a task has been unfreezed but the
    > corresponding cgroup hasn't been changed to FROZEN state.


    yes. thanks,

    Acked-by: Cedric Le Goater

    > Signed-off-by: Li Zefan
    > ---
    > kernel/cgroup_freezer.c | 3 ++-
    > 1 files changed, 2 insertions(+), 1 deletions(-)
    >
    > diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
    > index e950569..7f54d1c 100644
    > --- a/kernel/cgroup_freezer.c
    > +++ b/kernel/cgroup_freezer.c
    > @@ -190,8 +190,9 @@ static void freezer_fork(struct cgroup_subsys *ss, struct task_struct *task)
    > freezer = task_freezer(task);
    > task_unlock(task);
    >
    > - BUG_ON(freezer->state == CGROUP_FROZEN);
    > spin_lock_irq(&freezer->lock);
    > + BUG_ON(freezer->state == CGROUP_FROZEN);
    > +
    > /* Locking avoids race with FREEZING -> THAWED transitions. */
    > if (freezer->state == CGROUP_FREEZING)
    > freeze_task(task, true);


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [PATCH 3/4] freezer_cg: use thaw_process() in unfreeze_cgroup()

    Li Zefan wrote:
    > Don't duplicate the implementation of thaw_process().


    looks OK but you should remove __thaw_process() which is unused
    now.

    thanks,

    C.

    > Signed-off-by: Li Zefan
    > ---
    > kernel/cgroup_freezer.c | 15 ++++-----------
    > 1 files changed, 4 insertions(+), 11 deletions(-)
    >
    > diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
    > index 6fadafe..3ea57e4 100644
    > --- a/kernel/cgroup_freezer.c
    > +++ b/kernel/cgroup_freezer.c
    > @@ -275,25 +275,18 @@ static int try_to_freeze_cgroup(struct cgroup *cgroup, struct freezer *freezer)
    > return num_cant_freeze_now ? -EBUSY : 0;
    > }
    >
    > -static int unfreeze_cgroup(struct cgroup *cgroup, struct freezer *freezer)
    > +static void unfreeze_cgroup(struct cgroup *cgroup, struct freezer *freezer)
    > {
    > struct cgroup_iter it;
    > struct task_struct *task;
    >
    > cgroup_iter_start(cgroup, &it);
    > while ((task = cgroup_iter_next(cgroup, &it))) {
    > - int do_wake;
    > -
    > - task_lock(task);
    > - do_wake = __thaw_process(task);
    > - task_unlock(task);
    > - if (do_wake)
    > - wake_up_process(task);
    > + thaw_process(task);
    > }
    > cgroup_iter_end(cgroup, &it);
    > - freezer->state = CGROUP_THAWED;
    >
    > - return 0;
    > + freezer->state = CGROUP_THAWED;
    > }
    >
    > static int freezer_change_state(struct cgroup *cgroup,
    > @@ -320,7 +313,7 @@ static int freezer_change_state(struct cgroup *cgroup,
    > }
    > /* state == FREEZING and goal_state == THAWED, so unfreeze */
    > case CGROUP_FROZEN:
    > - retval = unfreeze_cgroup(cgroup, freezer);
    > + unfreeze_cgroup(cgroup, freezer);
    > break;
    > default:
    > break;


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. Re: [PATCH 2/4] freezer_cg: remove redundant check in freezer_can_attach()

    Li Zefan wrote:
    > It is sufficient to check if @task is frozen, and no need to check if
    > the original freezer is frozen.


    hmm, a task being frozen does not mean that its freezer cgroup is
    frozen. So the extra check seems valid but looking at the comment :

    /*
    * The call to cgroup_lock() in the freezer.state write method prevents
    * a write to that file racing against an attach, and hence the
    * can_attach() result will remain valid until the attach completes.
    */
    static int freezer_can_attach(struct cgroup_subsys *ss,

    how do we know that the task_freezer(task), which is not protected by
    the cgroup_lock(), is not going to change its state to CGROUP_FROZEN
    it looks racy.

    C.

    > Signed-off-by: Li Zefan
    > ---
    > kernel/cgroup_freezer.c | 16 +++++++---------
    > 1 files changed, 7 insertions(+), 9 deletions(-)
    >
    > diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
    > index 7f54d1c..6fadafe 100644
    > --- a/kernel/cgroup_freezer.c
    > +++ b/kernel/cgroup_freezer.c
    > @@ -162,9 +162,13 @@ static int freezer_can_attach(struct cgroup_subsys *ss,
    > struct task_struct *task)
    > {
    > struct freezer *freezer;
    > - int retval;
    >
    > - /* Anything frozen can't move or be moved to/from */
    > + /*
    > + * Anything frozen can't move or be moved to/from.
    > + *
    > + * Since orig_freezer->state == FROZEN means that @task has been
    > + * frozen, so it's sufficient to check the latter condition.
    > + */
    >
    > if (is_task_frozen_enough(task))
    > return -EBUSY;
    > @@ -173,13 +177,7 @@ static int freezer_can_attach(struct cgroup_subsys *ss,
    > if (freezer->state == CGROUP_FROZEN)
    > return -EBUSY;
    >
    > - retval = 0;
    > - task_lock(task);
    > - freezer = task_freezer(task);
    > - if (freezer->state == CGROUP_FROZEN)
    > - retval = -EBUSY;
    > - task_unlock(task);
    > - return retval;
    > + return 0;
    > }
    >
    > static void freezer_fork(struct cgroup_subsys *ss, struct task_struct *task)


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  8. Re: [PATCH 3/4] freezer_cg: use thaw_process() in unfreeze_cgroup()

    Cedric Le Goater wrote:
    > Li Zefan wrote:
    >> Don't duplicate the implementation of thaw_process().

    >
    > looks OK but you should remove __thaw_process() which is unused
    > now.
    >


    Then I'll make it a static function and remove the declaration from .h file.

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  9. Re: [PATCH 2/4] freezer_cg: remove redundant check in freezer_can_attach()

    Cedric Le Goater wrote:
    > Li Zefan wrote:
    >> It is sufficient to check if @task is frozen, and no need to check if
    >> the original freezer is frozen.

    >
    > hmm, a task being frozen does not mean that its freezer cgroup is
    > frozen.


    If a task has being frozen, then can_attach() returns -EBUSY at once.
    If a task isn't frozen, then we have orig_freezer->state == THAWED.

    So we need to check the state of the task only.

    > So the extra check seems valid but looking at the comment :
    >
    > /*
    > * The call to cgroup_lock() in the freezer.state write method prevents
    > * a write to that file racing against an attach, and hence the
    > * can_attach() result will remain valid until the attach completes.
    > */
    > static int freezer_can_attach(struct cgroup_subsys *ss,
    >
    > how do we know that the task_freezer(task), which is not protected by
    > the cgroup_lock(), is not going to change its state to CGROUP_FROZEN
    > it looks racy.
    >


    Since freezer_can_attach() is called by cgroup core with cgroup_lock held, there is
    no race with task attaching and state changing.

    > C.
    >
    >> Signed-off-by: Li Zefan
    >> ---
    >> kernel/cgroup_freezer.c | 16 +++++++---------
    >> 1 files changed, 7 insertions(+), 9 deletions(-)
    >>
    >> diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
    >> index 7f54d1c..6fadafe 100644
    >> --- a/kernel/cgroup_freezer.c
    >> +++ b/kernel/cgroup_freezer.c
    >> @@ -162,9 +162,13 @@ static int freezer_can_attach(struct cgroup_subsys *ss,
    >> struct task_struct *task)
    >> {
    >> struct freezer *freezer;
    >> - int retval;
    >>
    >> - /* Anything frozen can't move or be moved to/from */
    >> + /*
    >> + * Anything frozen can't move or be moved to/from.
    >> + *
    >> + * Since orig_freezer->state == FROZEN means that @task has been
    >> + * frozen, so it's sufficient to check the latter condition.
    >> + */
    >>
    >> if (is_task_frozen_enough(task))
    >> return -EBUSY;
    >> @@ -173,13 +177,7 @@ static int freezer_can_attach(struct cgroup_subsys *ss,
    >> if (freezer->state == CGROUP_FROZEN)
    >> return -EBUSY;
    >>
    >> - retval = 0;
    >> - task_lock(task);
    >> - freezer = task_freezer(task);
    >> - if (freezer->state == CGROUP_FROZEN)
    >> - retval = -EBUSY;
    >> - task_unlock(task);
    >> - return retval;
    >> + return 0;
    >> }
    >>
    >> static void freezer_fork(struct cgroup_subsys *ss, struct task_struct *task)

    >
    >

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  10. Re: [PATCH 2/4] freezer_cg: remove redundant check in freezer_can_attach()

    Li Zefan wrote:
    > Cedric Le Goater wrote:
    >> Li Zefan wrote:
    >>> It is sufficient to check if @task is frozen, and no need to check if
    >>> the original freezer is frozen.

    >> hmm, a task being frozen does not mean that its freezer cgroup is
    >> frozen.

    >
    > If a task has being frozen, then can_attach() returns -EBUSY at once.
    > If a task isn't frozen, then we have orig_freezer->state == THAWED.
    >
    > So we need to check the state of the task only.
    >
    >> So the extra check seems valid but looking at the comment :
    >>
    >> /*
    >> * The call to cgroup_lock() in the freezer.state write method prevents
    >> * a write to that file racing against an attach, and hence the
    >> * can_attach() result will remain valid until the attach completes.
    >> */
    >> static int freezer_can_attach(struct cgroup_subsys *ss,
    >>
    >> how do we know that the task_freezer(task), which is not protected by
    >> the cgroup_lock(), is not going to change its state to CGROUP_FROZEN
    >> it looks racy.
    >>

    >
    > Since freezer_can_attach() is called by cgroup core with cgroup_lock held, there is
    > no race with task attaching and state changing.


    ok I see. cgroup_mutex is global, I thought it had changed and that we
    were only locking the cgroup the task was being attached to.

    Acked-by: Cedric Le Goater

    thanks,

    C.


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  11. Re: [PATCH 4/4] freezer_cg: simplify freezer_change_state()

    Li Zefan wrote:
    > Just call unfreeze_cgroup() if goal_state == THAWED, and call
    > try_to_freeze_cgroup() if goal_state == FROZEN.
    >
    > No behavior has been changed.


    Looks good.

    Acked-by: Cedric Le Goater

    thanks,

    C.

    >
    > Signed-off-by: Li Zefan
    > ---
    > kernel/cgroup_freezer.c | 19 +++++++------------
    > 1 files changed, 7 insertions(+), 12 deletions(-)
    >
    > diff --git a/kernel/cgroup_freezer.c b/kernel/cgroup_freezer.c
    > index 3ea57e4..cdef2d8 100644
    > --- a/kernel/cgroup_freezer.c
    > +++ b/kernel/cgroup_freezer.c
    > @@ -296,27 +296,22 @@ static int freezer_change_state(struct cgroup *cgroup,
    > int retval = 0;
    >
    > freezer = cgroup_freezer(cgroup);
    > +
    > spin_lock_irq(&freezer->lock);
    > +
    > update_freezer_state(cgroup, freezer);
    > if (goal_state == freezer->state)
    > goto out;
    > - switch (freezer->state) {
    > +
    > + switch (goal_state) {
    > case CGROUP_THAWED:
    > - retval = try_to_freeze_cgroup(cgroup, freezer);
    > + unfreeze_cgroup(cgroup, freezer);
    > break;
    > - case CGROUP_FREEZING:
    > - if (goal_state == CGROUP_FROZEN) {
    > - /* Userspace is retrying after
    > - * "/bin/echo FROZEN > freezer.state" returned -EBUSY */
    > - retval = try_to_freeze_cgroup(cgroup, freezer);
    > - break;
    > - }
    > - /* state == FREEZING and goal_state == THAWED, so unfreeze */
    > case CGROUP_FROZEN:
    > - unfreeze_cgroup(cgroup, freezer);
    > + retval = try_to_freeze_cgroup(cgroup, freezer);
    > break;
    > default:
    > - break;
    > + BUG();
    > }
    > out:
    > spin_unlock_irq(&freezer->lock);


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  12. Re: [PATCH 3/4] freezer_cg: use thaw_process() in unfreeze_cgroup()

    On Tue, 21 Oct 2008 09:16:08 +0200
    Cedric Le Goater wrote:

    > Li Zefan wrote:
    > > Don't duplicate the implementation of thaw_process().

    >
    > looks OK but you should remove __thaw_process() which is unused
    > now.


    It's called by thaw_process().

    But that's the only callsite, I believe, so...

    --- a/include/linux/freezer.h~freezer_cg-use-thaw_process-in-unfreeze_cgroup-fix
    +++ a/include/linux/freezer.h
    @@ -44,11 +44,6 @@ static inline bool should_send_signal(st
    return !(p->flags & PF_FREEZER_NOSIG);
    }

    -/*
    - * Wake up a frozen process
    - */
    -extern int __thaw_process(struct task_struct *p);
    -
    /* Takes and releases task alloc lock using task_lock() */
    extern int thaw_process(struct task_struct *p);

    diff -puN kernel/freezer.c~freezer_cg-use-thaw_process-in-unfreeze_cgroup-fix kernel/freezer.c
    --- a/kernel/freezer.c~freezer_cg-use-thaw_process-in-unfreeze_cgroup-fix
    +++ a/kernel/freezer.c
    @@ -121,16 +121,7 @@ void cancel_freezing(struct task_struct
    }
    }

    -/*
    - * Wake up a frozen process
    - *
    - * task_lock() is needed to prevent the race with refrigerator() which may
    - * occur if the freezing of tasks fails. Namely, without the lock, if the
    - * freezing of tasks failed, thaw_tasks() might have run before a task in
    - * refrigerator() could call frozen_process(), in which case the task would be
    - * frozen and no one would thaw it.
    - */
    -int __thaw_process(struct task_struct *p)
    +static int __thaw_process(struct task_struct *p)
    {
    if (frozen(p)) {
    p->flags &= ~PF_FROZEN;
    @@ -140,6 +131,15 @@ int __thaw_process(struct task_struct *p
    return 0;
    }

    +/*
    + * Wake up a frozen process
    + *
    + * task_lock() is needed to prevent the race with refrigerator() which may
    + * occur if the freezing of tasks fails. Namely, without the lock, if the
    + * freezing of tasks failed, thaw_tasks() might have run before a task in
    + * refrigerator() could call frozen_process(), in which case the task would be
    + * frozen and no one would thaw it.
    + */
    int thaw_process(struct task_struct *p)
    {
    task_lock(p);
    _

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  13. Re: [PATCH 3/4] freezer_cg: use thaw_process() in unfreeze_cgroup()

    Andrew Morton wrote:
    > On Tue, 21 Oct 2008 09:16:08 +0200
    > Cedric Le Goater wrote:
    >
    >> Li Zefan wrote:
    >>> Don't duplicate the implementation of thaw_process().

    >> looks OK but you should remove __thaw_process() which is unused
    >> now.

    >
    > It's called by thaw_process().
    >
    > But that's the only callsite, I believe, so...


    yes it is. it was added by dc52ddc0e6f45b04780b26fc0813509f8e798c42
    and was inline before.

    thanks,

    C.

    >
    > --- a/include/linux/freezer.h~freezer_cg-use-thaw_process-in-unfreeze_cgroup-fix
    > +++ a/include/linux/freezer.h
    > @@ -44,11 +44,6 @@ static inline bool should_send_signal(st
    > return !(p->flags & PF_FREEZER_NOSIG);
    > }
    >
    > -/*
    > - * Wake up a frozen process
    > - */
    > -extern int __thaw_process(struct task_struct *p);
    > -
    > /* Takes and releases task alloc lock using task_lock() */
    > extern int thaw_process(struct task_struct *p);
    >
    > diff -puN kernel/freezer.c~freezer_cg-use-thaw_process-in-unfreeze_cgroup-fix kernel/freezer.c
    > --- a/kernel/freezer.c~freezer_cg-use-thaw_process-in-unfreeze_cgroup-fix
    > +++ a/kernel/freezer.c
    > @@ -121,16 +121,7 @@ void cancel_freezing(struct task_struct
    > }
    > }
    >
    > -/*
    > - * Wake up a frozen process
    > - *
    > - * task_lock() is needed to prevent the race with refrigerator() which may
    > - * occur if the freezing of tasks fails. Namely, without the lock, if the
    > - * freezing of tasks failed, thaw_tasks() might have run before a task in
    > - * refrigerator() could call frozen_process(), in which case the task would be
    > - * frozen and no one would thaw it.
    > - */
    > -int __thaw_process(struct task_struct *p)
    > +static int __thaw_process(struct task_struct *p)
    > {
    > if (frozen(p)) {
    > p->flags &= ~PF_FROZEN;
    > @@ -140,6 +131,15 @@ int __thaw_process(struct task_struct *p
    > return 0;
    > }
    >
    > +/*
    > + * Wake up a frozen process
    > + *
    > + * task_lock() is needed to prevent the race with refrigerator() which may
    > + * occur if the freezing of tasks fails. Namely, without the lock, if the
    > + * freezing of tasks failed, thaw_tasks() might have run before a task in
    > + * refrigerator() could call frozen_process(), in which case the task would be
    > + * frozen and no one would thaw it.
    > + */
    > int thaw_process(struct task_struct *p)
    > {
    > task_lock(p);
    > _
    >
    > --
    > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    > the body of a message to majordomo@vger.kernel.org
    > More majordomo info at http://vger.kernel.org/majordomo-info.html
    > Please read the FAQ at http://www.tux.org/lkml/
    >


    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread