lkml.org 
[lkml]   [2017]   [Sep]   [29]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH for-next 3/9] RDMA/hns: Add return statement when kzalloc return NULL in hns_roce_v1_recreate_lp_qp
From
Date


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

\
 
 \ /
  Last update: 2017-09-29 15:17    [W:0.063 / U:0.244 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site