[PATCH for-next 0/9] Bug fixes & Code improvements in hip06 and hip08 RoCE driver

[PATCH for-next 0/9] Bug fixes & Code improvements in hip06 and hip08 RoCE driver

[Wei Hu (Xavier)]

This patch-set introduces some bug fixes and code improvements
for hip06 and hip08 RoCE driver. It includes a patch for fixing
the assign algorithm of qp_attr->max_rd_atomic and
qp_attr->max_dest_rd_atomic, three patches for static check errors,
one for setting attr mask, one for returning RoCE device ah_attr
type when querying qp, one for the abi structure between libhns and
hns_roce drvier, one for unregistering inet addr, and the last one
for command queue delay processing in hip08 driver.

Lijun Ou (8):
  RDMA/hns: Modify the value with rd&dest_rd of qp_attr
  RDMA/hns: Factor out the code for checking sdb status into a new
    function
  RDMA/hns: Add return statement when kzalloc return NULL in
    hns_roce_v1_recreate_lp_qp
  RDMA/hns: Set mask for destination qp field of qp context assignment
  RDMA/hns: Set rdma_ah_attr type for querying qp
  RDMA/hns: Remove unnecessarily calling unregister_inetaddr_notifier
    function
  RDMA/hns: Remove unused struct members in hns-abi.h
  RDMA/hns: Replace usleep_range with udelay when checking command
    status

Wei Hu (Xavier) (1):
  RDMA/hns: Add return statement when checking error in
    hns_roce_v1_mr_free_work_fn

 drivers/infiniband/hw/hns/hns_roce_device.h |   1 -
 drivers/infiniband/hw/hns/hns_roce_hw_v1.c  | 117 ++++++++++++++++------------
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c  |   3 +-
 drivers/infiniband/hw/hns/hns_roce_main.c   |   1 -
 include/uapi/rdma/hns-abi.h                 |   2 -
 5 files changed, 69 insertions(+), 55 deletions(-)

-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH for-next 1/9] RDMA/hns: Modify the value with rd&dest_rd of qp_attr

[Wei Hu (Xavier)]

From: Lijun Ou <oulijun@huawei.com>

The value of max_rd_atomic and max_dest_rd_atomic in query_qp
are incorrect. It should be assigned by left shifting of
the bit in hip06 SoC.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
index 426f55a..6e9acfd 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
@@ -3484,10 +3484,10 @@ static int hns_roce_v1_q_qp(struct ib_qp *ibqp, struct ib_qp_attr *qp_attr,
 			      QP_CONTEXT_QPC_BYTES_12_P_KEY_INDEX_S);
 	qp_attr->port_num = hr_qp->port + 1;
 	qp_attr->sq_draining = 0;
