[PATCH 1/1] InfiniBand: Deletion of unnecessary checks before two function calls

[PATCH 1/1] InfiniBand: Deletion of unnecessary checks before two function calls

[SF Markus Elfring]

From: Markus Elfring <elfring-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
Date: Wed, 19 Nov 2014 19:19:21 +0100

The functions kfree() and pci_dev_put() test whether their argument is NULL
and then return immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
---
 drivers/infiniband/hw/mlx4/main.c         | 3 +--
 drivers/infiniband/hw/mthca/mthca_reset.c | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
index 8b72cf3..50dee1a 100644
--- a/drivers/infiniband/hw/mlx4/main.c
+++ b/drivers/infiniband/hw/mlx4/main.c
@@ -2512,8 +2512,7 @@ static void do_slave_init(struct mlx4_ib_dev *ibdev, int slave, int do_init)
 		if (!dm[i]) {
 			pr_err("failed to allocate memory for tunneling qp update work struct\n");
 			for (i = 0; i < dev->caps.num_ports; i++) {
-				if (dm[i])
-					kfree(dm[i]);
+				kfree(dm[i]);
 			}
 			goto out;
 		}
diff --git a/drivers/infiniband/hw/mthca/mthca_reset.c b/drivers/infiniband/hw/mthca/mthca_reset.c
index 74c6a94..c521654 100644
--- a/drivers/infiniband/hw/mthca/mthca_reset.c
+++ b/drivers/infiniband/hw/mthca/mthca_reset.c
@@ -279,8 +279,7 @@ good:
 	}
 
 out:
-	if (bridge)
-		pci_dev_put(bridge);
+	pci_dev_put(bridge);
 	kfree(bridge_header);
 	kfree(hca_header);
 
-- 
2.1.3

--
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

[PATCH] net: Mellanox: Delete unnecessary checks before the function call "vunmap"

[SF Markus Elfring]

From: Markus Elfring <elfring-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
Date: Wed, 4 Feb 2015 15:17:00 +0100

The vunmap() function performs also input parameter validation.
Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
---
 drivers/net/ethernet/mellanox/mlx4/alloc.c      | 2 +-
 drivers/net/ethernet/mellanox/mlx5/core/alloc.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/alloc.c b/drivers/net/ethernet/mellanox/mlx4/alloc.c
index 963dd7e..06faa51 100644
--- a/drivers/net/ethernet/mellanox/mlx4/alloc.c
+++ b/drivers/net/ethernet/mellanox/mlx4/alloc.c
@@ -660,7 +660,7 @@ void mlx4_buf_free(struct mlx4_dev *dev, int size, struct mlx4_buf *buf)
 		dma_free_coherent(&dev->pdev->dev, size, buf->direct.buf,
 				  buf->direct.map);
 	else {
-		if (BITS_PER_LONG == 64 && buf->direct.buf)
+		if (BITS_PER_LONG == 64)
 			vunmap(buf->direct.buf);
 
 		for (i = 0; i < buf->nbufs; ++i)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/alloc.c b/drivers/net/ethernet/mellanox/mlx5/core/alloc.c
index 56779c1..201ca6d 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/alloc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/alloc.c
@@ -121,7 +121,7 @@ void mlx5_buf_free(struct mlx5_core_dev *dev, struct mlx5_buf *buf)
 		dma_free_coherent(&dev->pdev->dev, buf->size, buf->direct.buf,
 				  buf->direct.map);
 	else {
-		if (BITS_PER_LONG == 64 && buf->direct.buf)
+		if (BITS_PER_LONG == 64)
 			vunmap(buf->direct.buf);
 
 		for (i = 0; i < buf->nbufs; i++)
-- 
2.2.2

--
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] net: Mellanox: Delete unnecessary checks before the function call "vunmap"

[Eli Cohen]

Acked-by: Eli Cohen <eli@mellanox.com>

On Wed, Feb 04, 2015 at 03:22:33PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Wed, 4 Feb 2015 15:17:00 +0100
> 
> The vunmap() function performs also input parameter validation.
> Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Re: [PATCH] net: Mellanox: Delete unnecessary checks before the function call "vunmap"

[David Miller]

From: SF Markus Elfring <elfring-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
Date: Wed, 04 Feb 2015 15:22:33 +0100

> From: Markus Elfring <elfring-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
> Date: Wed, 4 Feb 2015 15:17:00 +0100
> 
> The vunmap() function performs also input parameter validation.
> Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>

This does not apply cleanly to the net-next tree, please respin.
--
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

[PATCH] InfiniBand: Delete unnecessary checks before the function, call "srp_destroy_fr_pool"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 5 Feb 2015 13:20:42 +0100

The srp_destroy_fr_pool() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/ulp/srp/ib_srp.c | 12 +++++-------
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
index 0747c05..6f5dfa1 100644
--- a/drivers/infiniband/ulp/srp/ib_srp.c
+++ b/drivers/infiniband/ulp/srp/ib_srp.c
@@ -549,8 +549,7 @@ static int srp_create_ch_ib(struct srp_rdma_ch *ch)
 				     "FR pool allocation failed (%d)\n", ret);
 			goto err_qp;
 		}
