All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH][next] RDMA/nes: remove redundant check on udata
@ 2019-03-02 23:06 ` Colin King
  0 siblings, 0 replies; 10+ messages in thread
From: Colin King @ 2019-03-02 23:06 UTC (permalink / raw)
  To: Faisal Latif, Doug Ledford, Jason Gunthorpe, linux-rdma
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

The non-null check on udata is redundant as this check was performed
just a few statements earlier and the check is always true as udata
must be non-null at this point. Remove redundant the check on udata
and the redundant else part that can never be executed.

Detected by CoverityScan, CID#1477317 ("Logically dead code")

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/infiniband/hw/nes/nes_verbs.c | 73 +++++++++++++--------------
 1 file changed, 34 insertions(+), 39 deletions(-)

diff --git a/drivers/infiniband/hw/nes/nes_verbs.c b/drivers/infiniband/hw/nes/nes_verbs.c
index 828e4af3f951..526092d435df 100644
--- a/drivers/infiniband/hw/nes/nes_verbs.c
+++ b/drivers/infiniband/hw/nes/nes_verbs.c
@@ -1039,53 +1039,48 @@ static struct ib_qp *nes_create_qp(struct ib_pd *ibpd,
 				}
 				if (req.user_qp_buffer)
 					nesqp->nesuqp_addr = req.user_qp_buffer;
-				if (udata) {
-					nesqp->user_mode = 1;
-					if (virt_wqs) {
-						err = 1;
-						list_for_each_entry(nespbl, &nes_ucontext->qp_reg_mem_list, list) {
-							if (nespbl->user_base == (unsigned long )req.user_wqe_buffers) {
-								list_del(&nespbl->list);
-								err = 0;
-								nes_debug(NES_DBG_QP, "Found PBL for virtual QP. nespbl=%p. user_base=0x%lx\n",
-									  nespbl, nespbl->user_base);
-								break;
-							}
-						}
-						if (err) {
-							nes_debug(NES_DBG_QP, "Didn't Find PBL for virtual QP. address = %llx.\n",
-								  (long long unsigned int)req.user_wqe_buffers);
-							nes_free_resource(nesadapter, nesadapter->allocated_qps, qp_num);
-							kfree(nesqp->allocated_buffer);
-							return ERR_PTR(-EFAULT);
+
+				nesqp->user_mode = 1;
+				if (virt_wqs) {
+					err = 1;
+					list_for_each_entry(nespbl, &nes_ucontext->qp_reg_mem_list, list) {
+						if (nespbl->user_base == (unsigned long )req.user_wqe_buffers) {
+							list_del(&nespbl->list);
+							err = 0;
+							nes_debug(NES_DBG_QP, "Found PBL for virtual QP. nespbl=%p. user_base=0x%lx\n",
+								  nespbl, nespbl->user_base);
+							break;
 						}
 					}
-
-					nesqp->mmap_sq_db_index =
-						find_next_zero_bit(nes_ucontext->allocated_wqs,
-								   NES_MAX_USER_WQ_REGIONS, nes_ucontext->first_free_wq);
-					/* nes_debug(NES_DBG_QP, "find_first_zero_biton wqs returned %u\n",
-							nespd->mmap_db_index); */
-					if (nesqp->mmap_sq_db_index >= NES_MAX_USER_WQ_REGIONS) {
-						nes_debug(NES_DBG_QP,
-							  "db index > max user regions, failing create QP\n");
+					if (err) {
+						nes_debug(NES_DBG_QP, "Didn't Find PBL for virtual QP. address = %llx.\n",
+							  (long long unsigned int)req.user_wqe_buffers);
 						nes_free_resource(nesadapter, nesadapter->allocated_qps, qp_num);
-						if (virt_wqs) {
-							pci_free_consistent(nesdev->pcidev, nespbl->pbl_size, nespbl->pbl_vbase,
-									    nespbl->pbl_pbase);
-							kfree(nespbl);
-						}
 						kfree(nesqp->allocated_buffer);
-						return ERR_PTR(-ENOMEM);
+						return ERR_PTR(-EFAULT);
 					}
-					set_bit(nesqp->mmap_sq_db_index, nes_ucontext->allocated_wqs);
-					nes_ucontext->mmap_nesqp[nesqp->mmap_sq_db_index] = nesqp;
-					nes_ucontext->first_free_wq = nesqp->mmap_sq_db_index + 1;
-				} else {
+				}
+
+				nesqp->mmap_sq_db_index =
+					find_next_zero_bit(nes_ucontext->allocated_wqs,
+							   NES_MAX_USER_WQ_REGIONS, nes_ucontext->first_free_wq);
+				/* nes_debug(NES_DBG_QP, "find_first_zero_biton wqs returned %u\n",
+						nespd->mmap_db_index); */
+				if (nesqp->mmap_sq_db_index >= NES_MAX_USER_WQ_REGIONS) {
+					nes_debug(NES_DBG_QP,
+						  "db index > max user regions, failing create QP\n");
 					nes_free_resource(nesadapter, nesadapter->allocated_qps, qp_num);
+					if (virt_wqs) {
+						pci_free_consistent(nesdev->pcidev, nespbl->pbl_size, nespbl->pbl_vbase,
+								    nespbl->pbl_pbase);
+						kfree(nespbl);
+					}
 					kfree(nesqp->allocated_buffer);
-					return ERR_PTR(-EFAULT);
+					return ERR_PTR(-ENOMEM);
 				}
+				set_bit(nesqp->mmap_sq_db_index, nes_ucontext->allocated_wqs);
+				nes_ucontext->mmap_nesqp[nesqp->mmap_sq_db_index] = nesqp;
+				nes_ucontext->first_free_wq = nesqp->mmap_sq_db_index + 1;
 			}
 			err = (!virt_wqs) ? nes_setup_mmap_qp(nesqp, nesvnic, sq_size, rq_size) :
 					nes_setup_virt_qp(nesqp, nespbl, nesvnic, sq_size, rq_size);
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH][next] RDMA/nes: remove redundant check on udata
@ 2019-03-02 23:06 ` Colin King
  0 siblings, 0 replies; 10+ messages in thread