-	qp_attr->max_rd_atomic = roce_get_field(context->qpc_bytes_156,
+	qp_attr->max_rd_atomic = 1 << roce_get_field(context->qpc_bytes_156,
 				 QP_CONTEXT_QPC_BYTES_156_INITIATOR_DEPTH_M,
 				 QP_CONTEXT_QPC_BYTES_156_INITIATOR_DEPTH_S);
-	qp_attr->max_dest_rd_atomic = roce_get_field(context->qpc_bytes_32,
+	qp_attr->max_dest_rd_atomic = 1 << roce_get_field(context->qpc_bytes_32,
 				 QP_CONTEXT_QPC_BYTES_32_RESPONDER_RESOURCES_M,
 				 QP_CONTEXT_QPC_BYTES_32_RESPONDER_RESOURCES_S);
 	qp_attr->min_rnr_timer = (u8)(roce_get_field(context->qpc_bytes_24,
-- 
1.9.1

[PATCH for-next 2/9] RDMA/hns: Factor out the code for checking sdb status into a new function

[Wei Hu (Xavier)]

From: Lijun Ou <oulijun@huawei.com>

It mainly places the lines for checking send doorbell status
into a special functions. As a result, we can directly call it in
check_qp_db_process_status function and keep consistent indenting
style.

It fixes: 5f110ac4bed8 ("IB/hns: Fix for checkpatch.pl comment style)
The warning from static checker:
drivers/infiniband/hw/hns/hns_roce_hw_v1.c:3562 check_qp_db_process_status()
warn: inconsistent indenting

Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
Signed-off-by: Shaobo Xu <xushaobo2@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 95 ++++++++++++++++--------------
 1 file changed, 51 insertions(+), 44 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
index 6e9acfd..95f5c88 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
@@ -3532,6 +3532,53 @@ int hns_roce_v1_query_qp(struct ib_qp *ibqp, struct ib_qp_attr *qp_attr,
 		hns_roce_v1_q_qp(ibqp, qp_attr, qp_attr_mask, qp_init_attr);
 }
 
+static void hns_roce_check_sdb_status(struct hns_roce_dev *hr_dev,
+				      u32 *old_send, u32 *old_retry,
+				      u32 *tsp_st, u32 *success_flags)
+{
+	u32 sdb_retry_cnt;
+	u32 sdb_send_ptr;
+	u32 cur_cnt, old_cnt;
+	u32 send_ptr;
+
+	sdb_send_ptr = roce_read(hr_dev, ROCEE_SDB_SEND_PTR_REG);
+	sdb_retry_cnt =	roce_read(hr_dev, ROCEE_SDB_RETRY_CNT_REG);
+	cur_cnt = roce_get_field(sdb_send_ptr,
+				 ROCEE_SDB_SEND_PTR_SDB_SEND_PTR_M,
+				 ROCEE_SDB_SEND_PTR_SDB_SEND_PTR_S) +
+		  roce_get_field(sdb_retry_cnt,
+				 ROCEE_SDB_RETRY_CNT_SDB_RETRY_CT_M,
+				 ROCEE_SDB_RETRY_CNT_SDB_RETRY_CT_S);
+	if (!roce_get_bit(*tsp_st, ROCEE_CNT_CLR_CE_CNT_CLR_CE_S)) {
+		old_cnt = roce_get_field(*old_send,
+					 ROCEE_SDB_SEND_PTR_SDB_SEND_PTR_M,
+					 ROCEE_SDB_SEND_PTR_SDB_SEND_PTR_S) +
+			  roce_get_field(*old_retry,
+					 ROCEE_SDB_RETRY_CNT_SDB_RETRY_CT_M,
+					 ROCEE_SDB_RETRY_CNT_SDB_RETRY_CT_S);
+		if (cur_cnt - old_cnt > SDB_ST_CMP_VAL)
+			*success_flags = 1;
+	} else {
+		old_cnt = roce_get_field(*old_send,
+					 ROCEE_SDB_SEND_PTR_SDB_SEND_PTR_M,
+					 ROCEE_SDB_SEND_PTR_SDB_SEND_PTR_S);
+		if (cur_cnt - old_cnt > SDB_ST_CMP_VAL) {
+			*success_flags = 1;
+		} else {
+			send_ptr = roce_get_field(*old_send,
+					    ROCEE_SDB_SEND_PTR_SDB_SEND_PTR_M,
+					    ROCEE_SDB_SEND_PTR_SDB_SEND_PTR_S) +
+				   roce_get_field(sdb_retry_cnt,
+					    ROCEE_SDB_RETRY_CNT_SDB_RETRY_CT_M,
+					    ROCEE_SDB_RETRY_CNT_SDB_RETRY_CT_S);
+			roce_set_field(*old_send,
+				       ROCEE_SDB_SEND_PTR_SDB_SEND_PTR_M,
+				       ROCEE_SDB_SEND_PTR_SDB_SEND_PTR_S,
+				       send_ptr);
+		}
+	}
+}
+
 static int check_qp_db_process_status(struct hns_roce_dev *hr_dev,
 				      struct hns_roce_qp *hr_qp,
 				      u32 sdb_issue_ptr,
@@ -3539,12 +3586,10 @@ static int check_qp_db_process_status(struct hns_roce_dev *hr_dev,
 				      u32 *wait_stage)
 {
 	struct device *dev = &hr_dev->pdev->dev;
-	u32 sdb_retry_cnt, old_retry;
 	u32 sdb_send_ptr, old_send;
 	u32 success_flags = 0;
-	u32 cur_cnt, old_cnt;
 	unsigned long end;
-	u32 send_ptr;
+	u32 old_retry;
 	u32 inv_cnt;
 	u32 tsp_st;
 
@@ -3602,47 +3647,9 @@ static int check_qp_db_process_status(struct hns_roce_dev *hr_dev,
 
 				msleep(HNS_ROCE_V1_CHECK_DB_SLEEP_MSECS);
 
-				sdb_send_ptr = roce_read(hr_dev,
-							ROCEE_SDB_SEND_PTR_REG);
-				sdb_retry_cnt =	roce_read(hr_dev,
-						       ROCEE_SDB_RETRY_CNT_REG);
-				cur_cnt = roce_get_field(sdb_send_ptr,
-					ROCEE_SDB_SEND_PTR_SDB_SEND_PTR_M,
-					ROCEE_SDB_SEND_PTR_SDB_SEND_PTR_S) +
-					roce_get_field(sdb_retry_cnt,
-					ROCEE_SDB_RETRY_CNT_SDB_RETRY_CT_M,
-					ROCEE_SDB_RETRY_CNT_SDB_RETRY_CT_S);
-				if (!roce_get_bit(tsp_st,
-					ROCEE_CNT_CLR_CE_CNT_CLR_CE_S)) {
-					old_cnt = roce_get_field(old_send,
-					ROCEE_SDB_SEND_PTR_SDB_SEND_PTR_M,
-					ROCEE_SDB_SEND_PTR_SDB_SEND_PTR_S) +
-					roce_get_field(old_retry,
-					ROCEE_SDB_RETRY_CNT_SDB_RETRY_CT_M,
-					ROCEE_SDB_RETRY_CNT_SDB_RETRY_CT_S);
-					if (cur_cnt - old_cnt > SDB_ST_CMP_VAL)
-						success_flags = 1;
-				} else {
-					old_cnt = roce_get_field(old_send,
-					ROCEE_SDB_SEND_PTR_SDB_SEND_PTR_M,
-					ROCEE_SDB_SEND_PTR_SDB_SEND_PTR_S);
-					if (cur_cnt - old_cnt >
-					    SDB_ST_CMP_VAL) {
-						success_flags = 1;
-					} else {
-						send_ptr =
-							roce_get_field(old_send,
-					    ROCEE_SDB_SEND_PTR_SDB_SEND_PTR_M,
-					    ROCEE_SDB_SEND_PTR_SDB_SEND_PTR_S) +
-					    roce_get_field(sdb_retry_cnt,
-					    ROCEE_SDB_RETRY_CNT_SDB_RETRY_CT_M,
-					    ROCEE_SDB_RETRY_CNT_SDB_RETRY_CT_S);
-					    roce_set_field(old_send,
-					    ROCEE_SDB_SEND_PTR_SDB_SEND_PTR_M,
-					    ROCEE_SDB_SEND_PTR_SDB_SEND_PTR_S,
-						send_ptr);
-					}
-				}
+				hns_roce_check_sdb_status(hr_dev, &old_send,
+							  &old_retry, &tsp_st,
+							  &success_flags);
 			} while (!success_flags);
 		}
 
-- 
1.9.1

Re: [PATCH for-next 2/9] RDMA/hns: Factor out the code for checking sdb status into a new function

[Leon Romanovsky]

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

On Thu, Sep 28, 2017 at 12:57:27PM +0800, Wei Hu (Xavier) wrote:
> From: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>
> It mainly places the lines for checking send doorbell status
> into a special functions. As a result, we can directly call it in
> check_qp_db_process_status function and keep consistent indenting
> style.
>
> It fixes: 5f110ac4bed8 ("IB/hns: Fix for checkpatch.pl comment style)

You forgot " at the end of the line, and there is need to put fixes
(should be Fixes) in the line before Signed-off-by.

> The warning from static checker:
> drivers/infiniband/hw/hns/hns_roce_hw_v1.c:3562 check_qp_db_process_status()
> warn: inconsistent indenting
>
> Signed-off-by: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Wei Hu (Xavier) <xavier.huwei-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Shaobo Xu <xushaobo2-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 95 ++++++++++++++++--------------
>  1 file changed, 51 insertions(+), 44 deletions(-)
>

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

Re: [PATCH for-next 2/9] RDMA/hns: Factor out the code for checking sdb status into a new function

[Wei Hu (Xavier)]

On 2017/9/28 21:50, Leon Romanovsky wrote:
> On Thu, Sep 28, 2017 at 12:57:27PM +0800, Wei Hu (Xavier) wrote:
>> From: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>>
>> It mainly places the lines for checking send doorbell status
>> into a special functions. As a result, we can directly call it in
>> check_qp_db_process_status function and keep consistent indenting
>> style.
>>
>> It fixes: 5f110ac4bed8 ("IB/hns: Fix for checkpatch.pl comment style)
> You forgot " at the end of the line, and there is need to put fixes
> (should be Fixes) in the line before Signed-off-by.
Thanks, Leon
We will modify the statement(Fixes: xx)  and put it before signed-off-by 
in patch v2.

>> The warning from static checker:
>> drivers/infiniband/hw/hns/hns_roce_hw_v1.c:3562 check_qp_db_process_status()
>> warn: inconsistent indenting
>>
>> Signed-off-by: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>> Signed-off-by: Wei Hu (Xavier) <xavier.huwei-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>> Signed-off-by: Shaobo Xu <xushaobo2-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>> ---
>>   drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 95 ++++++++++++++++--------------
>>   1 file changed, 51 insertions(+), 44 deletions(-)
>>


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH for-next 4/9] RDMA/hns: Set mask for destination qp field of qp context assignment

[Wei Hu (Xavier)]

From: Lijun Ou <oulijun@huawei.com>

When only set IB_QP_DEST_QPN flag for attr_mask, the operation of
assigning the dest_qp_num for dest_qp field of qp context is valid.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
Signed-off-by: Shaobo Xu <xushaobo2@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
index 1071fa2..4fb7844 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
@@ -2880,10 +2880,11 @@ static int hns_roce_v1_m_qp(struct ib_qp *ibqp, const struct ib_qp_attr *attr,
 			       QP_CONTEXT_QPC_BYTES_32_RESPONDER_RESOURCES_S,
 			       ilog2((unsigned int)attr->max_dest_rd_atomic));
 
-		roce_set_field(context->qpc_bytes_36,
-			       QP_CONTEXT_QPC_BYTES_36_DEST_QP_M,
-			       QP_CONTEXT_QPC_BYTES_36_DEST_QP_S,
-			       attr->dest_qp_num);
+		if (attr_mask & IB_QP_DEST_QPN)
+			roce_set_field(context->qpc_bytes_36,
+				       QP_CONTEXT_QPC_BYTES_36_DEST_QP_M,
+				       QP_CONTEXT_QPC_BYTES_36_DEST_QP_S,
+				       attr->dest_qp_num);
 
 		/* Configure GID index */
 		port_num = rdma_ah_get_port_num(&attr->ah_attr);
-- 
1.9.1

[PATCH for-next 5/9] RDMA/hns: Set rdma_ah_attr type for querying qp

[Wei Hu (Xavier)]

From: Lijun Ou <oulijun@huawei.com>

When querying qp, It needs to return RoCE device ah_attr type
that may be specific to RoCE devices.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
Signed-off-by: Shaobo Xu <xushaobo2@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 2 ++
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
index 4fb7844..3496f39 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
@@ -3351,6 +3351,7 @@ static int hns_roce_v1_q_sqp(struct ib_qp *ibqp, struct ib_qp_attr *qp_attr,
 	qp_attr->path_mtu	= IB_MTU_256;
 	qp_attr->path_mig_state	= IB_MIG_ARMED;
 	qp_attr->qkey		= QKEY_VAL;
+	qp_attr->ah_attr.type   = RDMA_AH_ATTR_TYPE_ROCE;
 	qp_attr->rq_psn		= 0;
 	qp_attr->sq_psn		= 0;
 	qp_attr->dest_qp_num	= 1;
@@ -3432,6 +3433,7 @@ static int hns_roce_v1_q_qp(struct ib_qp *ibqp, struct ib_qp_attr *qp_attr,
 					       QP_CONTEXT_QPC_BYTES_48_MTU_M,
 					       QP_CONTEXT_QPC_BYTES_48_MTU_S);
 	qp_attr->path_mig_state = IB_MIG_ARMED;
+	qp_attr->ah_attr.type   = RDMA_AH_ATTR_TYPE_ROCE;
 	if (hr_qp->ibqp.qp_type == IB_QPT_UD)
 		qp_attr->qkey = QKEY_VAL;
 
diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
index 4171f73..90ef5c6 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
@@ -2829,6 +2829,7 @@ static int hns_roce_v2_query_qp(struct ib_qp *ibqp, struct ib_qp_attr *qp_attr,
 							V2_QPC_BYTE_24_MTU_M,
 							V2_QPC_BYTE_24_MTU_S);
 	qp_attr->path_mig_state = IB_MIG_ARMED;
+	qp_attr->ah_attr.type   = RDMA_AH_ATTR_TYPE_ROCE;
 	if (hr_qp->ibqp.qp_type == IB_QPT_UD)
 		qp_attr->qkey = V2_QKEY_VAL;
 
-- 
1.9.1

[PATCH for-next 3/9] RDMA/hns: Add return statement when kzalloc return NULL in hns_roce_v1_recreate_lp_qp

[Wei Hu (Xavier)]

From: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>

When lp_qp_work is NULL, it should be returned ENOMEM. This patch
mainly fixes it.

Ihis patch fixes the smatch error as below:
drivers/infiniband/hw/hns/hns_roce_hw_v1.c:918 hns_roce_v1_recreate_lp_qp()
error: potential null dereference 'lp_qp_work'.  (kzalloc returns null)

Signed-off-by: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Shaobo Xu <xushaobo2-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
index 95f5c88..1071fa2 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
@@ -912,6 +912,8 @@ static int hns_roce_v1_recreate_lp_qp(struct hns_roce_dev *hr_dev)
 
 	lp_qp_work = kzalloc(sizeof(struct hns_roce_recreate_lp_qp_work),
 			     GFP_KERNEL);
+	if (!lp_qp_work)
+		return -ENOMEM;
 
 	INIT_WORK(&(lp_qp_work->work), hns_roce_v1_recreate_lp_qp_work_fn);
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 3/9] RDMA/hns: Add return statement when kzalloc return NULL in hns_roce_v1_recreate_lp_qp

[Leon Romanovsky]

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

On Thu, Sep 28, 2017 at 12:57:28PM +0800, Wei Hu (Xavier) wrote:
> From: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>
> When lp_qp_work is NULL, it should be returned ENOMEM. This patch
> mainly fixes it.
>
> Ihis patch fixes the smatch error as below:
> drivers/infiniband/hw/hns/hns_roce_hw_v1.c:918 hns_roce_v1_recreate_lp_qp()
> error: potential null dereference 'lp_qp_work'.  (kzalloc returns null)
>
> Signed-off-by: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Wei Hu (Xavier) <xavier.huwei-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Shaobo Xu <xushaobo2-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> index 95f5c88..1071fa2 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> @@ -912,6 +912,8 @@ static int hns_roce_v1_recreate_lp_qp(struct hns_roce_dev *hr_dev)
>
>  	lp_qp_work = kzalloc(sizeof(struct hns_roce_recreate_lp_qp_work),
>  			     GFP_KERNEL);
> +	if (!lp_qp_work)
> +		return -ENOMEM;
>

You will treat this error in the same was as you will treat timeout,
which is wrong.

1656          */
1657         if (hr_dev->hw->dereg_mr && hns_roce_v1_recreate_lp_qp(hr_dev))
1658                 dev_warn(&hr_dev->pdev->dev, "recreate lp qp timeout!\n");
1659
1660         p = (u32 *)(&addr[0]);


>  	INIT_WORK(&(lp_qp_work->work), hns_roce_v1_recreate_lp_qp_work_fn);
>
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Re: [PATCH for-next 3/9] RDMA/hns: Add return statement when kzalloc return NULL in hns_roce_v1_recreate_lp_qp

[Wei Hu (Xavier)]

On 2017/9/28 17:13, Leon Romanovsky wrote:
> On Thu, Sep 28, 2017 at 12:57:28PM +0800, Wei Hu (Xavier) wrote:
>> From: Lijun Ou <oulijun@huawei.com>
>>
>> When lp_qp_work is NULL, it should be returned ENOMEM. This patch
>> mainly fixes it.
>>
>> Ihis patch fixes the smatch error as below:
>> drivers/infiniband/hw/hns/hns_roce_hw_v1.c:918 hns_roce_v1_recreate_lp_qp()
>> error: potential null dereference 'lp_qp_work'.  (kzalloc returns null)
>>
>> Signed-off-by: Lijun Ou <oulijun@huawei.com>
>> Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
>> Signed-off-by: Shaobo Xu <xushaobo2@huawei.com>
>> ---
>>   drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
>> index 95f5c88..1071fa2 100644
>> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
>> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
>> @@ -912,6 +912,8 @@ static int hns_roce_v1_recreate_lp_qp(struct hns_roce_dev *hr_dev)
>>
>>   	lp_qp_work = kzalloc(sizeof(struct hns_roce_recreate_lp_qp_work),
>>   			     GFP_KERNEL);
>> +	if (!lp_qp_work)
>> +		return -ENOMEM;
>>
> You will treat this error in the same was as you will treat timeout,
> which is wrong.
Thanks,  Leon
We will send v2 to fix the compatible warn info.
> 1656          */
> 1657         if (hr_dev->hw->dereg_mr && hns_roce_v1_recreate_lp_qp(hr_dev))
> 1658                 dev_warn(&hr_dev->pdev->dev, "recreate lp qp timeout!\n");
> 1659
> 1660         p = (u32 *)(&addr[0]);
>
>
>>   	INIT_WORK(&(lp_qp_work->work), hns_roce_v1_recreate_lp_qp_work_fn);
>>
>> --
>> 1.9.1
>>
>> --
>> 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

Re: [PATCH for-next 3/9] RDMA/hns: Add return statement when kzalloc return NULL in hns_roce_v1_recreate_lp_qp

[Leon Romanovsky]

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

On Thu, Sep 28, 2017 at 07:56:59PM +0800, Wei Hu (Xavier) wrote:
>
>
> On 2017/9/28 17:13, Leon Romanovsky wrote:
> > On Thu, Sep 28, 2017 at 12:57:28PM +0800, Wei Hu (Xavier) wrote:
> > > From: Lijun Ou <oulijun@huawei.com>
> > >
> > > When lp_qp_work is NULL, it should be returned ENOMEM. This patch
> > > mainly fixes it.
> > >
> > > Ihis patch fixes the smatch error as below:
> > > drivers/infiniband/hw/hns/hns_roce_hw_v1.c:918 hns_roce_v1_recreate_lp_qp()
> > > error: potential null dereference 'lp_qp_work'.  (kzalloc returns null)
> > >
> > > Signed-off-by: Lijun Ou <oulijun@huawei.com>
> > > Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> > > Signed-off-by: Shaobo Xu <xushaobo2@huawei.com>
> > > ---
> > >   drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 2 ++
> > >   1 file changed, 2 insertions(+)
> > >
> > > diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > > index 95f5c88..1071fa2 100644
> > > --- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > > +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > > @@ -912,6 +912,8 @@ static int hns_roce_v1_recreate_lp_qp(struct hns_roce_dev *hr_dev)
> > >
> > >   	lp_qp_work = kzalloc(sizeof(struct hns_roce_recreate_lp_qp_work),
> > >   			     GFP_KERNEL);
> > > +	if (!lp_qp_work)
> > > +		return -ENOMEM;
> > >
> > You will treat this error in the same was as you will treat timeout,
> > which is wrong.
> Thanks,  Leon
> We will send v2 to fix the compatible warn info.

No, you missed the point.
From the code flow below the behavior of hns_roce_v1_recreate_lp_qp
for ENOMEM and ETIMEOUT returns will be the same and it is wrong.

For the ETIMEOUT, you can continue, for ENOMEM, you should properly
unfold the whole flow.

Thanks


> > 1656          */
> > 1657         if (hr_dev->hw->dereg_mr && hns_roce_v1_recreate_lp_qp(hr_dev))
> > 1658                 dev_warn(&hr_dev->pdev->dev, "recreate lp qp timeout!\n");
> > 1659
> > 1660         p = (u32 *)(&addr[0]);
> >
> >
> > >   	INIT_WORK(&(lp_qp_work->work), hns_roce_v1_recreate_lp_qp_work_fn);
> > >
> > > --
> > > 1.9.1
> > >
> > > --
> > > 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
>
>
> --
> 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

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

Re: [PATCH for-next 3/9] RDMA/hns: Add return statement when kzalloc return NULL in hns_roce_v1_recreate_lp_qp

[Wei Hu (Xavier)]

On 2017/9/28 20:59, Leon Romanovsky wrote:
> On Thu, Sep 28, 2017 at 07:56:59PM +0800, Wei Hu (Xavier) wrote:
>>
>> On 2017/9/28 17:13, Leon Romanovsky wrote:
>>> On Thu, Sep 28, 2017 at 12:57:28PM +0800, Wei Hu (Xavier) wrote:
>>>> From: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>>>>
>>>> When lp_qp_work is NULL, it should be returned ENOMEM. This patch
>>>> mainly fixes it.
>>>>
>>>> Ihis patch fixes the smatch error as below:
>>>> drivers/infiniband/hw/hns/hns_roce_hw_v1.c:918 hns_roce_v1_recreate_lp_qp()
>>>> error: potential null dereference 'lp_qp_work'.  (kzalloc returns null)
>>>>
>>>> Signed-off-by: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>>>> Signed-off-by: Wei Hu (Xavier) <xavier.huwei-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>>>> Signed-off-by: Shaobo Xu <xushaobo2-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>>>> ---
>>>>    drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
>>>> index 95f5c88..1071fa2 100644
>>>> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
>>>> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
>>>> @@ -912,6 +912,8 @@ static int hns_roce_v1_recreate_lp_qp(struct hns_roce_dev *hr_dev)
>>>>
>>>>    	lp_qp_work = kzalloc(sizeof(struct hns_roce_recreate_lp_qp_work),
>>>>    			     GFP_KERNEL);
>>>> +	if (!lp_qp_work)
>>>> +		return -ENOMEM;
>>>>
>>> You will treat this error in the same was as you will treat timeout,
>>> which is wrong.
>> Thanks,  Leon
>> We will send v2 to fix the compatible warn info.
> No, you missed the point.
>  From the code flow below the behavior of hns_roce_v1_recreate_lp_qp
> for ENOMEM and ETIMEOUT returns will be the same and it is wrong.
>
> For the ETIMEOUT, you can continue, for ENOMEM, you should properly
> unfold the whole flow.
>
> Thanks
>
Hi, Leon
     We prepare to modify the warn info as bleow:

         if (hr_dev->hw->dereg_mr && hns_roce_v1_recreate_lp_qp(hr_dev))
             dev_warn(&hr_dev->pdev->dev, "recreate lp qp failed!\n");

     for -ETIMEDOUT,  there is a warn info as blow, but there isn't this 
one for -ENOMEM.
         dev_warn(dev, "recreate lp qp failed 20s timeout and return 
failed!\n");

         static int hns_roce_v1_recreate_lp_qp(struct hns_roce_dev *hr_dev)
         {
             <snip>
             lp_qp_work = kzalloc(sizeof(struct 
hns_roce_recreate_lp_qp_work),
                  GFP_KERNEL);
             if (!lp_qp_work)
                 return -ENOMEM;

             <snip>
             dev_warn(dev, "recreate lp qp failed 20s timeout and return 
failed!\n");
             return -ETIMEDOUT;
         }

     Regards
Wei Hu
>>> 1656          */
>>> 1657         if (hr_dev->hw->dereg_mr && hns_roce_v1_recreate_lp_qp(hr_dev))
>>> 1658                 dev_warn(&hr_dev->pdev->dev, "recreate lp qp timeout!\n");
>>> 1659
>>> 1660         p = (u32 *)(&addr[0]);
>>>
>>>
>>>>    	INIT_WORK(&(lp_qp_work->work), hns_roce_v1_recreate_lp_qp_work_fn);
>>>>
>>>> --
>>>> 1.9.1
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>>>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 3/9] RDMA/hns: Add return statement when kzalloc return NULL in hns_roce_v1_recreate_lp_qp

[Leon Romanovsky]

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

On Fri, Sep 29, 2017 at 02:07:22PM +0800, Wei Hu (Xavier) wrote:
>
>
> On 2017/9/28 20:59, Leon Romanovsky wrote:
> > On Thu, Sep 28, 2017 at 07:56:59PM +0800, Wei Hu (Xavier) wrote:
> > >
> > > On 2017/9/28 17:13, Leon Romanovsky wrote:
> > > > On Thu, Sep 28, 2017 at 12:57:28PM +0800, Wei Hu (Xavier) wrote:
> > > > > From: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> > > > >
> > > > > When lp_qp_work is NULL, it should be returned ENOMEM. This patch
> > > > > mainly fixes it.
> > > > >
> > > > > Ihis patch fixes the smatch error as below:
> > > > > drivers/infiniband/hw/hns/hns_roce_hw_v1.c:918 hns_roce_v1_recreate_lp_qp()
> > > > > error: potential null dereference 'lp_qp_work'.  (kzalloc returns null)
> > > > >
> > > > > Signed-off-by: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> > > > > Signed-off-by: Wei Hu (Xavier) <xavier.huwei-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> > > > > Signed-off-by: Shaobo Xu <xushaobo2-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> > > > > ---
> > > > >    drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 2 ++
> > > > >    1 file changed, 2 insertions(+)
> > > > >
> > > > > diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > > > > index 95f5c88..1071fa2 100644
> > > > > --- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > > > > +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> > > > > @@ -912,6 +912,8 @@ static int hns_roce_v1_recreate_lp_qp(struct hns_roce_dev *hr_dev)
> > > > >
> > > > >    	lp_qp_work = kzalloc(sizeof(struct hns_roce_recreate_lp_qp_work),
> > > > >    			     GFP_KERNEL);
> > > > > +	if (!lp_qp_work)
> > > > > +		return -ENOMEM;
> > > > >
> > > > You will treat this error in the same was as you will treat timeout,
> > > > which is wrong.
> > > Thanks,  Leon
> > > We will send v2 to fix the compatible warn info.
> > No, you missed the point.
> >  From the code flow below the behavior of hns_roce_v1_recreate_lp_qp
> > for ENOMEM and ETIMEOUT returns will be the same and it is wrong.
> >
> > For the ETIMEOUT, you can continue, for ENOMEM, you should properly
> > unfold the whole flow.
> >
> > Thanks
> >
> Hi, Leon
>     We prepare to modify the warn info as bleow:
>
>         if (hr_dev->hw->dereg_mr && hns_roce_v1_recreate_lp_qp(hr_dev))
>             dev_warn(&hr_dev->pdev->dev, "recreate lp qp failed!\n");
>
>     for -ETIMEDOUT,  there is a warn info as blow, but there isn't this one
> for -ENOMEM.
>         dev_warn(dev, "recreate lp qp failed 20s timeout and return
> failed!\n");
>
>         static int hns_roce_v1_recreate_lp_qp(struct hns_roce_dev *hr_dev)
>         {
>             <snip>
>             lp_qp_work = kzalloc(sizeof(struct
> hns_roce_recreate_lp_qp_work),
>                  GFP_KERNEL);
>             if (!lp_qp_work)
>                 return -ENOMEM;
>
>             <snip>
>             dev_warn(dev, "recreate lp qp failed 20s timeout and return
> failed!\n");
>             return -ETIMEDOUT;
>         }
>
>     Regards
> Wei Hu

Hi Wei,

It will be helpful, if you post your suggestions in git diff format.

My expectation is to see the following code:

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
index 747efd1ae5a6..0b9ec7c24f2d 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
@@ -1648,14 +1648,18 @@ void hns_roce_v1_set_mac(struct hns_roce_dev *hr_dev, u8 phy_port, u8 *addr)
 	u16 *p_h;
 	u32 *p;
 	u32 val;

 	/*
 	 * When mac changed, loopback may fail
 	 * because of smac not equal to dmac.
 	 * We Need to release and create reserved qp again.
 	 */
-	if (hr_dev->hw->dereg_mr && hns_roce_v1_recreate_lp_qp(hr_dev))
-		dev_warn(&hr_dev->pdev->dev, "recreate lp qp timeout!\n");
+	if (hr_dev->hw->dereg_mr) {
+		int ret;
+		ret = hns_roce_v1_recreate_lp_qp(hr_dev);
+		if (ret && ret != -ETIMEDOUT)
+			return ret;
+	}

 	p = (u32 *)(&addr[0]);
 	reg_smac_l = *p;

Thanks

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

Re: [PATCH for-next 3/9] RDMA/hns: Add return statement when kzalloc return NULL in hns_roce_v1_recreate_lp_qp

[Wei Hu (Xavier)]

On 2017/9/29 18:23, Leon Romanovsky wrote:
> On Fri, Sep 29, 2017 at 02:07:22PM +0800, Wei Hu (Xavier) wrote:
>>
>> On 2017/9/28 20:59, Leon Romanovsky wrote:
>>> On Thu, Sep 28, 2017 at 07:56:59PM +0800, Wei Hu (Xavier) wrote:
>>>> On 2017/9/28 17:13, Leon Romanovsky wrote:
>>>>> On Thu, Sep 28, 2017 at 12:57:28PM +0800, Wei Hu (Xavier) wrote:
>>>>>> From: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>>>>>>
>>>>>> When lp_qp_work is NULL, it should be returned ENOMEM. This patch
>>>>>> mainly fixes it.
>>>>>>
>>>>>> Ihis patch fixes the smatch error as below:
>>>>>> drivers/infiniband/hw/hns/hns_roce_hw_v1.c:918 hns_roce_v1_recreate_lp_qp()
>>>>>> error: potential null dereference 'lp_qp_work'.  (kzalloc returns null)
>>>>>>
>>>>>> Signed-off-by: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>>>>>> Signed-off-by: Wei Hu (Xavier) <xavier.huwei-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>>>>>> Signed-off-by: Shaobo Xu <xushaobo2-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>>>>>> ---
>>>>>>     drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 2 ++
>>>>>>     1 file changed, 2 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
>>>>>> index 95f5c88..1071fa2 100644
>>>>>> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
>>>>>> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
>>>>>> @@ -912,6 +912,8 @@ static int hns_roce_v1_recreate_lp_qp(struct hns_roce_dev *hr_dev)
>>>>>>
>>>>>>     	lp_qp_work = kzalloc(sizeof(struct hns_roce_recreate_lp_qp_work),
>>>>>>     			     GFP_KERNEL);
>>>>>> +	if (!lp_qp_work)
>>>>>> +		return -ENOMEM;
>>>>>>
>>>>> You will treat this error in the same was as you will treat timeout,
>>>>> which is wrong.
>>>> Thanks,  Leon
>>>> We will send v2 to fix the compatible warn info.
>>> No, you missed the point.
>>>   From the code flow below the behavior of hns_roce_v1_recreate_lp_qp
>>> for ENOMEM and ETIMEOUT returns will be the same and it is wrong.
>>>
>>> For the ETIMEOUT, you can continue, for ENOMEM, you should properly
>>> unfold the whole flow.
>>>
>>> Thanks
>>>
>> Hi, Leon
>>      We prepare to modify the warn info as bleow:
>>
>>          if (hr_dev->hw->dereg_mr && hns_roce_v1_recreate_lp_qp(hr_dev))
>>              dev_warn(&hr_dev->pdev->dev, "recreate lp qp failed!\n");
>>
>>      for -ETIMEDOUT,  there is a warn info as blow, but there isn't this one
>> for -ENOMEM.
>>          dev_warn(dev, "recreate lp qp failed 20s timeout and return
>> failed!\n");
>>
>>          static int hns_roce_v1_recreate_lp_qp(struct hns_roce_dev *hr_dev)
>>          {
>>              <snip>
>>              lp_qp_work = kzalloc(sizeof(struct
>> hns_roce_recreate_lp_qp_work),
>>                   GFP_KERNEL);
>>              if (!lp_qp_work)
>>                  return -ENOMEM;
>>
>>              <snip>
>>              dev_warn(dev, "recreate lp qp failed 20s timeout and return
>> failed!\n");
>>              return -ETIMEDOUT;
>>          }
>>
>>      Regards
>> Wei Hu
> Hi Wei,
>
> It will be helpful, if you post your suggestions in git diff format.
>
> My expectation is to see the following code:
>
> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> index 747efd1ae5a6..0b9ec7c24f2d 100644
> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
> @@ -1648,14 +1648,18 @@ void hns_roce_v1_set_mac(struct hns_roce_dev *hr_dev, u8 phy_port, u8 *addr)
>   	u16 *p_h;
>   	u32 *p;
>   	u32 val;
>
>   	/*
>   	 * When mac changed, loopback may fail
>   	 * because of smac not equal to dmac.
>   	 * We Need to release and create reserved qp again.
>   	 */
> -	if (hr_dev->hw->dereg_mr && hns_roce_v1_recreate_lp_qp(hr_dev))
> -		dev_warn(&hr_dev->pdev->dev, "recreate lp qp timeout!\n");
> +	if (hr_dev->hw->dereg_mr) {
> +		int ret;
> +		ret = hns_roce_v1_recreate_lp_qp(hr_dev);
> +		if (ret && ret != -ETIMEDOUT)
> +			return ret;
> +	}
>
>   	p = (u32 *)(&addr[0]);
>   	reg_smac_l = *p;
>
> Thanks
Hi, Leon
     Thanks for your suggestion. We will send patch v2 to fix it.

     Best Regard
Wei Hu

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH for-next 6/9] RDMA/hns: Add return statement when checking error in hns_roce_v1_mr_free_work_fn

[Wei Hu (Xavier)]

After the loop in hns_roce_v1_mr_free_work_fn function, it is possible that
the local variable named hr_qp is NULL, the operation "hr_qp->qpn" will
result in the exception. As a result, we add return statement when checking
error.

This patch fixes the smatch error as below:
drivers/infiniband/hw/hns/hns_roce_hw_v1.c:1009 hns_roce_v1_mr_free_work_fn()
error: we previously assumed 'hr_qp' could be null

Signed-off-by: Wei Hu (Xavier) <xavier.huwei-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Shaobo Xu <xushaobo2-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
index 3496f39..c08822b 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
@@ -1004,6 +1004,11 @@ static void hns_roce_v1_mr_free_work_fn(struct work_struct *work)
 		}
 	}
 
+	if (!ne) {
+		dev_err(dev, "Reseved loop qp is absent!\n");
+		goto free_work;
+	}
+
 	do {
 		ret = hns_roce_v1_poll_cq(&mr_free_cq->ib_cq, ne, wc);
 		if (ret < 0) {
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH for-next 8/9] RDMA/hns: Remove unused struct members in hns-abi.h

[Wei Hu (Xavier)]

From: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>

This patch mainly deletes some unused struct members for
hns_roce_ib_create_qp in order to match libhns, because
the num of struct members of hns_roce_ib_create_qp must
be the same with hns_roce_create_qp in libhns.

Signed-off-by: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Shaobo Xu <xushaobo2-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 include/uapi/rdma/hns-abi.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/include/uapi/rdma/hns-abi.h b/include/uapi/rdma/hns-abi.h
index 5d74019..79251b6 100644
--- a/include/uapi/rdma/hns-abi.h
+++ b/include/uapi/rdma/hns-abi.h
@@ -41,10 +41,8 @@ struct hns_roce_ib_create_cq {
 
 struct hns_roce_ib_create_qp {
 	__u64	buf_addr;
-	__u64   db_addr;
 	__u8    log_sq_bb_count;
 	__u8    log_sq_stride;
-	__u8    sq_no_prefetch;
 	__u8    reserved[5];
 };
 
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 8/9] RDMA/hns: Remove unused struct members in hns-abi.h

[Leon Romanovsky]

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

On Thu, Sep 28, 2017 at 12:57:33PM +0800, Wei Hu (Xavier) wrote:
> From: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>
> This patch mainly deletes some unused struct members for
> hns_roce_ib_create_qp in order to match libhns, because
> the num of struct members of hns_roce_ib_create_qp must
> be the same with hns_roce_create_qp in libhns.
>
> Signed-off-by: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Wei Hu (Xavier) <xavier.huwei-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> Signed-off-by: Shaobo Xu <xushaobo2-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
>  include/uapi/rdma/hns-abi.h | 2 --
>  1 file changed, 2 deletions(-)
>
> diff --git a/include/uapi/rdma/hns-abi.h b/include/uapi/rdma/hns-abi.h
> index 5d74019..79251b6 100644
> --- a/include/uapi/rdma/hns-abi.h
> +++ b/include/uapi/rdma/hns-abi.h
> @@ -41,10 +41,8 @@ struct hns_roce_ib_create_cq {
>
>  struct hns_roce_ib_create_qp {
>  	__u64	buf_addr;
> -	__u64   db_addr;
>  	__u8    log_sq_bb_count;
>  	__u8    log_sq_stride;
> -	__u8    sq_no_prefetch;
>  	__u8    reserved[5];
>  };

It is classical UAPI breakage which kernel tries to avoid.

In RDMA, we do allow rename of fields from reserved to something, but
don't allow binary layout change.

NAK to this change.

Thanks

>
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

Re: [PATCH for-next 8/9] RDMA/hns: Remove unused struct members in hns-abi.h

[Wei Hu (Xavier)]

On 2017/9/28 17:02, Leon Romanovsky wrote:
> On Thu, Sep 28, 2017 at 12:57:33PM +0800, Wei Hu (Xavier) wrote:
>> From: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>>
>> This patch mainly deletes some unused struct members for
>> hns_roce_ib_create_qp in order to match libhns, because
>> the num of struct members of hns_roce_ib_create_qp must
>> be the same with hns_roce_create_qp in libhns.
>>
>> Signed-off-by: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>> Signed-off-by: Wei Hu (Xavier) <xavier.huwei-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>> Signed-off-by: Shaobo Xu <xushaobo2-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>> ---
>>   include/uapi/rdma/hns-abi.h | 2 --
>>   1 file changed, 2 deletions(-)
>>
>> diff --git a/include/uapi/rdma/hns-abi.h b/include/uapi/rdma/hns-abi.h
>> index 5d74019..79251b6 100644
>> --- a/include/uapi/rdma/hns-abi.h
>> +++ b/include/uapi/rdma/hns-abi.h
>> @@ -41,10 +41,8 @@ struct hns_roce_ib_create_cq {
>>
>>   struct hns_roce_ib_create_qp {
>>   	__u64	buf_addr;
>> -	__u64   db_addr;
>>   	__u8    log_sq_bb_count;
>>   	__u8    log_sq_stride;
>> -	__u8    sq_no_prefetch;
>>   	__u8    reserved[5];
>>   };
> It is classical UAPI breakage which kernel tries to avoid.
>
> In RDMA, we do allow rename of fields from reserved to something, but
> don't allow binary layout change.
>
> NAK to this change.
>
> Thanks
Hi, Leon
     Now there is an inconsistency between hns_roce_ib_create_qp in 
kernel driver and hns_roce_create_qp  in libhns as below:

struct hns_roce_create_qp {
     struct ibv_create_qp        ibv_cmd;
     __u64                buf_addr;
     __u8                log_sq_bb_count;
     __u8                log_sq_stride;
     __u8                reserved[5];
};
     It is better to modify hns_roce_create_qp in libhns, right?
     Thanks

     Regards
Wei Hu
>> --
>> 1.9.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html


--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH for-next 8/9] RDMA/hns: Remove unused struct members in hns-abi.h

[Leon Romanovsky]

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

On Thu, Sep 28, 2017 at 07:56:40PM +0800, Wei Hu (Xavier) wrote:
>
>
> On 2017/9/28 17:02, Leon Romanovsky wrote:
> > On Thu, Sep 28, 2017 at 12:57:33PM +0800, Wei Hu (Xavier) wrote:
> > > From: Lijun Ou <oulijun@huawei.com>
> > >
> > > This patch mainly deletes some unused struct members for
> > > hns_roce_ib_create_qp in order to match libhns, because
> > > the num of struct members of hns_roce_ib_create_qp must
> > > be the same with hns_roce_create_qp in libhns.
> > >
> > > Signed-off-by: Lijun Ou <oulijun@huawei.com>
> > > Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
> > > Signed-off-by: Shaobo Xu <xushaobo2@huawei.com>
> > > ---
> > >   include/uapi/rdma/hns-abi.h | 2 --
> > >   1 file changed, 2 deletions(-)
> > >
> > > diff --git a/include/uapi/rdma/hns-abi.h b/include/uapi/rdma/hns-abi.h
> > > index 5d74019..79251b6 100644
> > > --- a/include/uapi/rdma/hns-abi.h
> > > +++ b/include/uapi/rdma/hns-abi.h
> > > @@ -41,10 +41,8 @@ struct hns_roce_ib_create_cq {
> > >
> > >   struct hns_roce_ib_create_qp {
> > >   	__u64	buf_addr;
> > > -	__u64   db_addr;
> > >   	__u8    log_sq_bb_count;
> > >   	__u8    log_sq_stride;
> > > -	__u8    sq_no_prefetch;
> > >   	__u8    reserved[5];
> > >   };
> > It is classical UAPI breakage which kernel tries to avoid.
> >
> > In RDMA, we do allow rename of fields from reserved to something, but
> > don't allow binary layout change.
> >
> > NAK to this change.
> >
> > Thanks
> Hi, Leon
>     Now there is an inconsistency between hns_roce_ib_create_qp in kernel
> driver and hns_roce_create_qp  in libhns as below:
>
> struct hns_roce_create_qp {
>     struct ibv_create_qp        ibv_cmd;
>     __u64                buf_addr;
>     __u8                log_sq_bb_count;
>     __u8                log_sq_stride;
>     __u8                reserved[5];
> };
>     It is better to modify hns_roce_create_qp in libhns, right?

Yes, it can work, because it is user space problem, where you used wrong
structure.

Thanks

>     Thanks
>
>     Regards
> Wei Hu
> > > --
> > > 1.9.1
> > >
> > > --
> > > 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
>
>
> --
> 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

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

Re: [PATCH for-next 8/9] RDMA/hns: Remove unused struct members in hns-abi.h

[Wei Hu (Xavier)]

On 2017/9/28 21:04, Leon Romanovsky wrote:
> On Thu, Sep 28, 2017 at 07:56:40PM +0800, Wei Hu (Xavier) wrote:
>>
>> On 2017/9/28 17:02, Leon Romanovsky wrote:
>>> On Thu, Sep 28, 2017 at 12:57:33PM +0800, Wei Hu (Xavier) wrote:
>>>> From: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>>>>
>>>> This patch mainly deletes some unused struct members for
>>>> hns_roce_ib_create_qp in order to match libhns, because
>>>> the num of struct members of hns_roce_ib_create_qp must
>>>> be the same with hns_roce_create_qp in libhns.
>>>>
>>>> Signed-off-by: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>>>> Signed-off-by: Wei Hu (Xavier) <xavier.huwei-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>>>> Signed-off-by: Shaobo Xu <xushaobo2-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
>>>> ---
>>>>    include/uapi/rdma/hns-abi.h | 2 --
>>>>    1 file changed, 2 deletions(-)
>>>>
>>>> diff --git a/include/uapi/rdma/hns-abi.h b/include/uapi/rdma/hns-abi.h
>>>> index 5d74019..79251b6 100644
>>>> --- a/include/uapi/rdma/hns-abi.h
>>>> +++ b/include/uapi/rdma/hns-abi.h
>>>> @@ -41,10 +41,8 @@ struct hns_roce_ib_create_cq {
>>>>
>>>>    struct hns_roce_ib_create_qp {
>>>>    	__u64	buf_addr;
>>>> -	__u64   db_addr;
>>>>    	__u8    log_sq_bb_count;
>>>>    	__u8    log_sq_stride;
>>>> -	__u8    sq_no_prefetch;
>>>>    	__u8    reserved[5];
>>>>    };
>>> It is classical UAPI breakage which kernel tries to avoid.
>>>
>>> In RDMA, we do allow rename of fields from reserved to something, but
>>> don't allow binary layout change.
>>>
>>> NAK to this change.
>>>
>>> Thanks
>> Hi, Leon
>>      Now there is an inconsistency between hns_roce_ib_create_qp in kernel
>> driver and hns_roce_create_qp  in libhns as below:
>>
>> struct hns_roce_create_qp {
>>      struct ibv_create_qp        ibv_cmd;
>>      __u64                buf_addr;
>>      __u8                log_sq_bb_count;
>>      __u8                log_sq_stride;
>>      __u8                reserved[5];
>> };
>>      It is better to modify hns_roce_create_qp in libhns, right?
> Yes, it can work, because it is user space problem, where you used wrong
> structure.
>
> Thanks
Thanks, Leon
I will pull it out off this series in patch v2.
>>      Thanks
>>
>>      Regards
>> Wei Hu
>>>> --
>>>> 1.9.1
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>>>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
>> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH for-next 9/9] RDMA/hns: Replace usleep_range with udelay when checking command status

[Wei Hu (Xavier)]

From: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>

It replaces usleep_range with udelay to avoid using usleep_range function
in spin_lock_bh spin region, because it probably cause calltrace.

BUG: scheduling while atomic: insmod/1428/0x00000002
Modules linked in: hns-roce-hw-v2(+) hns_roce rdma_ucm rdma_cm iw_cm ib_uverbs ib_cm ib_core
CPU: 0 PID: 1428 Comm: insmod Not tainted 4.12.0-rc1-00677-g252e8fd-dirty #43
Hardware name: (null) (DT)
Call trace:
[<ffff000008089d20>] dump_backtrace+0x0/0x274
[<ffff00000808a068>] show_stack+0x20/0x28
[<ffff00000844ea58>] dump_stack+0x94/0xb4
[<ffff0000080f975c>] __schedule_bug+0x68/0x84
[<ffff000008a988d4>] __schedule+0x5fc/0x70c
[<ffff000008a98a24>] schedule+0x40/0xa4
[<ffff000008a9c6f0>] schedule_hrtimeout_range_clock+0x98/0xfc
[<ffff000008a9c788>] schedule_hrtimeout_range+0x34/0x40
[<ffff000008a9c098>] usleep_range+0x6c/0x80
[<ffff000000b9ae68>] hns_roce_cmd_send+0xe4/0x264 [hns-roce-hw-v2]
[<ffff000000b9b748>] hns_roce_cmd_query_hw_info+0x40/0x60 [hns-roce-hw-v2]
[<ffff000000b9b790>] hns_roce_v2_profile+0x28/0x668 [hns-roce-hw-v2]
[<ffff000000b6b1f4>] hns_roce_init+0x6c/0x948 [hns-roce-hw-v2]

Signed-off-by: Lijun Ou <oulijun-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Signed-off-by: Shaobo Xu <xushaobo2-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
index 90ef5c6..915d1d5 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v2.c
@@ -589,7 +589,7 @@ int hns_roce_cmq_send(struct hns_roce_dev *hr_dev,
 		do {
 			if (hns_roce_cmq_csq_done(hr_dev))
 				break;
-			usleep_range(1000, 2000);
+			udelay(1);
 			timeout++;
 		} while (timeout < priv->cmq.tx_timeout);
 	}
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH for-next 7/9] RDMA/hns: Remove unnecessarily calling unregister_inetaddr_notifier function

[Wei Hu (Xavier)]

From: Lijun Ou <oulijun@huawei.com>

When the driver doesn't call register_inetaddr_notifier function, it need
not call unregister_inetaddr_notifier to unregister inet addr. This patch
fixes it.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Wei Hu (Xavier) <xavier.huwei@huawei.com>
Signed-off-by: Shaobo Xu <xushaobo2@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_device.h | 1 -
 drivers/infiniband/hw/hns/hns_roce_main.c   | 1 -
 2 files changed, 2 deletions(-)

diff --git a/drivers/infiniband/hw/hns/hns_roce_device.h b/drivers/infiniband/hw/hns/hns_roce_device.h
index 4f43c91..06f3dad 100644
--- a/drivers/infiniband/hw/hns/hns_roce_device.h
+++ b/drivers/infiniband/hw/hns/hns_roce_device.h
@@ -474,7 +474,6 @@ struct hns_roce_ib_iboe {
 	spinlock_t		lock;
 	struct net_device      *netdevs[HNS_ROCE_MAX_PORTS];
 	struct notifier_block	nb;
-	struct notifier_block	nb_inet;
 	u8			phy_port[HNS_ROCE_MAX_PORTS];
 };
 
diff --git a/drivers/infiniband/hw/hns/hns_roce_main.c b/drivers/infiniband/hw/hns/hns_roce_main.c
index 7a0c1e8..8fe8247 100644
--- a/drivers/infiniband/hw/hns/hns_roce_main.c
+++ b/drivers/infiniband/hw/hns/hns_roce_main.c
@@ -419,7 +419,6 @@ static void hns_roce_unregister_device(struct hns_roce_dev *hr_dev)
 {
 	struct hns_roce_ib_iboe *iboe = &hr_dev->iboe;
 
-	unregister_inetaddr_notifier(&iboe->nb_inet);
 	unregister_netdevice_notifier(&iboe->nb);
 	ib_unregister_device(&hr_dev->ib_dev);
 }
-- 
1.9.1