Re: [PATCH v3 06/15] NTB: ntb_transport: Avoid losing QP link-up requests

[Logan Gunthorpe]

On 2026-09-28 09:25, Koichiro Den wrote:
> ntb_netdev_open() can call ntb_transport_link_up() while the transport
> worker is completing setup on another CPU. Concurrent transport setup
> and a client link-up request can both read the other's flag as false and
> leave QP link work unqueued. The QP then stays down until another link
> event or client link-up request.
> 
> This is the store-buffering pattern described in
> tools/memory-model/Documentation/recipes.txt ("Store buffering").
> 
> Add a full barrier between the store and load on each side.
> 
> Fixes: fce8a7bb5b4b ("PCI-Express Non-Transparent Bridge Support")
> Cc: stable@xxxxxxxxxxxxxxx
> Reported-by: Sashiko <sashiko-bot@xxxxxxxxxx>
> Link: https://lore.kernel.org/r/20260907144701.702E41F00A3A@xxxxxxxxxxxxxxx/
> Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>

Thanks, I find smp_mb calls difficult to understand, but I think these
are correct. I expect I ran into this problem a few times back when I
was working on this code and had no idea the cause or how to fix it.

I have one minor suggestion below for the comment, other than that:

Reviewed-by: Logan Gunthorpe <logang@xxxxxxxxxxxx>


> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index 51d9e9969065..d290e5869c21 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -1101,6 +1101,12 @@ static void ntb_transport_link_work(struct work_struct *work)
>  	/* Publish the link only after every QP has been set up. */
>  	atomic_set_release(&nt->link_is_up, true);
>  
> +	/*
> +	 * Prevent both sides from missing each other's flag. Pairs with
> +	 * the barrier in ntb_transport_link_up().
> +	 */
> +	smp_mb();
> +

I don't find this comment all that easy to understand. Can we expand it
a little? Maybe something like:

 Order the link_is_up store before the client_ready loads below, so
 that this path or ntb_transport_link_up() is guaranteed to see the
 other's flag. Pairs with smp_mb() in ntb_transport_link_up().


Thanks,

Logan