lkml.org 
[lkml]   [2007]   [Apr]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    SubjectRe: [RFC PATCH(experimental) 2/2] Fix freezer-kthread_stop race
    Date
    On Thursday, 19 April 2007 23:31, Andrew Morton wrote:
    > On Thu, 19 Apr 2007 17:34:19 +0530
    > Gautham R Shenoy <ego@in.ibm.com> wrote:
    >
    > > Threads which wait for completion on a frozen thread might result in
    > > causing the freezer to fail, if the waiting thread is freezeable.
    > >
    > > There are some well known cases where it's preferable to temporarily thaw
    > > the frozen process, finish the wait for completion and allow both the
    > > processes to call try_to_freeze.
    > >
    > > kthread_stop is one such case.
    >
    > hm.
    >
    > > flush_workqueue might be another.
    >
    > flush_workqueue() just needs to die. I think there are (almost) no
    > legitimate users of it once cancel_work_sync() is merged.
    >
    > > This patch attempts to address such a situation with a fix for kthread_stop.
    >
    > Via wholly undescribed means :(

    Yeah, I have the same problem with it. :-)

    > > Strictly experimental. Compile tested on i386.
    >
    > Rather than doing <whatever you did>, perhaps we could make the freezing
    > process a dual-pass thing. On pass 1, mark all the threads as "we'll be
    > freezing you soon" and on the second pass, do the actual freezing. Then,
    > in problematic places such as kthread_stop() we can look to see if we'll
    > soon be asked to freeze and if so, run try_to_freeze().
    >
    > Of course, running try_to_freeze() in kthread_stop() would be very wrong,
    > so we'd actually need to do it in callers, preferably via a new
    > kthread_stop_freezeable() wrapper.
    >
    > And the two-pass-freeze thing is of course racy. It's also unnecessary:
    > setting a flag on every task in the machine is equivalent to setting a
    > global variable. So perhaps just use a global variable?
    >
    > int kthread_stop_freezeable(struct task_struct *k)
    > {
    > if (freeze_state == ABOUT_TO_START) {
    > wait_for(freeze_state == STARTED);
    > try_to_freeze();
    > }
    > kthread_stop(k);
    > }
    >
    > which is theoretically racy if another freeze_processes() starts
    > immediately. Anyway - please have a think about it ;)

    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);
    + }
    wake_up_process(k);
    put_task_struct(k);

    -
    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 10:53    [W:0.025 / U:98.296 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site