[PATCH 0/5] IB/nes: Fine-tuning for some function implementations

[PATCH 0/5] IB/nes: Fine-tuning for some function implementations

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 6 Mar 2017 18:26:34 +0100

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (5):
  Use kcalloc() in nes_init_mgt_qp()
  Use kcalloc() in nes_reg_user_mr()
  Adjust 16 checks for null pointers
  Delete unnecessary braces
  Improve size determinations in two functions

 drivers/infiniband/hw/nes/nes_mgt.c   |   2 +-
 drivers/infiniband/hw/nes/nes_verbs.c | 128 +++++++++++++---------------------
 2 files changed, 50 insertions(+), 80 deletions(-)

-- 
2.12.0

[PATCH 1/5] IB/nes: Use kcalloc() in nes_init_mgt_qp()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 6 Mar 2017 16:07:41 +0100

* A multiplication for the size determination of a memory allocation
  indicated that an array data structure should be processed.
  Thus reuse the corresponding function "kcalloc".

  This issue was detected by using the Coccinelle software.

* Replace the specification of a data type by a pointer dereference
  to make the corresponding size determination a bit safer according to
  the Linux coding style convention.

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

diff --git a/drivers/infiniband/hw/nes/nes_mgt.c b/drivers/infiniband/hw/nes/nes_mgt.c
index 33624f17c347..a84fffd33c21 100644
--- a/drivers/infiniband/hw/nes/nes_mgt.c
+++ b/drivers/infiniband/hw/nes/nes_mgt.c
@@ -877,7 +877,7 @@ int nes_init_mgt_qp(struct nes_device *nesdev, struct net_device *netdev, struct
 	int ret;
 
 	/* Allocate space the all mgt QPs once */
-	mgtvnic = kzalloc(NES_MGT_QP_COUNT * sizeof(struct nes_vnic_mgt), GFP_KERNEL);
+	mgtvnic = kcalloc(NES_MGT_QP_COUNT, sizeof(*mgtvnic), GFP_KERNEL);
 	if (!mgtvnic)
 		return -ENOMEM;
 
-- 
2.12.0

[PATCH 2/5] IB/nes: Use kcalloc() in nes_reg_user_mr()

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 6 Mar 2017 16:26:25 +0100

A multiplication for the size determination of a memory allocation
indicated that an array data structure should be processed.
Thus reuse the corresponding function "kcalloc".

This issue was detected by using the Coccinelle software.

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

diff --git a/drivers/infiniband/hw/nes/nes_verbs.c b/drivers/infiniband/hw/nes/nes_verbs.c
index d3eae2f3e9f5..a28dfeaa1f45 100644
--- a/drivers/infiniband/hw/nes/nes_verbs.c
+++ b/drivers/infiniband/hw/nes/nes_verbs.c
@@ -2260,8 +2260,9 @@ static struct ib_mr *nes_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
 								ibmr = ERR_PTR(-ENOMEM);
 								goto reg_user_mr_err;
 							}