From: Colin King @ 2019-03-02 23:06 UTC (permalink / raw)
  To: Faisal Latif, Doug Ledford, Jason Gunthorpe, linux-rdma
  Cc: kernel-janitors, linux-kernel

From: Colin Ian King <colin.king@canonical.com>

The non-null check on udata is redundant as this check was performed
just a few statements earlier and the check is always true as udata
must be non-null at this point. Remove redundant the check on udata
and the redundant else part that can never be executed.

Detected by CoverityScan, CID#1477317 ("Logically dead code")

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 drivers/infiniband/hw/nes/nes_verbs.c | 73 +++++++++++++--------------
 1 file changed, 34 insertions(+), 39 deletions(-)

diff --git a/drivers/infiniband/hw/nes/nes_verbs.c b/drivers/infiniband/hw/nes/nes_verbs.c
index 828e4af3f951..526092d435df 100644
--- a/drivers/infiniband/hw/nes/nes_verbs.c
+++ b/drivers/infiniband/hw/nes/nes_verbs.c
@@ -1039,53 +1039,48 @@ static struct ib_qp *nes_create_qp(struct ib_pd *ibpd,
 				}
 				if (req.user_qp_buffer)
 					nesqp->nesuqp_addr = req.user_qp_buffer;
-				if (udata) {
-					nesqp->user_mode = 1;
-					if (virt_wqs) {
-						err = 1;
-						list_for_each_entry(nespbl, &nes_ucontext->qp_reg_mem_list, list) {
-							if (nespbl->user_base = (unsigned long )req.user_wqe_buffers) {
-								list_del(&nespbl->list);
-								err = 0;
-								nes_debug(NES_DBG_QP, "Found PBL for virtual QP. nespbl=%p. user_base=0x%lx\n",
-									  nespbl, nespbl->user_base);
-								break;
-							}
-						}
-						if (err) {
-							nes_debug(NES_DBG_QP, "Didn't Find PBL for virtual QP. address = %llx.\n",
-								  (long long unsigned int)req.user_wqe_buffers);
-							nes_free_resource(nesadapter, nesadapter->allocated_qps, qp_num);
-							kfree(nesqp->allocated_buffer);
-							return ERR_PTR(-EFAULT);
+
+				nesqp->user_mode = 1;
+				if (virt_wqs) {
+					err = 1;
+					list_for_each_entry(nespbl, &nes_ucontext->qp_reg_mem_list, list) {
+						if (nespbl->user_base = (unsigned long )req.user_wqe_buffers) {
+							list_del(&nespbl->list);
+							err = 0;
+							nes_debug(NES_DBG_QP, "Found PBL for virtual QP. nespbl=%p. user_base=0x%lx\n",
+								  nespbl, nespbl->user_base);
+							break;
 						}
 					}
-
-					nesqp->mmap_sq_db_index -						find_next_zero_bit(nes_ucontext->allocated_wqs,
-								   NES_MAX_USER_WQ_REGIONS, nes_ucontext->first_free_wq);
-					/* nes_debug(NES_DBG_QP, "find_first_zero_biton wqs returned %u\n",
-							nespd->mmap_db_index); */
-					if (nesqp->mmap_sq_db_index >= NES_MAX_USER_WQ_REGIONS) {
-						nes_debug(NES_DBG_QP,
-							  "db index > max user regions, failing create QP\n");
+					if (err) {
+						nes_debug(NES_DBG_QP, "Didn't Find PBL for virtual QP. address = %llx.\n",
+							  (long long unsigned int)req.user_wqe_buffers);
 						nes_free_resource(nesadapter, nesadapter->allocated_qps, qp_num);
-						if (virt_wqs) {
-							pci_free_consistent(nesdev->pcidev, nespbl->pbl_size, nespbl->pbl_vbase,
-									    nespbl->pbl_pbase);
-							kfree(nespbl);
-						}
 						kfree(nesqp->allocated_buffer);
-						return ERR_PTR(-ENOMEM);
+						return ERR_PTR(-EFAULT);
 					}
-					set_bit(nesqp->mmap_sq_db_index, nes_ucontext->allocated_wqs);
-					nes_ucontext->mmap_nesqp[nesqp->mmap_sq_db_index] = nesqp;
-					nes_ucontext->first_free_wq = nesqp->mmap_sq_db_index + 1;
-				} else {
+				}
+
+				nesqp->mmap_sq_db_index +					find_next_zero_bit(nes_ucontext->allocated_wqs,
+							   NES_MAX_USER_WQ_REGIONS, nes_ucontext->first_free_wq);
+				/* nes_debug(NES_DBG_QP, "find_first_zero_biton wqs returned %u\n",
+						nespd->mmap_db_index); */
+				if (nesqp->mmap_sq_db_index >= NES_MAX_USER_WQ_REGIONS) {
+					nes_debug(NES_DBG_QP,
+						  "db index > max user regions, failing create QP\n");
 					nes_free_resource(nesadapter, nesadapter->allocated_qps, qp_num);
+					if (virt_wqs) {
+						pci_free_consistent(nesdev->pcidev, nespbl->pbl_size, nespbl->pbl_vbase,
+								    nespbl->pbl_pbase);
+						kfree(nespbl);
+					}
 					kfree(nesqp->allocated_buffer);
-					return ERR_PTR(-EFAULT);
+					return ERR_PTR(-ENOMEM);
 				}
+				set_bit(nesqp->mmap_sq_db_index, nes_ucontext->allocated_wqs);
+				nes_ucontext->mmap_nesqp[nesqp->mmap_sq_db_index] = nesqp;
+				nes_ucontext->first_free_wq = nesqp->mmap_sq_db_index + 1;
 			}
 			err = (!virt_wqs) ? nes_setup_mmap_qp(nesqp, nesvnic, sq_size, rq_size) :
 					nes_setup_virt_qp(nesqp, nespbl, nesvnic, sq_size, rq_size);
-- 
2.20.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: [PATCH][next] RDMA/nes: remove redundant check on udata
  2019-03-02 23:06 ` Colin King
@ 2019-03-04  6:36   ` Leon Romanovsky
  -1 siblings, 0 replies; 10+ messages in thread
From: Leon Romanovsky @ 2019-03-04  6:36 UTC (permalink / raw)
  To: Colin King
  Cc: Faisal Latif, Doug Ledford, Jason Gunthorpe, linux-rdma,
	kernel-janitors, linux-kernel

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

On Sat, Mar 02, 2019 at 11:06:36PM +0000, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The non-null check on udata is redundant as this check was performed
> just a few statements earlier and the check is always true as udata
> must be non-null at this point. Remove redundant the check on udata
> and the redundant else part that can never be executed.
>
> Detected by CoverityScan, CID#1477317 ("Logically dead code")
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  drivers/infiniband/hw/nes/nes_verbs.c | 73 +++++++++++++--------------
>  1 file changed, 34 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/infiniband/hw/nes/nes_verbs.c b/drivers/infiniband/hw/nes/nes_verbs.c
> index 828e4af3f951..526092d435df 100644
> --- a/drivers/infiniband/hw/nes/nes_verbs.c
> +++ b/drivers/infiniband/hw/nes/nes_verbs.c
> @@ -1039,53 +1039,48 @@ static struct ib_qp *nes_create_qp(struct ib_pd *ibpd,
>  				}
>  				if (req.user_qp_buffer)
>  					nesqp->nesuqp_addr = req.user_qp_buffer;
> -				if (udata) {
> -					nesqp->user_mode = 1;

It was single place where user_mode was set, you should continue and
remove all code which depends on "if (nesqp->user_mode)".

I looked on the change which triggered Coverity warning and looks like
your commit message should include this Fixes line:
Fixes: 899444505473 ("IB/{hw,sw}: Remove 'uobject->context' dependency in object creation APIs")

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH][next] RDMA/nes: remove redundant check on udata
@ 2019-03-04  6:36   ` Leon Romanovsky
  0 siblings, 0 replies; 10+ messages in thread
From: Leon Romanovsky @ 2019-03-04  6:36 UTC (permalink / raw)
  To: Colin King
  Cc: Faisal Latif, Doug Ledford, Jason Gunthorpe, linux-rdma,
	kernel-janitors, linux-kernel

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

On Sat, Mar 02, 2019 at 11:06:36PM +0000, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The non-null check on udata is redundant as this check was performed
> just a few statements earlier and the check is always true as udata
> must be non-null at this point. Remove redundant the check on udata
> and the redundant else part that can never be executed.
>
> Detected by CoverityScan, CID#1477317 ("Logically dead code")
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  drivers/infiniband/hw/nes/nes_verbs.c | 73 +++++++++++++--------------
>  1 file changed, 34 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/infiniband/hw/nes/nes_verbs.c b/drivers/infiniband/hw/nes/nes_verbs.c
> index 828e4af3f951..526092d435df 100644
> --- a/drivers/infiniband/hw/nes/nes_verbs.c
> +++ b/drivers/infiniband/hw/nes/nes_verbs.c
> @@ -1039,53 +1039,48 @@ static struct ib_qp *nes_create_qp(struct ib_pd *ibpd,
>  				}
>  				if (req.user_qp_buffer)
>  					nesqp->nesuqp_addr = req.user_qp_buffer;
> -				if (udata) {
> -					nesqp->user_mode = 1;

It was single place where user_mode was set, you should continue and
remove all code which depends on "if (nesqp->user_mode)".

I looked on the change which triggered Coverity warning and looks like
your commit message should include this Fixes line:
Fixes: 899444505473 ("IB/{hw,sw}: Remove 'uobject->context' dependency in object creation APIs")

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH][next] RDMA/nes: remove redundant check on udata
  2019-03-04  6:36   ` Leon Romanovsky
@ 2019-03-07 10:23     ` Colin Ian King
  -1 siblings, 0 replies; 10+ messages in thread
From: Colin Ian King @ 2019-03-07 10:23 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Faisal Latif, Doug Ledford, Jason Gunthorpe, linux-rdma,
	kernel-janitors, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 2639 bytes --]

On 04/03/2019 06:36, Leon Romanovsky wrote:
> On Sat, Mar 02, 2019 at 11:06:36PM +0000, Colin King wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> The non-null check on udata is redundant as this check was performed
>> just a few statements earlier and the check is always true as udata
>> must be non-null at this point. Remove redundant the check on udata
>> and the redundant else part that can never be executed.
>>
>> Detected by CoverityScan, CID#1477317 ("Logically dead code")
>>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>>  drivers/infiniband/hw/nes/nes_verbs.c | 73 +++++++++++++--------------
>>  1 file changed, 34 insertions(+), 39 deletions(-)
>>
>> diff --git a/drivers/infiniband/hw/nes/nes_verbs.c b/drivers/infiniband/hw/nes/nes_verbs.c
>> index 828e4af3f951..526092d435df 100644
>> --- a/drivers/infiniband/hw/nes/nes_verbs.c
>> +++ b/drivers/infiniband/hw/nes/nes_verbs.c
>> @@ -1039,53 +1039,48 @@ static struct ib_qp *nes_create_qp(struct ib_pd *ibpd,
>>  				}
>>  				if (req.user_qp_buffer)
>>  					nesqp->nesuqp_addr = req.user_qp_buffer;
>> -				if (udata) {
>> -					nesqp->user_mode = 1;
> 
> It was single place where user_mode was set, you should continue and
> remove all code which depends on "if (nesqp->user_mode)".

This is not the only place where user_mode is being set; we currently have:


        if (udata) {
                if (ib_copy_from_udata(&req, udata, sizeof(struct
nes_create_qp_req))) {
                        nes_free_resource(nesadapter,
nesadapter->allocated_qps, qp_num);
                        kfree(nesqp->allocated_buffer);
                        nes_debug(NES_DBG_QP, "ib_copy_from_udata()
Failed \n");
                        return ERR_PTR(-EFAULT);
                }
                if (req.user_wqe_buffers) {
                        virt_wqs = 1;
                }
                if (req.user_qp_buffer)
                        nesqp->nesuqp_addr = req.user_qp_buffer;
                if (udata) {
                        nesqp->user_mode = 1;
			...


and I'm just removing the redudant 2nd if (udata) check, so the
nesqp->user_mode = 1 assignment is still being kept hence it's not
correct to remove all the if (nesqp->user_mode) releated code.


> 
> I looked on the change which triggered Coverity warning and looks like
> your commit message should include this Fixes line:
> Fixes: 899444505473 ("IB/{hw,sw}: Remove 'uobject->context' dependency in object creation APIs")

Can that be added to the patch before it's applied?

> 
> Thanks
> 

Colin


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH][next] RDMA/nes: remove redundant check on udata
@ 2019-03-07 10:23     ` Colin Ian King
  0 siblings, 0 replies; 10+ messages in thread
From: Colin Ian King @ 2019-03-07 10:23 UTC (permalink / raw)
  To: Leon Romanovsky
  Cc: Faisal Latif, Doug Ledford, Jason Gunthorpe, linux-rdma,
	kernel-janitors, linux-kernel


[-- Attachment #1.1: Type: text/plain, Size: 2639 bytes --]

On 04/03/2019 06:36, Leon Romanovsky wrote:
> On Sat, Mar 02, 2019 at 11:06:36PM +0000, Colin King wrote:
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> The non-null check on udata is redundant as this check was performed
>> just a few statements earlier and the check is always true as udata
>> must be non-null at this point. Remove redundant the check on udata
>> and the redundant else part that can never be executed.
>>
>> Detected by CoverityScan, CID#1477317 ("Logically dead code")
>>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>>  drivers/infiniband/hw/nes/nes_verbs.c | 73 +++++++++++++--------------
>>  1 file changed, 34 insertions(+), 39 deletions(-)
>>
>> diff --git a/drivers/infiniband/hw/nes/nes_verbs.c b/drivers/infiniband/hw/nes/nes_verbs.c
>> index 828e4af3f951..526092d435df 100644
>> --- a/drivers/infiniband/hw/nes/nes_verbs.c
>> +++ b/drivers/infiniband/hw/nes/nes_verbs.c
>> @@ -1039,53 +1039,48 @@ static struct ib_qp *nes_create_qp(struct ib_pd *ibpd,
>>  				}
>>  				if (req.user_qp_buffer)
>>  					nesqp->nesuqp_addr = req.user_qp_buffer;
>> -				if (udata) {
>> -					nesqp->user_mode = 1;
> 
> It was single place where user_mode was set, you should continue and
> remove all code which depends on "if (nesqp->user_mode)".

This is not the only place where user_mode is being set; we currently have:


        if (udata) {
                if (ib_copy_from_udata(&req, udata, sizeof(struct
nes_create_qp_req))) {
                        nes_free_resource(nesadapter,
nesadapter->allocated_qps, qp_num);
                        kfree(nesqp->allocated_buffer);
                        nes_debug(NES_DBG_QP, "ib_copy_from_udata()
Failed \n");
                        return ERR_PTR(-EFAULT);
                }
                if (req.user_wqe_buffers) {
                        virt_wqs = 1;
                }
                if (req.user_qp_buffer)
                        nesqp->nesuqp_addr = req.user_qp_buffer;
                if (udata) {
                        nesqp->user_mode = 1;
			...


and I'm just removing the redudant 2nd if (udata) check, so the
nesqp->user_mode = 1 assignment is still being kept hence it's not
correct to remove all the if (nesqp->user_mode) releated code.


> 
> I looked on the change which triggered Coverity warning and looks like
> your commit message should include this Fixes line:
> Fixes: 899444505473 ("IB/{hw,sw}: Remove 'uobject->context' dependency in object creation APIs")

Can that be added to the patch before it's applied?

> 
> Thanks
> 

Colin


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH][next] RDMA/nes: remove redundant check on udata
  2019-03-07 10:23     ` Colin Ian King
@ 2019-03-07 17:01       ` Leon Romanovsky
  -1 siblings, 0 replies; 10+ messages in thread
From: Leon Romanovsky @ 2019-03-07 17:01 UTC (permalink / raw)
  To: Colin Ian King
  Cc: Faisal Latif, Doug Ledford, Jason Gunthorpe, linux-rdma,
	kernel-janitors, linux-kernel

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

On Thu, Mar 07, 2019 at 10:23:28AM +0000, Colin Ian King wrote:
> On 04/03/2019 06:36, Leon Romanovsky wrote:
> > On Sat, Mar 02, 2019 at 11:06:36PM +0000, Colin King wrote:
> >> From: Colin Ian King <colin.king@canonical.com>
> >>
> >> The non-null check on udata is redundant as this check was performed
> >> just a few statements earlier and the check is always true as udata
> >> must be non-null at this point. Remove redundant the check on udata
> >> and the redundant else part that can never be executed.
> >>
> >> Detected by CoverityScan, CID#1477317 ("Logically dead code")
> >>
> >> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> >> ---
> >>  drivers/infiniband/hw/nes/nes_verbs.c | 73 +++++++++++++--------------
> >>  1 file changed, 34 insertions(+), 39 deletions(-)
> >>
> >> diff --git a/drivers/infiniband/hw/nes/nes_verbs.c b/drivers/infiniband/hw/nes/nes_verbs.c
> >> index 828e4af3f951..526092d435df 100644
> >> --- a/drivers/infiniband/hw/nes/nes_verbs.c
> >> +++ b/drivers/infiniband/hw/nes/nes_verbs.c
> >> @@ -1039,53 +1039,48 @@ static struct ib_qp *nes_create_qp(struct ib_pd *ibpd,
> >>  				}
> >>  				if (req.user_qp_buffer)
> >>  					nesqp->nesuqp_addr = req.user_qp_buffer;
> >> -				if (udata) {
> >> -					nesqp->user_mode = 1;
> >
> > It was single place where user_mode was set, you should continue and
> > remove all code which depends on "if (nesqp->user_mode)".
>
> This is not the only place where user_mode is being set; we currently have:

ok, It doesn't matter, I'll clean it later with my memory allocation
conversion patches.

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH][next] RDMA/nes: remove redundant check on udata
@ 2019-03-07 17:01       ` Leon Romanovsky
  0 siblings, 0 replies; 10+ messages in thread
From: Leon Romanovsky @ 2019-03-07 17:01 UTC (permalink / raw)
  To: Colin Ian King
  Cc: Faisal Latif, Doug Ledford, Jason Gunthorpe, linux-rdma,
	kernel-janitors, linux-kernel

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

On Thu, Mar 07, 2019 at 10:23:28AM +0000, Colin Ian King wrote:
> On 04/03/2019 06:36, Leon Romanovsky wrote:
> > On Sat, Mar 02, 2019 at 11:06:36PM +0000, Colin King wrote:
> >> From: Colin Ian King <colin.king@canonical.com>
> >>
> >> The non-null check on udata is redundant as this check was performed
> >> just a few statements earlier and the check is always true as udata
> >> must be non-null at this point. Remove redundant the check on udata
> >> and the redundant else part that can never be executed.
> >>
> >> Detected by CoverityScan, CID#1477317 ("Logically dead code")
> >>
> >> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> >> ---
> >>  drivers/infiniband/hw/nes/nes_verbs.c | 73 +++++++++++++--------------
> >>  1 file changed, 34 insertions(+), 39 deletions(-)
> >>
> >> diff --git a/drivers/infiniband/hw/nes/nes_verbs.c b/drivers/infiniband/hw/nes/nes_verbs.c
> >> index 828e4af3f951..526092d435df 100644
> >> --- a/drivers/infiniband/hw/nes/nes_verbs.c
> >> +++ b/drivers/infiniband/hw/nes/nes_verbs.c
> >> @@ -1039,53 +1039,48 @@ static struct ib_qp *nes_create_qp(struct ib_pd *ibpd,
> >>  				}
> >>  				if (req.user_qp_buffer)
> >>  					nesqp->nesuqp_addr = req.user_qp_buffer;
> >> -				if (udata) {
> >> -					nesqp->user_mode = 1;
> >
> > It was single place where user_mode was set, you should continue and
> > remove all code which depends on "if (nesqp->user_mode)".
>
> This is not the only place where user_mode is being set; we currently have:

ok, It doesn't matter, I'll clean it later with my memory allocation
conversion patches.

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH][next] RDMA/nes: remove redundant check on udata
  2019-03-02 23:06 ` Colin King
@ 2019-03-26 15:10   ` Jason Gunthorpe
  -1 siblings, 0 replies; 10+ messages in thread
From: Jason Gunthorpe @ 2019-03-26 15:10 UTC (permalink / raw)
  To: Colin King
  Cc: Faisal Latif, Doug Ledford, linux-rdma, kernel-janitors, linux-kernel

On Sat, Mar 02, 2019 at 11:06:36PM +0000, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> The non-null check on udata is redundant as this check was performed
> just a few statements earlier and the check is always true as udata
> must be non-null at this point. Remove redundant the check on udata
> and the redundant else part that can never be executed.
> 
> Detected by CoverityScan, CID#1477317 ("Logically dead code")
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  drivers/infiniband/hw/nes/nes_verbs.c | 73 +++++++++++++--------------
>  1 file changed, 34 insertions(+), 39 deletions(-)

Applied to for-next

Thanks,
Jason

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH][next] RDMA/nes: remove redundant check on udata
@ 2019-03-26 15:10   ` Jason Gunthorpe
  0 siblings, 0 replies; 10+ messages in thread
From: Jason Gunthorpe @ 2019-03-26 15:10 UTC (permalink / raw)
  To: Colin King
  Cc: Faisal Latif, Doug Ledford, linux-rdma, kernel-janitors, linux-kernel

On Sat, Mar 02, 2019 at 11:06:36PM +0000, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> The non-null check on udata is redundant as this check was performed
> just a few statements earlier and the check is always true as udata
> must be non-null at this point. Remove redundant the check on udata
> and the redundant else part that can never be executed.
> 
> Detected by CoverityScan, CID#1477317 ("Logically dead code")
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  drivers/infiniband/hw/nes/nes_verbs.c | 73 +++++++++++++--------------
>  1 file changed, 34 insertions(+), 39 deletions(-)

Applied to for-next

Thanks,
Jason

^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2019-03-26 15:10 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-02 23:06 [PATCH][next] RDMA/nes: remove redundant check on udata Colin King
2019-03-02 23:06 ` Colin King
2019-03-04  6:36 ` Leon Romanovsky
2019-03-04  6:36   ` Leon Romanovsky
2019-03-07 10:23   ` Colin Ian King
2019-03-07 10:23     ` Colin Ian King
2019-03-07 17:01     ` Leon Romanovsky
2019-03-07 17:01       ` Leon Romanovsky
2019-03-26 15:10 ` Jason Gunthorpe
2019-03-26 15:10   ` Jason Gunthorpe

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.