On 06/20, Oleg Nesterov wrote:
>
> On 06/20, Ingo Molnar wrote:
> >
> > --- a/kernel/sched.c
> > +++ b/kernel/sched.c
> > @@ -4405,6 +4405,16 @@ do_wait_for_common(struct completion *x, long
> > timeout, int state)
> > spin_unlock_irq(&x->wait.lock);
> > timeout = schedule_timeout(timeout);
> > spin_lock_irq(&x->wait.lock);
> > +
> > + /*
> > + * If the completion has arrived meanwhile
> > + * then return 1 jiffy time left:
> > + */
> > + if (x->done && !timeout) {
> > + timeout = 1;
> > + break;
> > + }
> > +
> > if (!timeout) {
> > __remove_wait_queue(&x->wait, &wait);
> > return timeout;
>
> This is the real nitpick, but I wonder what is the right behaviour
> of wait_for_completion_timeout(x, 0) when x->done != 0. Perhaps we
> can return 1 in that case too, just for the consistency?
>
> IOW, how about the patch below? this also makes the code a bit
> simpler because we factor out __remove_wait_queue().
Even better, we can kill the first __remove_wait_queue() as well.
Oleg.
--- kernel/sched.c
+++ kernel/sched.c
@@ -4739,22 +4739,20 @@ do_wait_for_common(struct completion *x,
signal_pending(current)) ||
(state == TASK_KILLABLE &&
fatal_signal_pending(current))) {
- __remove_wait_queue(&x->wait, &wait);
- return -ERESTARTSYS;
+ timeout = -ERESTARTSYS;
+ break;
}
__set_current_state(state);
spin_unlock_irq(&x->wait.lock);
timeout = schedule_timeout(timeout);
spin_lock_irq(&x->wait.lock);
- if (!timeout) {
- __remove_wait_queue(&x->wait, &wait);
- return timeout;
- }
- } while (!x->done);
+ } while (!x->done && timeout);
__remove_wait_queue(&x->wait, &wait);
+ if (!x->done)
+ return timeout;
}
x->done--;
- return timeout;
+ return timeout ?: 1;
}
static long __sched
_______________________________________________
general mailing list
[email protected]
http://lists.openfabrics.org/cgi-bin/mailman/listinfo/general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
