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




    \
     
     \ /
      Last update: 2012-03-27 10:15    [W:0.026 / U:1.504 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site