lkml.org 
[lkml]   [2008]   [Jun]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: wait_for_completion_timeout() spurious failure under heavy load?
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


\
 
 \ /
  Last update: 2008-06-20 16:33    [W:0.055 / U:0.424 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site