-		if (ch->fr_pool)
-			srp_destroy_fr_pool(ch->fr_pool);
+		srp_destroy_fr_pool(ch->fr_pool);
 		ch->fr_pool = fr_pool;
 	} else if (!dev->use_fast_reg && dev->has_fmr) {
 		fmr_pool = srp_alloc_fmr_pool(target);
@@ -615,13 +614,12 @@ static void srp_free_ch_ib(struct srp_target_port *target,
 	if (!ch->qp)
 		return;
 
-	if (dev->use_fast_reg) {
-		if (ch->fr_pool)
-			srp_destroy_fr_pool(ch->fr_pool);
-	} else {
+	if (dev->use_fast_reg)
+		srp_destroy_fr_pool(ch->fr_pool);
+	else
 		if (ch->fmr_pool)
 			ib_destroy_fmr_pool(ch->fmr_pool);
-	}
+
 	srp_destroy_qp(ch);
 	ib_destroy_cq(ch->send_cq);
 	ib_destroy_cq(ch->recv_cq);
-- 
2.2.2

Re: [PATCH] InfiniBand: Delete unnecessary checks before the function, call "srp_destroy_fr_pool"

[Bart Van Assche]

On 02/05/15 13:26, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 5 Feb 2015 13:20:42 +0100
> 
> The srp_destroy_fr_pool() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/infiniband/ulp/srp/ib_srp.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/infiniband/ulp/srp/ib_srp.c b/drivers/infiniband/ulp/srp/ib_srp.c
> index 0747c05..6f5dfa1 100644
> --- a/drivers/infiniband/ulp/srp/ib_srp.c
> +++ b/drivers/infiniband/ulp/srp/ib_srp.c
> @@ -549,8 +549,7 @@ static int srp_create_ch_ib(struct srp_rdma_ch *ch)
>  				     "FR pool allocation failed (%d)\n", ret);
>  			goto err_qp;
>  		}
> -		if (ch->fr_pool)
> -			srp_destroy_fr_pool(ch->fr_pool);
> +		srp_destroy_fr_pool(ch->fr_pool);
>  		ch->fr_pool = fr_pool;
>  	} else if (!dev->use_fast_reg && dev->has_fmr) {
>  		fmr_pool = srp_alloc_fmr_pool(target);
> @@ -615,13 +614,12 @@ static void srp_free_ch_ib(struct srp_target_port *target,
>  	if (!ch->qp)
>  		return;
>  
> -	if (dev->use_fast_reg) {
> -		if (ch->fr_pool)
> -			srp_destroy_fr_pool(ch->fr_pool);
> -	} else {
> +	if (dev->use_fast_reg)
> +		srp_destroy_fr_pool(ch->fr_pool);
> +	else
>  		if (ch->fmr_pool)
>  			ib_destroy_fmr_pool(ch->fmr_pool);
> -	}
> +
>  	srp_destroy_qp(ch);
>  	ib_destroy_cq(ch->send_cq);
>  	ib_destroy_cq(ch->recv_cq);

This patch makes the code in srp_free_ch_ib() asymmetric which is
unfortunate. Please add a test in ib_destroy_fmr_pool() such that the
check before ib_destroy_fmr_pool() can also be eliminated.

Bart.

[PATCH v2] net: Mellanox: Delete unnecessary checks before the function call "vunmap"

[SF Markus Elfring]

From: Markus Elfring <elfring-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
Date: Mon, 9 Feb 2015 11:10:41 +0100

The vunmap() function performs also input parameter validation.
Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
Acked-by: Eli Cohen <eli-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>
---
 drivers/net/ethernet/mellanox/mlx4/alloc.c      | 2 +-
 drivers/net/ethernet/mellanox/mlx5/core/alloc.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/mellanox/mlx4/alloc.c b/drivers/net/ethernet/mellanox/mlx4/alloc.c
index a716c26..0c51c69 100644
--- a/drivers/net/ethernet/mellanox/mlx4/alloc.c
+++ b/drivers/net/ethernet/mellanox/mlx4/alloc.c
@@ -662,7 +662,7 @@ void mlx4_buf_free(struct mlx4_dev *dev, int size, struct mlx4_buf *buf)
 				  buf->direct.buf,
 				  buf->direct.map);
 	else {
-		if (BITS_PER_LONG == 64 && buf->direct.buf)
+		if (BITS_PER_LONG == 64)
 			vunmap(buf->direct.buf);
 
 		for (i = 0; i < buf->nbufs; ++i)
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/alloc.c b/drivers/net/ethernet/mellanox/mlx5/core/alloc.c
index 56779c1..201ca6d 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/alloc.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/alloc.c
@@ -121,7 +121,7 @@ void mlx5_buf_free(struct mlx5_core_dev *dev, struct mlx5_buf *buf)
 		dma_free_coherent(&dev->pdev->dev, buf->size, buf->direct.buf,
 				  buf->direct.map);
 	else {
-		if (BITS_PER_LONG == 64 && buf->direct.buf)
+		if (BITS_PER_LONG == 64)
 			vunmap(buf->direct.buf);
 
 		for (i = 0; i < buf->nbufs; i++)
-- 
2.2.2

--
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 v2] net: Mellanox: Delete unnecessary checks before the function call "vunmap"

[David Miller]

From: SF Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 09 Feb 2015 11:21:26 +0100

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 9 Feb 2015 11:10:41 +0100
> 
> The vunmap() function performs also input parameter validation.
> Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> Acked-by: Eli Cohen <eli@mellanox.com>

Applied.

Re: [PATCH] InfiniBand: Deletion of unnecessary checks before two function calls

[SF Markus Elfring]

> From: Markus Elfring <elfring-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
> Date: Wed, 19 Nov 2014 19:19:21 +0100
> 
> The functions kfree() and pci_dev_put() test whether their argument is NULL
> and then return immediately. Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
> ---
>  drivers/infiniband/hw/mlx4/main.c         | 3 +--
>  drivers/infiniband/hw/mthca/mthca_reset.c | 3 +--
>  2 files changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/mlx4/main.c b/drivers/infiniband/hw/mlx4/main.c
> index 8b72cf3..50dee1a 100644
> --- a/drivers/infiniband/hw/mlx4/main.c
> +++ b/drivers/infiniband/hw/mlx4/main.c
> @@ -2512,8 +2512,7 @@ static void do_slave_init(struct mlx4_ib_dev *ibdev, int slave, int do_init)
>  		if (!dm[i]) {
>  			pr_err("failed to allocate memory for tunneling qp update work struct\n");
>  			for (i = 0; i < dev->caps.num_ports; i++) {
> -				if (dm[i])
> -					kfree(dm[i]);
> +				kfree(dm[i]);
>  			}
>  			goto out;
>  		}
> diff --git a/drivers/infiniband/hw/mthca/mthca_reset.c b/drivers/infiniband/hw/mthca/mthca_reset.c
> index 74c6a94..c521654 100644
> --- a/drivers/infiniband/hw/mthca/mthca_reset.c
> +++ b/drivers/infiniband/hw/mthca/mthca_reset.c
> @@ -279,8 +279,7 @@ good:
>  	}
>  
>  out:
> -	if (bridge)
> -		pci_dev_put(bridge);
> +	pci_dev_put(bridge);
>  	kfree(bridge_header);
>  	kfree(hca_header);
>  
> 

More software developers became interested in similar
source code improvements.

infiniband: Remove redundant NULL check before kfree
https://lkml.org/lkml/2015/6/26/45
https://www.mail-archive.com/linux-kernel%40vger.kernel.org/msg919966.html

Regards,
Markus
--
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

[PATCH] staging/rdma/hfi1: Delete unnecessary checks before two function calls

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 5 Nov 2015 08:41:00 +0100

The functions "sc_return_credits" and "vfree" perform also input
parameter validation. Thus the tests around their calls are not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/staging/rdma/hfi1/file_ops.c  | 2 +-
 drivers/staging/rdma/hfi1/user_sdma.c | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/rdma/hfi1/file_ops.c b/drivers/staging/rdma/hfi1/file_ops.c
index aae9826..204d1d0 100644
--- a/drivers/staging/rdma/hfi1/file_ops.c
+++ b/drivers/staging/rdma/hfi1/file_ops.c
@@ -313,7 +313,7 @@ static ssize_t hfi1_file_write(struct file *fp, const char __user *data,
 	case HFI1_CMD_SDMA_STATUS_UPD:
 		break;
 	case HFI1_CMD_CREDIT_UPD:
-		if (uctxt && uctxt->sc)
+		if (uctxt)
 			sc_return_credits(uctxt->sc);
 		break;
 	case HFI1_CMD_TID_UPDATE:
diff --git a/drivers/staging/rdma/hfi1/user_sdma.c b/drivers/staging/rdma/hfi1/user_sdma.c
index 36c838d..45f3797 100644
--- a/drivers/staging/rdma/hfi1/user_sdma.c
+++ b/drivers/staging/rdma/hfi1/user_sdma.c
@@ -473,8 +473,7 @@ int hfi1_user_sdma_free_queues(struct hfi1_filedata *fd)
 		fd->pq = NULL;
 	}
 	if (fd->cq) {
-		if (fd->cq->comps)
-			vfree(fd->cq->comps);
+		vfree(fd->cq->comps);
 		kfree(fd->cq);
 		fd->cq = NULL;
 	}
-- 
2.6.2

Re: [PATCH] staging/rdma/hfi1: Delete unnecessary checks before two function calls

[Dan Carpenter]

On Thu, Nov 05, 2015 at 09:10:47AM +0100, SF Markus Elfring wrote:
> diff --git a/drivers/staging/rdma/hfi1/file_ops.c b/drivers/staging/rdma/hfi1/file_ops.c
> index aae9826..204d1d0 100644
> --- a/drivers/staging/rdma/hfi1/file_ops.c
> +++ b/drivers/staging/rdma/hfi1/file_ops.c
> @@ -313,7 +313,7 @@ static ssize_t hfi1_file_write(struct file *fp, const char __user *data,
>  	case HFI1_CMD_SDMA_STATUS_UPD:
>  		break;
>  	case HFI1_CMD_CREDIT_UPD:
> -		if (uctxt && uctxt->sc)
> +		if (uctxt)
>  			sc_return_credits(uctxt->sc);

Relying on hidden sanity checks makes the code harder to read.

regards,
dan carpenter

[PATCH] staging-EHCA: Delete unnecessary checks before the function call "kmem_cache_destroy"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 15 Nov 2015 21:58:45 +0100

The kmem_cache_destroy() function tests whether its argument is NULL
and then returns immediately. Thus the test around the calls is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/staging/rdma/ehca/ehca_av.c   | 3 +--
 drivers/staging/rdma/ehca/ehca_cq.c   | 3 +--
 drivers/staging/rdma/ehca/ehca_main.c | 3 +--
 drivers/staging/rdma/ehca/ehca_mrmw.c | 6 ++----
 drivers/staging/rdma/ehca/ehca_pd.c   | 3 +--
 drivers/staging/rdma/ehca/ehca_qp.c   | 3 +--
 6 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/rdma/ehca/ehca_av.c b/drivers/staging/rdma/ehca/ehca_av.c
index 4659263..04b9398 100644
--- a/drivers/staging/rdma/ehca/ehca_av.c
+++ b/drivers/staging/rdma/ehca/ehca_av.c
@@ -272,6 +272,5 @@ int ehca_init_av_cache(void)
 
 void ehca_cleanup_av_cache(void)
 {
-	if (av_cache)
-		kmem_cache_destroy(av_cache);
+	kmem_cache_destroy(av_cache);
 }
diff --git a/drivers/staging/rdma/ehca/ehca_cq.c b/drivers/staging/rdma/ehca/ehca_cq.c
index ea1b5c1..1aa7931 100644
--- a/drivers/staging/rdma/ehca/ehca_cq.c
+++ b/drivers/staging/rdma/ehca/ehca_cq.c
@@ -393,6 +393,5 @@ int ehca_init_cq_cache(void)
 
 void ehca_cleanup_cq_cache(void)
 {
-	if (cq_cache)
-		kmem_cache_destroy(cq_cache);
+	kmem_cache_destroy(cq_cache);
 }
diff --git a/drivers/staging/rdma/ehca/ehca_main.c b/drivers/staging/rdma/ehca/ehca_main.c
index 8246418..860b974 100644
--- a/drivers/staging/rdma/ehca/ehca_main.c
+++ b/drivers/staging/rdma/ehca/ehca_main.c
@@ -245,8 +245,7 @@ static void ehca_destroy_slab_caches(void)
 	ehca_cleanup_cq_cache();
 	ehca_cleanup_pd_cache();
 #ifdef CONFIG_PPC_64K_PAGES
-	if (ctblk_cache)
-		kmem_cache_destroy(ctblk_cache);
+	kmem_cache_destroy(ctblk_cache);
 #endif
 }
 
diff --git a/drivers/staging/rdma/ehca/ehca_mrmw.c b/drivers/staging/rdma/ehca/ehca_mrmw.c
index f914b30..553e883 100644
--- a/drivers/staging/rdma/ehca/ehca_mrmw.c
+++ b/drivers/staging/rdma/ehca/ehca_mrmw.c
@@ -2251,10 +2251,8 @@ int ehca_init_mrmw_cache(void)
 
 void ehca_cleanup_mrmw_cache(void)
 {
-	if (mr_cache)
-		kmem_cache_destroy(mr_cache);
-	if (mw_cache)
-		kmem_cache_destroy(mw_cache);
+	kmem_cache_destroy(mr_cache);
+	kmem_cache_destroy(mw_cache);
 }
 
 static inline int ehca_init_top_bmap(struct ehca_top_bmap *ehca_top_bmap,
diff --git a/drivers/staging/rdma/ehca/ehca_pd.c b/drivers/staging/rdma/ehca/ehca_pd.c
index 351577a..2a8aae4 100644
--- a/drivers/staging/rdma/ehca/ehca_pd.c
+++ b/drivers/staging/rdma/ehca/ehca_pd.c
@@ -119,6 +119,5 @@ int ehca_init_pd_cache(void)
 
 void ehca_cleanup_pd_cache(void)
 {
-	if (pd_cache)
-		kmem_cache_destroy(pd_cache);
+	kmem_cache_destroy(pd_cache);
 }
diff --git a/drivers/staging/rdma/ehca/ehca_qp.c b/drivers/staging/rdma/ehca/ehca_qp.c
index 2e89356..896c01f 100644
--- a/drivers/staging/rdma/ehca/ehca_qp.c
+++ b/drivers/staging/rdma/ehca/ehca_qp.c
@@ -2252,6 +2252,5 @@ int ehca_init_qp_cache(void)
 
 void ehca_cleanup_qp_cache(void)
 {
-	if (qp_cache)
-		kmem_cache_destroy(qp_cache);
+	kmem_cache_destroy(qp_cache);
 }
-- 
2.6.2

[PATCH] IB/hfi1: Delete an unnecessary check before the function call "sc_return_credits"

[SF Markus Elfring]

From: Markus Elfring <elfring-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
Date: Sat, 23 Jul 2016 08:30:52 +0200

The sc_return_credits() function tests whether its argument is NULL
and then returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
---
 drivers/infiniband/hw/hfi1/file_ops.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/hfi1/file_ops.c b/drivers/infiniband/hw/hfi1/file_ops.c
index c702a00..32c19fa 100644
--- a/drivers/infiniband/hw/hfi1/file_ops.c
+++ b/drivers/infiniband/hw/hfi1/file_ops.c
@@ -228,7 +228,7 @@ static long hfi1_file_ioctl(struct file *fp, unsigned int cmd,
 				    sizeof(struct hfi1_base_info));
 		break;
 	case HFI1_IOCTL_CREDIT_UPD:
-		if (uctxt && uctxt->sc)
+		if (uctxt)
 			sc_return_credits(uctxt->sc);
 		break;
 
-- 
2.9.2

--
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

[PATCH 0/3] IB/mthca: Fine-tuning for mthca_reset()

[SF Markus Elfring]

From: Markus Elfring <elfring-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
Date: Sat, 23 Jul 2016 12:50:12 +0200

Further update suggestions were taken into account
after a patch was applied from static source code analysis.

Markus Elfring (3):
  Delete an unnecessary check before the function call "pci_dev_put"
  Less function calls after error detection
  Delete unnecessary variable initialisations

 drivers/infiniband/hw/mthca/mthca_reset.c | 48 +++++++++++++++----------------
 1 file changed, 23 insertions(+), 25 deletions(-)

-- 
2.9.2

--
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

[PATCH 1/3] IB/mthca: Delete an unnecessary check before the function call "pci_dev_put"

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 23 Jul 2016 10:05:12 +0200

The pci_dev_put() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/hw/mthca/mthca_reset.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/mthca/mthca_reset.c b/drivers/infiniband/hw/mthca/mthca_reset.c
index 74c6a94..c521654 100644
--- a/drivers/infiniband/hw/mthca/mthca_reset.c
+++ b/drivers/infiniband/hw/mthca/mthca_reset.c
@@ -279,8 +279,7 @@ good:
 	}
 
 out:
-	if (bridge)
-		pci_dev_put(bridge);
+	pci_dev_put(bridge);
 	kfree(bridge_header);
 	kfree(hca_header);
 
-- 
2.9.2

[PATCH 2/3] IB/mthca: Less function calls in mthca_reset() after error detection

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 23 Jul 2016 11:28:30 +0200

The kfree() function was called in a few cases by the mthca_reset()
function during error handling even if the passed variables "bridge_header"
and "hca_header" contained a null pointer.

Adjust jump targets according to the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/hw/mthca/mthca_reset.c | 41 +++++++++++++++----------------
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/drivers/infiniband/hw/mthca/mthca_reset.c b/drivers/infiniband/hw/mthca/mthca_reset.c
index c521654..6727af2 100644
--- a/drivers/infiniband/hw/mthca/mthca_reset.c
+++ b/drivers/infiniband/hw/mthca/mthca_reset.c
@@ -98,7 +98,7 @@ int mthca_reset(struct mthca_dev *mdev)
 		err = -ENOMEM;
 		mthca_err(mdev, "Couldn't allocate memory to save HCA "
 			  "PCI header, aborting.\n");
-		goto out;
+		goto put_dev;
 	}
 
 	for (i = 0; i < 64; ++i) {
@@ -108,7 +108,7 @@ int mthca_reset(struct mthca_dev *mdev)
 			err = -ENODEV;
 			mthca_err(mdev, "Couldn't save HCA "
 				  "PCI header, aborting.\n");
-			goto out;
+			goto free_hca;
 		}
 	}
 
@@ -121,7 +121,7 @@ int mthca_reset(struct mthca_dev *mdev)
 			err = -ENOMEM;
 			mthca_err(mdev, "Couldn't allocate memory to save HCA "
 				  "bridge PCI header, aborting.\n");
-			goto out;
+			goto free_hca;
 		}
 
 		for (i = 0; i < 64; ++i) {
@@ -131,7 +131,7 @@ int mthca_reset(struct mthca_dev *mdev)
 				err = -ENODEV;
 				mthca_err(mdev, "Couldn't save HCA bridge "
 					  "PCI header, aborting.\n");
-				goto out;
+				goto free_bh;
 			}
 		}
 		bridge_pcix_cap = pci_find_capability(bridge, PCI_CAP_ID_PCIX);
@@ -139,7 +139,7 @@ int mthca_reset(struct mthca_dev *mdev)
 				err = -ENODEV;
 				mthca_err(mdev, "Couldn't locate HCA bridge "
 					  "PCI-X capability, aborting.\n");
-				goto out;
+				goto free_bh;
 		}
 	}
 
@@ -152,7 +152,7 @@ int mthca_reset(struct mthca_dev *mdev)
 			err = -ENOMEM;
 			mthca_err(mdev, "Couldn't map HCA reset register, "
 				  "aborting.\n");
