Re: [PATCH RFC 01/12] RDMA/rtrs-srv: Remove ib_dev_count from rtrs_srv_ib_ctx

Guoqing Jiang
2022-11-14T08:45:43Z

On 11/14/22 4:24 PM, Jinpu Wang wrote:
> On Mon, Nov 14, 2022 at 9:00 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>> Hi Jinpu,
>>
>> On 11/14/22 3:39 PM, Jinpu Wang wrote:
>>> Hi Guoqing,
>>>
>>> Thx for the patch, see comments below.
>>> On Sun, Nov 13, 2022 at 2:08 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>>>> The ib_dev_count is supposed to track the number of added ib devices
>>>> which is only used in rtrs_srv_{add,remove}_one.
>>>>
>>>> However we only trigger rtrs_srv_add_one from rnbd_srv_init_module
>>>> -> rtrs_srv_open -> ib_register_client -> client->add which should
>>>> happen only once.
>>> client->add is call per ib_device, eg:
>>> jwang@ps404a-1.stg:~$ ls -l /sys/class/infiniband/mlx5_*
>>> lrwxrwxrwx 1 root root 0 Nov 8 13:49 /sys/class/infiniband/mlx5_0 ->
>>> ../../devices/pci0000:ae/0000:ae:00.0/0000:af:00.0/infiniband/mlx5_0
>>> lrwxrwxrwx 1 root root 0 Nov 8 13:49 /sys/class/infiniband/mlx5_1 ->
>>> ../../devices/pci0000:ae/0000:ae:00.0/0000:af:00.1/infiniband/mlx5_1
>>> rtrs will be call twice for mlx5_0 and mlx5_1 devices
>> Ah, yes.
>>
>> But still we can only load/unload module once, I guess it was used to avoid
>> racy condition (concurrent loading/unloading module?), could you elaborate
>> why it is needed?
> The change was introduced due to a bug report, you can follow the
> discussion here for the history and reason:
> https://lore.kernel.org/linux-rdma/20200617103732.10356-1-haris.iqbal@cloud.ionos.com/

Thanks for the link.

I probably missed something but I don't know how rnbd_server module can be
initialized before cma module since we have the dependency chain as
follows.

INFINIBAND_RTRS_SERVER depends on INFINIBAND_ADDR_TRANS
BLK_DEV_RNBD_SERVER depends on INFINIBAND_RTRS_SERVER

But commit 558d52b2976b ("RDMA/rtrs-srv: Incorporate ib_register_client into
rtrs server init") did mention this.

"and if the rnbd_server module is initialized before RDMA cma module, a
null ptr
dereference occurs during the RDMA bind operation."

Thanks,
Guoqing