-							root_vpbl.leaf_vpbl = kzalloc(sizeof(*root_vpbl.leaf_vpbl)*1024,
-									GFP_KERNEL);
+							root_vpbl.leaf_vpbl = kcalloc(1024,
+										      sizeof(*root_vpbl.leaf_vpbl),
+										      GFP_KERNEL);
 							if (!root_vpbl.leaf_vpbl) {
 								ib_umem_release(region);
 								pci_free_consistent(nesdev->pcidev, 8192, root_vpbl.pbl_vbase,
-- 
2.12.0

[PATCH 3/5] IB/nes: Adjust 16 checks for null pointers

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 6 Mar 2017 16:54:03 +0100
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The script "checkpatch.pl" pointed information out like the following.

Comparison to NULL could be written !…

Thus fix the affected source code places.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/hw/nes/nes_verbs.c | 35 +++++++++++++++++------------------
 1 file changed, 17 insertions(+), 18 deletions(-)

diff --git a/drivers/infiniband/hw/nes/nes_verbs.c b/drivers/infiniband/hw/nes/nes_verbs.c
index a28dfeaa1f45..f79ea50e3ab5 100644
--- a/drivers/infiniband/hw/nes/nes_verbs.c
+++ b/drivers/infiniband/hw/nes/nes_verbs.c
@@ -106,7 +106,7 @@ static struct ib_mw *nes_alloc_mw(struct ib_pd *ibpd, enum ib_mw_type type,
 
 	/* Register the region with the adapter */
 	cqp_request = nes_get_cqp_request(nesdev);
-	if (cqp_request == NULL) {
+	if (!cqp_request) {
 		kfree(nesmr);
 		nes_free_resource(nesadapter, nesadapter->allocated_mrs, stag_index);
 		return ERR_PTR(-ENOMEM);
@@ -171,7 +171,7 @@ static int nes_dealloc_mw(struct ib_mw *ibmw)
 
 	/* Deallocate the window with the adapter */
 	cqp_request = nes_get_cqp_request(nesdev);
-	if (cqp_request == NULL) {
+	if (!cqp_request) {
 		nes_debug(NES_DBG_MR, "Failed to get a cqp_request.\n");
 		return -ENOMEM;
 	}
@@ -224,7 +224,7 @@ static int alloc_fast_reg_mr(struct nes_device *nesdev, struct nes_pd *nespd,
 
 
 	cqp_request = nes_get_cqp_request(nesdev);
-	if (cqp_request == NULL) {
+	if (!cqp_request) {
 		nes_debug(NES_DBG_MR, "Failed to get a cqp_request.\n");
 		return -ENOMEM;
 	}
@@ -627,7 +627,7 @@ static int nes_mmap(struct ib_ucontext *context, struct vm_area_struct *vma)
 			return -EFAULT;
 		}
 		nesqp = nes_ucontext->mmap_nesqp[index];
-		if (nesqp == NULL) {
+		if (!nesqp) {
 			nes_debug(NES_DBG_MMAP, "wq %lu has a NULL QP base.\n", index);
 			return -EFAULT;
 		}
@@ -1228,7 +1228,7 @@ static struct ib_qp *nes_create_qp(struct ib_pd *ibpd,
 
 			/* Create the QP */
 			cqp_request = nes_get_cqp_request(nesdev);
-			if (cqp_request == NULL) {
+			if (!cqp_request) {
 				nes_debug(NES_DBG_QP, "Failed to get a cqp_request\n");
 				nes_free_resource(nesadapter, nesadapter->allocated_qps, qp_num);
 				nes_free_qp_mem(nesdev, nesqp,virt_wqs);
@@ -1539,7 +1539,7 @@ static struct ib_cq *nes_create_cq(struct ib_device *ibdev,
 
 	/* send CreateCQ request to CQP */
 	cqp_request = nes_get_cqp_request(nesdev);
-	if (cqp_request == NULL) {
+	if (!cqp_request) {
 		nes_debug(NES_DBG_CQ, "Failed to get a cqp_request.\n");
 		if (!context)
 			pci_free_consistent(nesdev->pcidev, nescq->cq_mem_size, mem,
@@ -1697,7 +1697,7 @@ static int nes_destroy_cq(struct ib_cq *ib_cq)
 	u32 opcode = 0;
 	int ret;
 
-	if (ib_cq == NULL)
+	if (!ib_cq)
 		return 0;
 
 	nescq = to_nescq(ib_cq);
@@ -1709,7 +1709,7 @@ static int nes_destroy_cq(struct ib_cq *ib_cq)
 
 	/* Send DestroyCQ request to CQP */
 	cqp_request = nes_get_cqp_request(nesdev);
-	if (cqp_request == NULL) {
+	if (!cqp_request) {
 		nes_debug(NES_DBG_CQ, "Failed to get a cqp_request.\n");
 		return -ENOMEM;
 	}
@@ -1788,8 +1788,7 @@ static u32 root_256(struct nes_device *nesdev,
 	if (pbl_count_4k == 1) {
 		new_root->pbl_vbase = pci_alloc_consistent(nesdev->pcidev,
 						512, &new_root->pbl_pbase);
-
-		if (new_root->pbl_vbase == NULL)
+		if (!new_root->pbl_vbase)
 			return 0;
 
 		leaf_pbl = (u64)root_vpbl->pbl_pbase;
@@ -1847,7 +1846,7 @@ static int nes_reg_mr(struct nes_device *nesdev, struct nes_pd *nespd,
 
 	/* Register the region with the adapter */
 	cqp_request = nes_get_cqp_request(nesdev);
-	if (cqp_request == NULL) {
+	if (!cqp_request) {
 		nes_debug(NES_DBG_MR, "Failed to get a cqp_request.\n");
 		return -ENOMEM;
 	}
@@ -2509,7 +2508,7 @@ static int nes_dereg_mr(struct ib_mr *ib_mr)
 	/* Deallocate the region with the adapter */
 
 	cqp_request = nes_get_cqp_request(nesdev);
-	if (cqp_request == NULL) {
+	if (!cqp_request) {
 		nes_debug(NES_DBG_MR, "Failed to get a cqp_request.\n");
 		return -ENOMEM;
 	}
@@ -2673,7 +2672,7 @@ int nes_hw_modify_qp(struct nes_device *nesdev, struct nes_qp *nesqp,
 			nesqp->hwqp.qp_id, atomic_read(&nesqp->refcount));
 
 	cqp_request = nes_get_cqp_request(nesdev);
-	if (cqp_request == NULL) {
+	if (!cqp_request) {
 		nes_debug(NES_DBG_MOD_QP, "Failed to get a cqp_request.\n");
 		return -ENOMEM;
 	}
@@ -2793,7 +2792,7 @@ int nes_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
 					spin_unlock_irqrestore(&nesqp->lock, qplockflags);
 					return -EINVAL;
 				}
-				if (nesqp->cm_id == NULL) {
+				if (!nesqp->cm_id) {
 					nes_debug(NES_DBG_MOD_QP, "QP%u: Failing attempt to move QP to RTS without a CM_ID. \n",
 							nesqp->hwqp.qp_id );
 					spin_unlock_irqrestore(&nesqp->lock, qplockflags);
@@ -3696,9 +3695,9 @@ struct nes_ib_device *nes_init_ofa_device(struct net_device *netdev)
 	struct nes_device *nesdev = nesvnic->nesdev;
 
 	nesibdev = (struct nes_ib_device *)ib_alloc_device(sizeof(struct nes_ib_device));
-	if (nesibdev == NULL) {
+	if (!nesibdev)
 		return NULL;
-	}
+
 	strlcpy(nesibdev->ibdev.name, "nes%d", IB_DEVICE_NAME_MAX);
 	nesibdev->ibdev.owner = THIS_MODULE;
 
@@ -3772,7 +3771,7 @@ struct nes_ib_device *nes_init_ofa_device(struct net_device *netdev)
 	nesibdev->ibdev.drain_rq = nes_drain_rq;
 
 	nesibdev->ibdev.iwcm = kzalloc(sizeof(*nesibdev->ibdev.iwcm), GFP_KERNEL);
-	if (nesibdev->ibdev.iwcm == NULL) {
+	if (!nesibdev->ibdev.iwcm) {
 		ib_dealloc_device(&nesibdev->ibdev);
 		return NULL;
 	}
@@ -3844,7 +3843,7 @@ void  nes_port_ibevent(struct nes_vnic *nesvnic)
  */
 void nes_destroy_ofa_device(struct nes_ib_device *nesibdev)
 {
-	if (nesibdev == NULL)
+	if (!nesibdev)
 		return;
 
 	nes_unregister_ofa_device(nesibdev);
-- 
2.12.0

[PATCH 4/5] IB/nes: Delete unnecessary braces

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 6 Mar 2017 17:55:43 +0100

Do not use curly brackets at some source code places
where a single statement should be sufficient.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/hw/nes/nes_verbs.c | 84 +++++++++++------------------------
 1 file changed, 27 insertions(+), 57 deletions(-)

diff --git a/drivers/infiniband/hw/nes/nes_verbs.c b/drivers/infiniband/hw/nes/nes_verbs.c
index f79ea50e3ab5..7764ba423c1c 100644
--- a/drivers/infiniband/hw/nes/nes_verbs.c
+++ b/drivers/infiniband/hw/nes/nes_verbs.c
@@ -87,9 +87,8 @@ static struct ib_mw *nes_alloc_mw(struct ib_pd *ibpd, enum ib_mw_type type,
 
 	ret = nes_alloc_resource(nesadapter, nesadapter->allocated_mrs,
 			nesadapter->max_mr, &stag_index, &next_stag_index, NES_RESOURCE_MW);
-	if (ret) {
+	if (ret)
 		return ERR_PTR(ret);
-	}
 
 	nesmr = kzalloc(sizeof(*nesmr), GFP_KERNEL);
 	if (!nesmr) {
@@ -137,11 +136,7 @@ static struct ib_mw *nes_alloc_mw(struct ib_pd *ibpd, enum ib_mw_type type,
 		nes_put_cqp_request(nesdev, cqp_request);
 		kfree(nesmr);
 		nes_free_resource(nesadapter, nesadapter->allocated_mrs, stag_index);
-		if (!ret) {
-			return ERR_PTR(-ETIME);
-		} else {
-			return ERR_PTR(-ENOMEM);
-		}
+		return ERR_PTR(ret ? -ENOMEM : -ETIME);
 	}
 	nes_put_cqp_request(nesdev, cqp_request);
 
@@ -680,9 +675,8 @@ static struct ib_pd *nes_alloc_pd(struct ib_device *ibdev,
 
 	err = nes_alloc_resource(nesadapter, nesadapter->allocated_pds,
 			nesadapter->max_pd, &pd_num, &nesadapter->next_pd, NES_RESOURCE_PD);
-	if (err) {
+	if (err)
 		return ERR_PTR(err);
-	}
 
 	nespd = kzalloc(sizeof (struct nes_pd), GFP_KERNEL);
 	if (!nespd) {
@@ -743,9 +737,8 @@ static int nes_dealloc_pd(struct ib_pd *ibpd)
 				nespd->mmap_db_index);
 		clear_bit(nespd->mmap_db_index, nesucontext->allocated_doorbells);
 		nesucontext->mmap_db_index[nespd->mmap_db_index] = 0;
-		if (nesucontext->first_free_db > nespd->mmap_db_index) {
+		if (nesucontext->first_free_db > nespd->mmap_db_index)
 			nesucontext->first_free_db = nespd->mmap_db_index;
-		}
 	}
 
 	nes_debug(NES_DBG_PD, "Deallocating PD%u structure located @%p.\n",
@@ -1034,11 +1027,11 @@ static struct ib_qp *nes_create_qp(struct ib_pd *ibpd,
 	atomic_inc(&qps_created);
 	switch (init_attr->qp_type) {
 		case IB_QPT_RC:
-			if (nes_drv_opt & NES_DRV_OPT_NO_INLINE_DATA) {
+			if (nes_drv_opt & NES_DRV_OPT_NO_INLINE_DATA)
 				init_attr->cap.max_inline_data = 0;
-			} else {
+			else
 				init_attr->cap.max_inline_data = 64;
-			}
+
 			sq_size = init_attr->cap.max_send_wr;
 			rq_size = init_attr->cap.max_recv_wr;
 
@@ -1058,9 +1051,8 @@ static struct ib_qp *nes_create_qp(struct ib_pd *ibpd,
 
 			ret = nes_alloc_resource(nesadapter, nesadapter->allocated_qps,
 					nesadapter->max_qp, &qp_num, &nesadapter->next_qp, NES_RESOURCE_QP);
-			if (ret) {
+			if (ret)
 				return ERR_PTR(ret);
-			}
 
 			/* Need 512 (actually now 1024) byte alignment on this structure */
 			mem = kzalloc(sizeof(*nesqp)+NES_SW_CONTEXT_ALIGN-1, GFP_KERNEL);
@@ -1084,9 +1076,8 @@ static struct ib_qp *nes_create_qp(struct ib_pd *ibpd,
 					nes_debug(NES_DBG_QP, "ib_copy_from_udata() Failed \n");
 					return ERR_PTR(-EFAULT);
 				}
-				if (req.user_wqe_buffers) {
+				if (req.user_wqe_buffers)
 					virt_wqs = 1;
-				}
 				if (req.user_qp_buffer)
 					nesqp->nesuqp_addr = req.user_qp_buffer;
 				if ((ibpd->uobject) && (ibpd->uobject->context)) {
@@ -1271,11 +1262,7 @@ static struct ib_qp *nes_create_qp(struct ib_pd *ibpd,
 				nes_free_resource(nesadapter, nesadapter->allocated_qps, qp_num);
 				nes_free_qp_mem(nesdev, nesqp,virt_wqs);
 				kfree(nesqp->allocated_buffer);
-				if (!ret) {
-					return ERR_PTR(-ETIME);
-				} else {
-					return ERR_PTR(-EIO);
-				}
+				return ERR_PTR(ret ? -EIO : -ETIME);
 			}
 
 			nes_put_cqp_request(nesdev, cqp_request);
@@ -1401,9 +1388,8 @@ static int nes_destroy_qp(struct ib_qp *ibqp)
 			nes_ucontext = to_nesucontext(ibqp->uobject->context);
 			clear_bit(nesqp->mmap_sq_db_index, nes_ucontext->allocated_wqs);
 			nes_ucontext->mmap_nesqp[nesqp->mmap_sq_db_index] = NULL;
-			if (nes_ucontext->first_free_wq > nesqp->mmap_sq_db_index) {
+			if (nes_ucontext->first_free_wq > nesqp->mmap_sq_db_index)
 				nes_ucontext->first_free_wq = nesqp->mmap_sq_db_index;
-			}
 		}
 		if (nesqp->pbl_pbase && nesqp->sq_kmapped) {
 			nesqp->sq_kmapped = 0;
@@ -1458,9 +1444,8 @@ static struct ib_cq *nes_create_cq(struct ib_device *ibdev,
 
 	err = nes_alloc_resource(nesadapter, nesadapter->allocated_cqs,
 			nesadapter->max_cq, &cq_num, &nesadapter->next_cq, NES_RESOURCE_CQ);
-	if (err) {
+	if (err)
 		return ERR_PTR(err);
-	}
 
 	nescq = kzalloc(sizeof(struct nes_cq), GFP_KERNEL);
 	if (!nescq) {
@@ -2031,9 +2016,8 @@ struct ib_mr *nes_reg_phys_mr(struct ib_pd *ib_pd, u64 addr, u64 size,
 
 	err = nes_alloc_resource(nesadapter, nesadapter->allocated_mrs, nesadapter->max_mr,
 			&stag_index, &next_stag_index, NES_RESOURCE_PHYS_MR);
-	if (err) {
+	if (err)
 		return ERR_PTR(err);
-	}
 
 	nesmr = kzalloc(sizeof(*nesmr), GFP_KERNEL);
 	if (!nesmr) {
@@ -2079,11 +2063,7 @@ struct ib_mr *nes_reg_phys_mr(struct ib_pd *ib_pd, u64 addr, u64 size,
 	/* Make the leaf PBL the root if only one PBL */
 	root_vpbl.pbl_pbase = vpbl.pbl_pbase;
 
-	if (single_page) {
-		pbl_count = 0;
-	} else {
-		pbl_count = 1;
-	}
+	pbl_count = single_page ? 0 : 1;
 	ret = nes_reg_mr(nesdev, nespd, stag, region_length, &root_vpbl,
 			addr, pbl_count, 1, acc, iova_start,
 			&nesmr->pbls_used, &nesmr->pbl_4k);
@@ -2159,9 +2139,8 @@ static struct ib_mr *nes_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
 	int first_page = 1;
 
 	region = ib_umem_get(pd->uobject->context, start, length, acc, 0);
-	if (IS_ERR(region)) {
+	if (IS_ERR(region))
 		return (struct ib_mr *)region;
-	}
 
 	nes_debug(NES_DBG_MR, "User base = 0x%lX, Virt base = 0x%lX, length = %u,"
 			" offset = %u, page size = %u.\n",
@@ -2337,9 +2316,8 @@ static struct ib_mr *nes_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
 
 			iova_start = virt;
 			/* Make the leaf PBL the root if only one PBL */
-			if (root_pbl_index == 1) {
+			if (root_pbl_index == 1)
 				root_vpbl.pbl_pbase = vpbl.pbl_pbase;
-			}
 
 			if (single_page) {
 				pbl_count = 0;
@@ -2411,11 +2389,10 @@ static struct ib_mr *nes_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
 			pbl_depth = region->length >> 12;
 			pbl_depth += (region->length & (4096-1)) ? 1 : 0;
 			nespbl->pbl_size = pbl_depth*sizeof(u64);
-			if (req.reg_type == IWNES_MEMREG_TYPE_QP) {
+			if (req.reg_type == IWNES_MEMREG_TYPE_QP)
 				nes_debug(NES_DBG_MR, "Attempting to allocate QP PBL memory");
-			} else {
+			else
 				nes_debug(NES_DBG_MR, "Attempting to allocate CP PBL memory");
-			}
 
 			nes_debug(NES_DBG_MR, " %u bytes, %u entries.\n",
 					nespbl->pbl_size, pbl_depth);
@@ -2458,11 +2435,11 @@ static struct ib_mr *nes_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
 				}
 			}
 
-			if (req.reg_type == IWNES_MEMREG_TYPE_QP) {
+			if (req.reg_type == IWNES_MEMREG_TYPE_QP)
 				list_add_tail(&nespbl->list, &nes_ucontext->qp_reg_mem_list);
-			} else {
+			else
 				list_add_tail(&nespbl->list, &nes_ucontext->cq_reg_mem_list);
-			}
+
 			nesmr->ibmr.rkey = -1;
 			nesmr->ibmr.lkey = -1;
 			nesmr->mode = req.reg_type;
@@ -2497,9 +2474,9 @@ static int nes_dereg_mr(struct ib_mr *ib_mr)
 				    nesmr->pages,
 				    nesmr->paddr);
 
-	if (nesmr->region) {
+	if (nesmr->region)
 		ib_umem_release(nesmr->region);
-	}
+
 	if (nesmr->mode != IWNES_MEMREG_TYPE_MEM) {
 		kfree(nesmr);
 		return 0;
@@ -2676,11 +2653,7 @@ int nes_hw_modify_qp(struct nes_device *nesdev, struct nes_qp *nesqp,
 		nes_debug(NES_DBG_MOD_QP, "Failed to get a cqp_request.\n");
 		return -ENOMEM;
 	}
-	if (wait_completion) {
-		cqp_request->waiting = 1;
-	} else {
-		cqp_request->waiting = 0;
-	}
+	cqp_request->waiting = wait_completion ? 1 : 0;
 	cqp_wqe = &cqp_request->cqp_wqe;
 
 	set_wqe_32bit_value(cqp_wqe->wqe_words, NES_CQP_WQE_OPCODE_IDX,
@@ -3864,9 +3837,8 @@ int nes_register_ofa_device(struct nes_ib_device *nesibdev)
 	int i, ret;
 
 	ret = ib_register_device(&nesvnic->nesibdev->ibdev, NULL);
-	if (ret) {
+	if (ret)
 		return ret;
-	}
 
 	/* Get the resources allocated to this device */
 	nesibdev->max_cq = (nesadapter->max_cq-NES_FIRST_QPN) / nesadapter->port_count;
@@ -3901,13 +3873,11 @@ static void nes_unregister_ofa_device(struct nes_ib_device *nesibdev)
 	struct nes_vnic *nesvnic = nesibdev->nesvnic;
 	int i;
 
-	for (i = 0; i < ARRAY_SIZE(nes_dev_attributes); ++i) {
+	for (i = 0; i < ARRAY_SIZE(nes_dev_attributes); ++i)
 		device_remove_file(&nesibdev->ibdev.dev, nes_dev_attributes[i]);
-	}
 
-	if (nesvnic->of_device_registered) {
+	if (nesvnic->of_device_registered)
 		ib_unregister_device(&nesibdev->ibdev);
-	}
 
 	nesvnic->of_device_registered = 0;
 }
-- 
2.12.0

[PATCH 5/5] IB/nes: Improve size determinations in two functions

[SF Markus Elfring]

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Mon, 6 Mar 2017 18:12:37 +0100

Replace the specification of two data structures by pointer dereferences
as the parameter for the operator "sizeof" to make the corresponding size
determinations a bit safer according to the Linux coding style convention.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/infiniband/hw/nes/nes_verbs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/nes/nes_verbs.c b/drivers/infiniband/hw/nes/nes_verbs.c
index 7764ba423c1c..5cbb62e44d84 100644
--- a/drivers/infiniband/hw/nes/nes_verbs.c
+++ b/drivers/infiniband/hw/nes/nes_verbs.c
@@ -678,7 +678,7 @@ static struct ib_pd *nes_alloc_pd(struct ib_device *ibdev,
 	if (err)
 		return ERR_PTR(err);
 
-	nespd = kzalloc(sizeof (struct nes_pd), GFP_KERNEL);
+	nespd = kzalloc(sizeof(*nespd), GFP_KERNEL);
 	if (!nespd) {
 		nes_free_resource(nesadapter, nesadapter->allocated_pds, pd_num);
 		return ERR_PTR(-ENOMEM);
@@ -1447,7 +1447,7 @@ static struct ib_cq *nes_create_cq(struct ib_device *ibdev,
 	if (err)
 		return ERR_PTR(err);
 
-	nescq = kzalloc(sizeof(struct nes_cq), GFP_KERNEL);
+	nescq = kzalloc(sizeof(*nescq), GFP_KERNEL);
 	if (!nescq) {
 		nes_free_resource(nesadapter, nesadapter->allocated_cqs, cq_num);
 		return ERR_PTR(-ENOMEM);
-- 
2.12.0

Re: [PATCH 4/5] IB/nes: Delete unnecessary braces

[Joe Perches]

On Mon, 2017-03-06 at 18:43 +0100, SF Markus Elfring wrote:
> Do not use curly brackets at some source code places
> where a single statement should be sufficient.
[]
> diff --git a/drivers/infiniband/hw/nes/nes_verbs.c b/drivers/infiniband/hw/nes/nes_verbs.c
[]
> @@ -137,11 +136,7 @@ static struct ib_mw *nes_alloc_mw(struct ib_pd *ibpd, enum ib_mw_type type,
>  		nes_put_cqp_request(nesdev, cqp_request);
>  		kfree(nesmr);
>  		nes_free_resource(nesadapter, nesadapter->allocated_mrs, stag_index);
> -		if (!ret) {
> -			return ERR_PTR(-ETIME);
> -		} else {
> -			return ERR_PTR(-ENOMEM);
> -		}
> +		return ERR_PTR(ret ? -ENOMEM : -ETIME);

Using a goto error label for this block would
probably be more common kernel style ere.

Re: [PATCH 4/5] IB/nes: Delete unnecessary braces

[Julia Lawall]

On Mon, 6 Mar 2017, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Mon, 6 Mar 2017 17:55:43 +0100
>
> Do not use curly brackets at some source code places
> where a single statement should be sufficient.

The introduction of ? is not mentioned in the commit log.  I'm not sure
that it is suggested Linux kernel coding style either.  One could
furthermore wonder whether those 0s and 1s could be booleans.

julia

>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/infiniband/hw/nes/nes_verbs.c | 84 +++++++++++------------------------
>  1 file changed, 27 insertions(+), 57 deletions(-)
>
> diff --git a/drivers/infiniband/hw/nes/nes_verbs.c b/drivers/infiniband/hw/nes/nes_verbs.c
> index f79ea50e3ab5..7764ba423c1c 100644
> --- a/drivers/infiniband/hw/nes/nes_verbs.c
> +++ b/drivers/infiniband/hw/nes/nes_verbs.c
> @@ -87,9 +87,8 @@ static struct ib_mw *nes_alloc_mw(struct ib_pd *ibpd, enum ib_mw_type type,
>
>  	ret = nes_alloc_resource(nesadapter, nesadapter->allocated_mrs,
>  			nesadapter->max_mr, &stag_index, &next_stag_index, NES_RESOURCE_MW);
> -	if (ret) {
> +	if (ret)
>  		return ERR_PTR(ret);
> -	}
>
>  	nesmr = kzalloc(sizeof(*nesmr), GFP_KERNEL);
>  	if (!nesmr) {
> @@ -137,11 +136,7 @@ static struct ib_mw *nes_alloc_mw(struct ib_pd *ibpd, enum ib_mw_type type,
>  		nes_put_cqp_request(nesdev, cqp_request);
>  		kfree(nesmr);
>  		nes_free_resource(nesadapter, nesadapter->allocated_mrs, stag_index);
> -		if (!ret) {
> -			return ERR_PTR(-ETIME);
> -		} else {
> -			return ERR_PTR(-ENOMEM);
> -		}
> +		return ERR_PTR(ret ? -ENOMEM : -ETIME);
>  	}
>  	nes_put_cqp_request(nesdev, cqp_request);
>
> @@ -680,9 +675,8 @@ static struct ib_pd *nes_alloc_pd(struct ib_device *ibdev,
>
>  	err = nes_alloc_resource(nesadapter, nesadapter->allocated_pds,
>  			nesadapter->max_pd, &pd_num, &nesadapter->next_pd, NES_RESOURCE_PD);
> -	if (err) {
> +	if (err)
>  		return ERR_PTR(err);
> -	}
>
>  	nespd = kzalloc(sizeof (struct nes_pd), GFP_KERNEL);
>  	if (!nespd) {
> @@ -743,9 +737,8 @@ static int nes_dealloc_pd(struct ib_pd *ibpd)
>  				nespd->mmap_db_index);
>  		clear_bit(nespd->mmap_db_index, nesucontext->allocated_doorbells);
>  		nesucontext->mmap_db_index[nespd->mmap_db_index] = 0;
> -		if (nesucontext->first_free_db > nespd->mmap_db_index) {
> +		if (nesucontext->first_free_db > nespd->mmap_db_index)
>  			nesucontext->first_free_db = nespd->mmap_db_index;
> -		}
>  	}
>
>  	nes_debug(NES_DBG_PD, "Deallocating PD%u structure located @%p.\n",
> @@ -1034,11 +1027,11 @@ static struct ib_qp *nes_create_qp(struct ib_pd *ibpd,
>  	atomic_inc(&qps_created);
>  	switch (init_attr->qp_type) {
>  		case IB_QPT_RC:
> -			if (nes_drv_opt & NES_DRV_OPT_NO_INLINE_DATA) {
> +			if (nes_drv_opt & NES_DRV_OPT_NO_INLINE_DATA)
>  				init_attr->cap.max_inline_data = 0;
> -			} else {
> +			else
>  				init_attr->cap.max_inline_data = 64;
> -			}
> +
>  			sq_size = init_attr->cap.max_send_wr;
>  			rq_size = init_attr->cap.max_recv_wr;
>
> @@ -1058,9 +1051,8 @@ static struct ib_qp *nes_create_qp(struct ib_pd *ibpd,
>
>  			ret = nes_alloc_resource(nesadapter, nesadapter->allocated_qps,
>  					nesadapter->max_qp, &qp_num, &nesadapter->next_qp, NES_RESOURCE_QP);
> -			if (ret) {
> +			if (ret)
>  				return ERR_PTR(ret);
> -			}
>
>  			/* Need 512 (actually now 1024) byte alignment on this structure */
>  			mem = kzalloc(sizeof(*nesqp)+NES_SW_CONTEXT_ALIGN-1, GFP_KERNEL);
> @@ -1084,9 +1076,8 @@ static struct ib_qp *nes_create_qp(struct ib_pd *ibpd,
>  					nes_debug(NES_DBG_QP, "ib_copy_from_udata() Failed \n");
>  					return ERR_PTR(-EFAULT);
>  				}
> -				if (req.user_wqe_buffers) {
> +				if (req.user_wqe_buffers)
>  					virt_wqs = 1;
> -				}
>  				if (req.user_qp_buffer)
>  					nesqp->nesuqp_addr = req.user_qp_buffer;
>  				if ((ibpd->uobject) && (ibpd->uobject->context)) {
> @@ -1271,11 +1262,7 @@ static struct ib_qp *nes_create_qp(struct ib_pd *ibpd,
>  				nes_free_resource(nesadapter, nesadapter->allocated_qps, qp_num);
>  				nes_free_qp_mem(nesdev, nesqp,virt_wqs);
>  				kfree(nesqp->allocated_buffer);
> -				if (!ret) {
> -					return ERR_PTR(-ETIME);
> -				} else {
> -					return ERR_PTR(-EIO);
> -				}
> +				return ERR_PTR(ret ? -EIO : -ETIME);
>  			}
>
>  			nes_put_cqp_request(nesdev, cqp_request);
> @@ -1401,9 +1388,8 @@ static int nes_destroy_qp(struct ib_qp *ibqp)
>  			nes_ucontext = to_nesucontext(ibqp->uobject->context);
>  			clear_bit(nesqp->mmap_sq_db_index, nes_ucontext->allocated_wqs);
>  			nes_ucontext->mmap_nesqp[nesqp->mmap_sq_db_index] = NULL;
> -			if (nes_ucontext->first_free_wq > nesqp->mmap_sq_db_index) {
> +			if (nes_ucontext->first_free_wq > nesqp->mmap_sq_db_index)
>  				nes_ucontext->first_free_wq = nesqp->mmap_sq_db_index;
> -			}
>  		}
>  		if (nesqp->pbl_pbase && nesqp->sq_kmapped) {
>  			nesqp->sq_kmapped = 0;
> @@ -1458,9 +1444,8 @@ static struct ib_cq *nes_create_cq(struct ib_device *ibdev,
>
>  	err = nes_alloc_resource(nesadapter, nesadapter->allocated_cqs,
>  			nesadapter->max_cq, &cq_num, &nesadapter->next_cq, NES_RESOURCE_CQ);
> -	if (err) {
> +	if (err)
>  		return ERR_PTR(err);
> -	}
>
>  	nescq = kzalloc(sizeof(struct nes_cq), GFP_KERNEL);
>  	if (!nescq) {
> @@ -2031,9 +2016,8 @@ struct ib_mr *nes_reg_phys_mr(struct ib_pd *ib_pd, u64 addr, u64 size,
>
>  	err = nes_alloc_resource(nesadapter, nesadapter->allocated_mrs, nesadapter->max_mr,
>  			&stag_index, &next_stag_index, NES_RESOURCE_PHYS_MR);
> -	if (err) {
> +	if (err)
>  		return ERR_PTR(err);
> -	}
>
>  	nesmr = kzalloc(sizeof(*nesmr), GFP_KERNEL);
>  	if (!nesmr) {
> @@ -2079,11 +2063,7 @@ struct ib_mr *nes_reg_phys_mr(struct ib_pd *ib_pd, u64 addr, u64 size,
>  	/* Make the leaf PBL the root if only one PBL */
>  	root_vpbl.pbl_pbase = vpbl.pbl_pbase;
>
> -	if (single_page) {
> -		pbl_count = 0;
> -	} else {
> -		pbl_count = 1;
> -	}
> +	pbl_count = single_page ? 0 : 1;
>  	ret = nes_reg_mr(nesdev, nespd, stag, region_length, &root_vpbl,
>  			addr, pbl_count, 1, acc, iova_start,
>  			&nesmr->pbls_used, &nesmr->pbl_4k);
> @@ -2159,9 +2139,8 @@ static struct ib_mr *nes_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
>  	int first_page = 1;
>
>  	region = ib_umem_get(pd->uobject->context, start, length, acc, 0);
> -	if (IS_ERR(region)) {
> +	if (IS_ERR(region))
>  		return (struct ib_mr *)region;
> -	}
>
>  	nes_debug(NES_DBG_MR, "User base = 0x%lX, Virt base = 0x%lX, length = %u,"
>  			" offset = %u, page size = %u.\n",
> @@ -2337,9 +2316,8 @@ static struct ib_mr *nes_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
>
>  			iova_start = virt;
>  			/* Make the leaf PBL the root if only one PBL */
> -			if (root_pbl_index == 1) {
> +			if (root_pbl_index == 1)
>  				root_vpbl.pbl_pbase = vpbl.pbl_pbase;
> -			}
>
>  			if (single_page) {
>  				pbl_count = 0;
> @@ -2411,11 +2389,10 @@ static struct ib_mr *nes_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
>  			pbl_depth = region->length >> 12;
>  			pbl_depth += (region->length & (4096-1)) ? 1 : 0;
>  			nespbl->pbl_size = pbl_depth*sizeof(u64);
> -			if (req.reg_type == IWNES_MEMREG_TYPE_QP) {
> +			if (req.reg_type == IWNES_MEMREG_TYPE_QP)
>  				nes_debug(NES_DBG_MR, "Attempting to allocate QP PBL memory");
> -			} else {
> +			else
>  				nes_debug(NES_DBG_MR, "Attempting to allocate CP PBL memory");
> -			}
>
>  			nes_debug(NES_DBG_MR, " %u bytes, %u entries.\n",
>  					nespbl->pbl_size, pbl_depth);
> @@ -2458,11 +2435,11 @@ static struct ib_mr *nes_reg_user_mr(struct ib_pd *pd, u64 start, u64 length,
>  				}
>  			}
>
> -			if (req.reg_type == IWNES_MEMREG_TYPE_QP) {
> +			if (req.reg_type == IWNES_MEMREG_TYPE_QP)
>  				list_add_tail(&nespbl->list, &nes_ucontext->qp_reg_mem_list);
> -			} else {
> +			else
>  				list_add_tail(&nespbl->list, &nes_ucontext->cq_reg_mem_list);
> -			}
> +
>  			nesmr->ibmr.rkey = -1;
>  			nesmr->ibmr.lkey = -1;
>  			nesmr->mode = req.reg_type;
> @@ -2497,9 +2474,9 @@ static int nes_dereg_mr(struct ib_mr *ib_mr)
>  				    nesmr->pages,
>  				    nesmr->paddr);
>
> -	if (nesmr->region) {
> +	if (nesmr->region)
>  		ib_umem_release(nesmr->region);
> -	}
> +
>  	if (nesmr->mode != IWNES_MEMREG_TYPE_MEM) {
>  		kfree(nesmr);
>  		return 0;
> @@ -2676,11 +2653,7 @@ int nes_hw_modify_qp(struct nes_device *nesdev, struct nes_qp *nesqp,
>  		nes_debug(NES_DBG_MOD_QP, "Failed to get a cqp_request.\n");
>  		return -ENOMEM;
>  	}
> -	if (wait_completion) {
> -		cqp_request->waiting = 1;
> -	} else {
> -		cqp_request->waiting = 0;
> -	}
> +	cqp_request->waiting = wait_completion ? 1 : 0;
>  	cqp_wqe = &cqp_request->cqp_wqe;
>
>  	set_wqe_32bit_value(cqp_wqe->wqe_words, NES_CQP_WQE_OPCODE_IDX,
> @@ -3864,9 +3837,8 @@ int nes_register_ofa_device(struct nes_ib_device *nesibdev)
>  	int i, ret;
>
>  	ret = ib_register_device(&nesvnic->nesibdev->ibdev, NULL);
> -	if (ret) {
> +	if (ret)
>  		return ret;
> -	}
>
>  	/* Get the resources allocated to this device */
>  	nesibdev->max_cq = (nesadapter->max_cq-NES_FIRST_QPN) / nesadapter->port_count;
> @@ -3901,13 +3873,11 @@ static void nes_unregister_ofa_device(struct nes_ib_device *nesibdev)
>  	struct nes_vnic *nesvnic = nesibdev->nesvnic;
>  	int i;
>
> -	for (i = 0; i < ARRAY_SIZE(nes_dev_attributes); ++i) {
> +	for (i = 0; i < ARRAY_SIZE(nes_dev_attributes); ++i)
>  		device_remove_file(&nesibdev->ibdev.dev, nes_dev_attributes[i]);
> -	}
>
> -	if (nesvnic->of_device_registered) {
> +	if (nesvnic->of_device_registered)
>  		ib_unregister_device(&nesibdev->ibdev);
> -	}
>
>  	nesvnic->of_device_registered = 0;
>  }
> --
> 2.12.0
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

[PATCH] IB/nes: Neaten nes_alloc_mw

[Joe Perches]

Convert the function to follow more typical kernel style.

Use an error return block instead of individual returns.

Reduces the object code size a trivial amount.
$ size drivers/infiniband/hw/nes/nes_verbs.o*
   text	   data	    bss	    dec	    hex	filename
  26864	     96	     12	  26972	   695c	drivers/infiniband/hw/nes/nes_verbs.o.new
  26932	     96	     12	  27040	   69a0	drivers/infiniband/hw/nes/nes_verbs.o.old

Signed-off-by: Joe Perches <joe@perches.com>
---

Markus, please try to improve the code for
human readers instead of doing these mechanical
conversions.

Something like the below seems nicer to me.

I hope one day it'll seem nicer to you too
and you make the effort to understand and
then fix the code style a little more than
doing those apparently completely mindless
conversions.

 drivers/infiniband/hw/nes/nes_verbs.c | 73 ++++++++++++++++++++---------------
 1 file changed, 42 insertions(+), 31 deletions(-)

diff --git a/drivers/infiniband/hw/nes/nes_verbs.c b/drivers/infiniband/hw/nes/nes_verbs.c
index ccf0a4cffe9c..5fe5af39f834 100644
--- a/drivers/infiniband/hw/nes/nes_verbs.c
+++ b/drivers/infiniband/hw/nes/nes_verbs.c
@@ -86,15 +86,15 @@ static struct ib_mw *nes_alloc_mw(struct ib_pd *ibpd, enum ib_mw_type type,
 	next_stag_index %= nesadapter->max_mr;
 
 	ret = nes_alloc_resource(nesadapter, nesadapter->allocated_mrs,
-			nesadapter->max_mr, &stag_index, &next_stag_index, NES_RESOURCE_MW);
-	if (ret) {
+				 nesadapter->max_mr, &stag_index,
+				 &next_stag_index, NES_RESOURCE_MW);
+	if (ret)
 		return ERR_PTR(ret);
-	}
 
 	nesmr = kzalloc(sizeof(*nesmr), GFP_KERNEL);
 	if (!nesmr) {
-		nes_free_resource(nesadapter, nesadapter->allocated_mrs, stag_index);
-		return ERR_PTR(-ENOMEM);
+		ret = -ENOMEM;
+		goto err_free_res;
 	}
 
 	stag = stag_index << 8;
@@ -102,49 +102,55 @@ static struct ib_mw *nes_alloc_mw(struct ib_pd *ibpd, enum ib_mw_type type,
 	stag += (u32)stag_key;
 
 	nes_debug(NES_DBG_MR, "Registering STag 0x%08X, index = 0x%08X\n",
-			stag, stag_index);
+		  stag, stag_index);
 
 	/* Register the region with the adapter */
 	cqp_request = nes_get_cqp_request(nesdev);
-	if (cqp_request == NULL) {
-		kfree(nesmr);
-		nes_free_resource(nesadapter, nesadapter->allocated_mrs, stag_index);
-		return ERR_PTR(-ENOMEM);
+	if (!cqp_request) {
+		ret = -ENOMEM;
+		goto err_kfree;
 	}
 
 	cqp_request->waiting = 1;
 	cqp_wqe = &cqp_request->cqp_wqe;
 
 	cqp_wqe->wqe_words[NES_CQP_WQE_OPCODE_IDX] =
-			cpu_to_le32( NES_CQP_ALLOCATE_STAG | NES_CQP_STAG_RIGHTS_REMOTE_READ |
-			NES_CQP_STAG_RIGHTS_REMOTE_WRITE | NES_CQP_STAG_VA_TO |
-			NES_CQP_STAG_REM_ACC_EN);
+		cpu_to_le32((NES_CQP_ALLOCATE_STAG |
+			     NES_CQP_STAG_RIGHTS_REMOTE_READ |
+			     NES_CQP_STAG_RIGHTS_REMOTE_WRITE |
+			     NES_CQP_STAG_VA_TO |
+			     NES_CQP_STAG_REM_ACC_EN));
 
 	nes_fill_init_cqp_wqe(cqp_wqe, nesdev);
-	set_wqe_32bit_value(cqp_wqe->wqe_words, NES_CQP_STAG_WQE_LEN_HIGH_PD_IDX, (nespd->pd_id & 0x00007fff));
-	set_wqe_32bit_value(cqp_wqe->wqe_words, NES_CQP_STAG_WQE_STAG_IDX, stag);
+	set_wqe_32bit_value(cqp_wqe->wqe_words,
+			    NES_CQP_STAG_WQE_LEN_HIGH_PD_IDX,
+			    nespd->pd_id & 0x00007fff);
+	set_wqe_32bit_value(cqp_wqe->wqe_words,
+			    NES_CQP_STAG_WQE_STAG_IDX,
+			    stag);
 
 	atomic_set(&cqp_request->refcount, 2);
 	nes_post_cqp_request(nesdev, cqp_request);
 
 	/* Wait for CQP */
-	ret = wait_event_timeout(cqp_request->waitq, (cqp_request->request_done != 0),
-			NES_EVENT_TIMEOUT);
-	nes_debug(NES_DBG_MR, "Register STag 0x%08X completed, wait_event_timeout ret = %u,"
-			" CQP Major:Minor codes = 0x%04X:0x%04X.\n",
-			stag, ret, cqp_request->major_code, cqp_request->minor_code);
-	if ((!ret) || (cqp_request->major_code)) {
-		nes_put_cqp_request(nesdev, cqp_request);
-		kfree(nesmr);
-		nes_free_resource(nesadapter, nesadapter->allocated_mrs, stag_index);
-		if (!ret) {
-			return ERR_PTR(-ETIME);
-		} else {
-			return ERR_PTR(-ENOMEM);
-		}
-	}
+	ret = wait_event_timeout(cqp_request->waitq,
+				 cqp_request->request_done != 0,
+				 NES_EVENT_TIMEOUT);
+
+	nes_debug(NES_DBG_MR, "Register STag 0x%08X completed, wait_event_timeout ret = %u, CQP Major:Minor codes = 0x%04X:0x%04X\n",
+		  stag, ret, cqp_request->major_code, cqp_request->minor_code);
+
 	nes_put_cqp_request(nesdev, cqp_request);
 
+	if (!ret) {
+		ret = -ETIME;
+		goto err_kfree;
+	}
+	if (cqp_request->major_code) {
+		ret = -ENOMEM;
+		goto err_kfree;
+	}
+
 	nesmr->ibmw.rkey = stag;
 	nesmr->mode = IWNES_MEMREG_TYPE_MW;
 	ibmw = &nesmr->ibmw;
@@ -152,8 +158,13 @@ static struct ib_mw *nes_alloc_mw(struct ib_pd *ibpd, enum ib_mw_type type,
 	nesmr->pbls_used = 0;
 
 	return ibmw;
-}
 
+err_kfree:
+	kfree(nesmr);
+err_free_res:
+	nes_free_resource(nesadapter, nesadapter->allocated_mrs, stag_index);
+	return ERR_PTR(ret);
+}
 
 /**
  * nes_dealloc_mw
-- 
2.10.0.rc2.1.g053435c