Re: [PATCH rdma-next v2 09/11] RDMA/erdma: Add the erdma module

[Cheng Xu <chengyou@linux.alibaba.com>]

On 1/18/22 4:28 PM, Leon Romanovsky wrote:
> On Tue, Jan 18, 2022 at 11:29:34AM +0800, Cheng Xu wrote:
>>
>>
>> On 1/17/22 11:22 PM, Jason Gunthorpe wrote:
>>> On Mon, Jan 17, 2022 at 04:48:26PM +0800, Cheng Xu wrote:
>>>> Add the main erdma module and debugfs files. The main module provides
>>>> interface to infiniband subsytem, and the debugfs module provides a way
>>>> to allow user can get the core status of the device and set the preferred
>>>> congestion control algorithm.
> 
> <...>
> 
>>>
>>>> +static __init int erdma_init_module(void)
>>>> +{
>>>> +	int ret;
>>>> +
>>>> +	ret = erdma_cm_init();
>>>> +	if (ret)
>>>> +		return ret;
>>>> +
>>>> +	ret = pci_register_driver(&erdma_pci_driver);
>>>> +	if (ret) {
>>>> +		pr_err("Couldn't register erdma driver.\n");
>>>> +		goto uninit_cm;
>>>> +	}
>>>> +
>>>> +	ret = register_netdevice_notifier(&erdma_netdev_nb);
>>>> +	if (ret)
>>>> +		goto unregister_driver;
>>>
>>> And notifiers should not be registered without devices.
>>
>> I'm confused about this. irdma/bnxt_re/siw/rxe register net notifiers in
>> their module_init, and get their ibdev structures by function
>> 'ib_device_get_by_netdev'. Other drivers (mlx4/mlx5/hns) register notifiers
>> with devices.
> 
> Let's put siw and RXE aside, they are special. Regarding irdma - it is a
> bug and its register notifier logic should be in driver init code. It
> will ensure that notifications are received only when the ib device is
> ready.
> 
> And for the bnxt_re case, I didn't look too closely on why it is written
> how it is written.
> 
> Thanks

Get it, thanks.

Cheng Xu