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

Hi,

On Tue, Aug 18, 2026 at 08:11:09AM +0200, Sven Peter wrote:
> 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/.

Cool! Thanks for sharing.

> 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.

Understood.