[PATCH] NTB: ntb_transport: Mark the transport link down on cleanup

[PATCH] NTB: ntb_transport: Mark the transport link down on cleanup

[Koichiro Den]

Before the NTB hardware and transport drivers were split,
ntb_transport_link_cleanup() changed the transport link state to down on
the first cleanup and cancelled pending link work on repeated cleanup.

The conversion to struct ntb_transport_ctx retained the repeated-cleanup
branch but dropped the state transition. Once a link has come up,
nt->link_is_up consequently remains true across link-down cleanup, allowing
QP cleanup workers to retry the handshake against a peer that is down.

Restore the missing transition.

Fixes: e26a5843f7f5 ("NTB: Split ntb_hw_intel and ntb_transport drivers")
Signed-off-by: Koichiro Den <den@valinux.co.jp>
---
 drivers/ntb/ntb_transport.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
index f59f926d4bfa..5b9fccd1383d 100644
--- a/drivers/ntb/ntb_transport.c
+++ b/drivers/ntb/ntb_transport.c
@@ -974,6 +974,8 @@ static void ntb_transport_link_cleanup(struct ntb_transport_ctx *nt)
 
 	if (!nt->link_is_up)
 		cancel_delayed_work_sync(&nt->link_work);
+	else
+		nt->link_is_up = false;
 
 	for (i = 0; i < nt->mw_count; i++)
 		ntb_free_mw(nt, i);
-- 
2.51.0

Re: [PATCH] NTB: ntb_transport: Mark the transport link down on cleanup

[Koichiro Den]

On Fri, Jul 17, 2026 at 03:12:23PM +0900, Koichiro Den wrote:
> Before the NTB hardware and transport drivers were split,
> ntb_transport_link_cleanup() changed the transport link state to down on
> the first cleanup and cancelled pending link work on repeated cleanup.
> 
> The conversion to struct ntb_transport_ctx retained the repeated-cleanup
> branch but dropped the state transition. Once a link has come up,
> nt->link_is_up consequently remains true across link-down cleanup, allowing
> QP cleanup workers to retry the handshake against a peer that is down.
> 
> Restore the missing transition.
> 
> Fixes: e26a5843f7f5 ("NTB: Split ntb_hw_intel and ntb_transport drivers")
> Signed-off-by: Koichiro Den <den@valinux.co.jp>

I thought this was worth posting as a standalone patch, but after another look,
now I don't think it stands on its own.

Keeping nt->link_is_up set after clean-up, and letting the link work keep
retrying as a result, might be odd. But at least with current mainline, I
couldn't find a convincing user-visible problem caused by that.

So, please disregard this submission. Sorry for the noise.

P.S. regarding what Sashiko pointed out as "New issues":

> 
> New issues:
> - [High] Calling cancel_delayed_work_sync(&nt->link_work) while holding the nt->link_event_lock mutex causes an AB-BA deadlock.
> - [Medium] Leaving nt->link_work pending when transitioning nt->link_is_up to false allows a race condition that incorrectly re-establishes the link.

Both look real to me.

IIUC, the first one is not entirely new, although this patch makes it easier to
hit. Fixing it would be a small change, but that would be a separate fix and
doesn't justify keeping this patch. The second one, on the other hand, is
introduced by this patch. I simply missed that.

> ---
>  drivers/ntb/ntb_transport.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index f59f926d4bfa..5b9fccd1383d 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -974,6 +974,8 @@ static void ntb_transport_link_cleanup(struct ntb_transport_ctx *nt)
>  
>  	if (!nt->link_is_up)
>  		cancel_delayed_work_sync(&nt->link_work);
> +	else
> +		nt->link_is_up = false;
>  
>  	for (i = 0; i < nt->mw_count; i++)
>  		ntb_free_mw(nt, i);
> -- 
> 2.51.0
> 
>