Messages in this thread | | | From | (Eric W. Biederman) | Subject | Re: [PATCH 3/3] make kthread_stop() scalable | Date | Fri, 13 Apr 2007 17:44:41 -0600 |
| |
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/
| |