Re: [PATCH rdma-next v10 9/9] RDMA/mana_ib: Handle error CQEs for kernel QPs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] RDMA/mana_ib: race in mana_remove_qp_from_cqs() when send_cq
  == recv_cq
--

commit a21cd9814010904d171e9602dc8b61668d553074
Author: Konstantin Taranov <kotaranov@microsoft.com>

RDMA/mana_ib: Handle error CQEs for kernel QPs

This commit decodes hardware error CQEs on kernel QPs, defers moving
failing QPs to the ERR state via a work item, and cleans pending CQEs on
QP reset.

> diff --git a/drivers/infiniband/hw/mana/qp.c b/drivers/infiniband/hw/mana/qp.c
> index c021e58817628..837e5c46ed778 100644
> --- a/drivers/infiniband/hw/mana/qp.c
> +++ b/drivers/infiniband/hw/mana/qp.c
[ ... ]
> @@ -774,15 +777,19 @@ static void mana_remove_qp_from_cqs(struct mana_ib_qp *qp, bool reset)
>  	list_del_init(&qp->send_err_node);
>  	/* Keep shadow reset serialized with hardware polling and SW flushing. */
>  	if (reset) {
> +		mana_ib_cq_clean(send_cq, qp);
>  		reset_shadow_queue(&qp->shadow_sq);
>  		reset_shadow_queue(&qp->shadow_mmq);
>  	}
>  	spin_unlock_irqrestore(&send_cq->cq_lock, flags);
>  
>  	spin_lock_irqsave(&recv_cq->cq_lock, flags);
>  	list_del_init(&qp->recv_err_node);
> -	if (reset)
> +	if (reset) {
> +		if (recv_cq != send_cq)
> +			mana_ib_cq_clean(recv_cq, qp);
>  		reset_shadow_queue(&qp->shadow_rq);
> +	}
>  	spin_unlock_irqrestore(&recv_cq->cq_lock, flags);
>  }

[Severity: Medium]
When send_cq == recv_cq, can a concurrent mana_ib_poll_cq() between these
two critical sections flush receive shadow WQEs whose hardware CQEs were
already invalidated?

mana_ib_cq_clean() checks mana_ib_cqe_matches_qp() using cqe_info.is_sq,
so the first call invalidates both SQ and RQ CQEs for qp in the shared CQ.
Dropping cq_lock before unlinking qp->recv_err_node and resetting
qp->shadow_rq opens a window for mana_ib_poll_cq():

CPU0                                CPU1
mana_remove_qp_from_cqs(qp, true)
  spin_lock(&cq->cq_lock)
  list_del_init(&qp->send_err_node)
  mana_ib_cq_clean(cq, qp)
    /* invalidates SQ and RQ CQEs */
  reset_shadow_queue(&qp->shadow_sq)
  reset_shadow_queue(&qp->shadow_mmq)
  spin_unlock(&cq->cq_lock)
                                    mana_ib_poll_cq()
                                      spin_lock(&cq->cq_lock)
                                      mana_poll_cqe()
                                        /* skips invalidated RQ CQEs */
                                      mana_flush_completions()
                                        /* flushes qp->shadow_rq as
                                         * IB_WC_WR_FLUSH_ERR */
                                      spin_unlock(&cq->cq_lock)
  spin_lock(&cq->cq_lock)
  list_del_init(&qp->recv_err_node)
  reset_shadow_queue(&qp->shadow_rq)
  spin_unlock(&cq->cq_lock)

If recv_cq == send_cq, unlinking qp->recv_err_node and resetting
qp->shadow_rq inside the first critical section (and skipping the second
lock acquisition) would keep the CQ clean and shadow queue resets atomic.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111121417.1265896-1-kotaranov@linux.microsoft.com?part=9