Re: [PATCH 1/5] thunderbolt: Fix tunnel reference leak when the DPRX work is not started

Hi,

On 8/18/26 08:00, Mika Westerberg wrote:
> Hi,
>
> On Tue, Aug 18, 2026 at 07:44:35AM +0200, Sven Peter wrote:
>> Hi,
>>
>> On 8/18/26 06:42, Mika Westerberg wrote:
>>> Hi,
>>>
>>> On Mon, Aug 17, 2026 at 09:53:58PM +0200, Sven Peter wrote:
>>>> tb_dp_dprx_start always takes a tunnel reference which is only dropped
>>>> by dprx_work eventually. Tunnels that have no callback don't ever queue
>>>> that work and tb_dp_dprx_stop then has nothing to cancel. It however only
>>>> releases the reference if cancel_delayed_work returned true and the
>>>> reference is leaked then.
>>> Okay but we always actually pass that callback there so I guess you are
>>> hitting this because you have modified the caller in tb.c not to pass the
>>> callback, right? If that's the case then I suggest mention how you actually
>>> reproduced this whole issue.
>>>
>>> I'm thinking we should make the callback mandatory instead as we always
>>> need it for DP tunnels anyway. It should work the same also in Apple
>>> silicon (one you have the DP tunneling in place).
>> As mentioned a few lines below,
>>> ---
>>> I didn't actually hit this on hardware but found it while fixing a domain
>>> leak in the same area and that fix depends on this one.
>>> ---
> Hehe, sorry missed that one.

No worries, maybe I should've put it into the commit message and/or 
cover letter as well!
>
>> ^-- there, I didn't actually hit this. It's just that there's also a
>> tb_domain leak here (see patch 3) and when fixing that one the asymmetry
>> here just jumps out. There's nothing special my code does to tb.c , the only
>> reason DP tunnels don't work yet is because they need two separate MMIO
>> blocks (what macOS calls "DP IN PHY" and "display crossbar") and possibly
>> also the display co-processor to be up. Once that's done they should come up
>> normally.
>>
>> Tunnels discovered in tb_tunnel_discover_dp setup a DP tunnel with callback
>> = NULL but also never start the dptx_work there and I'm not familiar enough
>> with the code to know if it's possible to ever have those end up in the
>> "normal" paths which queue the dprx_work then.
>> I'm happy to also just make the callback mandatory though and just bail if
>> it's not set.
> For the discovery (happens when the boot firmware/kexec sets up the
> tunnels) DPRX negotiation is already done so we never need to do that for
> those. I think it simplifies this if we just make it mandatory.

Okay, great, I'll look into making it mandatory for v2 then.

>
> BTW, is this bringup stuff available somewhere already? I have M1 Mac here
> so perhaps I can at least help testing things (assuming setting these up is
> not super complex).

Sure, the current WIP code is in 
https://github.com/AsahiLinux/linux/commits/tbt-reset-wip/.
It's not quite in a good shape yet and I have a few improvements in my 
local tree that I can push this evening after $work. Right now only 
XDomain works since pcie, dp and usb3 tunnels need more bringup work and 
not all cables work because I'm not forwarding the information like 
Apple's NHI expects them just yet.


Best,

Sven