Re: [PATCH for-next] RDMA/hns: Optimize eqe buffer allocation flow

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 2020/2/10 18:21, Leon Romanovsky wrote:
> On Mon, Feb 10, 2020 at 05:48:05PM +0800, Weihang Li wrote:
>>
>>
>> On 2020/2/10 17:25, Leon Romanovsky wrote:
>>>>>> -		if (!eq->bt_l0)
>>>>>> -			return -ENOMEM;
>>>>>> -
>>>>>> -		eq->cur_eqe_ba = eq->l0_dma;
>>>>>> -		eq->nxt_eqe_ba = 0;
>>>>>> +	/* alloc a tmp list for storing eq buf address */
>>>>>> +	ret = hns_roce_alloc_buf_list(&region, &buf_list, 1);
>>>>>> +	if (ret) {
>>>>>> +		dev_err(hr_dev->dev, "alloc eq buf_list error\n");
>>>>> The same comment like we gave for bnxt driver, no dev_* prints inside
>>>>> driver, use ibdev_*.
>>>>>
>>>>> Thanks
>>>>>
>>>> Hi Leon,
>>>>
>>>> map_eq_buf() is called before ib_register_device(), so we can't use
>>>> ibdev_* here.
>>> As long as map_eq_buf() is called after ib_alloc_device(), you will be fine.
>>>
>>> Thanks
>>
>> Hi Leon,
>>
>> eq is used to queue hardware event, it should be ready before hardware is initialized.
>> So we can't call map_eq_buf() after ib_alloc_device().
> 
> How can it be that your newly added function has hns_roce_dev in the
> signature and you didn't call to ib_alloc_device()?
> 
>  +static int map_eq_buf(struct hns_roce_dev *hr_dev, struct hns_roce_eq *eq,
>  +                u32 page_shift)
> 
> Thanks
> 

Sorry, I confused ib_alloc_device() and ib_register_device(). What I was about to say is
ib_register_device().

Order of these functions in hns driver is:

1. ib_alloc_device()
2. map_eq_buf()
3. ib_register_device()

Refer to code in __ibdev_printk():

	else if (ibdev)
		printk("%s%s: %pV",
		       level, dev_name(&ibdev->dev), vaf);


If we called ibdev_*() before ib_register_device(), it will print "null" for the device
name. And I make a simple test, it will print like this:

[   41.400347] (null): -------------- This is a test!----------

Because map_eq_buf() should be finished before ib_register_device(), so I think we have
to use dev_*() in it.

>>
>> Thanks
>> Weihang
>>
>>>
>>>> Thanks for your reminder, another patch that replace other dev_* in
>>>> hns driver with ibdev_* is on preparing.
>>>>
>>>> Weihang
>>>>
>>>>> .
>>>>>
>>> .
>>>
>>
> 
> .
> 




[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux