Re: [PATCH v3 11/15] NTB: ntb_transport: Wait for RX completions before resetting a QP

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ntb_transport: unbounded wait in ntb_qp_link_cleanup() can deadlock
--

commit a765c073e53bc7d3e624d4540c20299c1d832801
Author: Koichiro Den <den@valinux.co.jp>

NTB: ntb_transport: Wait for RX completions before resetting a QP

This commit adds a wait loop to ntb_qp_link_cleanup() to ensure the rx_post_q
is fully drained before tearing down the connection. This prevents memory
windows from being freed while they are still actively in use by the DMA
hardware on systems that lack proper terminate and synchronize operations.

Link: https://lore.kernel.org/r/20260907144257.767281F00A3A@smtp.kernel.org/

> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index 9f71af97a8ef9..8f1acf44bb53c 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c

[ ... ]

> @@ -960,6 +967,17 @@ static void ntb_qp_link_cleanup(struct ntb_transport_qp *qp)
>  	disable_delayed_work_sync(&qp->link_work);
>  	ntb_transport_set_qp_active(qp, false);
>  	tasklet_kill(&qp->rxc_db_work);
> +	/*
> +	 * Some DMA engines lack terminate/synchronize ops (e.g. IOAT), and
> +	 * DMA_COMPLETION_NO_ORDER rules out cookie-based waits.
> +	 *
> +	 * Waiting for rx_post_q to empty suffices: ntb_complete_rxc() finishes
> +	 * its MW accesses before removing each entry under ntb_rx_q_lock.
> +	 * qp->active is false and rxc_db_work is stopped, so no new RX DMA
> +	 * can be submitted.
> +	 */
> +	while (!ntb_transport_rx_idle(qp))
> +		fsleep(1000);

[Severity: High]
Could this loop wait indefinitely and cause a deadlock?

If a pending RX DMA operation never completes due to a wedged DMA engine,
hardware fault, or dropped interrupt, ntb_complete_rxc() will never be called
to remove the entry from rx_post_q.

Since the caller of this function, ntb_transport_link_cleanup(), acquires
nt->link_event_lock prior to calling ntb_qp_link_cleanup(), an unbounded wait
here could permanently deadlock the NTB link state machine and result in a
hung task.

Would it make sense to add a timeout mechanism to this wait?

>  
>  	ntb_qp_link_down_reset(qp);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928152550.3354675-1-den@valinux.co.jp?part=11