[PATCH] thunderbolt: Do not RPM complete unrelated subtrees on unplug

[PATCH] thunderbolt: Do not RPM complete unrelated subtrees on unplug

[Jonathan Gopel]

When a Thunderbolt device is unplugged, that switch and every switch
subsequent to it on the bus device list is `complete()`d, not just its
child devices. In the case of multiple domains, it may cause, eg, an
attempt to `complete()` the uninitialized domain 1 root switch RPM
completion on unplug of a switch from domain 0 (though ordering is not
guaranteed numerically sorted).

I initially noticed this issue on a Dell XPS9720 when the system would
seem to sporadically not wake after unplugging a Thunderbolt dock. I
tried multiple kernel versions including v7.2.7 and v7.3-rc4 to see if
that resolved the issue, but the issue persisted. Eventually I found
that I could consistently generate a kernel Oops reporting a page fault
at 0xffff_ffff_ffff_fff8 on unplug from one side of the laptop and went
bug hunting. I became suspicious of the `bus_for_each_dev()` walk that I
ultimately ended up changing. To confirm this issue, I did an eBPF trace
of `bus_for_each_dev()`, `complete_rpm()`, and `complete()` while
unplugging in a variety of situations and found that an unplug from
domain 0 tries to `complete_rpm()` domain 1's switch, whose RPM
completion is not initialized and thus is holding a null wait-list
pointer, triggering the page fault.

Here are the key sections of that eBPF trace. Note that HEAD denotes the
completion's own wait-list HEAD:

```
RESCAN_ENTER domain=0

COMPLETE rescan_domain=0 target=0-1 target_domain=0
         route=0x1 parent=0-0 unplugged=1
COMPLETION done=0 head=HEAD next=HEAD prev=HEAD

BUS_WALK domain=0 start=0-1 callback=complete_rpm
CALLBACK_ENTER rescan_domain=0 dev=1-0 parent=domain1 is_switch=1

COMPLETE rescan_domain=0 target=1-0 target_domain=1
         route=0x0 parent=domain1 unplugged=0
COMPLETION done=0 head=HEAD next=0 prev=0

        complete+5
        complete_rpm+43
        bus_for_each_dev+133
        icm_free_unplugged_children+250
        icm_rescan_work+42
```

To fix this, I have reused some of the adjacent logic to recursively
walk and `complete()` only the unplugged switch and its descendants.

I only have a single system with multiple Thunderbolt domains - the Dell
XPS9720. I am consistently able to reproduce the Oops on that system.
Since running with this patch, I have not seen the Oops reoccur. I have
also tested it on a Thinkpad X1-Gen12, and that system boots and runs
well with it, but it only has a single Thunderbolt domain, so it does
not exercise the multi-domain failure case.

Signed-off-by: Jonathan Gopel <jgopel@gmail.com>
---
 drivers/thunderbolt/icm.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/drivers/thunderbolt/icm.c b/drivers/thunderbolt/icm.c
index 669807f0eaf8..d6801d8b669b 100644
--- a/drivers/thunderbolt/icm.c
+++ b/drivers/thunderbolt/icm.c
@@ -2068,13 +2068,16 @@ static void icm_unplug_children(struct tb_switch *sw)
 	}
 }
 
-static int complete_rpm(struct device *dev, void *data)
+static void complete_rpm(struct tb_switch *sw)
 {
-	struct tb_switch *sw = tb_to_switch(dev);
+	struct tb_port *port;
 
-	if (sw)
-		complete(&sw->rpm_complete);
-	return 0;
+	complete(&sw->rpm_complete);
+
+	tb_switch_for_each_port(sw, port) {
+		if (tb_port_has_remote(port))
+			complete_rpm(port->remote->sw);
+	}
 }
 
 static void remove_unplugged_switch(struct tb_switch *sw)
@@ -2088,8 +2091,7 @@ static void remove_unplugged_switch(struct tb_switch *sw)
 	 * tb_switch_remove() calls pm_runtime_get_sync() that then waits
 	 * for it.
 	 */
-	complete_rpm(&sw->dev, NULL);
-	bus_for_each_dev(&tb_bus_type, &sw->dev, NULL, complete_rpm);
+	complete_rpm(sw);
 	tb_switch_remove(sw);
 
 	pm_runtime_mark_last_busy(parent);
-- 
2.55.0

Re: [PATCH] thunderbolt: Do not RPM complete unrelated subtrees on unplug

[Mika Westerberg]

Hi,

On Sat, Oct 03, 2026 at 08:26:04PM -0600, Jonathan Gopel wrote:
> When a Thunderbolt device is unplugged, that switch and every switch
> subsequent to it on the bus device list is `complete()`d, not just its
> child devices. In the case of multiple domains, it may cause, eg, an
> attempt to `complete()` the uninitialized domain 1 root switch RPM
> completion on unplug of a switch from domain 0 (though ordering is not
> guaranteed numerically sorted).

