Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 7 Sep 2022 15:37:11 +0300
From:   Leon Romanovsky <leon@kernel.org>
To:     Li zeming <zeming@nfschina.com>
Cc:     jgg@ziepe.ca, linux-rdma@vger.kernel.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH] rdma/rdmavt_qp: Remove unnecessary 'NULL' values from qp
Message-ID: <YxiQd+sO8OfYkmJq@unreal>
References: <20220907033604.4637-1-zeming@nfschina.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220907033604.4637-1-zeming@nfschina.com>
Precedence: bulk

On Wed, Sep 07, 2022 at 11:36:04AM +0800, Li zeming wrote:
> The pointer qp is assigned before it is used, it does not need to be
> initialized and assigned.
> 
> Signed-off-by: Li zeming <zeming@nfschina.com>
> ---
>  include/rdma/rdmavt_qp.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/rdma/rdmavt_qp.h b/include/rdma/rdmavt_qp.h
> index 2e58d5e6ac0e..2afc3300d618 100644
> --- a/include/rdma/rdmavt_qp.h
> +++ b/include/rdma/rdmavt_qp.h
> @@ -699,7 +699,7 @@ static inline struct rvt_qp *rvt_lookup_qpn(struct rvt_dev_info *rdi,
>  					    struct rvt_ibport *rvp,
>  					    u32 qpn) __must_hold(RCU)
>  {
> -	struct rvt_qp *qp = NULL;
> +	struct rvt_qp *qp;
>  
>  	if (unlikely(qpn <= 1)) {
>  		qp = rcu_dereference(rvp->qp[qpn]);

This function is completely wrong, most likely it never returns NULL
otherwise, we would crash in "if (qp->ibqp.qp_num == qpn)" line.

The proper change will be something like this:

diff --git a/include/rdma/rdmavt_qp.h b/include/rdma/rdmavt_qp.h
index 2e58d5e6ac0e..883c328e06b6 100644
--- a/include/rdma/rdmavt_qp.h
+++ b/include/rdma/rdmavt_qp.h
@@ -699,19 +699,19 @@ static inline struct rvt_qp *rvt_lookup_qpn(struct rvt_dev_info *rdi,
                                            struct rvt_ibport *rvp,
                                            u32 qpn) __must_hold(RCU)
 {
-       struct rvt_qp *qp = NULL;
+       struct rvt_qp *qp;
+       u32 n;

-       if (unlikely(qpn <= 1)) {
-               qp = rcu_dereference(rvp->qp[qpn]);
-       } else {
-               u32 n = hash_32(qpn, rdi->qp_dev->qp_table_bits);
+       if (unlikely(qpn <= 1))
+               return rcu_dereference(rvp->qp[qpn]);

-               for (qp = rcu_dereference(rdi->qp_dev->qp_table[n]); qp;
-                       qp = rcu_dereference(qp->next))
-                       if (qp->ibqp.qp_num == qpn)
-                               break;
-       }
-       return qp;
+       n = hash_32(qpn, rdi->qp_dev->qp_table_bits);
+
+       for (qp = rcu_dereference(rdi->qp_dev->qp_table[n]); qp;
+            qp = rcu_dereference(qp->next))
+               if (qp->ibqp.qp_num == qpn)
+                       return qp;
+       return NULL;
 }

 /**


> -- 
> 2.18.2
>