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

[Dave Jiang]

On 9/24/26 8:56 AM, Dave Jiang wrote:
> 
> 
> 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>

Probably should take a look at the sashiko raised issue.

> 
>> ---
>> 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;
>>  }
>