Re: [PATCH 1/5] thunderbolt: Fix tunnel reference leak when the DPRX work is not started
Mika Westerberg <[email protected]>
| Newsgroups | dev.linux.lists.asahi,org.kernel.vger.linux-kernel,org.kernel.vger.linux-usb,org.kernel.vger.stable |
|---|---|
| Message-ID | <[email protected]> |
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).