Good catch!

> I initially noticed this issue on a Dell XPS9720 when the system would
> seem to sporadically not wake after unplugging a Thunderbolt dock. I
> tried multiple kernel versions including v7.2.7 and v7.3-rc4 to see if
> that resolved the issue, but the issue persisted. Eventually I found
> that I could consistently generate a kernel Oops reporting a page fault
> at 0xffff_ffff_ffff_fff8 on unplug from one side of the laptop and went
> bug hunting. I became suspicious of the `bus_for_each_dev()` walk that I
> ultimately ended up changing. To confirm this issue, I did an eBPF trace
> of `bus_for_each_dev()`, `complete_rpm()`, and `complete()` while
> unplugging in a variety of situations and found that an unplug from
> domain 0 tries to `complete_rpm()` domain 1's switch, whose RPM
> completion is not initialized and thus is holding a null wait-list
> pointer, triggering the page fault.
> 
> Here are the key sections of that eBPF trace. Note that HEAD denotes the
> completion's own wait-list HEAD:
> 
> ```
> RESCAN_ENTER domain=0
> 
> COMPLETE rescan_domain=0 target=0-1 target_domain=0
>          route=0x1 parent=0-0 unplugged=1
> COMPLETION done=0 head=HEAD next=HEAD prev=HEAD
> 
> BUS_WALK domain=0 start=0-1 callback=complete_rpm
> CALLBACK_ENTER rescan_domain=0 dev=1-0 parent=domain1 is_switch=1
> 
> COMPLETE rescan_domain=0 target=1-0 target_domain=1
>          route=0x0 parent=domain1 unplugged=0
> COMPLETION done=0 head=HEAD next=0 prev=0
> 
>         complete+5
>         complete_rpm+43
>         bus_for_each_dev+133
>         icm_free_unplugged_children+250
>         icm_rescan_work+42
> ```
> 
> To fix this, I have reused some of the adjacent logic to recursively
> walk and `complete()` only the unplugged switch and its descendants.
> 
> I only have a single system with multiple Thunderbolt domains - the Dell
> XPS9720. I am consistently able to reproduce the Oops on that system.
> Since running with this patch, I have not seen the Oops reoccur. I have
> also tested it on a Thinkpad X1-Gen12, and that system boots and runs
> well with it, but it only has a single Thunderbolt domain, so it does
> not exercise the multi-domain failure case.

This looks like LLM was involved and if that's the case then please add
Assisted-by: LLM.

> 
> Signed-off-by: Jonathan Gopel <jgopel@gmail.com>
> ---
>  drivers/thunderbolt/icm.c | 16 +++++++++-------
>  1 file changed, 9 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/thunderbolt/icm.c b/drivers/thunderbolt/icm.c
> index 669807f0eaf8..d6801d8b669b 100644
> --- a/drivers/thunderbolt/icm.c
> +++ b/drivers/thunderbolt/icm.c
> @@ -2068,13 +2068,16 @@ static void icm_unplug_children(struct tb_switch *sw)
>  	}
>  }
>  
> -static int complete_rpm(struct device *dev, void *data)
> +static void complete_rpm(struct tb_switch *sw)
>  {
> -	struct tb_switch *sw = tb_to_switch(dev);
> +	struct tb_port *port;

So instead of this, just check that sw->tb matches what you pass in..

>  
> -	if (sw)
> -		complete(&sw->rpm_complete);
> -	return 0;
> +	complete(&sw->rpm_complete);
> +
> +	tb_switch_for_each_port(sw, port) {
> +		if (tb_port_has_remote(port))
> +			complete_rpm(port->remote->sw);
> +	}
>  }
>  
>  static void remove_unplugged_switch(struct tb_switch *sw)
> @@ -2088,8 +2091,7 @@ static void remove_unplugged_switch(struct tb_switch *sw)
>  	 * tb_switch_remove() calls pm_runtime_get_sync() that then waits
>  	 * for it.
>  	 */
> -	complete_rpm(&sw->dev, NULL);
> -	bus_for_each_dev(&tb_bus_type, &sw->dev, NULL, complete_rpm);

.. here pass sw->tb 

Re: [PATCH] thunderbolt: Do not RPM complete unrelated subtrees on unplug

[Jonathan Gopel]

Hi Mika,

Thank you for the prompt response, it's much appreciated. I've left a
few questions inline.

