[PATCH] ib_srpt: wait_for_completion_timeout does not return negativ status

[PATCH] ib_srpt: wait_for_completion_timeout does not return negativ status

[Nicholas Mc Guire]

Signed-off-by: Nicholas Mc Guire <der.herr-kA1LtwSENNE@public.gmane.org>
---

Patch is against 3.19.0-rc3 -next-20150109

Patch was compiletested only with x86_64_defconfig +
CONFIG_TARGET_CORE=m, CONFIG_INFINIBAND=m, CONFIG_INFINIBAND_SRPT=m

 drivers/infiniband/ulp/srpt/ib_srpt.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
index eb694dd..4e58c76 100644
--- a/drivers/infiniband/ulp/srpt/ib_srpt.c
+++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
@@ -3533,7 +3533,7 @@ static void srpt_close_session(struct se_session *se_sess)
 	spin_unlock_irq(&sdev->spinlock);
 
 	res = wait_for_completion_timeout(&release_done, 60 * HZ);
-	WARN_ON(res <= 0);
+	WARN_ON(res == 0);
 }
 
 /**
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ib_srpt: wait_for_completion_timeout does not return negativ status

[Bart Van Assche]

On 01/16/15 12:20, Nicholas Mc Guire wrote:
> Signed-off-by: Nicholas Mc Guire <der.herr-kA1LtwSENNE@public.gmane.org>
> ---
> 
> Patch is against 3.19.0-rc3 -next-20150109
> 
> Patch was compiletested only with x86_64_defconfig +
> CONFIG_TARGET_CORE=m, CONFIG_INFINIBAND=m, CONFIG_INFINIBAND_SRPT=m
> 
>  drivers/infiniband/ulp/srpt/ib_srpt.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
> index eb694dd..4e58c76 100644
> --- a/drivers/infiniband/ulp/srpt/ib_srpt.c
> +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
> @@ -3533,7 +3533,7 @@ static void srpt_close_session(struct se_session *se_sess)
>  	spin_unlock_irq(&sdev->spinlock);
>  
>  	res = wait_for_completion_timeout(&release_done, 60 * HZ);
> -	WARN_ON(res <= 0);
> +	WARN_ON(res == 0);
>  }
>  
>  /**

The description of this patch explains why you would like to change this
code but not why this change is useful. Does building the current code
e.g. trigger a compiler warning ? If so, which warning ? If not, why
would you like to change this code and why do you think this change is
an improvement ?

Bart.

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ib_srpt: wait_for_completion_timeout does not return negativ status

[Nicholas Mc Guire]

On Fri, 16 Jan 2015, Bart Van Assche wrote:

> On 01/16/15 12:20, Nicholas Mc Guire wrote:
> > Signed-off-by: Nicholas Mc Guire <der.herr-kA1LtwSENNE@public.gmane.org>
> > ---
> > 
> > Patch is against 3.19.0-rc3 -next-20150109
> > 
> > Patch was compiletested only with x86_64_defconfig +
> > CONFIG_TARGET_CORE=m, CONFIG_INFINIBAND=m, CONFIG_INFINIBAND_SRPT=m
> > 
> >  drivers/infiniband/ulp/srpt/ib_srpt.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
> > index eb694dd..4e58c76 100644
> > --- a/drivers/infiniband/ulp/srpt/ib_srpt.c
> > +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
> > @@ -3533,7 +3533,7 @@ static void srpt_close_session(struct se_session *se_sess)
> >  	spin_unlock_irq(&sdev->spinlock);
> >  
> >  	res = wait_for_completion_timeout(&release_done, 60 * HZ);
> > -	WARN_ON(res <= 0);
> > +	WARN_ON(res == 0);
> >  }
> >  
> >  /**
> 
> The description of this patch explains why you would like to change this
> code but not why this change is useful. Does building the current code
> e.g. trigger a compiler warning ? If so, which warning ? If not, why
> would you like to change this code and why do you think this change is
> an improvement ?
>
the code will not fail in its urrent form but it will trigger 
static code chekcers (actually this was found by a static code checker)

so its a QA issue  not a bug fix in any way.

thx!
hofrat
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] ib_srpt: wait_for_completion_timeout does not return negativ status

[Nicholas A. Bellinger]

Hi Nicholas,

On Fri, 2015-01-16 at 12:20 +0100, Nicholas Mc Guire wrote:
> Signed-off-by: Nicholas Mc Guire <der.herr@hofr.at>
> ---
> 
> Patch is against 3.19.0-rc3 -next-20150109
> 
> Patch was compiletested only with x86_64_defconfig +
> CONFIG_TARGET_CORE=m, CONFIG_INFINIBAND=m, CONFIG_INFINIBAND_SRPT=m
> 
>  drivers/infiniband/ulp/srpt/ib_srpt.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/ulp/srpt/ib_srpt.c b/drivers/infiniband/ulp/srpt/ib_srpt.c
> index eb694dd..4e58c76 100644
> --- a/drivers/infiniband/ulp/srpt/ib_srpt.c
> +++ b/drivers/infiniband/ulp/srpt/ib_srpt.c
> @@ -3533,7 +3533,7 @@ static void srpt_close_session(struct se_session *se_sess)
>  	spin_unlock_irq(&sdev->spinlock);
>  
>  	res = wait_for_completion_timeout(&release_done, 60 * HZ);
> -	WARN_ON(res <= 0);
> +	WARN_ON(res == 0);
>  }
>  
>  /**

Notice that 'res' here is still incorrectly defined as an 'int', instead
of 'unsigned long'.

I've updated your patch to use the proper type for res, and added a
short commit log to describe what it's doing.

Merged into target-pending/for-next.

Thanks!

--nab