diff mbox

[for-rc,11/11] RDMA/hns: Fix the bug with NULL pointer

Message ID 1525402633-119279-12-git-send-email-oulijun@huawei.com (mailing list archive)
State Accepted
Headers show

Commit Message

Lijun Ou May 4, 2018, 2:57 a.m. UTC
When the last QP of eight QPs is not exist in
hns_roce_v1_mr_free_work_fn function, the
print for qpn of hr_qp may introduce a
calltrace for NULL pointer.

Signed-off-by: Lijun Ou <oulijun@huawei.com>
---
 drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Michael J. Ruhl May 4, 2018, 12:38 p.m. UTC | #1
>-----Original Message-----
>From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
>owner@vger.kernel.org] On Behalf Of Lijun Ou
>Sent: Thursday, May 3, 2018 10:57 PM
>To: dledford@redhat.com; jgg@mellanox.com
>Cc: leon@kernel.org; linux-rdma@vger.kernel.org
>Subject: [PATCH for-rc 11/11] RDMA/hns: Fix the bug with NULL pointer
>
>When the last QP of eight QPs is not exist in
>hns_roce_v1_mr_free_work_fn function, the
>print for qpn of hr_qp may introduce a
>calltrace for NULL pointer.
>
>Signed-off-by: Lijun Ou <oulijun@huawei.com>
>---
> drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
>b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
>index b3417a9..8013d69 100644
>--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
>+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
>@@ -1037,7 +1037,7 @@ static void hns_roce_v1_mr_free_work_fn(struct
>work_struct *work)
>
> 	do {
> 		ret = hns_roce_v1_poll_cq(&mr_free_cq->ib_cq, ne, wc);
>-		if (ret < 0) {
>+		if (ret < 0 && hr_qp) {
> 			dev_err(dev,
> 			   "(qp:0x%lx) starts, Poll cqe failed(%d) for mr 0x%x
>free! Remain %d cqe\n",
> 			   hr_qp->qpn, ret, hr_mr->key, ne);

Hi Lijun,

Previous to your patch, the loop would exit if ret < 0.

Now, it will continue.  Is this what you wanted?

Mike

>--
>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
Lijun Ou May 5, 2018, 3:16 a.m. UTC | #2
在 2018/5/4 20:38, Ruhl, Michael J 写道:
>> -----Original Message-----
>> From: linux-rdma-owner@vger.kernel.org [mailto:linux-rdma-
>> owner@vger.kernel.org] On Behalf Of Lijun Ou
>> Sent: Thursday, May 3, 2018 10:57 PM
>> To: dledford@redhat.com; jgg@mellanox.com
>> Cc: leon@kernel.org; linux-rdma@vger.kernel.org
>> Subject: [PATCH for-rc 11/11] RDMA/hns: Fix the bug with NULL pointer
>>
>> When the last QP of eight QPs is not exist in
>> hns_roce_v1_mr_free_work_fn function, the
>> print for qpn of hr_qp may introduce a
>> calltrace for NULL pointer.
>>
>> Signed-off-by: Lijun Ou <oulijun@huawei.com>
>> ---
>> drivers/infiniband/hw/hns/hns_roce_hw_v1.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
>> b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
>> index b3417a9..8013d69 100644
>> --- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
>> +++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
>> @@ -1037,7 +1037,7 @@ static void hns_roce_v1_mr_free_work_fn(struct
>> work_struct *work)
>>
>> 	do {
>> 		ret = hns_roce_v1_poll_cq(&mr_free_cq->ib_cq, ne, wc);
>> -		if (ret < 0) {
>> +		if (ret < 0 && hr_qp) {
>> 			dev_err(dev,
>> 			   "(qp:0x%lx) starts, Poll cqe failed(%d) for mr 0x%x
>> free! Remain %d cqe\n",
>> 			   hr_qp->qpn, ret, hr_mr->key, ne);
> 
> Hi Lijun,
> 
> Previous to your patch, the loop would exit if ret < 0.
> 
> Now, it will continue.  Is this what you wanted?
> 
> Mike
> 
Hi,Mike
   I think it is. I have analysised the algorithm after received your review. The condition of loop exit is
poll ne cqe associated non empty hr_qp or timeout.The ret < 0 repsesents that the driver will not lookup the relatived
qp caused dirty memory or the hardware exception. As a result, the loop continue is not unncessary.

thanks
Lijun Ou



>> --
>> 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
diff mbox

Patch

diff --git a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
index b3417a9..8013d69 100644
--- a/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
+++ b/drivers/infiniband/hw/hns/hns_roce_hw_v1.c
@@ -1037,7 +1037,7 @@  static void hns_roce_v1_mr_free_work_fn(struct work_struct *work)
 
 	do {
 		ret = hns_roce_v1_poll_cq(&mr_free_cq->ib_cq, ne, wc);
-		if (ret < 0) {
+		if (ret < 0 && hr_qp) {
 			dev_err(dev,
 			   "(qp:0x%lx) starts, Poll cqe failed(%d) for mr 0x%x free! Remain %d cqe\n",
 			   hr_qp->qpn, ret, hr_mr->key, ne);