Messages in this thread | | | Date | Fri, 17 Feb 2012 15:44:58 +0100 | From | Oleg Nesterov <> | Subject | Re: [PATCH 2/4] vfork: make it killable |
| |
On 02/16, Andrew Morton wrote: > > On Thu, 16 Feb 2012 18:27:06 +0100 > Oleg Nesterov <oleg@redhat.com> wrote: > > > --- a/kernel/fork.c > > +++ b/kernel/fork.c > > @@ -669,10 +669,34 @@ struct mm_struct *mm_access(struct task_struct *task, unsigned int mode) > > > > void complete_vfork_done(struct task_struct *tsk) > > { > > - struct completion *vfork_done = tsk->vfork_done; > > + struct completion *vfork; > > > > - tsk->vfork_done = NULL; > > - complete(vfork_done); > > + task_lock(tsk); > > + vfork = tsk->vfork_done; > > + if (likely(vfork)) { > > + tsk->vfork_done = NULL; > > + complete(vfork); > > + } > > + task_unlock(tsk); > > +} > > OK, so now we don't need to test tsk->vfork_done in callers. But > mm_release() still does this, and it does it outside locks.
Yes, complete_vfork_done() can be called unconditionally,
> Mistake, > or micro-optimisation?
micro-optimisation to avoid the unnecessary task_lock().
> If the latter, why is the lockless peek > race-free?
If ->vfork_done != NULL, the child can never miss it. The parent sets this pointer before the first wakeup.
However. The killed parent can clear ->vfork_done, see "if (killed)" int wait_for_vfork_done(). That is why complete_vfork_done() should re-check under task_lock().
Oleg.
| |