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

[Dave Jiang]

On 9/28/26 1:51 AM, Koichiro Den wrote:
> On Thu, Sep 24, 2026 at 08:57:49AM -0700, Dave Jiang wrote:
>>
>>
>> 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.
> 
> I looked into it and thought we might as well fix the TX/MW teardown race too.
> In fact, with patch 14, transport removal stops netdev TX and its timer before
> freeing MWs, but ordinary link-down cleanup doesn't wait for them.
> I gave this a try locally, but it all ended up being quite a rework..
> 
> So for this series, I'd just drop WARN_ON_ONCE(!ntb_transport_tx_free_entry(qp))
> from ntb_async_tx().
> 
> The caller already checks for space. Clean-up can clear remote_rx_info before
> this second check, which now returns 0 and trips the WARN. The WARN doesn't stop
> TX anyway.
> 
> This won't fix the existing lifetime race, but it does remove the new warning
> Sashiko pointed out. I'd leave the broader fix for separate work. Still,
> returning 0 when we see a cleared pointer seems at least better than
> dereferencing a stale one.
> 
> I'm planning to send v3 with this one-line deletion, but if you have a better
> idea, please let me know.

Nope, this is fine.