lkml.org 
[lkml]   [2007]   [Apr]   [13]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [PATCH 3/3] make kthread_stop() scalable
    Date
    Oleg Nesterov <oleg@tv-sign.ru> writes:

    > It's a shame kthread_stop() (may take a while!) runs with a global semaphore
    > held. With this patch kthread() allocates all neccesary data (struct kthread)
    > on its own stack, globals kthread_stop_xxx are deleted.

    Oleg so fare you patches have been inspiring. However..

    > HACKS:
    >
    > - re-use task_struct->set_child_tid to point to "struct kthread"

    task_struct->vfork_done is a better cannidate.

    > - use do_exit() directly to preserve "struct kthread" on stack

    Calling do_exit directly like that is not a hack, as it appears the preferred
    way to exit is to call do_exit, or complete_and_exit.

    While this does improve the scalability and remove a global variable. It
    also introduces a complex special case in the form of struct kthread.

    It also doesn't solve the biggest problem with the current kthread interface
    in that calling kthread_stop does not cause the code to break out of
    interruptible sleeps.

    > static int kthread(void *_create)
    > {
    > - struct kthread_create_info *create = _create;
    > - int (*threadfn)(void *data);
    > - void *data;
    > - int ret = -EINTR;
    > + struct kthread self = {
    > + .task = current,
    > + .err = -EINTR,
    > + };
    >
    > /* Copy data: it's on kthread's stack */
    > - threadfn = create->threadfn;
    > - data = create->data;
    > + struct kthread_create_info *create = _create;
    > + int (*threadfn)(void *data) = create->threadfn;
    > + void *data = create->data;
    > +
    > + /*
    > + * This should be enough to assure that self is still on
    > + * stack when we enter do_exit()
    > + */
    > + set_kthread(&self);
    > + create->result = &self;
    >

    > @@ -91,7 +105,7 @@ static void create_kthread(struct kthrea
    >
    > /* We want our own signal handler (we take no signals by default). */
    > pid = kernel_thread(kthread, create, CLONE_FS | CLONE_FILES | SIGCHLD);
    > - create->result = pid;
    > + create->result = ERR_PTR(pid);

    Ouch. You have a nasty race here.

    If kthread runs before kernel_thread returns then setting
    "create->result = ERR_PTR(pid);" could easily stomp
    "create->result = &self".


    Eric

    -
    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-14 01:49    [W:0.022 / U:4.156 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site