Commit 071ba4cc authored by Jason Gunthorpe's avatar Jason Gunthorpe
Browse files

RDMA: Add rdma_connect_locked() 

There are two flows for handling RDMA_CM_EVENT_ROUTE_RESOLVED, either the
handler triggers a completion and another thread does rdma_connect() or
the handler directly calls rdma_connect().

In all cases rdma_connect() needs to hold the handler_mutex, but when
handler's are invoked this is already held by the core code. This causes
ULPs using the 2nd method to deadlock.

Provide a rdma_connect_locked() and have all ULPs call it from their
handlers.

Link: https://lore.kernel.org/r/0-v2-53c22d5c1405+33-rdma_connect_locking_jgg@nvidia.com


Reported-and-tested-by: default avatarGuoqing Jiang <guoqing.jiang@cloud.ionos.com>
Fixes: 2a7cec53 ("RDMA/cma: Fix locking for the RDMA_CM_CONNECT state")
Acked-by: default avatarSantosh Shilimkar <santosh.shilimkar@oracle.com>
Acked-by: default avatarJack Wang <jinpu.wang@cloud.ionos.com>
Reviewed-by: default avatarChristoph Hellwig <hch@lst.de>
Reviewed-by: default avatarMax Gurtovoy <mgurtovoy@nvidia.com>
Reviewed-by: default avatarSagi Grimberg <sagi@grimberg.me>
Signed-off-by: default avatarJason Gunthorpe <jgg@nvidia.com>
parent 7d66a714

drivers/infiniband/core/cma.c

+38 −10
Original line number Diff line number Diff line
@@ -405,10 +405,10 @@ static int cma_comp_exch(struct rdma_id_private *id_priv, 
	/*

	 * The FSM uses a funny double locking where state is protected by both

	 * the handler_mutex and the spinlock. State is not allowed to change

	 * away from a handler_mutex protected value without also holding

	 * to/from a handler_mutex protected value without also holding

	 * handler_mutex.

	 */

	if (comp == RDMA_CM_CONNECT)

	if (comp == RDMA_CM_CONNECT || exch == RDMA_CM_CONNECT)

		lockdep_assert_held(&id_priv->handler_mutex);



	spin_lock_irqsave(&id_priv->lock, flags);
@@ -4038,17 +4038,23 @@ out: 
	return ret;

}



int rdma_connect(struct rdma_cm_id *id, struct rdma_conn_param *conn_param)

/**

 * rdma_connect_locked - Initiate an active connection request.

 * @id: Connection identifier to connect.

 * @conn_param: Connection information used for connected QPs.

 *

 * Same as rdma_connect() but can only be called from the

 * RDMA_CM_EVENT_ROUTE_RESOLVED handler callback.

 */

int rdma_connect_locked(struct rdma_cm_id *id,

			struct rdma_conn_param *conn_param)

{

	struct rdma_id_private *id_priv =

		container_of(id, struct rdma_id_private, id);

	int ret;



	mutex_lock(&id_priv->handler_mutex);

	if (!cma_comp_exch(id_priv, RDMA_CM_ROUTE_RESOLVED, RDMA_CM_CONNECT)) {

		ret = -EINVAL;

		goto err_unlock;

	}

	if (!cma_comp_exch(id_priv, RDMA_CM_ROUTE_RESOLVED, RDMA_CM_CONNECT))

		return -EINVAL;



