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

Groups > linux.kernel > #1309493 > unrolled thread

[PATCH v3 0/6] InfiniBand-ocrdma: Fine-tuning for some function implementations

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v3 0/6] InfiniBand-ocrdma: Fine-tuning for some function  implementations SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-14 18:20 +0100
    [PATCH v3 1/6] InfiniBand-ocrdma: One jump label less in  ocrdma_alloc_ucontext_pd() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-14 18:40 +0100
    [PATCH v3 3/6] InfiniBand-ocrdma: Returning only value constants in  ocrdma_qp_state_change() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-14 18:50 +0100
    [PATCH v3 2/6] InfiniBand-ocrdma: Delete unnecessary variable  initialisations in 11 functions SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-14 18:50 +0100
      Re: [PATCH v3 2/6] InfiniBand-ocrdma: Delete unnecessary variable  initialisations in 11 functions Leon Romanovsky <leon@leon.nu> - 2016-01-15 14:30 +0100
        Re: InfiniBand-ocrdma: Delete unnecessary variable initialisations in  11 functions SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-15 16:00 +0100
          Re: InfiniBand-ocrdma: Delete unnecessary variable initialisations  in 11 functions Leon Romanovsky <leon@leon.nu> - 2016-01-15 16:10 +0100
            Re: InfiniBand-ocrdma: Delete unnecessary variable initialisations in  11 functions SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-15 16:30 +0100
              Re: InfiniBand-ocrdma: Delete unnecessary variable initialisations  in 11 functions Leon Romanovsky <leon@leon.nu> - 2016-01-15 17:00 +0100
                Re: InfiniBand-ocrdma: Delete unnecessary variable initialisations  in 11 functions Dan Carpenter <dan.carpenter@oracle.com> - 2016-01-15 17:20 +0100
                Re: InfiniBand-ocrdma: Delete unnecessary variable initialisations in  11 functions SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-15 17:30 +0100
                  Re: InfiniBand-ocrdma: Delete unnecessary variable initialisations  in 11 functions Leon Romanovsky <leon@leon.nu> - 2016-01-15 18:10 +0100
                    Re: InfiniBand-ocrdma: Delete unnecessary variable initialisations  in 11 functions Leon Romanovsky <leon@leon.nu> - 2016-01-15 18:50 +0100
                    Re: InfiniBand-ocrdma: Delete unnecessary variable initialisations in  11 functions SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-15 19:30 +0100
                      Re: InfiniBand-ocrdma: Delete unnecessary variable initialisations  in 11 functions Leon Romanovsky <leon@leon.nu> - 2016-01-16 07:20 +0100
                        Re: InfiniBand-ocrdma: Delete unnecessary variable initialisations in  11 functions SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-16 09:40 +0100
    [PATCH v3 4/6] InfiniBand-ocrdma: Return a value from a function call  in _ocrdma_modify_qp() directly SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-14 18:50 +0100
    [PATCH v3 6/6] InfiniBand-ocrdma: Delete an unnecessary variable in  ocrdma_dealloc_pd() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-14 19:00 +0100
    [PATCH v3 5/6] InfiniBand-ocrdma: Returning only value constants in  ocrdma_resize_cq() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-01-14 19:00 +0100

#1309493 — [PATCH v3 0/6] InfiniBand-ocrdma: Fine-tuning for some function implementations

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 14 Jan 2016 18:15:54 +0100

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

Markus Elfring (6):
  One variable and jump label less in ocrdma_alloc_ucontext_pd()
  Delete unnecessary variable initialisations in 11 functions
  Returning only value constants in ocrdma_qp_state_change()
  Return a value from a function call in _ocrdma_modify_qp() directly
  Returning only value constants in ocrdma_resize_cq()
  Delete an unnecessary variable in ocrdma_dealloc_pd()

---

v3: Rebase proposed changes on the source files for the software
    "Linux next-20160114".
    
