Commit a099b085 authored by Bob Pearson's avatar Bob Pearson Committed by Jason Gunthorpe
Browse files

RDMA/rxe: Revert changes from irqsave to bh locks 

A previous patch replaced all irqsave locks in rxe with bh locks.  This
ran into problems because rdmacm has a bad habit of calling rdma verbs
APIs while disabling irqs. This is not allowed during spin_unlock_bh()
causing programs that use rdmacm to fail.  This patch reverts the changes
to locks that had this problem or got dragged into the same mess. After
this patch blktests/check -q srp now runs correctly.

Link: https://lore.kernel.org/r/20220215194448.44369-1-rpearsonhpe@gmail.com


Fixes: 21adfa7a ("RDMA/rxe: Replace irqsave locks with bh locks")
Reported-by: default avatarGuoqing Jiang <guoqing.jiang@linux.dev>
Reported-by: default avatarBart Van Assche <bvanassche@acm.org>
Signed-off-by: default avatarBob Pearson <rpearsonhpe@gmail.com>
Tested-by: default avatarBart Van Assche <bvanassche@acm.org>
Acked-by: default avatarZhu Yanjun <zyjzyj2000@gmail.com>
Signed-off-by: default avatarJason Gunthorpe <jgg@nvidia.com>
parent 3c8bc395

drivers/infiniband/sw/rxe/rxe_cq.c

