Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1593557 > unrolled thread

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

Started bySF Markus Elfring <elfring@users.sourceforge.net>
First post2017-03-06 19:00 +0100
Last post2017-03-06 19:30 +0100
Articles 6 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/5] IB/nes: Fine-tuning for some function implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2017-03-06 19:00 +0100
    [PATCH 4/5] IB/nes: Delete unnecessary braces SF Markus Elfring <elfring@users.sourceforge.net> - 2017-03-06 19:10 +0100
      Re: [PATCH 4/5] IB/nes: Delete unnecessary braces Joe Perches <joe@perches.com> - 2017-03-06 19:40 +0100
        [PATCH] IB/nes: Neaten nes_alloc_mw Joe Perches <joe@perches.com> - 2017-03-07 01:40 +0100
      Re: [PATCH 4/5] IB/nes: Delete unnecessary braces Julia Lawall <julia.lawall@lip6.fr> - 2017-03-06 22:40 +0100
    [PATCH 5/5] IB/nes: Improve size determinations in two functions SF Markus Elfring <elfring@users.sourceforge.net> - 2017-03-06 19:30 +0100

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

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2017-03-06 19:00 +0100
Subject[PATCH 0/5] IB/nes: Fine-tuning for some function implementations
Message-ID<ti4VI-33v-7@gated-at.bofh.it>
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

[toc] | [next] | [standalone]

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

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2017-03-06 19:10 +0100
Subject[PATCH 4/5] IB/nes: Delete unnecessary braces
Message-ID<ti5f3-3x2-3@gated-at.bofh.it>
In reply to#1593557
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

[toc] | [prev] | [next] | [standalone]

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

FromJoe Perches <joe@perches.com>
Date2017-03-06 19:40 +0100
SubjectRe: [PATCH 4/5] IB/nes: Delete unnecessary braces
Message-ID<ti5I6-3Hy-11@gated-at.bofh.it>
In reply to#1593558
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.

[toc] | [prev] | [next] | [standalone]

#1593817 — [PATCH] IB/nes: Neaten nes_alloc_mw

FromJoe Perches <joe@perches.com>
Date2017-03-07 01:40 +0100
Subject[PATCH] IB/nes: Neaten nes_alloc_mw
Message-ID<tibkt-7Hd-7@gated-at.bofh.it>
In reply to#1593573
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

[toc] | [prev] | [next] | [standalone]

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

FromJulia Lawall <julia.lawall@lip6.fr>
Date2017-03-06 22:40 +0100
SubjectRe: [PATCH 4/5] IB/nes: Delete unnecessary braces
Message-ID<ti8wh-5Kt-13@gated-at.bofh.it>
In reply to#1593558

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
>

[toc] | [prev] | [next] | [standalone]

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

FromSF Markus Elfring <elfring@users.sourceforge.net>
Date2017-03-06 19:30 +0100
Subject[PATCH 5/5] IB/nes: Improve size determinations in two functions
Message-ID<ti5yp-3DN-5@gated-at.bofh.it>
In reply to#1593557
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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web