Re: [PATCH v2 11/14] NTB: ntb_transport: Prepare remote RX info accesses for MW teardown

[Dave Jiang]

On 9/9/26 9:08 PM, Koichiro Den wrote:
> The next patch clears remote_rx_info when freeing its MW.
> ntb_transport_tx_free_entry() and debugfs stats reads can run during
> link cleanup, so make them handle a NULL pointer.
> 
> The pointer is accessed locklessly. Use READ_ONCE() and WRITE_ONCE()
> to prevent compiler-induced tearing, and retain the read value so
> the NULL check and dereference use the same pointer.
> 
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Koichiro Den <den@xxxxxxxxxxxxx>

Reviewed-by: Dave Jiang <dave.jiang@xxxxxxxxx>

> ---
> Changes in v2:
>   - No changes.
> 
>  drivers/ntb/ntb_transport.c | 22 +++++++++++++++++-----
>  1 file changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/ntb/ntb_transport.c b/drivers/ntb/ntb_transport.c
> index 7ccba2c04f54..b949f36a4f2d 100644
> --- a/drivers/ntb/ntb_transport.c
> +++ b/drivers/ntb/ntb_transport.c
> @@ -489,6 +489,7 @@ EXPORT_SYMBOL_GPL(ntb_transport_unregister_client);
>  static int ntb_qp_debugfs_stats_show(struct seq_file *s, void *v)
>  {
>  	struct ntb_transport_qp *qp = s->private;
> +	struct ntb_rx_info *remote_rx_info;
>  
>  	if (!qp || !qp->link_is_up)
>  		return 0;
> @@ -516,7 +517,9 @@ static int ntb_qp_debugfs_stats_show(struct seq_file *s, void *v)
>  	seq_printf(s, "tx_err_no_buf - %llu\n", qp->tx_err_no_buf);
>  	seq_printf(s, "tx_mw - \t0x%p\n", qp->tx_mw);
>  	seq_printf(s, "tx_index (H) - \t%u\n", qp->tx_index);
> -	seq_printf(s, "RRI (T) - \t%u\n", qp->remote_rx_info->entry);
> +	remote_rx_info = READ_ONCE(qp->remote_rx_info);
> +	if (remote_rx_info)
> +		seq_printf(s, "RRI (T) - \t%u\n", remote_rx_info->entry);
>  	seq_printf(s, "tx_max_entry - \t%u\n", qp->tx_max_entry);
>  	seq_printf(s, "free tx - \t%u\n", ntb_transport_tx_free_entry(qp));
>  	seq_putc(s, '\n');
> @@ -611,7 +614,7 @@ static int ntb_transport_setup_qp_mw(struct ntb_transport_ctx *nt,
>  	qp->rx_buff = mw->virt_addr + rx_size * (qp_num / mw_count);
>  	rx_size -= sizeof(struct ntb_rx_info);
>  
> -	qp->remote_rx_info = qp->rx_buff + rx_size;
> +	WRITE_ONCE(qp->remote_rx_info, qp->rx_buff + rx_size);
>  
>  	/* Due to housekeeping, there must be atleast 2 buffs */
>  	qp->rx_max_frame = min(transport_mtu, rx_size / 2);
> @@ -934,9 +937,12 @@ static void ntb_qp_link_context_reset(struct ntb_transport_qp *qp)
>  
>  static void ntb_qp_link_down_reset(struct ntb_transport_qp *qp)
>  {
> +	struct ntb_rx_info *remote_rx_info;
> +
>  	ntb_qp_link_context_reset(qp);
> -	if (qp->remote_rx_info)
> -		qp->remote_rx_info->entry = qp->rx_max_entry - 1;
> +	remote_rx_info = READ_ONCE(qp->remote_rx_info);
> +	if (remote_rx_info)
> +		remote_rx_info->entry = qp->rx_max_entry - 1;
>  }
>  
>  static void ntb_transport_schedule_qp_link(struct ntb_transport_qp *qp,
> @@ -2558,8 +2564,14 @@ EXPORT_SYMBOL_GPL(ntb_transport_max_size);
>  
>  unsigned int ntb_transport_tx_free_entry(struct ntb_transport_qp *qp)
>  {
> +	struct ntb_rx_info *remote_rx_info = READ_ONCE(qp->remote_rx_info);
>  	unsigned int head = qp->tx_index;
> -	unsigned int tail = qp->remote_rx_info->entry;
> +	unsigned int tail;
> +
> +	if (!remote_rx_info)
> +		return 0;
> +
> +	tail = remote_rx_info->entry;
>  
>  	return tail >= head ? tail - head : qp->tx_max_entry + tail - head;
>  }