+12 −8
Original line number Diff line number Diff line
@@ -42,13 +42,14 @@ int rxe_cq_chk_attr(struct rxe_dev *rxe, struct rxe_cq *cq, 
static void rxe_send_complete(struct tasklet_struct *t)

{

	struct rxe_cq *cq = from_tasklet(cq, t, comp_task);

	unsigned long flags;



	spin_lock_bh(&cq->cq_lock);

	spin_lock_irqsave(&cq->cq_lock, flags);

	if (cq->is_dying) {

		spin_unlock_bh(&cq->cq_lock);

		spin_unlock_irqrestore(&cq->cq_lock, flags);

		return;

	}

	spin_unlock_bh(&cq->cq_lock);

	spin_unlock_irqrestore(&cq->cq_lock, flags);



	cq->ibcq.comp_handler(&cq->ibcq, cq->ibcq.cq_context);

}
@@ -107,12 +108,13 @@ int rxe_cq_post(struct rxe_cq *cq, struct rxe_cqe *cqe, int solicited) 
	struct ib_event ev;

	int full;

	void *addr;

	unsigned long flags;



	spin_lock_bh(&cq->cq_lock);

	spin_lock_irqsave(&cq->cq_lock, flags);



	full = queue_full(cq->queue, QUEUE_TYPE_TO_CLIENT);

	if (unlikely(full)) {

		spin_unlock_bh(&cq->cq_lock);

		spin_unlock_irqrestore(&cq->cq_lock, flags);

		if (cq->ibcq.event_handler) {

			ev.device = cq->ibcq.device;

			ev.element.cq = &cq->ibcq;
@@ -128,7 +130,7 @@ int rxe_cq_post(struct rxe_cq *cq, struct rxe_cqe *cqe, int solicited) 


	queue_advance_producer(cq->queue, QUEUE_TYPE_TO_CLIENT);



	spin_unlock_bh(&cq->cq_lock);

	spin_unlock_irqrestore(&cq->cq_lock, flags);



	if ((cq->notify == IB_CQ_NEXT_COMP) ||

	    (cq->notify == IB_CQ_SOLICITED && solicited)) {
@@ -141,9 +143,11 @@ int rxe_cq_post(struct rxe_cq *cq, struct rxe_cqe *cqe, int solicited) 


void rxe_cq_disable(struct rxe_cq *cq)

{

	spin_lock_bh(&cq->cq_lock);

	unsigned long flags;



	spin_lock_irqsave(&cq->cq_lock, flags);

	cq->is_dying = true;

	spin_unlock_bh(&cq->cq_lock);

	spin_unlock_irqrestore(&cq->cq_lock, flags);

}



void rxe_cq_cleanup(struct rxe_pool_elem *elem)

drivers/infiniband/sw/rxe/rxe_mcast.c

+11 −8
Original line number Diff line number Diff line
@@ -58,11 +58,12 @@ static int rxe_mcast_get_grp(struct rxe_dev *rxe, union ib_gid *mgid, 
	int err;

	struct rxe_mcg *grp;

	struct rxe_pool *pool = &rxe->mc_grp_pool;

	unsigned long flags;



	if (rxe->attr.max_mcast_qp_attach == 0)

		return -EINVAL;



	write_lock_bh(&pool->pool_lock);

	write_lock_irqsave(&pool->pool_lock, flags);



	grp = rxe_pool_get_key_locked(pool, mgid);

	if (grp)
@@ -70,13 +71,13 @@ static int rxe_mcast_get_grp(struct rxe_dev *rxe, union ib_gid *mgid, 


	grp = create_grp(rxe, pool, mgid);

	if (IS_ERR(grp)) {

		write_unlock_bh(&pool->pool_lock);

		write_unlock_irqrestore(&pool->pool_lock, flags);

		err = PTR_ERR(grp);

		return err;

	}



done:

	write_unlock_bh(&pool->pool_lock);

	write_unlock_irqrestore(&pool->pool_lock, flags);

	*grp_p = grp;

	return 0;

}
@@ -86,9 +87,10 @@ static int rxe_mcast_add_grp_elem(struct rxe_dev *rxe, struct rxe_qp *qp, 
{

	int err;

	struct rxe_mca *elem;

	unsigned long flags;



	/* check to see of the qp is already a member of the group */

	spin_lock_bh(&grp->mcg_lock);

	spin_lock_irqsave(&grp->mcg_lock, flags);

	list_for_each_entry(elem, &grp->qp_list, qp_list) {

		if (elem->qp == qp) {

			err = 0;
@@ -118,7 +120,7 @@ static int rxe_mcast_add_grp_elem(struct rxe_dev *rxe, struct rxe_qp *qp, 


	err = 0;

out:

	spin_unlock_bh(&grp->mcg_lock);

	spin_unlock_irqrestore(&grp->mcg_lock, flags);

	return err;

}
@@ -127,12 +129,13 @@ static int rxe_mcast_drop_grp_elem(struct rxe_dev *rxe, struct rxe_qp *qp, 
{

	struct rxe_mcg *grp;

	struct rxe_mca *elem, *tmp;

	unsigned long flags;



	grp = rxe_pool_get_key(&rxe->mc_grp_pool, mgid);

	if (!grp)

		goto err1;



	spin_lock_bh(&grp->mcg_lock);

	spin_lock_irqsave(&grp->mcg_lock, flags);



	list_for_each_entry_safe(elem, tmp, &grp->qp_list, qp_list) {

		if (elem->qp == qp) {
@@ -140,7 +143,7 @@ static int rxe_mcast_drop_grp_elem(struct rxe_dev *rxe, struct rxe_qp *qp, 
			grp->num_qp--;

			atomic_dec(&qp->mcg_num);



			spin_unlock_bh(&grp->mcg_lock);

			spin_unlock_irqrestore(&grp->mcg_lock, flags);

			rxe_drop_ref(elem);

			rxe_drop_ref(grp);	/* ref held by QP */

			rxe_drop_ref(grp);	/* ref from get_key */
@@ -148,7 +151,7 @@ static int rxe_mcast_drop_grp_elem(struct rxe_dev *rxe, struct rxe_qp *qp, 
		}

	}



	spin_unlock_bh(&grp->mcg_lock);

	spin_unlock_irqrestore(&grp->mcg_lock, flags);

	rxe_drop_ref(grp);			/* ref from get_key */

err1:

	return -EINVAL;

drivers/infiniband/sw/rxe/rxe_pool.c

+18 −12
Original line number Diff line number Diff line
@@ -260,11 +260,12 @@ int __rxe_add_key_locked(struct rxe_pool_elem *elem, void *key) 
int __rxe_add_key(struct rxe_pool_elem *elem, void *key)

{

	struct rxe_pool *pool = elem->pool;

	unsigned long flags;

	int err;



	write_lock_bh(&pool->pool_lock);

	write_lock_irqsave(&pool->pool_lock, flags);

	err = __rxe_add_key_locked(elem, key);

	write_unlock_bh(&pool->pool_lock);

	write_unlock_irqrestore(&pool->pool_lock, flags);



	return err;

}
@@ -279,10 +280,11 @@ void __rxe_drop_key_locked(struct rxe_pool_elem *elem) 
void __rxe_drop_key(struct rxe_pool_elem *elem)

{

	struct rxe_pool *pool = elem->pool;

	unsigned long flags;



	write_lock_bh(&pool->pool_lock);

	write_lock_irqsave(&pool->pool_lock, flags);

	__rxe_drop_key_locked(elem);

	write_unlock_bh(&pool->pool_lock);

	write_unlock_irqrestore(&pool->pool_lock, flags);

}



int __rxe_add_index_locked(struct rxe_pool_elem *elem)
@@ -299,11 +301,12 @@ int __rxe_add_index_locked(struct rxe_pool_elem *elem) 
int __rxe_add_index(struct rxe_pool_elem *elem)

{

	struct rxe_pool *pool = elem->pool;

	unsigned long flags;

	int err;



	write_lock_bh(&pool->pool_lock);

	write_lock_irqsave(&pool->pool_lock, flags);

	err = __rxe_add_index_locked(elem);

	write_unlock_bh(&pool->pool_lock);

	write_unlock_irqrestore(&pool->pool_lock, flags);



	return err;

}
@@ -319,10 +322,11 @@ void __rxe_drop_index_locked(struct rxe_pool_elem *elem) 
void __rxe_drop_index(struct rxe_pool_elem *elem)

{

	struct rxe_pool *pool = elem->pool;

	unsigned long flags;



	write_lock_bh(&pool->pool_lock);

	write_lock_irqsave(&pool->pool_lock, flags);

	__rxe_drop_index_locked(elem);

	write_unlock_bh(&pool->pool_lock);

	write_unlock_irqrestore(&pool->pool_lock, flags);

}



void *rxe_alloc_locked(struct rxe_pool *pool)
@@ -440,11 +444,12 @@ void *rxe_pool_get_index_locked(struct rxe_pool *pool, u32 index) 


void *rxe_pool_get_index(struct rxe_pool *pool, u32 index)

{

	unsigned long flags;

	void *obj;



	read_lock_bh(&pool->pool_lock);

	read_lock_irqsave(&pool->pool_lock, flags);

	obj = rxe_pool_get_index_locked(pool, index);

	read_unlock_bh(&pool->pool_lock);

	read_unlock_irqrestore(&pool->pool_lock, flags);



	return obj;

}
@@ -484,11 +489,12 @@ void *rxe_pool_get_key_locked(struct rxe_pool *pool, void *key) 


void *rxe_pool_get_key(struct rxe_pool *pool, void *key)

{

	unsigned long flags;

	void *obj;



	read_lock_bh(&pool->pool_lock);

	read_lock_irqsave(&pool->pool_lock, flags);

	obj = rxe_pool_get_key_locked(pool, key);

	read_unlock_bh(&pool->pool_lock);

	read_unlock_irqrestore(&pool->pool_lock, flags);



	return obj;

}

drivers/infiniband/sw/rxe/rxe_queue.c

+6 −4
Original line number Diff line number Diff line
@@ -151,6 +151,8 @@ int rxe_queue_resize(struct rxe_queue *q, unsigned int *num_elem_p, 
	struct rxe_queue *new_q;

	unsigned int num_elem = *num_elem_p;

	int err;

	unsigned long producer_flags;

	unsigned long consumer_flags;



	new_q = rxe_queue_init(q->rxe, &num_elem, elem_size, q->type);

	if (!new_q)
@@ -164,17 +166,17 @@ int rxe_queue_resize(struct rxe_queue *q, unsigned int *num_elem_p, 
		goto err1;

	}



	spin_lock_bh(consumer_lock);

	spin_lock_irqsave(consumer_lock, consumer_flags);



	if (producer_lock) {

		spin_lock_bh(producer_lock);

		spin_lock_irqsave(producer_lock, producer_flags);

		err = resize_finish(q, new_q, num_elem);

		spin_unlock_bh(producer_lock);

		spin_unlock_irqrestore(producer_lock, producer_flags);

	} else {

		err = resize_finish(q, new_q, num_elem);

	}



	spin_unlock_bh(consumer_lock);

	spin_unlock_irqrestore(consumer_lock, consumer_flags);



	rxe_queue_cleanup(new_q);	/* new/old dep on err */

	if (err)

drivers/infiniband/sw/rxe/rxe_resp.c

+6 −5
Original line number Diff line number Diff line
@@ -297,21 +297,22 @@ static enum resp_states get_srq_wqe(struct rxe_qp *qp) 
	struct ib_event ev;

	unsigned int count;

	size_t size;

	unsigned long flags;



	if (srq->error)

		return RESPST_ERR_RNR;



	spin_lock_bh(&srq->rq.consumer_lock);

	spin_lock_irqsave(&srq->rq.consumer_lock, flags);



	wqe = queue_head(q, QUEUE_TYPE_FROM_CLIENT);

	if (!wqe) {

		spin_unlock_bh(&srq->rq.consumer_lock);

		spin_unlock_irqrestore(&srq->rq.consumer_lock, flags);

		return RESPST_ERR_RNR;

	}



	/* don't trust user space data */

	if (unlikely(wqe->dma.num_sge > srq->rq.max_sge)) {

		spin_unlock_bh(&srq->rq.consumer_lock);

		spin_unlock_irqrestore(&srq->rq.consumer_lock, flags);

		pr_warn("%s: invalid num_sge in SRQ entry\n", __func__);

		return RESPST_ERR_MALFORMED_WQE;

	}
@@ -327,11 +328,11 @@ static enum resp_states get_srq_wqe(struct rxe_qp *qp) 
		goto event;

	}



	spin_unlock_bh(&srq->rq.consumer_lock);

	spin_unlock_irqrestore(&srq->rq.consumer_lock, flags);

	return RESPST_CHK_LENGTH;



event:

	spin_unlock_bh(&srq->rq.consumer_lock);

	spin_unlock_irqrestore(&srq->rq.consumer_lock, flags);

	ev.device = qp->ibqp.device;

	ev.element.srq = qp->ibqp.srq;

	ev.event = IB_EVENT_SRQ_LIMIT_REACHED;