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    [from the cache]
©2003-2011 Jasper Spaans. Advertise on this site