* [PATCH] thunderbolt: Do not RPM complete unrelated subtrees on unplug
@ 2026-10-04 2:26 Jonathan Gopel
2026-10-04 12:02 ` Mika Westerberg
0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Gopel @ 2026-10-04 2:26 UTC (permalink / raw)
To: andreas.noever, westeri, YehezkelShB
Cc: linux-usb, linux-kernel, 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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] thunderbolt: Do not RPM complete unrelated subtrees on unplug 2026-10-04 2:26 [PATCH] thunderbolt: Do not RPM complete unrelated subtrees on unplug Jonathan Gopel @ 2026-10-04 12:02 ` Mika Westerberg 2026-10-04 17:11 ` Jonathan Gopel 2026-10-04 17:13 ` [PATCH v2] " Jonathan Gopel 0 siblings, 2 replies; 6+ messages in thread From: Mika Westerberg @ 2026-10-04 12:02 UTC (permalink / raw) To: Jonathan Gopel Cc: andreas.noever, westeri, YehezkelShB, linux-usb, linux-kernel 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] thunderbolt: Do not RPM complete unrelated subtrees on unplug 2026-10-04 12:02 ` Mika Westerberg @ 2026-10-04 17:11 ` Jonathan Gopel 2026-10-05 4:30 ` Mika Westerberg 2026-10-04 17:13 ` [PATCH v2] " Jonathan Gopel 1 sibling, 1 reply; 6+ messages in thread From: Jonathan Gopel @ 2026-10-04 17:11 UTC (permalink / raw) To: Mika Westerberg Cc: andreas.noever, westeri, YehezkelShB, linux-usb, linux-kernel 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. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] thunderbolt: Do not RPM complete unrelated subtrees on unplug 2026-10-04 17:11 ` Jonathan Gopel @ 2026-10-05 4:30 ` Mika Westerberg 0 siblings, 0 replies; 6+ messages in thread From: Mika Westerberg @ 2026-10-05 4:30 UTC (permalink / raw) To: Jonathan Gopel Cc: andreas.noever, westeri, YehezkelShB, linux-usb, linux-kernel Hi, On Sun, Oct 04, 2026 at 11:11:07AM -0600, Jonathan Gopel wrote: > 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? If the LLM did not create the patch then I don't think you need to add the tag. At least this is my interpretation of: https://docs.kernel.org/process/coding-assistants.html > > > > > > 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. The latter (what I'm suggesting) should be slightly simpler since it just adds a check that makes sure the domain is the same. I always prefer simplicity :) ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] thunderbolt: Do not RPM complete unrelated subtrees on unplug 2026-10-04 12:02 ` Mika Westerberg 2026-10-04 17:11 ` Jonathan Gopel @ 2026-10-04 17:13 ` Jonathan Gopel 2026-10-05 11:38 ` Mika Westerberg 1 sibling, 1 reply; 6+ messages in thread From: Jonathan Gopel @ 2026-10-04 17:13 UTC (permalink / raw) To: mika.westerberg Cc: andreas.noever, westeri, YehezkelShB, linux-usb, linux-kernel, 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] thunderbolt: Do not RPM complete unrelated subtrees on unplug 2026-10-04 17:13 ` [PATCH v2] " Jonathan Gopel @ 2026-10-05 11:38 ` Mika Westerberg 0 siblings, 0 replies; 6+ messages in thread From: Mika Westerberg @ 2026-10-05 11:38 UTC (permalink / raw) To: Jonathan Gopel Cc: andreas.noever, westeri, YehezkelShB, linux-usb, linux-kernel Hi, On Sun, Oct 04, 2026 at 11:13:50AM -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). > > 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> I dropped the Assisted-by per your previous email, did a couple of small cosmetic changes to the patch, added Fixes and stable tags and applied to thunderbolt.git/fixes, thanks! ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-10-05 11:38 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-10-04 2:26 [PATCH] thunderbolt: Do not RPM complete unrelated subtrees on unplug Jonathan Gopel 2026-10-04 12:02 ` Mika Westerberg 2026-10-04 17:11 ` Jonathan Gopel 2026-10-05 4:30 ` Mika Westerberg 2026-10-04 17:13 ` [PATCH v2] " Jonathan Gopel 2026-10-05 11:38 ` Mika Westerberg
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®