-			goto out;
+			goto free_bh;
 		}
 
 		writel(MTHCA_RESET_VALUE, reset);
@@ -172,7 +172,7 @@ int mthca_reset(struct mthca_dev *mdev)
 				err = -ENODEV;
 				mthca_err(mdev, "Couldn't access HCA after reset, "
 					  "aborting.\n");
-				goto out;
+				goto free_bh;
 			}
 
 			if (v != 0xffffffff)
@@ -184,7 +184,7 @@ int mthca_reset(struct mthca_dev *mdev)
 		err = -ENODEV;
 		mthca_err(mdev, "PCI device did not come back after reset, "
 			  "aborting.\n");
-		goto out;
+		goto free_bh;
 	}
 
 good:
@@ -195,14 +195,14 @@ good:
 			err = -ENODEV;
 			mthca_err(mdev, "Couldn't restore HCA bridge Upstream "
 				  "split transaction control, aborting.\n");
-			goto out;
+			goto free_bh;
 		}
 		if (pci_write_config_dword(bridge, bridge_pcix_cap + 0xc,
 				 bridge_header[(bridge_pcix_cap + 0xc) / 4])) {
 			err = -ENODEV;
 			mthca_err(mdev, "Couldn't restore HCA bridge Downstream "
 				  "split transaction control, aborting.\n");
-			goto out;
+			goto free_bh;
 		}
 		/*
 		 * Bridge control register is at 0x3e, so we'll
@@ -216,7 +216,7 @@ good:
 				err = -ENODEV;
 				mthca_err(mdev, "Couldn't restore HCA bridge reg %x, "
 					  "aborting.\n", i);
-				goto out;
+				goto free_bh;
 			}
 		}
 
@@ -225,7 +225,7 @@ good:
 			err = -ENODEV;
 			mthca_err(mdev, "Couldn't restore HCA bridge COMMAND, "
 				  "aborting.\n");
-			goto out;
+			goto free_bh;
 		}
 	}
 
@@ -235,7 +235,7 @@ good:
 			err = -ENODEV;
 			mthca_err(mdev, "Couldn't restore HCA PCI-X "
 				  "command register, aborting.\n");
-			goto out;
+			goto free_bh;
 		}
 	}
 
@@ -246,7 +246,7 @@ good:
 			err = -ENODEV;
 			mthca_err(mdev, "Couldn't restore HCA PCI Express "
 				  "Device Control register, aborting.\n");
-			goto out;
+			goto free_bh;
 		}
 		linkctl = hca_header[(hca_pcie_cap + PCI_EXP_LNKCTL) / 4];
 		if (pcie_capability_write_word(mdev->pdev, PCI_EXP_LNKCTL,
@@ -254,7 +254,7 @@ good:
 			err = -ENODEV;
 			mthca_err(mdev, "Couldn't restore HCA PCI Express "
 				  "Link control register, aborting.\n");
-			goto out;
+			goto free_bh;
 		}
 	}
 
@@ -266,7 +266,7 @@ good:
 			err = -ENODEV;
 			mthca_err(mdev, "Couldn't restore HCA reg %x, "
 				  "aborting.\n", i);
-			goto out;
+			goto free_bh;
 		}
 	}
 
@@ -275,13 +275,12 @@ good:
 		err = -ENODEV;
 		mthca_err(mdev, "Couldn't restore HCA COMMAND, "
 			  "aborting.\n");
-		goto out;
 	}
-
-out:
-	pci_dev_put(bridge);
+free_bh:
 	kfree(bridge_header);
+free_hca:
 	kfree(hca_header);
-
+put_dev:
+	pci_dev_put(bridge);
 	return err;
 }
-- 
2.9.2

[PATCH 3/3] IB/mthca: Delete unnecessary variable initialisations in mthca_reset()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sat, 23 Jul 2016 11:54:06 +0200

Three local variables will be set to an appropriate value a bit later.
Thus omit the explicit initialisation at the beginning.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/hw/mthca/mthca_reset.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/infiniband/hw/mthca/mthca_reset.c b/drivers/infiniband/hw/mthca/mthca_reset.c
index 6727af2..0a458c6 100644
--- a/drivers/infiniband/hw/mthca/mthca_reset.c
+++ b/drivers/infiniband/hw/mthca/mthca_reset.c
@@ -42,12 +42,12 @@ int mthca_reset(struct mthca_dev *mdev)
 {
 	int i;
 	int err = 0;
-	u32 *hca_header    = NULL;
+	u32 *hca_header;
 	u32 *bridge_header = NULL;
 	struct pci_dev *bridge = NULL;
 	int bridge_pcix_cap = 0;
-	int hca_pcie_cap = 0;
-	int hca_pcix_cap = 0;
+	int hca_pcie_cap;
+	int hca_pcix_cap;
 
 	u16 devctl;
 	u16 linkctl;
-- 
2.9.2

Re: [PATCH 1/3] IB/mthca: Delete an unnecessary check before the function call "pci_dev_put"

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 509 bytes --]

On Sat, Jul 23, 2016 at 12:58:08PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 23 Jul 2016 10:05:12 +0200
> 
> The pci_dev_put() function tests whether its argument is NULL and then
> returns immediately. Thus the test around the call is not needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>

Thanks,
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 2/3] IB/mthca: Less function calls in mthca_reset() after error detection

[Leon Romanovsky]

[-- Attachment #1: Type: text/plain, Size: 655 bytes --]

On Sat, Jul 23, 2016 at 12:59:15PM +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
> Date: Sat, 23 Jul 2016 11:28:30 +0200
> 
> The kfree() function was called in a few cases by the mthca_reset()
> function during error handling even if the passed variables "bridge_header"
> and "hca_header" contained a null pointer.
> 
> Adjust jump targets according to the Linux coding style convention.
> 
> Signed-off-by: Markus Elfring <elfring-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>

Thanks,
Reviewed-by: Leon Romanovsky <leonro-VPRAkNaXOzVWk0Htik3J/w@public.gmane.org>

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH] IB/hfi1: Delete an unnecessary check before the function call "sc_return_credits"

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 1258 bytes --]

On Sat, 2016-07-23 at 09:42 +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
> Date: Sat, 23 Jul 2016 08:30:52 +0200
> 
> The sc_return_credits() function tests whether its argument is NULL
> and then returns immediately. Thus the test around the call is not
> needed.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring-Rn4VEauK+AKRv+LV9MX5uipxlwaOVQ5f@public.gmane.org>
> ---
>  drivers/infiniband/hw/hfi1/file_ops.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/infiniband/hw/hfi1/file_ops.c
> b/drivers/infiniband/hw/hfi1/file_ops.c
> index c702a00..32c19fa 100644
> --- a/drivers/infiniband/hw/hfi1/file_ops.c
> +++ b/drivers/infiniband/hw/hfi1/file_ops.c
> @@ -228,7 +228,7 @@ static long hfi1_file_ioctl(struct file *fp,
> unsigned int cmd,
>  				    sizeof(struct hfi1_base_info));
>  		break;
>  	case HFI1_IOCTL_CREDIT_UPD:
> -		if (uctxt && uctxt->sc)
> +		if (uctxt)
>  			sc_return_credits(uctxt->sc);
>  		break;
>  

Thanks, applied.

-- 
Doug Ledford <dledford-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
              GPG KeyID: 0E572FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 0/3] IB/mthca: Fine-tuning for mthca_reset()

[Doug Ledford]

[-- Attachment #1: Type: text/plain, Size: 1124 bytes --]

On Sat, 2016-07-23 at 12:55 +0200, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sat, 23 Jul 2016 12:50:12 +0200
> 
> Further update suggestions were taken into account
> after a patch was applied from static source code analysis.
> 
> Markus Elfring (3):
>   Delete an unnecessary check before the function call "pci_dev_put"
>   Less function calls after error detection
>   Delete unnecessary variable initialisations
> 
>  drivers/infiniband/hw/mthca/mthca_reset.c | 48 +++++++++++++++----
> ------------
>  1 file changed, 23 insertions(+), 25 deletions(-)
> 

I applied 1 and 2, but skipped 3.  My memory from long ago is that
those variables didn't used to have initializers, but the compiler
complained, so the initialization was added at some point.  This is a
dead driver, only around for the few systems out there using out dated
hardware.  I don't want to change this and then have to revert it
because the old warnings came back, so I just dropped that patch.

-- 
Doug Ledford <dledford@redhat.com>
              GPG KeyID: 0E572FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]