On Mon, 2012-03-26 at 19:33 -0700, 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; > + case TASK_KILLABLE: > + case TASK_INTERRUPTIBLE: > + break; > + } Don't do this, just pass in TASK_UNINTERRUPTIBLE (or TASK_NORMAL) whatever you like/need. Don't fudge it by over-loading TASK_RUNNING (aka. 0). > + t = wait_for_common(x, timeout, state); > + if (likely(t > 0)) { > + ret = 0; > + } else { > + if (t < 0) > + ret = t; > + else > + ret = -ETIMEDOUT; > + } > + return ret; Again, why wreck an entirely reasonable return code and break with the rest of the API. > +} > +EXPORT_SYMBOL(wait_for_completion_timeout_state); So I'm fine with adding wait_for_completion_timeout_state(), but make it look and smell like wait_for_completion_timeout() and use a proper state, like wake_up_state(). IOW: unsigned long __sched wait_for_completion_timeout_state(struct completion *x, unsigned long timeout, unsigned int state) { return wait_for_common(x, timeout, state); } EXPORT_SYMBOL(wait_for_completion_timeout_state); -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html