On 04/13, Eric W. Biederman wrote:
>
> 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.
I can't say I agree. I thought it is good to have a struct which represents
a kernel thread. Actually, I thought we can have __kthread_create() which
returns "struct kthread". May be I am wrong, because yes, ->set_child_tid can
point right to completion, and we can use some TIF flag instead of ->should_stop.
This needs to update a lot of include/asm/ files.
> 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.
Hm? kthread_stop() does wake_up_process(), it wakes up TASK_INTERRUPTIBLE tasks.
> > @@ -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".
Yes, thanks... Can't understand how I was soooo stupid!!! thanks...
Damn. We don't need 2 completions! just one.
create_kthread:
pid = kernel_thread(...);
if (pid < 0) {
create->result = ERR_PTR(pid);
complete(create->started);
}
// else: kthread() will do complete()
return;
Oleg.
-
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/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