On Sun, Oct 04, 2026 at 02:02:55PM +0200, Mika Westerberg wrote:
> Hi,
> 
> On Sat, Oct 03, 2026 at 08:26:04PM -0600, Jonathan Gopel wrote:
> > When a Thunderbolt device is unplugged, that switch and every switch
> > subsequent to it on the bus device list is `complete()`d, not just its
> > child devices. In the case of multiple domains, it may cause, eg, an
> > attempt to `complete()` the uninitialized domain 1 root switch RPM
> > completion on unplug of a switch from domain 0 (though ordering is not
> > guaranteed numerically sorted).
> 
> Good catch!
> 
> > I initially noticed this issue on a Dell XPS9720 when the system would
> > seem to sporadically not wake after unplugging a Thunderbolt dock. I
> > tried multiple kernel versions including v7.2.7 and v7.3-rc4 to see if
> > that resolved the issue, but the issue persisted. Eventually I found
> > that I could consistently generate a kernel Oops reporting a page fault
> > at 0xffff_ffff_ffff_fff8 on unplug from one side of the laptop and went
> > bug hunting. I became suspicious of the `bus_for_each_dev()` walk that I
> > ultimately ended up changing. To confirm this issue, I did an eBPF trace
> > of `bus_for_each_dev()`, `complete_rpm()`, and `complete()` while
> > unplugging in a variety of situations and found that an unplug from
> > domain 0 tries to `complete_rpm()` domain 1's switch, whose RPM
> > completion is not initialized and thus is holding a null wait-list
> > pointer, triggering the page fault.
> > 
> > Here are the key sections of that eBPF trace. Note that HEAD denotes the
> > completion's own wait-list HEAD:
> > 
> > ```
> > RESCAN_ENTER domain=0
> > 
> > COMPLETE rescan_domain=0 target=0-1 target_domain=0
> >          route=0x1 parent=0-0 unplugged=1
> > COMPLETION done=0 head=HEAD next=HEAD prev=HEAD
> > 
> > BUS_WALK domain=0 start=0-1 callback=complete_rpm
> > CALLBACK_ENTER rescan_domain=0 dev=1-0 parent=domain1 is_switch=1
> > 
> > COMPLETE rescan_domain=0 target=1-0 target_domain=1
> >          route=0x0 parent=domain1 unplugged=0
> > COMPLETION done=0 head=HEAD next=0 prev=0
> > 
> >         complete+5
> >         complete_rpm+43
> >         bus_for_each_dev+133
> >         icm_free_unplugged_children+250
> >         icm_rescan_work+42
> > ```
> > 
> > To fix this, I have reused some of the adjacent logic to recursively
> > walk and `complete()` only the unplugged switch and its descendants.
> > 
> > I only have a single system with multiple Thunderbolt domains - the Dell
> > XPS9720. I am consistently able to reproduce the Oops on that system.
> > Since running with this patch, I have not seen the Oops reoccur. I have
> > also tested it on a Thinkpad X1-Gen12, and that system boots and runs
> > well with it, but it only has a single Thunderbolt domain, so it does
> > not exercise the multi-domain failure case.
> 
> This looks like LLM was involved and if that's the case then please add
> Assisted-by: LLM.
> 

I'm happy to add an annotation here, but I was unclear on what the
policy was. I have a few other issues with this system since adding a
TB5 dock to my setup that I may also pursue patching, so if it's ok with
you I'd love to understand the expectation a bit better be expected.
I've read the policy, but I didn't come away with a clear understanding
of what exactly counts as assistance.

I had an LLM do the initial portion of the investigation here, basically
going from symptoms to identifying the Oops, which pointed at the
Thunderbolt unplug code. From there, I wanted to make sure I delivered a
high quality fix, so I did the remainder by hand. Does this warrant the
annotation?

> > 
> > Signed-off-by: Jonathan Gopel <jgopel@gmail.com>
> > ---
> >  drivers/thunderbolt/icm.c | 16 +++++++++-------
> >  1 file changed, 9 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/thunderbolt/icm.c b/drivers/thunderbolt/icm.c
> > index 669807f0eaf8..d6801d8b669b 100644
> > --- a/drivers/thunderbolt/icm.c
> > +++ b/drivers/thunderbolt/icm.c
> > @@ -2068,13 +2068,16 @@ static void icm_unplug_children(struct tb_switch *sw)
> >  	}
> >  }
> >  
> > -static int complete_rpm(struct device *dev, void *data)
> > +static void complete_rpm(struct tb_switch *sw)
> >  {
> > -	struct tb_switch *sw = tb_to_switch(dev);
> > +	struct tb_port *port;}
> 
> So instead of this, just check that sw->tb matches what you pass in..
> 
> >  
> > -	if (sw)
> > -		complete(&sw->rpm_complete);
> > -	return 0;
> > +	complete(&sw->rpm_complete);
> > +
> > +	tb_switch_for_each_port(sw, port) {
> > +		if (tb_port_has_remote(port))
> > +			complete_rpm(port->remote->sw);
> > +	}
> >  }
> >  
> >  static void remove_unplugged_switch(struct tb_switch *sw)
> > @@ -2088,8 +2091,7 @@ static void remove_unplugged_switch(struct tb_switch *sw)
> >  	 * tb_switch_remove() calls pm_runtime_get_sync() that then waits
> >  	 * for it.
> >  	 */
> > -	complete_rpm(&sw->dev, NULL);
> > -	bus_for_each_dev(&tb_bus_type, &sw->dev, NULL, complete_rpm);
> 
> .. here pass sw->tb 

