On 03/26, Boaz Harrosh wrote: > > +int __sched > +wait_for_completion_timeout_state(struct completion *x, > + unsigned long timeout, int state) > +{ > + long t; > + int ret; > + > + if (!timeout) > + timeout = MAX_SCHEDULE_TIMEOUT; > + > + switch (state) { > + default: > + WARN_ON_ONCE(1); > + /* fall through */ > + case 0: > + state = TASK_UNINTERRUPTIBLE; > + break; Well, this looks strange, imho. If the caller wants TASK_UNINTERRUPTIBLE, it should simply pass TASK_UNINTERRUPTIBLE. wait_for_completion_timeout_state(state => 0) looks confusing, and this is not symmetrical wrt other states. > + t = wait_for_common(x, timeout, state); > + if (likely(t > 0)) { > + ret = 0; > + } else { > + if (t < 0) > + ret = t; > + else > + ret = -ETIMEDOUT; > + } > + return ret; I tend to agree with Peter. This is the common helper, probably it will have more users. We shouldn't throw out the positive return value, it can be useful. call_usermodehelper_exec() can simply do retval = wait_for_common(...); if (retval > 0) retval = sub_info->retval; else if (!retval) retval = -ETIMEDOUT; Oleg. -- To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html