* [PATCH for-next 0/9] Bug fixes & Code improvements in hip06 and hip08 RoCE driver
@ 2017-09-28 4:57 Wei Hu (Xavier)
2017-09-28 4:57 ` [PATCH for-next 1/9] RDMA/hns: Modify the value with rd&dest_rd of qp_attr Wei Hu (Xavier)
` (8 more replies)
0 siblings, 9 replies; 22+ messages in thread
From: Wei Hu (Xavier) @ 2017-09-28 4:57 UTC (permalink / raw)
To: dledford
Cc: linux-rdma, xavier.huwei, lijun_nudt, oulijun, charles.chenxin,
liuyixian, xushaobo2, zhangxiping3, xavier.huwei, linuxarm,
linux-kernel, shaobohsu
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* [PATCH for-next 1/9] RDMA/hns: Modify the value with rd&dest_rd of qp_attr
2017-09-28 4:57 [PATCH for-next 0/9] Bug fixes & Code improvements in hip06 and hip08 RoCE driver Wei Hu (Xavier)
@ 2017-09-28 4:57 ` Wei Hu (Xavier)
2017-09-28 4:57 ` [PATCH for-next 2/9] RDMA/hns: Factor out the code for checking sdb status into a new function Wei Hu (Xavier)
` (7 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Wei Hu (Xavier) @ 2017-09-28 4:57 UTC (permalink / raw)
To: dledford
Cc: linux-rdma, xavier.huwei, lijun_nudt, oulijun, charles.chenxin,
liuyixian, xushaobo2, zhangxiping3, xavier.huwei, linuxarm,
linux-kernel, shaobohsu
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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH for-next 2/9] RDMA/hns: Factor out the code for checking sdb status into a new function
2017-09-28 4:57 [PATCH for-next 0/9] Bug fixes & Code improvements in hip06 and hip08 RoCE driver Wei Hu (Xavier)
2017-09-28 4:57 ` [PATCH for-next 1/9] RDMA/hns: Modify the value with rd&dest_rd of qp_attr Wei Hu (Xavier)
@ 2017-09-28 4:57 ` Wei Hu (Xavier)
2017-09-28 13:50 ` Leon Romanovsky
2017-09-28 4:57 ` [PATCH for-next 3/9] RDMA/hns: Add return statement when kzalloc return NULL in hns_roce_v1_recreate_lp_qp Wei Hu (Xavier)
` (6 subsequent siblings)
8 siblings, 1 reply; 22+ messages in thread
From: Wei Hu (Xavier) @ 2017-09-28 4:57 UTC (permalink / raw)
To: dledford
Cc: linux-rdma, xavier.huwei, lijun_nudt, oulijun, charles.chenxin,
liuyixian, xushaobo2, zhangxiping3, xavier.huwei, linuxarm,
linux-kernel, shaobohsu
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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH for-next 3/9] RDMA/hns: Add return statement when kzalloc return NULL in hns_roce_v1_recreate_lp_qp
2017-09-28 4:57 [PATCH for-next 0/9] Bug fixes & Code improvements in hip06 and hip08 RoCE driver Wei Hu (Xavier)
2017-09-28 4:57 ` [PATCH for-next 1/9] RDMA/hns: Modify the value with rd&dest_rd of qp_attr Wei Hu (Xavier)
2017-09-28 4:57 ` [PATCH for-next 2/9] RDMA/hns: Factor out the code for checking sdb status into a new function Wei Hu (Xavier)
@ 2017-09-28 4:57 ` Wei Hu (Xavier)
2017-09-28 9:13 ` Leon Romanovsky
2017-09-28 4:57 ` [PATCH for-next 4/9] RDMA/hns: Set mask for destination qp field of qp context assignment Wei Hu (Xavier)
` (5 subsequent siblings)
8 siblings, 1 reply; 22+ messages in thread
From: Wei Hu (Xavier) @ 2017-09-28 4:57 UTC (permalink / raw)
To: dledford
Cc: linux-rdma, xavier.huwei, lijun_nudt, oulijun, charles.chenxin,
liuyixian, xushaobo2, zhangxiping3, xavier.huwei, linuxarm,
linux-kernel, shaobohsu
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;
INIT_WORK(&(lp_qp_work->work), hns_roce_v1_recreate_lp_qp_work_fn);
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH for-next 4/9] RDMA/hns: Set mask for destination qp field of qp context assignment
2017-09-28 4:57 [PATCH for-next 0/9] Bug fixes & Code improvements in hip06 and hip08 RoCE driver Wei Hu (Xavier)
` (2 preceding siblings ...)
2017-09-28 4:57 ` [PATCH for-next 3/9] RDMA/hns: Add return statement when kzalloc return NULL in hns_roce_v1_recreate_lp_qp Wei Hu (Xavier)
@ 2017-09-28 4:57 ` Wei Hu (Xavier)
2017-09-28 4:57 ` [PATCH for-next 5/9] RDMA/hns: Set rdma_ah_attr type for querying qp Wei Hu (Xavier)
` (4 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Wei Hu (Xavier) @ 2017-09-28 4:57 UTC (permalink / raw)
To: dledford
Cc: linux-rdma, xavier.huwei, lijun_nudt, oulijun, charles.chenxin,
liuyixian, xushaobo2, zhangxiping3, xavier.huwei, linuxarm,
linux-kernel, shaobohsu
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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH for-next 5/9] RDMA/hns: Set rdma_ah_attr type for querying qp
2017-09-28 4:57 [PATCH for-next 0/9] Bug fixes & Code improvements in hip06 and hip08 RoCE driver Wei Hu (Xavier)
` (3 preceding siblings ...)
2017-09-28 4:57 ` [PATCH for-next 4/9] RDMA/hns: Set mask for destination qp field of qp context assignment Wei Hu (Xavier)
@ 2017-09-28 4:57 ` Wei Hu (Xavier)
2017-09-28 4:57 ` [PATCH for-next 6/9] RDMA/hns: Add return statement when checking error in hns_roce_v1_mr_free_work_fn Wei Hu (Xavier)
` (3 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Wei Hu (Xavier) @ 2017-09-28 4:57 UTC (permalink / raw)
To: dledford
Cc: linux-rdma, xavier.huwei, lijun_nudt, oulijun, charles.chenxin,
liuyixian, xushaobo2, zhangxiping3, xavier.huwei, linuxarm,
linux-kernel, shaobohsu
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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH for-next 6/9] RDMA/hns: Add return statement when checking error in hns_roce_v1_mr_free_work_fn
2017-09-28 4:57 [PATCH for-next 0/9] Bug fixes & Code improvements in hip06 and hip08 RoCE driver Wei Hu (Xavier)
` (4 preceding siblings ...)
2017-09-28 4:57 ` [PATCH for-next 5/9] RDMA/hns: Set rdma_ah_attr type for querying qp Wei Hu (Xavier)
@ 2017-09-28 4:57 ` Wei Hu (Xavier)
2017-09-28 4:57 ` [PATCH for-next 7/9] RDMA/hns: Remove unnecessarily calling unregister_inetaddr_notifier function Wei Hu (Xavier)
` (2 subsequent siblings)
8 siblings, 0 replies; 22+ messages in thread
From: Wei Hu (Xavier) @ 2017-09-28 4:57 UTC (permalink / raw)
To: dledford
Cc: linux-rdma, xavier.huwei, lijun_nudt, oulijun, charles.chenxin,
liuyixian, xushaobo2, zhangxiping3, xavier.huwei, linuxarm,
linux-kernel, shaobohsu
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@huawei.com>
Signed-off-by: Lijun Ou <oulijun@huawei.com>
Signed-off-by: Shaobo Xu <xushaobo2@huawei.com>
---
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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH for-next 7/9] RDMA/hns: Remove unnecessarily calling unregister_inetaddr_notifier function
2017-09-28 4:57 [PATCH for-next 0/9] Bug fixes & Code improvements in hip06 and hip08 RoCE driver Wei Hu (Xavier)
` (5 preceding siblings ...)
2017-09-28 4:57 ` [PATCH for-next 6/9] RDMA/hns: Add return statement when checking error in hns_roce_v1_mr_free_work_fn Wei Hu (Xavier)
@ 2017-09-28 4:57 ` Wei Hu (Xavier)
2017-09-28 4:57 ` [PATCH for-next 8/9] RDMA/hns: Remove unused struct members in hns-abi.h Wei Hu (Xavier)
2017-09-28 4:57 ` [PATCH for-next 9/9] RDMA/hns: Replace usleep_range with udelay when checking command status Wei Hu (Xavier)
8 siblings, 0 replies; 22+ messages in thread
From: Wei Hu (Xavier) @ 2017-09-28 4:57 UTC (permalink / raw)
To: dledford
Cc: linux-rdma, xavier.huwei, lijun_nudt, oulijun, charles.chenxin,
liuyixian, xushaobo2, zhangxiping3, xavier.huwei, linuxarm,
linux-kernel, shaobohsu
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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH for-next 8/9] RDMA/hns: Remove unused struct members in hns-abi.h
2017-09-28 4:57 [PATCH for-next 0/9] Bug fixes & Code improvements in hip06 and hip08 RoCE driver Wei Hu (Xavier)
` (6 preceding siblings ...)
2017-09-28 4:57 ` [PATCH for-next 7/9] RDMA/hns: Remove unnecessarily calling unregister_inetaddr_notifier function Wei Hu (Xavier)
@ 2017-09-28 4:57 ` Wei Hu (Xavier)
2017-09-28 9:02 ` Leon Romanovsky
2017-09-28 4:57 ` [PATCH for-next 9/9] RDMA/hns: Replace usleep_range with udelay when checking command status Wei Hu (Xavier)
8 siblings, 1 reply; 22+ messages in thread
From: Wei Hu (Xavier) @ 2017-09-28 4:57 UTC (permalink / raw)
To: dledford
Cc: linux-rdma, xavier.huwei, lijun_nudt, oulijun, charles.chenxin,
liuyixian, xushaobo2, zhangxiping3, xavier.huwei, linuxarm,
linux-kernel, shaobohsu
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];
};
--
1.9.1
^ permalink raw reply related [flat|nested] 22+ messages in thread
* [PATCH for-next 9/9] RDMA/hns: Replace usleep_range with udelay when checking command status
2017-09-28 4:57 [PATCH for-next 0/9] Bug fixes & Code improvements in hip06 and hip08 RoCE driver Wei Hu (Xavier)
` (7 preceding siblings ...)
2017-09-28 4:57 ` [PATCH for-next 8/9] RDMA/hns: Remove unused struct members in hns-abi.h Wei Hu (Xavier)
@ 2017-09-28 4:57 ` Wei Hu (Xavier)
8 siblings, 0 replies; 22+ messages in thread
From: Wei Hu (Xavier) @ 2017-09-28 4:57 UTC (permalink / raw)
To: dledford
Cc: linux-rdma, xavier.huwei, lijun_nudt, oulijun, charles.chenxin,
liuyixian, xushaobo2, zhangxiping3, xavier.huwei, linuxarm,
linux-kernel, shaobohsu
From: Lijun Ou <oulijun@huawei.com>
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@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_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
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH for-next 8/9] RDMA/hns: Remove unused struct members in hns-abi.h
2017-09-28 4:57 ` [PATCH for-next 8/9] RDMA/hns: Remove unused struct members in hns-abi.h Wei Hu (Xavier)
@ 2017-09-28 9:02 ` Leon Romanovsky
2017-09-28 11:56 ` Wei Hu (Xavier)
0 siblings, 1 reply; 22+ messages in thread
From: Leon Romanovsky @ 2017-09-28 9:02 UTC (permalink / raw)
To: Wei Hu (Xavier)
Cc: dledford, linux-rdma, lijun_nudt, oulijun, charles.chenxin,
liuyixian, xushaobo2, zhangxiping3, xavier.huwei, linuxarm,
linux-kernel, shaobohsu
[-- Attachment #1: Type: text/plain, Size: 1402 bytes --]
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
>
> --
> 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH for-next 3/9] RDMA/hns: Add return statement when kzalloc return NULL in hns_roce_v1_recreate_lp_qp
2017-09-28 4:57 ` [PATCH for-next 3/9] RDMA/hns: Add return statement when kzalloc return NULL in hns_roce_v1_recreate_lp_qp Wei Hu (Xavier)
@ 2017-09-28 9:13 ` Leon Romanovsky
2017-09-28 11:56 ` Wei Hu (Xavier)
0 siblings, 1 reply; 22+ messages in thread
From: Leon Romanovsky @ 2017-09-28 9:13 UTC (permalink / raw)
To: Wei Hu (Xavier)
Cc: dledford, linux-rdma, lijun_nudt, oulijun, charles.chenxin,
liuyixian, xushaobo2, zhangxiping3, xavier.huwei, linuxarm,
linux-kernel, shaobohsu
[-- Attachment #1: Type: text/plain, Size: 1707 bytes --]
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.
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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH for-next 8/9] RDMA/hns: Remove unused struct members in hns-abi.h
2017-09-28 9:02 ` Leon Romanovsky
@ 2017-09-28 11:56 ` Wei Hu (Xavier)
2017-09-28 13:04 ` Leon Romanovsky
0 siblings, 1 reply; 22+ messages in thread
From: Wei Hu (Xavier) @ 2017-09-28 11:56 UTC (permalink / raw)
To: Leon Romanovsky
Cc: dledford, linux-rdma, lijun_nudt, oulijun, charles.chenxin,
liuyixian, xushaobo2, zhangxiping3, xavier.huwei, linuxarm,
linux-kernel, shaobohsu
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?
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH for-next 3/9] RDMA/hns: Add return statement when kzalloc return NULL in hns_roce_v1_recreate_lp_qp
2017-09-28 9:13 ` Leon Romanovsky
@ 2017-09-28 11:56 ` Wei Hu (Xavier)
2017-09-28 12:59 ` Leon Romanovsky
0 siblings, 1 reply; 22+ messages in thread
From: Wei Hu (Xavier) @ 2017-09-28 11:56 UTC (permalink / raw)
To: Leon Romanovsky
Cc: dledford, linux-rdma, lijun_nudt, oulijun, charles.chenxin,
liuyixian, xushaobo2, zhangxiping3, xavier.huwei, linuxarm,
linux-kernel, shaobohsu
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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH for-next 3/9] RDMA/hns: Add return statement when kzalloc return NULL in hns_roce_v1_recreate_lp_qp
2017-09-28 11:56 ` Wei Hu (Xavier)
@ 2017-09-28 12:59 ` Leon Romanovsky
2017-09-29 6:07 ` Wei Hu (Xavier)
0 siblings, 1 reply; 22+ messages in thread
From: Leon Romanovsky @ 2017-09-28 12:59 UTC (permalink / raw)
To: Wei Hu (Xavier)
Cc: dledford, linux-rdma, lijun_nudt, oulijun, charles.chenxin,
liuyixian, xushaobo2, zhangxiping3, xavier.huwei, linuxarm,
linux-kernel, shaobohsu
[-- 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 --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH for-next 8/9] RDMA/hns: Remove unused struct members in hns-abi.h
2017-09-28 11:56 ` Wei Hu (Xavier)
@ 2017-09-28 13:04 ` Leon Romanovsky
2017-09-28 16:12 ` Wei Hu (Xavier)
0 siblings, 1 reply; 22+ messages in thread
From: Leon Romanovsky @ 2017-09-28 13:04 UTC (permalink / raw)
To: Wei Hu (Xavier)
Cc: dledford, linux-rdma, lijun_nudt, oulijun, charles.chenxin,
liuyixian, xushaobo2, zhangxiping3, xavier.huwei, linuxarm,
linux-kernel, shaobohsu
[-- 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 --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH for-next 2/9] RDMA/hns: Factor out the code for checking sdb status into a new function
2017-09-28 4:57 ` [PATCH for-next 2/9] RDMA/hns: Factor out the code for checking sdb status into a new function Wei Hu (Xavier)
@ 2017-09-28 13:50 ` Leon Romanovsky
2017-09-29 2:05 ` Wei Hu (Xavier)
0 siblings, 1 reply; 22+ messages in thread
From: Leon Romanovsky @ 2017-09-28 13:50 UTC (permalink / raw)
To: Wei Hu (Xavier)
Cc: dledford, linux-rdma, lijun_nudt, oulijun, charles.chenxin,
liuyixian, xushaobo2, zhangxiping3, xavier.huwei, linuxarm,
linux-kernel, shaobohsu
[-- Attachment #1: Type: text/plain, Size: 955 bytes --]
On Thu, Sep 28, 2017 at 12:57:27PM +0800, Wei Hu (Xavier) wrote:
> 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)
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@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(-)
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH for-next 8/9] RDMA/hns: Remove unused struct members in hns-abi.h
2017-09-28 13:04 ` Leon Romanovsky
@ 2017-09-28 16:12 ` Wei Hu (Xavier)
0 siblings, 0 replies; 22+ messages in thread
From: Wei Hu (Xavier) @ 2017-09-28 16:12 UTC (permalink / raw)
To: Leon Romanovsky, Wei Hu (Xavier)
Cc: dledford, linux-rdma, lijun_nudt, oulijun, charles.chenxin,
liuyixian, xushaobo2, zhangxiping3, linuxarm, linux-kernel,
shaobohsu
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@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, 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@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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH for-next 2/9] RDMA/hns: Factor out the code for checking sdb status into a new function
2017-09-28 13:50 ` Leon Romanovsky
@ 2017-09-29 2:05 ` Wei Hu (Xavier)
0 siblings, 0 replies; 22+ messages in thread
From: Wei Hu (Xavier) @ 2017-09-29 2:05 UTC (permalink / raw)
To: Leon Romanovsky
Cc: dledford, linux-rdma, lijun_nudt, oulijun, charles.chenxin,
liuyixian, xushaobo2, zhangxiping3, xavier.huwei, linuxarm,
linux-kernel, shaobohsu
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@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)
> 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@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(-)
>>
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH for-next 3/9] RDMA/hns: Add return statement when kzalloc return NULL in hns_roce_v1_recreate_lp_qp
2017-09-28 12:59 ` Leon Romanovsky
@ 2017-09-29 6:07 ` Wei Hu (Xavier)
2017-09-29 10:23 ` Leon Romanovsky
0 siblings, 1 reply; 22+ messages in thread
From: Wei Hu (Xavier) @ 2017-09-29 6:07 UTC (permalink / raw)
To: Leon Romanovsky
Cc: dledford, linux-rdma, lijun_nudt, oulijun, charles.chenxin,
liuyixian, xushaobo2, zhangxiping3, xavier.huwei, linuxarm,
linux-kernel, shaobohsu
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@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
>
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@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
^ permalink raw reply [flat|nested] 22+ messages in thread
* Re: [PATCH for-next 3/9] RDMA/hns: Add return statement when kzalloc return NULL in hns_roce_v1_recreate_lp_qp
2017-09-29 6:07 ` Wei Hu (Xavier)
@ 2017-09-29 10:23 ` Leon Romanovsky
2017-09-29 13:15 ` Wei Hu (Xavier)
0 siblings, 1 reply; 22+ messages in thread
From: Leon Romanovsky @ 2017-09-29 10:23 UTC (permalink / raw)
To: Wei Hu (Xavier)
Cc: dledford, linux-rdma, lijun_nudt, oulijun, charles.chenxin,
liuyixian, xushaobo2, zhangxiping3, xavier.huwei, linuxarm,
linux-kernel, shaobohsu
[-- Attachment #1: Type: text/plain, Size: 3883 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@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
> >
> 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 --]
^ permalink raw reply related [flat|nested] 22+ messages in thread
* Re: [PATCH for-next 3/9] RDMA/hns: Add return statement when kzalloc return NULL in hns_roce_v1_recreate_lp_qp
2017-09-29 10:23 ` Leon Romanovsky
@ 2017-09-29 13:15 ` Wei Hu (Xavier)
0 siblings, 0 replies; 22+ messages in thread
From: Wei Hu (Xavier) @ 2017-09-29 13:15 UTC (permalink / raw)
To: Leon Romanovsky, Wei Hu (Xavier)
Cc: dledford, linux-rdma, lijun_nudt, oulijun, charles.chenxin,
liuyixian, xushaobo2, zhangxiping3, linuxarm, linux-kernel,
shaobohsu
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@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
>>>
>> 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
^ permalink raw reply [flat|nested] 22+ messages in thread
end of thread, other threads:[~2017-09-29 13:15 UTC | newest]
Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-28 4:57 [PATCH for-next 0/9] Bug fixes & Code improvements in hip06 and hip08 RoCE driver Wei Hu (Xavier)
2017-09-28 4:57 ` [PATCH for-next 1/9] RDMA/hns: Modify the value with rd&dest_rd of qp_attr Wei Hu (Xavier)
2017-09-28 4:57 ` [PATCH for-next 2/9] RDMA/hns: Factor out the code for checking sdb status into a new function Wei Hu (Xavier)
2017-09-28 13:50 ` Leon Romanovsky
2017-09-29 2:05 ` Wei Hu (Xavier)
2017-09-28 4:57 ` [PATCH for-next 3/9] RDMA/hns: Add return statement when kzalloc return NULL in hns_roce_v1_recreate_lp_qp Wei Hu (Xavier)
2017-09-28 9:13 ` Leon Romanovsky
2017-09-28 11:56 ` Wei Hu (Xavier)
2017-09-28 12:59 ` Leon Romanovsky
2017-09-29 6:07 ` Wei Hu (Xavier)
2017-09-29 10:23 ` Leon Romanovsky
2017-09-29 13:15 ` Wei Hu (Xavier)
2017-09-28 4:57 ` [PATCH for-next 4/9] RDMA/hns: Set mask for destination qp field of qp context assignment Wei Hu (Xavier)
2017-09-28 4:57 ` [PATCH for-next 5/9] RDMA/hns: Set rdma_ah_attr type for querying qp Wei Hu (Xavier)
2017-09-28 4:57 ` [PATCH for-next 6/9] RDMA/hns: Add return statement when checking error in hns_roce_v1_mr_free_work_fn Wei Hu (Xavier)
2017-09-28 4:57 ` [PATCH for-next 7/9] RDMA/hns: Remove unnecessarily calling unregister_inetaddr_notifier function Wei Hu (Xavier)
2017-09-28 4:57 ` [PATCH for-next 8/9] RDMA/hns: Remove unused struct members in hns-abi.h Wei Hu (Xavier)
2017-09-28 9:02 ` Leon Romanovsky
2017-09-28 11:56 ` Wei Hu (Xavier)
2017-09-28 13:04 ` Leon Romanovsky
2017-09-28 16:12 ` Wei Hu (Xavier)
2017-09-28 4:57 ` [PATCH for-next 9/9] RDMA/hns: Replace usleep_range with udelay when checking command status Wei Hu (Xavier)
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).