I'm submitting a v2 patch with this revision. If you have time, I would
love to understand the reasoning for preferring one over the other - to
my eye they look equivalent.

[PATCH v2] thunderbolt: Do not RPM complete unrelated subtrees on unplug

[Jonathan Gopel]

When a Thunderbolt device is unplugged, that switch and every switch
subsequent to it on the bus device list is `complete()`d, not just its
child devices. In the case of multiple domains, it may cause, eg, an
attempt to `complete()` the uninitialized domain 1 root switch RPM
completion on unplug of a switch from domain 0 (though ordering is not
guaranteed numerically sorted).

I initially noticed this issue on a Dell XPS9720 when the system would
seem to sporadically not wake after unplugging a Thunderbolt dock. I
tried multiple kernel versions including v7.2.7 and v7.3-rc4 to see if
that resolved the issue, but the issue persisted. Eventually I found
that I could consistently generate a kernel Oops reporting a page fault
at 0xffff_ffff_ffff_fff8 on unplug from one side of the laptop and went
bug hunting. I became suspicious of the `bus_for_each_dev()` walk that I
ultimately ended up changing. To confirm this issue, I did an eBPF trace
of `bus_for_each_dev()`, `complete_rpm()`, and `complete()` while
unplugging in a variety of situations and found that an unplug from
domain 0 tries to `complete_rpm()` domain 1's switch, whose RPM
completion is not initialized and thus is holding a null wait-list
pointer, triggering the page fault.

Here are the key sections of that eBPF trace. Note that HEAD denotes the
completion's own wait-list HEAD:

```
RESCAN_ENTER domain=0

COMPLETE rescan_domain=0 target=0-1 target_domain=0
         route=0x1 parent=0-0 unplugged=1
COMPLETION done=0 head=HEAD next=HEAD prev=HEAD

BUS_WALK domain=0 start=0-1 callback=complete_rpm
CALLBACK_ENTER rescan_domain=0 dev=1-0 parent=domain1 is_switch=1

COMPLETE rescan_domain=0 target=1-0 target_domain=1
         route=0x0 parent=domain1 unplugged=0
COMPLETION done=0 head=HEAD next=0 prev=0

        complete+5
        complete_rpm+43
        bus_for_each_dev+133
        icm_free_unplugged_children+250
        icm_rescan_work+42
```

This patch gates the `complete()` on an additional condition - that the
Thunderbolt domain matches the domain of the device that was unplugged.

I only have a single system with multiple Thunderbolt domains - the Dell
XPS9720. I am consistently able to reproduce the Oops on that system.
Since running with this patch, I have not seen the Oops reoccur. I have
also tested it on a Thinkpad X1-Gen12, and that system boots and runs
well with it, but it only has a single Thunderbolt domain, so it does
not exercise the multi-domain failure case.

Assisted by: LLM
Signed-off-by: Jonathan Gopel <jgopel@gmail.com>
---
 drivers/thunderbolt/icm.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/thunderbolt/icm.c b/drivers/thunderbolt/icm.c
index 669807f0eaf8..c1e6b2ea63cb 100644
--- a/drivers/thunderbolt/icm.c
+++ b/drivers/thunderbolt/icm.c
@@ -2071,9 +2071,9 @@ static void icm_unplug_children(struct tb_switch *sw)
 static int complete_rpm(struct device *dev, void *data)
 {
 	struct tb_switch *sw = tb_to_switch(dev);
-
-	if (sw)
+	if (sw && sw->tb == (struct tb *)data) {
 		complete(&sw->rpm_complete);
+	}
 	return 0;
 }
 
@@ -2088,8 +2088,8 @@ static void remove_unplugged_switch(struct tb_switch *sw)
 	 * tb_switch_remove() calls pm_runtime_get_sync() that then waits
 	 * for it.
 	 */
-	complete_rpm(&sw->dev, NULL);
-	bus_for_each_dev(&tb_bus_type, &sw->dev, NULL, complete_rpm);
+	complete_rpm(&sw->dev, sw->tb);
+	bus_for_each_dev(&tb_bus_type, &sw->dev, sw->tb, complete_rpm);
 	tb_switch_remove(sw);
 
 	pm_runtime_mark_last_busy(parent);
-- 
2.55.0