lkml.org 
[lkml]   [2007]   [Apr]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [RFC PATCH(experimental) 2/2] Fix freezer-kthread_stop race
    Date
    On Friday, 20 April 2007 23:20, Oleg Nesterov wrote:
    > On 04/20, Gautham R Shenoy wrote:
    > >
    > > On Fri, Apr 20, 2007 at 10:54:36AM +0200, Rafael J. Wysocki wrote:
    > > >
    > > > Hmm, can't we do something like this instead:
    > > >
    > > > ---
    > > > kernel/kthread.c | 10 ++++++++++
    > > > 1 file changed, 10 insertions(+)
    > > >
    > > > Index: linux-2.6.21-rc7/kernel/kthread.c
    > > > ===================================================================
    > > > --- linux-2.6.21-rc7.orig/kernel/kthread.c
    > > > +++ linux-2.6.21-rc7/kernel/kthread.c
    > > > @@ -13,6 +13,7 @@
    > > > #include <linux/file.h>
    > > > #include <linux/module.h>
    > > > #include <linux/mutex.h>
    > > > +#include <linux/freezer.h>
    > > > #include <asm/semaphore.h>
    > > >
    > > > /*
    > > > @@ -232,6 +233,15 @@ int kthread_stop(struct task_struct *k)
    > > >
    > > > /* Now set kthread_should_stop() to true, and wake it up. */
    > > > kthread_stop_info.k = k;
    > > > + if (!(current->flags & PF_NOFREEZE)) {
    > > > + /* If we are freezable, the freezer will wait for us */
    > > > + task_lock(k);
    > > > + k->flags |= PF_NOFREEZE;
    > > > + if (frozen(k))
    > > > + k->flags &= ~PF_FROZEN;
    > > > +
    > > > + task_unlock(k);
    > > > + }
    > >
    > > Yes, we can do this for now since the tasks have only two freeze states,
    > > namely Freezeable and Non Freezeable.
    >
    > No, we can't change k->flags, k owns its ->flags, and it is not atomic.

    Yes, but if we move PF_FROZEN to a separate field in task_struct with
    appropriate locking, then it won't be a problem any more IMO.

    > Rafael, may I suggest you to document task_lock() in thaw_process() ? This
    > looks really confusing, as if task_lock() protects "p->flags &= ~PF_FROZEN".
    >
    > Actually, task_lock() is needed to prevent the race with refrigerator()
    > when the freezing fails, but this is not obvious.

    Sure, I will.

    Greetings,
    Rafael
    -
    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/

    \
     
     \ /
      Last update: 2007-04-20 23:45    [W:0.024 / U:61.740 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site