Messages in this thread | | | Date | Tue, 27 Mar 2012 17:43:05 +0200 | From | Oleg Nesterov <> | Subject | Re: [PATCH 5/6] kmod: Add new call_usermodehelper_timeout() API |
| |
Hi Boaz,
I'll read this series tomorrow, but
On 03/26, Boaz Harrosh wrote: > > int call_usermodehelper_exec(struct subprocess_info *sub_info, int wait) > { > DECLARE_COMPLETION_ONSTACK(done); > + int wait_state; > int retval = 0; > > helper_lock(); > @@ -540,19 +541,15 @@ int call_usermodehelper_exec(struct subprocess_info *sub_info, int wait) > if (wait == UMH_NO_WAIT) /* task has freed sub_info */ > goto unlock; > > - if (wait & UMH_KILLABLE) { > - retval = wait_for_completion_killable(&done); > - if (!retval) > - goto wait_done; > - > + wait_state = (wait & UMH_KILLABLE) ? TASK_KILLABLE : 0; > + retval = wait_for_completion_timeout_state(&done, sub_info->timeout, > + wait_state); > + if (unlikely(retval)) { > /* umh_complete() will see NULL and free sub_info */ > if (xchg(&sub_info->complete, NULL)) > goto unlock; > - /* fallthrough, umh_complete() was already called */ > } > > - wait_for_completion(&done);
at first glance this looks certainly wrong, or I misread the patch.
We can't remove the "fallback to wait_for_completion" logic until you move the completion into subprocess_info (the next patch seems to do this).
xchg() can race with umh_complete(). If it returns NULL, umh_complete() was already called and got ->complete != NULL, we must not return until umh_complete() finishes complete().
Oleg.
| |