v2: Unfortunately, the first update step from this series contained
    an inappropriate suggestion.
    Thus fix that.

 drivers/infiniband/hw/ocrdma/ocrdma_ah.c    |  2 +-
 drivers/infiniband/hw/ocrdma/ocrdma_hw.c    |  7 +++---
 drivers/infiniband/hw/ocrdma/ocrdma_stats.c |  4 +--
 drivers/infiniband/hw/ocrdma/ocrdma_verbs.c | 39 +++++++++++------------------
 4 files changed, 20 insertions(+), 32 deletions(-)

-- 
2.6.3

[toc] | [next] | [standalone]

#1309532 — [PATCH v3 1/6] InfiniBand-ocrdma: One jump label less in ocrdma_alloc_ucontext_pd()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 14 Jan 2016 17:17:44 +0100

This issue was detected by using the Coccinelle software.

* Let us return directly if a call of the _ocrdma_alloc_pd()
  function failed.

* Reduce the scope for the local variable "status" to one case
  of an if statement.

* Delete the jump label "err" then.

* Return zero as a constant at the end.

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

diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c b/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
index d4c687b..4caf167 100644
--- a/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
+++ b/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
@@ -483,19 +483,16 @@ static int ocrdma_alloc_ucontext_pd(struct ocrdma_dev *dev,
 				    struct ocrdma_ucontext *uctx,
 				    struct ib_udata *udata)
 {
-	int status = 0;
-
 	uctx->cntxt_pd = _ocrdma_alloc_pd(dev, uctx, udata);
 	if (IS_ERR(uctx->cntxt_pd)) {
-		status = PTR_ERR(uctx->cntxt_pd);
+		int status = PTR_ERR(uctx->cntxt_pd);
 		uctx->cntxt_pd = NULL;
-		goto err;
+		return status;
 	}
 
 	uctx->cntxt_pd->uctx = uctx;
 	uctx->cntxt_pd->ibpd.device = &dev->ibdev;
-err:
-	return status;
+	return 0;
 }
 
 static int ocrdma_dealloc_ucontext_pd(struct ocrdma_ucontext *uctx)
-- 
2.6.3

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

#1309536 — [PATCH v3 3/6] InfiniBand-ocrdma: Returning only value constants in ocrdma_qp_state_change()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 14 Jan 2016 17:54:45 +0100

Return zero at the end without using the local variable "status".

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

diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
index 159b1d5..3766927 100644
--- a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
+++ b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
@@ -2138,7 +2138,6 @@ int ocrdma_qp_state_change(struct ocrdma_qp *qp, enum ib_qp_state new_ib_state,
 			   enum ib_qp_state *old_ib_state)
 {
 	unsigned long flags;
-	int status = 0;
 	enum ocrdma_qp_state new_state;
 	new_state = get_ocrdma_qp_state(new_ib_state);
 
@@ -2163,7 +2162,7 @@ int ocrdma_qp_state_change(struct ocrdma_qp *qp, enum ib_qp_state new_ib_state,
 	qp->state = new_state;
 
 	spin_unlock_irqrestore(&qp->q_lock, flags);
-	return status;
+	return 0;
 }
 
 static u32 ocrdma_set_create_qp_mbx_access_flags(struct ocrdma_qp *qp)
-- 
2.6.3

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

#1309538 — [PATCH v3 2/6] InfiniBand-ocrdma: Delete unnecessary variable initialisations in 11 functions

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 14 Jan 2016 17:47:59 +0100

The variable "status" 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/ocrdma/ocrdma_ah.c    |  2 +-
 drivers/infiniband/hw/ocrdma/ocrdma_hw.c    |  4 ++--
 drivers/infiniband/hw/ocrdma/ocrdma_stats.c |  4 ++--
 drivers/infiniband/hw/ocrdma/ocrdma_verbs.c | 12 ++++++------
 4 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_ah.c b/drivers/infiniband/hw/ocrdma/ocrdma_ah.c
index a343e03..41f0171 100644
--- a/drivers/infiniband/hw/ocrdma/ocrdma_ah.c
+++ b/drivers/infiniband/hw/ocrdma/ocrdma_ah.c
@@ -59,7 +59,7 @@ static inline int set_av_attr(struct ocrdma_dev *dev, struct ocrdma_ah *ah,
 			struct ib_ah_attr *attr, union ib_gid *sgid,
 			int pdid, bool *isvlan, u16 vlan_tag)
 {
-	int status = 0;
+	int status;
 	struct ocrdma_eth_vlan eth;
 	struct ocrdma_grh grh;
 	int eth_sz;
diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
index 283ca84..159b1d5 100644
--- a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
+++ b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
@@ -1113,7 +1113,7 @@ mbx_err:
 static int ocrdma_nonemb_mbx_cmd(struct ocrdma_dev *dev, struct ocrdma_mqe *mqe,
 				 void *payload_va)
 {
-	int status = 0;
+	int status;
 	struct ocrdma_mbx_rsp *rsp = payload_va;
 
 	if ((mqe->hdr.spcl_sge_cnt_emb & OCRDMA_MQE_HDR_EMB_MASK) >>
@@ -2871,7 +2871,7 @@ int ocrdma_mbx_destroy_srq(struct ocrdma_dev *dev, struct ocrdma_srq *srq)
 static int ocrdma_mbx_get_dcbx_config(struct ocrdma_dev *dev, u32 ptype,
 				      struct ocrdma_dcbx_cfg *dcbxcfg)
 {
-	int status = 0;
+	int status;
 	dma_addr_t pa;
 	struct ocrdma_mqe cmd;
 
diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_stats.c b/drivers/infiniband/hw/ocrdma/ocrdma_stats.c
index 86c303a..119baa3 100644
--- a/drivers/infiniband/hw/ocrdma/ocrdma_stats.c
+++ b/drivers/infiniband/hw/ocrdma/ocrdma_stats.c
@@ -608,7 +608,7 @@ static char *ocrdma_driver_dbg_stats(struct ocrdma_dev *dev)
 static void ocrdma_update_stats(struct ocrdma_dev *dev)
 {
 	ulong now = jiffies, secs;
-	int status = 0;
+	int status;
 	struct ocrdma_rdma_stats_resp *rdma_stats =
 		      (struct ocrdma_rdma_stats_resp *)dev->stats_mem.va;
 	struct ocrdma_rsrc_stats *rsrc_stats = &rdma_stats->act_rsrc_stats;
@@ -639,7 +639,7 @@ static ssize_t ocrdma_dbgfs_ops_write(struct file *filp,
 {
 	char tmp_str[32];
 	long reset;
-	int status = 0;
+	int status;
 	struct ocrdma_stats *pstats = filp->private_data;
 	struct ocrdma_dev *dev = pstats->dev;
 
diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c b/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
index 4caf167..1d90d18 100644
--- a/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
+++ b/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
@@ -419,7 +419,7 @@ static struct ocrdma_pd *_ocrdma_alloc_pd(struct ocrdma_dev *dev,
 					  struct ib_udata *udata)
 {
 	struct ocrdma_pd *pd = NULL;
-	int status = 0;
+	int status;
 
 	pd = kzalloc(sizeof(*pd), GFP_KERNEL);
 	if (!pd)
@@ -468,7 +468,7 @@ static inline int is_ucontext_pd(struct ocrdma_ucontext *uctx,
 static int _ocrdma_dealloc_pd(struct ocrdma_dev *dev,
 			      struct ocrdma_pd *pd)
 {
-	int status = 0;
+	int status;
 
 	if (dev->pd_mgr->pd_prealloc_valid)
 		status = ocrdma_put_pd_num(dev, pd->id, pd->dpp_enabled);
@@ -593,7 +593,7 @@ map_err:
 
 int ocrdma_dealloc_ucontext(struct ib_ucontext *ibctx)
 {
-	int status = 0;
+	int status;
 	struct ocrdma_mm *mm, *tmp;
 	struct ocrdma_ucontext *uctx = get_ocrdma_ucontext(ibctx);
 	struct ocrdma_dev *dev = get_ocrdma_dev(ibctx->device);
@@ -620,7 +620,7 @@ int ocrdma_mmap(struct ib_ucontext *context, struct vm_area_struct *vma)
 	unsigned long vm_page = vma->vm_pgoff << PAGE_SHIFT;
 	u64 unmapped_db = (u64) dev->nic_info.unmapped_db;
 	unsigned long len = (vma->vm_end - vma->vm_start);
-	int status = 0;
+	int status;
 	bool found;
 
 	if (vma->vm_start & (PAGE_SIZE - 1))
@@ -1283,7 +1283,7 @@ static int ocrdma_copy_qp_uresp(struct ocrdma_qp *qp,
 				struct ib_udata *udata, int dpp_offset,
 				int dpp_credit_lmt, int srq)
 {
-	int status = 0;
+	int status;
 	u64 usr_db;
 	struct ocrdma_create_qp_uresp uresp;
 	struct ocrdma_pd *pd = qp->pd;
@@ -1947,7 +1947,7 @@ int ocrdma_modify_srq(struct ib_srq *ibsrq,
 		      enum ib_srq_attr_mask srq_attr_mask,
 		      struct ib_udata *udata)
 {
-	int status = 0;
+	int status;
 	struct ocrdma_srq *srq;
 
 	srq = get_ocrdma_srq(ibsrq);
-- 
2.6.3

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

#1310105 — Re: [PATCH v3 2/6] InfiniBand-ocrdma: Delete unnecessary variable initialisations in 11 functions

On Thu, Jan 14, 2016 at 06:43:13PM +0100, SF Markus Elfring wrote:
> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Thu, 14 Jan 2016 17:47:59 +0100
> 
> The variable "status" will be set to an appropriate value a bit later.
> Thus omit the explicit initialisation at the beginning.

What did you try to achieve by this patch?

> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/infiniband/hw/ocrdma/ocrdma_ah.c    |  2 +-
>  drivers/infiniband/hw/ocrdma/ocrdma_hw.c    |  4 ++--
>  drivers/infiniband/hw/ocrdma/ocrdma_stats.c |  4 ++--
>  drivers/infiniband/hw/ocrdma/ocrdma_verbs.c | 12 ++++++------
>  4 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_ah.c b/drivers/infiniband/hw/ocrdma/ocrdma_ah.c
> index a343e03..41f0171 100644
> --- a/drivers/infiniband/hw/ocrdma/ocrdma_ah.c
> +++ b/drivers/infiniband/hw/ocrdma/ocrdma_ah.c
> @@ -59,7 +59,7 @@ static inline int set_av_attr(struct ocrdma_dev *dev, struct ocrdma_ah *ah,
>  			struct ib_ah_attr *attr, union ib_gid *sgid,
>  			int pdid, bool *isvlan, u16 vlan_tag)
>  {
> -	int status = 0;
> +	int status;
>  	struct ocrdma_eth_vlan eth;
>  	struct ocrdma_grh grh;
>  	int eth_sz;
> diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> index 283ca84..159b1d5 100644
> --- a/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> +++ b/drivers/infiniband/hw/ocrdma/ocrdma_hw.c
> @@ -1113,7 +1113,7 @@ mbx_err:
>  static int ocrdma_nonemb_mbx_cmd(struct ocrdma_dev *dev, struct ocrdma_mqe *mqe,
>  				 void *payload_va)
>  {
> -	int status = 0;
> +	int status;
>  	struct ocrdma_mbx_rsp *rsp = payload_va;
>  
>  	if ((mqe->hdr.spcl_sge_cnt_emb & OCRDMA_MQE_HDR_EMB_MASK) >>
> @@ -2871,7 +2871,7 @@ int ocrdma_mbx_destroy_srq(struct ocrdma_dev *dev, struct ocrdma_srq *srq)
>  static int ocrdma_mbx_get_dcbx_config(struct ocrdma_dev *dev, u32 ptype,
>  				      struct ocrdma_dcbx_cfg *dcbxcfg)
>  {
> -	int status = 0;
> +	int status;
>  	dma_addr_t pa;
>  	struct ocrdma_mqe cmd;
>  
> diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_stats.c b/drivers/infiniband/hw/ocrdma/ocrdma_stats.c
> index 86c303a..119baa3 100644
> --- a/drivers/infiniband/hw/ocrdma/ocrdma_stats.c
> +++ b/drivers/infiniband/hw/ocrdma/ocrdma_stats.c
> @@ -608,7 +608,7 @@ static char *ocrdma_driver_dbg_stats(struct ocrdma_dev *dev)
>  static void ocrdma_update_stats(struct ocrdma_dev *dev)
>  {
>  	ulong now = jiffies, secs;
> -	int status = 0;
> +	int status;
>  	struct ocrdma_rdma_stats_resp *rdma_stats =
>  		      (struct ocrdma_rdma_stats_resp *)dev->stats_mem.va;
>  	struct ocrdma_rsrc_stats *rsrc_stats = &rdma_stats->act_rsrc_stats;
> @@ -639,7 +639,7 @@ static ssize_t ocrdma_dbgfs_ops_write(struct file *filp,
>  {
>  	char tmp_str[32];
>  	long reset;
> -	int status = 0;
> +	int status;
>  	struct ocrdma_stats *pstats = filp->private_data;
>  	struct ocrdma_dev *dev = pstats->dev;
>  
> diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c b/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
> index 4caf167..1d90d18 100644
> --- a/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
> +++ b/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
> @@ -419,7 +419,7 @@ static struct ocrdma_pd *_ocrdma_alloc_pd(struct ocrdma_dev *dev,
>  					  struct ib_udata *udata)
>  {
>  	struct ocrdma_pd *pd = NULL;
> -	int status = 0;
> +	int status;
>  
>  	pd = kzalloc(sizeof(*pd), GFP_KERNEL);
>  	if (!pd)
> @@ -468,7 +468,7 @@ static inline int is_ucontext_pd(struct ocrdma_ucontext *uctx,
>  static int _ocrdma_dealloc_pd(struct ocrdma_dev *dev,
>  			      struct ocrdma_pd *pd)
>  {
> -	int status = 0;
> +	int status;
>  
>  	if (dev->pd_mgr->pd_prealloc_valid)
>  		status = ocrdma_put_pd_num(dev, pd->id, pd->dpp_enabled);
> @@ -593,7 +593,7 @@ map_err:
>  
>  int ocrdma_dealloc_ucontext(struct ib_ucontext *ibctx)
>  {
> -	int status = 0;
> +	int status;
>  	struct ocrdma_mm *mm, *tmp;
>  	struct ocrdma_ucontext *uctx = get_ocrdma_ucontext(ibctx);
>  	struct ocrdma_dev *dev = get_ocrdma_dev(ibctx->device);
> @@ -620,7 +620,7 @@ int ocrdma_mmap(struct ib_ucontext *context, struct vm_area_struct *vma)
>  	unsigned long vm_page = vma->vm_pgoff << PAGE_SHIFT;
>  	u64 unmapped_db = (u64) dev->nic_info.unmapped_db;
>  	unsigned long len = (vma->vm_end - vma->vm_start);
> -	int status = 0;
> +	int status;
>  	bool found;
>  
>  	if (vma->vm_start & (PAGE_SIZE - 1))
> @@ -1283,7 +1283,7 @@ static int ocrdma_copy_qp_uresp(struct ocrdma_qp *qp,
>  				struct ib_udata *udata, int dpp_offset,
>  				int dpp_credit_lmt, int srq)
>  {
> -	int status = 0;
> +	int status;
>  	u64 usr_db;
>  	struct ocrdma_create_qp_uresp uresp;
>  	struct ocrdma_pd *pd = qp->pd;
> @@ -1947,7 +1947,7 @@ int ocrdma_modify_srq(struct ib_srq *ibsrq,
>  		      enum ib_srq_attr_mask srq_attr_mask,
>  		      struct ib_udata *udata)
>  {
> -	int status = 0;
> +	int status;
>  	struct ocrdma_srq *srq;
>  
>  	srq = get_ocrdma_srq(ibsrq);
> -- 
> 2.6.3
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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]

#1310175 — Re: InfiniBand-ocrdma: Delete unnecessary variable initialisations in 11 functions

>> The variable "status" will be set to an appropriate value a bit later.
>> Thus omit the explicit initialisation at the beginning.
> 
> What did you try to achieve by this patch?

I would like to optimise the affected source files a bit.
Would you like to clarify any measurable effects around the implementation
detail when various variables will only be initialised immediately
before they will be read again?

Regards,
Markus

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

#1310207 — Re: InfiniBand-ocrdma: Delete unnecessary variable initialisations in 11 functions

On Fri, Jan 15, 2016 at 03:50:27PM +0100, SF Markus Elfring wrote:
> >> The variable "status" will be set to an appropriate value a bit later.
> >> Thus omit the explicit initialisation at the beginning.
> > 
> > What did you try to achieve by this patch?
> 
> I would like to optimise the affected source files a bit.
> Would you like to clarify any measurable effects around the implementation
> detail when various variables will only be initialised immediately
> before they will be read again?

Compiler will drop this variable initialization by itself because
there are no reads between this variable initialization and write.

I recommend you to take a look on the assembly code and ensure it
by yourself.

The proposed change won't affect performance at all.

> 
> Regards,
> Markus

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

#1310222 — Re: InfiniBand-ocrdma: Delete unnecessary variable initialisations in 11 functions

>> Would you like to clarify any measurable effects around the implementation
>> detail when various variables will only be initialised immediately
>> before they will be read again?
> 
> Compiler will drop this variable initialization by itself because
> there are no reads between this variable initialization and write.

Which compiler variants would you to take into account for such an use case?


> I recommend you to take a look on the assembly code and ensure it
> by yourself.

Will any configuration parameters and command arguments become relevant
to improve also a corresponding software comparison?


> The proposed change won't affect performance at all.

Will unneeded variable assignments be really optimised away by default?


By the way:
Will a small source code reduction matter also a bit here?

Regards,
Markus

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

#1310235 — Re: InfiniBand-ocrdma: Delete unnecessary variable initialisations in 11 functions

On Fri, Jan 15, 2016 at 04:26:36PM +0100, SF Markus Elfring wrote:
> >> Would you like to clarify any measurable effects around the implementation
> >> detail when various variables will only be initialised immediately
> >> before they will be read again?
> > 
> > Compiler will drop this variable initialization by itself because
> > there are no reads between this variable initialization and write.
> 
> Which compiler variants would you to take into account for such an use case?

GCC supported it before 1999 when I saw it first time. My assumption
that in 2016 all compilers are doing such optimization now.
I would be glad to hear an example of modern compiler which doesn't
support this simple optimization.

> 
> 
> > I recommend you to take a look on the assembly code and ensure it
> > by yourself.
> 
> Will any configuration parameters and command arguments become relevant
> to improve also a corresponding software comparison?

Please suggest us, you are proposing this change, and not me.

> 
> 
> > The proposed change won't affect performance at all.
> 
> Will unneeded variable assignments be really optimised away by default?

Yes

> 
> 
> By the way:
> Will a small source code reduction matter also a bit here?

If you are interested in saving space of one latter, you need to take into
account git database increase, do you?

> 
> Regards,
> Markus

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

#1310244 — Re: InfiniBand-ocrdma: Delete unnecessary variable initialisations in 11 functions

Doing bogus initializations turns off GCC's checking for uninitialized
variables so it's a bad habbit.

On the other hand, GCC's checking is not perfect and it sometimes misses
bugs so these patches have to be reviewed manually which is maybe too
much work to be worthwhile.

regards,
dan carpenter

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

#1310253 — Re: InfiniBand-ocrdma: Delete unnecessary variable initialisations in 11 functions

> GCC supported it before 1999 when I saw it first time. My assumption
> that in 2016 all compilers are doing such optimization now.

Interesting …


> I would be glad to hear an example of modern compiler which doesn't
> support this simple optimization.

Would you like to take into account any other source code analysis approaches?


>> Will any configuration parameters and command arguments become relevant
>> to improve also a corresponding software comparison?
> 
> Please suggest us, you are proposing this change, and not me.

Which combination of hardware and software versions would you find representative
for a corresponding system check?


>>> The proposed change won't affect performance at all.
>>
>> Will unneeded variable assignments be really optimised away by default?
> 
> Yes

Can it be that this result will depend on special parameters so that data flow
analysis and optimisation will be performed in the way you seem to expect?


> If you are interested in saving space of one latter, you need to take into
> account git database increase, do you?

There are also other aspects to consider:

* Do you insist to initialise a return code at the beginning of every function
  with a non-void return type?

* Does each bit of extra information can result also in unwanted consequences?

* Is this a specific source code review concern?

* Can this software be improved a bit more only if we dare to talk about
  potential update candidates?

Regards,
Markus

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

#1310282 — Re: InfiniBand-ocrdma: Delete unnecessary variable initialisations in 11 functions

On Fri, Jan 15, 2016 at 05:24:50PM +0100, SF Markus Elfring wrote:

Since, you didn't answer to my original question, I will repeat it again.
[Q.] What did you try to achieve by this patch?

P.S. This is mailing list for developers and not for patch bots.
We are glad to see patches that clean the code, but they need to
be meaningful. Your automated patches add noise without any real
benefit.

You was suggested to be ignored in MTD mailing list exactly for this
type of patches, did you learn anything from that experience?

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

#1310311 — Re: InfiniBand-ocrdma: Delete unnecessary variable initialisations in 11 functions

On Fri, Jan 15, 2016 at 09:19:38AM -0800, Bart Van Assche wrote:
> On 01/15/2016 09:00 AM, Leon Romanovsky wrote:
> >On Fri, Jan 15, 2016 at 05:24:50PM +0100, SF Markus Elfring wrote:
> >
> >Since, you didn't answer to my original question, I will repeat it again.
> >[Q.] What did you try to achieve by this patch?
> 
> Hello Leon,
> 
> Have you noticed Dan's reply: "Doing bogus initializations turns off GCC's
> checking for uninitialized variables so it's a bad habit."

Yes and his second part of that message too, that uninitialized
checks in GCC work as not as expected [1, 2].

Stackoverflow site has a lot examples of these types of bugs [3].
These examples together with Dan's suggestion requires from all
reviewers to be extra cautions when removing variable initialization.

[1] https://gcc.gnu.org/wiki/Better_Uninitialized_Warnings
[2] https://gcc.gnu.org/bugzilla/buglist.cgi?quicksearch=may%20be%20uninitialized
[3] http://stackoverflow.com/questions/27063678/compiler-not-detecting-obviously-uninitialized-variable

> 
> Thanks,
> 
> Bart.
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" 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]

#1310359 — Re: InfiniBand-ocrdma: Delete unnecessary variable initialisations in 11 functions

> [Q.] What did you try to achieve by this patch?

I would appreciate a bit more fine-tuning in the affected source files.


> P.S. This is mailing list for developers

Do you try to express any further restrictions?


> and not for patch bots.

Would you like to explain such an information a bit more?


> We are glad to see patches that clean the code, but they need to
> be meaningful.

This is usual.


> Your automated patches add noise without any real benefit.

Are you expecting a kind of special proof?


> You was suggested to be ignored in MTD mailing list exactly
> for this type of patches,

Will the acceptance increase a bit for similar issues over time?


> did you learn anything from that experience?

Will another acknowledgement by Selvin Xavier influence any corresponding
software improvements?


How do you think about to add any further constructive comments
also for the other proposed update steps?

Regards,
Markus

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

#1310901 — Re: InfiniBand-ocrdma: Delete unnecessary variable initialisations in 11 functions

On Fri, Jan 15, 2016 at 07:19:31PM +0100, SF Markus Elfring wrote:
> > [Q.] What did you try to achieve by this patch?
> 
> I would appreciate a bit more fine-tuning in the affected source files.

Please provide the numbers BEFORE and AFTER your change which can
support that your so called "fine-tuning" worked.

We are waiting to see it together with Tested-By tag to emphasize
that your code was tested on real HW and passed minimal sanity checks.

NAK on this patch.

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

#1310909 — Re: InfiniBand-ocrdma: Delete unnecessary variable initialisations in 11 functions

> Please provide the numbers BEFORE and AFTER your change which can
> support that your so called "fine-tuning" worked.

For which combinations of hardware and software versions would you
like to see corresponding results from detailed system checks
and special benchmarks?


> We are waiting to see it together with Tested-By tag to emphasize
> that your code was tested on real HW and passed minimal sanity checks.

Are any other contributors interested to collaborate for such a task?


> NAK on this patch.

Thanks for your feedback.

Does it mean that you reject (only) the proposed source code adjustments
around the variable "status" in the shown function selection at the moment?

Would you like to clarify the other update steps from this patch series
a bit more?

Regards,
Markus

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

#1309541 — [PATCH v3 4/6] InfiniBand-ocrdma: Return a value from a function call in _ocrdma_modify_qp() directly

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 14 Jan 2016 18:00:23 +0100

Return the value from a call of the ocrdma_mbx_modify_qp() function
without using an extra assignment for the local variable "status".

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

diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c b/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
index 1d90d18..5f2a34a 100644
--- a/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
+++ b/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
@@ -1492,9 +1492,7 @@ int _ocrdma_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
 	 */
 	if (status < 0)
 		return status;
-	status = ocrdma_mbx_modify_qp(dev, qp, attr, attr_mask);
-
-	return status;
+	return ocrdma_mbx_modify_qp(dev, qp, attr, attr_mask);
 }
 
 int ocrdma_modify_qp(struct ib_qp *ibqp, struct ib_qp_attr *attr,
-- 
2.6.3

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

#1309545 — [PATCH v3 6/6] InfiniBand-ocrdma: Delete an unnecessary variable in ocrdma_dealloc_pd()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 14 Jan 2016 18:08:08 +0100

1. Return zero in one case directly.

2. Return the value from a call of the _ocrdma_dealloc_pd() function
   without using an extra assignment for the local variable.

3. Remove the variable "status" in this function then.

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

diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c b/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
index d87985b..e6e1b51 100644
--- a/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
+++ b/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
@@ -751,7 +751,6 @@ int ocrdma_dealloc_pd(struct ib_pd *ibpd)
 	struct ocrdma_pd *pd = get_ocrdma_pd(ibpd);
 	struct ocrdma_dev *dev = get_ocrdma_dev(ibpd->device);
 	struct ocrdma_ucontext *uctx = NULL;
-	int status = 0;
 	u64 usr_db;
 
 	uctx = pd->uctx;
@@ -765,11 +764,10 @@ int ocrdma_dealloc_pd(struct ib_pd *ibpd)
 
 		if (is_ucontext_pd(uctx, pd)) {
 			ocrdma_release_ucontext_pd(uctx);
-			return status;
+			return 0;
 		}
 	}
-	status = _ocrdma_dealloc_pd(dev, pd);
-	return status;
+	return _ocrdma_dealloc_pd(dev, pd);
 }
 
 static int ocrdma_alloc_lkey(struct ocrdma_dev *dev, struct ocrdma_mr *mr,
-- 
2.6.3

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

#1309548 — [PATCH v3 5/6] InfiniBand-ocrdma: Returning only value constants in ocrdma_resize_cq()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Thu, 14 Jan 2016 18:04:17 +0100

Return constant integer values without storing them in the local
variable "status".

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

diff --git a/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c b/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
index 5f2a34a..d87985b 100644
--- a/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
+++ b/drivers/infiniband/hw/ocrdma/ocrdma_verbs.c
@@ -1121,15 +1121,12 @@ ctx_err:
 int ocrdma_resize_cq(struct ib_cq *ibcq, int new_cnt,
 		     struct ib_udata *udata)
 {
-	int status = 0;
 	struct ocrdma_cq *cq = get_ocrdma_cq(ibcq);
 
-	if (new_cnt < 1 || new_cnt > cq->max_hw_cqe) {
-		status = -EINVAL;
-		return status;
-	}
+	if (new_cnt < 1 || new_cnt > cq->max_hw_cqe)
+		return -EINVAL;
 	ibcq->cqe = new_cnt;
-	return status;
+	return 0;
 }
 
 static void ocrdma_flush_cq(struct ocrdma_cq *cq)
-- 
2.6.3

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web