Re: [PATCH 3/4] kthreads: rework kthread_stop()

[Oleg Nesterov]

On 01/30, Andrew Morton wrote:
>
> On Fri, 30 Jan 2009 13:33:58 +0100
> Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
> 
> >  int kthread_stop(struct task_struct *k)
> >  {
> > +	struct kthread *kthread;
> >  	int ret;
> >  
> > -	mutex_lock(&kthread_stop_lock);
> > -
> > -	/* It could exit after stop_info.k set, but before wake_up_process. */
> > -	get_task_struct(k);
> > -
> >  	trace_sched_kthread_stop(k);
> > +	get_task_struct(k);
> >  
> > -	/* Must init completion *before* thread sees kthread_stop_info.k */
> > -	init_completion(&kthread_stop_info.done);
> > -	smp_wmb();
> > +	kthread = to_kthread(k);
> > +	barrier(); /* it might have exited */
> 
> Why the change from smp_wmb() to plain old barrier()?

These 2 barriers have nothing to do with each other.

Before this patch, smp_wmb() was needed to make sure kthread sees the
initialized &kthread_stop_info.done if it sees kthread_should_stop().

After the patch, this barrier() tells the compiler it must not move
"kthread = to_kthread(k)" down, under the "if (k->vfork_done)" check.
Because k->vfork_done is "volatile", it can be changed under us.

But, once we got the k->vfork_done != NULL we can use it safely, even
if the task exits in parallel. Because we have a reference to task_struct,
and thus to tsk->stack.

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/