Oleg Nesterov <[EMAIL PROTECTED]> 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 [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
