From: Mika Westerberg <mika.westerberg@linux.intel.com>
To: Sven Peter <sven@kernel.org>
Cc: Andreas Noever <andreas.noever@gmail.com>,
Mika Westerberg <westeri@kernel.org>,
Yehezkel Bernat <YehezkelShB@gmail.com>,
asahi@lists.linux.dev, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org,
Konrad Dybcio <konradybcio@kernel.org>,
stable@vger.kernel.org
Subject: Re: [PATCH 1/5] thunderbolt: Fix tunnel reference leak when the DPRX work is not started
Date: Tue, 18 Aug 2026 08:00:44 +0200 [thread overview]
Message-ID: <20260818060044.GT893316@black.igk.intel.com> (raw)
In-Reply-To: <b9100abf-b284-4e56-a4e7-9dfbf96734a8@kernel.org>
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).
next prev parent reply other threads:[~2026-08-18 6:00 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-17 19:53 [PATCH 0/5] thunderbolt: Fix DP tunnel teardown while an async DPRX read is running Sven Peter
2026-08-17 19:53 ` [PATCH 1/5] thunderbolt: Fix tunnel reference leak when the DPRX work is not started Sven Peter
2026-08-18 4:42 ` Mika Westerberg
2026-08-18 5:44 ` Sven Peter
2026-08-18 6:00 ` Mika Westerberg [this message]
2026-08-18 6:11 ` Sven Peter
2026-08-18 6:19 ` Mika Westerberg
2026-08-17 19:53 ` [PATCH 2/5] thunderbolt: Hold a switch reference for each path hop Sven Peter
2026-08-18 6:05 ` Mika Westerberg
2026-08-17 19:54 ` [PATCH 3/5] thunderbolt: Fix domain reference leak when DPRX read is canceled Sven Peter
2026-08-17 19:54 ` [PATCH 4/5] thunderbolt: Don't access a DP tunnel after its DPRX read was canceled Sven Peter
2026-08-18 6:09 ` Mika Westerberg
2026-08-17 19:54 ` [PATCH 5/5] thunderbolt: Cancel the DPRX read when the domain is stopped Sven Peter
2026-08-18 6:17 ` Mika Westerberg
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260818060044.GT893316@black.igk.intel.com \
--to=mika.westerberg@linux.intel.com \
--cc=YehezkelShB@gmail.com \
--cc=andreas.noever@gmail.com \
--cc=asahi@lists.linux.dev \
--cc=konradybcio@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=stable@vger.kernel.org \
--cc=sven@kernel.org \
--cc=westeri@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.