Hi Guoqing,
Thx for the patch, see comments below.
On Sun, Nov 13, 2022 at 2:08 AM Guoqing Jiang <guoqing.jiang@xxxxxxxxx> 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@xxxxxxxxxxxx:~$ 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
So this one is NACK
And so is rtrs_srv_close since it is only called
> by unload rnbd-server or failure case when load rnbd-server.
>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@xxxxxxxxx>
> ---
> drivers/infiniband/ulp/rtrs/rtrs-srv.c | 16 ----------------
> drivers/infiniband/ulp/rtrs/rtrs-srv.h | 1 -
> 2 files changed, 17 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> index 22d7ba05e9fe..79504aaef0cc 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> @@ -2097,8 +2097,6 @@ static int rtrs_srv_add_one(struct ib_device *device)
> int ret = 0;
>
> mutex_lock(&ib_ctx.ib_dev_mutex);
> - if (ib_ctx.ib_dev_count)
> - goto out;
>
> /*
> * Since our CM IDs are NOT bound to any ib device we will create them
> @@ -2108,21 +2106,12 @@ static int rtrs_srv_add_one(struct ib_device *device)
> ret = rtrs_srv_rdma_init(ctx, ib_ctx.port);
> if (ret) {
> /*
> - * We errored out here.
> * According to the ib code, if we encounter an error here then the
> * error code is ignored, and no more calls to our ops are made.
> */
> pr_err("Failed to initialize RDMA connection");
> - goto err_out;
> }
>
> -out:
> - /*
> - * Keep a track on the number of ib devices added
> - */
> - ib_ctx.ib_dev_count++;
> -
> -err_out:
> mutex_unlock(&ib_ctx.ib_dev_mutex);
> return ret;
> }
> @@ -2132,10 +2121,6 @@ static void rtrs_srv_remove_one(struct ib_device *device, void *client_data)
> struct rtrs_srv_ctx *ctx;
>
> mutex_lock(&ib_ctx.ib_dev_mutex);
> - ib_ctx.ib_dev_count--;
> -
> - if (ib_ctx.ib_dev_count)
> - goto out;
>
> /*
> * Since our CM IDs are NOT bound to any ib device we will remove them
> @@ -2145,7 +2130,6 @@ static void rtrs_srv_remove_one(struct ib_device *device, void *client_data)
> rdma_destroy_id(ctx->cm_id_ip);
> rdma_destroy_id(ctx->cm_id_ib);
>
> -out:
> mutex_unlock(&ib_ctx.ib_dev_mutex);
> }
>
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.h b/drivers/infiniband/ulp/rtrs/rtrs-srv.h
> index 2f8a638e36fa..eccc432b0715 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-srv.h
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.h
> @@ -126,7 +126,6 @@ struct rtrs_srv_ib_ctx {
> struct rtrs_srv_ctx *srv_ctx;
> u16 port;
> struct mutex ib_dev_mutex;
> - int ib_dev_count;
> };
>
> extern struct class *rtrs_dev_class;
> --
> 2.31.1
>
