* [PATCH] wait_for_completion_timeout does not return < 0
@ 2014-12-31 9:04 Nicholas Mc Guire
2014-12-31 10:31 ` Sudip Mukherjee
0 siblings, 1 reply; 4+ messages in thread
From: Nicholas Mc Guire @ 2014-12-31 9:04 UTC (permalink / raw)
To: Inaky Perez-Gonzalez; +Cc: linux-wimax, netdev, linux-kernel, Nicholas Mc Guire
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.
patch is against linux-next 3.19.0-rc1 -next-20141226
Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] wait_for_completion_timeout does not return < 0
2014-12-31 9:04 [PATCH] wait_for_completion_timeout does not return < 0 Nicholas Mc Guire
@ 2014-12-31 10:31 ` Sudip Mukherjee
2014-12-31 10:37 ` Nicholas Mc Guire
0 siblings, 1 reply; 4+ messages in thread
From: Sudip Mukherjee @ 2014-12-31 10:31 UTC (permalink / raw)
To: Nicholas Mc Guire; +Cc: Inaky Perez-Gonzalez, linux-wimax, netdev, linux-kernel
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() ?
sudip
>
> patch is against linux-next 3.19.0-rc1 -next-20141226
>
> Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
> ---
> 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] wait_for_completion_timeout does not return < 0
2014-12-31 10:31 ` Sudip Mukherjee
@ 2014-12-31 10:37 ` Nicholas Mc Guire
2014-12-31 10:50 ` Sudip Mukherjee
0 siblings, 1 reply; 4+ messages in thread
From: Nicholas Mc Guire @ 2014-12-31 10:37 UTC (permalink / raw)
To: Sudip Mukherjee; +Cc: Inaky Perez-Gonzalez, linux-wimax, netdev, linux-kernel
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
thx!
hofrat
> >
> > patch is against linux-next 3.19.0-rc1 -next-20141226
> >
> > Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
> > ---
> > 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@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] wait_for_completion_timeout does not return < 0
2014-12-31 10:37 ` Nicholas Mc Guire
@ 2014-12-31 10:50 ` Sudip Mukherjee
0 siblings, 0 replies; 4+ messages in thread
From: Sudip Mukherjee @ 2014-12-31 10:50 UTC (permalink / raw)
To: Nicholas Mc Guire; +Cc: Inaky Perez-Gonzalez, linux-wimax, netdev, linux-kernel
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@hofr.at>
> > > ---
> > > 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@vger.kernel.org
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > > Please read the FAQ at http://www.tux.org/lkml/
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-12-31 10:51 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-12-31 9:04 [PATCH] wait_for_completion_timeout does not return < 0 Nicholas Mc Guire
2014-12-31 10:31 ` Sudip Mukherjee
2014-12-31 10:37 ` Nicholas Mc Guire
2014-12-31 10:50 ` Sudip Mukherjee
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).