	if (!id->qp) {

		id_priv->qp_num = conn_param->qp_num;
@@ -4066,11 +4072,33 @@ int rdma_connect(struct rdma_cm_id *id, struct rdma_conn_param *conn_param) 
		ret = -ENOSYS;

	if (ret)

		goto err_state;

	mutex_unlock(&id_priv->handler_mutex);

	return 0;

err_state:

	cma_comp_exch(id_priv, RDMA_CM_CONNECT, RDMA_CM_ROUTE_RESOLVED);

err_unlock:

	return ret;

}

EXPORT_SYMBOL(rdma_connect_locked);



/**

 * rdma_connect - Initiate an active connection request.

 * @id: Connection identifier to connect.

 * @conn_param: Connection information used for connected QPs.

 *

 * Users must have resolved a route for the rdma_cm_id to connect with by having

 * called rdma_resolve_route before calling this routine.

 *

 * This call will either connect to a remote QP or obtain remote QP information

 * for unconnected rdma_cm_id's.  The actual operation is based on the

 * rdma_cm_id's port space.

 */

int rdma_connect(struct rdma_cm_id *id, struct rdma_conn_param *conn_param)

{

	struct rdma_id_private *id_priv =

		container_of(id, struct rdma_id_private, id);

	int ret;



	mutex_lock(&id_priv->handler_mutex);

	ret = rdma_connect_locked(id, conn_param);

	mutex_unlock(&id_priv->handler_mutex);

	return ret;

}

drivers/infiniband/ulp/iser/iser_verbs.c

+1 −1
Original line number Diff line number Diff line
@@ -620,7 +620,7 @@ static void iser_route_handler(struct rdma_cm_id *cma_id) 
	conn_param.private_data	= (void *)&req_hdr;

	conn_param.private_data_len = sizeof(struct iser_cm_hdr);



	ret = rdma_connect(cma_id, &conn_param);

	ret = rdma_connect_locked(cma_id, &conn_param);

	if (ret) {

		iser_err("failure connecting: %d\n", ret);

		goto failure;

drivers/infiniband/ulp/rtrs/rtrs-clt.c

+2 −2
Original line number Diff line number Diff line
@@ -1674,9 +1674,9 @@ static int rtrs_rdma_route_resolved(struct rtrs_clt_con *con) 
	uuid_copy(&msg.sess_uuid, &sess->s.uuid);

	uuid_copy(&msg.paths_uuid, &clt->paths_uuid);



	err = rdma_connect(con->c.cm_id, &param);

	err = rdma_connect_locked(con->c.cm_id, &param);

	if (err)

		rtrs_err(clt, "rdma_connect(): %d\n", err);

		rtrs_err(clt, "rdma_connect_locked(): %d\n", err);



	return err;

}

drivers/nvme/host/rdma.c

+2 −2
Original line number Diff line number Diff line
@@ -1890,10 +1890,10 @@ static int nvme_rdma_route_resolved(struct nvme_rdma_queue *queue) 
		priv.hsqsize = cpu_to_le16(queue->ctrl->ctrl.sqsize);

	}



	ret = rdma_connect(queue->cm_id, &param);

	ret = rdma_connect_locked(queue->cm_id, &param);

	if (ret) {

		dev_err(ctrl->ctrl.device,

			"rdma_connect failed (%d).\n", ret);

			"rdma_connect_locked failed (%d).\n", ret);

		goto out_destroy_queue_ib;

	}

include/rdma/rdma_cm.h

+2 −12
Original line number Diff line number Diff line
@@ -227,19 +227,9 @@ void rdma_destroy_qp(struct rdma_cm_id *id); 
int rdma_init_qp_attr(struct rdma_cm_id *id, struct ib_qp_attr *qp_attr,

		       int *qp_attr_mask);



/**

 * rdma_connect - Initiate an active connection request.

 * @id: Connection identifier to connect.

 * @conn_param: Connection information used for connected QPs.

 *

 * Users must have resolved a route for the rdma_cm_id to connect with

 * by having called rdma_resolve_route before calling this routine.

 *

 * This call will either connect to a remote QP or obtain remote QP

 * information for unconnected rdma_cm_id's.  The actual operation is

 * based on the rdma_cm_id's port space.

 */

int rdma_connect(struct rdma_cm_id *id, struct rdma_conn_param *conn_param);

int rdma_connect_locked(struct rdma_cm_id *id,

			struct rdma_conn_param *conn_param);



int rdma_connect_ece(struct rdma_cm_id *id, struct rdma_conn_param *conn_param,

		     struct rdma_ucm_ece *ece);

CRA Git | Maintained and supported by SUSTech CRA and CCSE