Re: [PATCH] wait_for_completion_timeout does not return < 0

From: Sudip Mukherjee
Date: Wed Dec 31 2014 - 05:51:37 EST


On Wed, Dec 31, 2014 at 11:37:11AM +0100, Nicholas Mc Guire wrote:
> On Wed, 31 Dec 2014, Sudip Mukherjee wrote:
>
> > On Wed, Dec 31, 2014 at 04:04:03AM -0500, Nicholas Mc Guire wrote:
> > > This is only removing the comment which is misleading as
> > > wait_for_completion_timeout does not return < 0 thus there
> > > never is anything to be passed on.
> >
> > a small doubt -
> > i am seeing that:
> > unsigned long wait_for_completion_timeout() is calling
> > long wait_for_common() which is again calling
> > long __wait_for_common which is again calling
> > long do_wait_for_common()
> >
> > now the return value from do_wait_for_common can be -ERESTARTSYS,
> > so then what happens when wait_for_completion_timeout return this -ERESTARTSYS as an unsigned value ?
> > it becomes a positive value, and ultimately ctx.result (which is 0) is returned.
> > so then are we just ignoring the error value from do_wait_for_common() ?
> >
>
> ESTARTSYS only can be returned if state matches in do_wait_for_common
> but wait_for_completion_timemout passes TASK_UNINTERRUPTIBLE
> so signal_pending_state will return 0 and thus it will never
> return -ERESTARTSYS.
>
> my understanding of the callchain is:
> wait_for_completion_timemout which is uninterruptible
> -> wait_for_common(...TASK_UNINTERRUPTIBLE)
> -> __wait_for_common(...TASK_UNINTERRUPTIBLE)
> -> do_wait_for_common(...TASK_UNINTERRUPTIBLE)
> -> signal_pending_state(TASK_UNINTERRUPTIBLE...)
>
> static inline int signal_pending_state(long state, struct task_struct *p)
> {
> if (!(state & (TASK_INTERRUPTIBLE | TASK_WAKEKILL)))
> return 0;
>
> so wait_for_completion_timemout should return >=0 only

doubt cleared.

thanks
sudip
>
> thx!
> hofrat
>
> > >
> > > patch is against linux-next 3.19.0-rc1 -next-20141226
> > >
> > > Signed-off-by: Nicholas Mc Guire <der.herr@xxxxxxx>
> > > ---
> > > drivers/net/wimax/i2400m/driver.c | 1 -
> > > 1 file changed, 1 deletion(-)
> > >
> > > diff --git a/drivers/net/wimax/i2400m/driver.c b/drivers/net/wimax/i2400m/driver.c
> > > index 9c78090..0a6384e 100644
> > > --- a/drivers/net/wimax/i2400m/driver.c
> > > +++ b/drivers/net/wimax/i2400m/driver.c
> > > @@ -197,7 +197,6 @@ int i2400m_op_reset(struct wimax_dev *wimax_dev)
> > > result = -ETIMEDOUT;
> > > else if (result > 0)
> > > result = ctx.result;
> > > - /* if result < 0, pass it on */
> > > mutex_lock(&i2400m->init_mutex);
> > > i2400m->reset_ctx = NULL;
> > > mutex_unlock(&i2400m->init_mutex);
> > > --
> > > 1.7.10.4
> > >
> > > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> > > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > > Please read the FAQ at http://www.tux.org/lkml/
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/