Re: [PATCH 1/5] thunderbolt: Fix tunnel reference leak when the DPRX work is not started
From: Mika Westerberg
Date: Tue Aug 18 2026 - 02:01:20 EST
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.
> ^-- 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.
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).