Re: [PATCH RFC 08/12] RDMA/rtrs: Kill recon_cnt from several structs

Haris Iqbal
2022-11-15T09:39:27Z

On Sun, Nov 13, 2022 at 2:08 AM Guoqing Jiang <guoqing.jiang@linux.dev> wrote:
>
> Seems the only relevant comment about recon_cnt is,
>
> /*
> * On every new session connections increase reconnect counter
> * to avoid clashes with previous sessions not yet closed
> * sessions on a server side.
> */
>
> However, it is not clear how the recon_cnt avoid clashed at these places
> in the commit since no where checks it.

It does seem redundant. This predates my time, so I don't know if
there was a change which removed the usage of this. I tried to search
in commit history, but couldn't.

@Jinpu Your thoughts?

>
> Signed-off-by: Guoqing Jiang <guoqing.jiang@linux.dev>
> ---
> drivers/infiniband/ulp/rtrs/rtrs-clt.c | 8 --------
> drivers/infiniband/ulp/rtrs/rtrs-pri.h | 3 ---
> drivers/infiniband/ulp/rtrs/rtrs-srv.c | 7 +------
> 3 files changed, 1 insertion(+), 17 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-clt.c b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> index 5ffc170dae8c..dcc8c041a141 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-clt.c
> @@ -1802,7 +1802,6 @@ static int rtrs_rdma_route_resolved(struct rtrs_clt_con *con)
> .version = cpu_to_le16(RTRS_PROTO_VER),
> .cid = cpu_to_le16(con->c.cid),
> .cid_num = cpu_to_le16(clt_path->s.con_num),
> - .recon_cnt = cpu_to_le16(clt_path->s.recon_cnt),
> };
> msg.first_conn = clt_path->for_new_clt ? FIRST_CONN : 0;
> uuid_copy(&msg.sess_uuid, &clt_path->s.uuid);
> @@ -2336,13 +2335,6 @@ static int init_conns(struct rtrs_clt_path *clt_path)
> unsigned int cid;
> int err;
>
> - /*
> - * On every new session connections increase reconnect counter
> - * to avoid clashes with previous sessions not yet closed
> - * sessions on a server side.
> - */
> - clt_path->s.recon_cnt++;
> -
> /* Establish all RDMA connections */
> for (cid = 0; cid < clt_path->s.con_num; cid++) {
> err = create_con(clt_path, cid);
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-pri.h b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
> index a2420eecaf5a..c4ddaeba1c59 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-pri.h
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-pri.h
> @@ -109,7 +109,6 @@ struct rtrs_path {
> struct rtrs_con **con;
> unsigned int con_num;
> unsigned int irq_con_num;
> - unsigned int recon_cnt;
> unsigned int signal_interval;
> struct rtrs_ib_dev *dev;
> int dev_ref;
> @@ -177,7 +176,6 @@ struct rtrs_sg_desc {
> * @version: RTRS protocol version
> * @cid: Current connection id
> * @cid_num: Number of connections per session
> - * @recon_cnt: Reconnections counter
> * @sess_uuid: UUID of a session (path)
> * @paths_uuid: UUID of a group of sessions (paths)
> *
> @@ -196,7 +194,6 @@ struct rtrs_msg_conn_req {
> __le16 version;
> __le16 cid;
> __le16 cid_num;
> - __le16 recon_cnt;
> uuid_t sess_uuid;
> uuid_t paths_uuid;
> u8 first_conn : 1;
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> index 4c883c57c2ef..e2ea09a8def7 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> @@ -1712,7 +1712,6 @@ static int create_con(struct rtrs_srv_path *srv_path,
> static struct rtrs_srv_path *__alloc_path(struct rtrs_srv_sess *srv,
> struct rdma_cm_id *cm_id,
> unsigned int con_num,
> - unsigned int recon_cnt,
> const uuid_t *uuid)
> {
> struct rtrs_srv_path *srv_path;
> @@ -1768,7 +1767,6 @@ static struct rtrs_srv_path *__alloc_path(struct rtrs_srv_sess *srv,
>
> srv_path->s.con_num = con_num;
> srv_path->s.irq_con_num = con_num;
> - srv_path->s.recon_cnt = recon_cnt;
> uuid_copy(&srv_path->s.uuid, uuid);
> spin_lock_init(&srv_path->state_lock);
> INIT_WORK(&srv_path->close_work, rtrs_srv_close_work);
> @@ -1818,7 +1816,6 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
> struct rtrs_srv_sess *srv;
>
> u16 version, con_num, cid;
> - u16 recon_cnt;
> int err = -ECONNRESET;
> bool alloc_path = false;
>
> @@ -1848,7 +1845,6 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
> pr_err("Incorrect cid: %d >= %d\n", cid, con_num);
> goto reject_w_err;
> }
> - recon_cnt = le16_to_cpu(msg->recon_cnt);
> srv = get_or_create_srv(ctx, &msg->paths_uuid, msg->first_conn);
> if (IS_ERR(srv)) {
> err = PTR_ERR(srv);
> @@ -1885,8 +1881,7 @@ static int rtrs_rdma_connect(struct rdma_cm_id *cm_id,
> goto reject_w_err;
> }
> } else {
> - srv_path = __alloc_path(srv, cm_id, con_num, recon_cnt,
> - &msg->sess_uuid);
> + srv_path = __alloc_path(srv, cm_id, con_num, &msg->sess_uuid);
> if (IS_ERR(srv_path)) {
> mutex_unlock(&srv->paths_mutex);
> put_srv(srv);
> --
> 2.31.1
>