Re: [PATCH rdma-next 07/10] RDMA/mana_ib: Poll RC completions using PSN and FSN progress

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] rdma/mana_ib: interrupt storm from psn+1 arming on unsignaled WQEs
--

commit f323a2eb29a1338a158b3fde42ecaf1e6e1cc432
Author: Konstantin Taranov <kotaranov@microsoft.com>
Subject: RDMA/mana_ib: Poll RC completions using PSN and FSN progress

This commit decodes RC receive completions, armed-PSN notifications, and
MMQ completions. It retires requester and MMQ shadow entries using sequence
numbers, honors cross-queue fence dependencies, and suppresses successful WCs
for unsignaled work requests.

> diff --git a/drivers/infiniband/hw/mana/cq.c b/drivers/infiniband/hw/mana/cq.c
> index 843e2c64b32bc..0ceb67ed62afa 100644
> --- a/drivers/infiniband/hw/mana/cq.c
> +++ b/drivers/infiniband/hw/mana/cq.c
[ ... ]
> @@ -422,17 +422,104 @@ static void handle_rq_cqe(struct mana_ib_qp *qp, struct gdma_comp *cqe,
[ ... ]
> +static bool handle_psn_cqe(struct mana_ib_qp *qp, u32 psn,
> +			   struct mana_cq_poll *poll)
> +{
> +	struct mana_ib_dev *mdev = container_of(qp->ibqp.device, struct mana_ib_dev, ib_dev);
> +	struct shadow_wqe_header *shadow_wqe;
> +	struct gdma_queue *recv_q;
> +	bool consumed;
> +	u32 arm_psn;
> +
> +	if (qp->ibqp.qp_type != IB_QPT_RC)
> +		return true;
> +
> +	recv_q = qp->rc_qp.queues[MANA_RC_RECV_QUEUE_REQUESTER].kmem;
> +
> +	consumed = mana_advance_send(qp, false, psn, poll);
> +	if (consumed) {
> +		shadow_wqe = shadow_queue_get_next_to_signal(&qp->shadow_sq);
> +		arm_psn = shadow_wqe ? shadow_wqe->wqe_offset_or_psn : psn + 1;
> +		mana_gd_wq_ring_doorbell_ext(mdev_to_gc(mdev), recv_q, arm_psn,
> +					     SET_ARM_BIT, MANA_PSN_CLIENT_OFFSET);
> +	}
> +
> +	return consumed;
> +}

[Severity: High]
Does unconditionally arming psn + 1 here when shadow_wqe is NULL trigger an
interrupt storm?

When processing an ARMED_CMPL CQE here in handle_psn_cqe(), if there are no
pending signaled WQEs (because shadow_queue_get_next_to_signal() skips over
unsignaled ones and returns NULL), the driver unconditionally arms psn + 1.

The hardware then generates a CQE for the very next packet. handle_psn_cqe()
will process this, find no signaled WQEs again, and arm psn + 2.

Does this cycle force the hardware to generate an interrupt and CQE for every
single packet of every unsignaled WQE, defeating the unsignaled optimization?

Could arming logic be initiated during post_send() when a signaled WQE is
posted, rather than relying on sequential fallback in the completion handler?

> diff --git a/drivers/infiniband/hw/mana/shadow_queue.h b/drivers/infiniband/hw/mana/shadow_queue.h
> index b5d6bb28bd452..9abf390cf3a30 100644
> --- a/drivers/infiniband/hw/mana/shadow_queue.h
> +++ b/drivers/infiniband/hw/mana/shadow_queue.h
[ ... ]
> @@ -105,4 +108,23 @@ static inline void shadow_queue_advance_consumer(struct shadow_queue *queue)
>  	smp_store_release(&queue->cons_idx, queue->cons_idx + 1);
>  }
>  
> +static inline struct shadow_wqe_header *
> +shadow_queue_get_next_to_signal(struct shadow_queue *queue)
> +{
> +	struct shadow_wqe_header *wqe = NULL;
> +	/* Inspect only entries that have been fully published by posting. */
> +	u64 prod_idx = smp_load_acquire(&queue->prod_idx);
> +
> +	queue->next_to_signal_idx = max(queue->next_to_signal_idx,
> +					queue->cons_idx);
> +	while (queue->next_to_signal_idx < prod_idx) {
> +		wqe = shadow_queue_get_element(queue, queue->next_to_signal_idx);
> +		queue->next_to_signal_idx++;
> +		if (!(wqe->flags & MANA_WQ_NO_SIGNAL_WC))
> +			return wqe;
> +	}
> +
> +	return NULL;
> +}

[Severity: High]
Since this loop skips unsignaled WQEs and returns NULL when only unsignaled
WQEs are pending, does it force the fallback psn + 1 arming behavior in
handle_psn_cqe() mentioned above?

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