lkml.org 
[lkml]   [2012]   [Mar]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 4/6 OPTION-A version 3] completion: Add new wait_for_completion_timeout_state
    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.



    \
     
     \ /
      Last update: 2012-03-28 19:49    [W:0.027 / U:0.212 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site