Messages in this thread | | | Date | Fri, 29 Jul 2011 16:32:54 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH 3/8] vfork: make it killable |
| |
On 07/29, Matt Fleming wrote: > > On Wed, 2011-07-27 at 18:32 +0200, Oleg Nesterov wrote: > > [...] > > > static long clone_vfork_finish(struct task_struct *child, > > struct completion *vfork_done, long pid) > > { > > - freezer_do_not_count(); > > - wait_for_completion(vfork_done); > > - freezer_count(); > > + int killed = wait_for_completion_killable(vfork_done); > > + > > + if (killed) { > > + struct completion *steal = xchg(&child->vfork_done, NULL); > > + /* if we race with complete_vfork_done() we have to wait */ > > + if (unlikely(!steal)) > > + wait_for_completion(vfork_done); > > + > > + return -EINTR; > > + } > > Hmm.. isn't this inherently racy anyway? Why go to the trouble of trying > to handle this race at all?
Suppose the child does xchg() and sees vfork_done != NULL. In this case the parent shouldn't return from do_fork() until the child does complete(), this "struct completion" was allocated on parent's stack.
OK, I am starting to agree this looks overcomplicated, task_lock() can make the code look simpler (see 0/8).
Oleg.
| |