* [PATCH AUTOSEL 6.18] driver core: Replace dev->can_match with dev_can_match()
[not found] <20260831133314.4125787-1-sashal@kernel.org>
@ 2026-08-31 13:26 ` Sasha Levin
2026-08-31 13:30 ` [PATCH AUTOSEL 6.18] driver core: Avoid warning when removing a device while its supplier is unbinding Sasha Levin
1 sibling, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2026-08-31 13:26 UTC (permalink / raw)
To: patches, stable
Cc: Douglas Anderson, Saravana Kannan, Rafael J. Wysocki (Intel),
Danilo Krummrich, Greg Kroah-Hartman, Marek Szyprowski,
Sasha Levin, driver-core, linux-kernel
From: Douglas Anderson <dianders@chromium.org>
[ Upstream commit f2648bb3150a71241a2254aa4ac10680d7f9fb16 ]
In C, bitfields are not necessarily safe to modify from multiple
threads without locking. Switch "can_match" over to the "flags" field
so modifications are safe.
Cc: Saravana Kannan <saravanak@kernel.org>
Reviewed-by: Rafael J. Wysocki (Intel) <rafael@kernel.org>
Reviewed-by: Danilo Krummrich <dakr@kernel.org>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Douglas Anderson <dianders@chromium.org>
Link: https://patch.msgid.link/20260406162231.v5.2.I54b3ae6311ff34ad30227659d91bb109911a4aea@changeid
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
# Stable Backport Analysis: `driver core: Replace dev->can_match with
dev_can_match()`
**Local tree:** `v6.18.43` (Makefile: 6.18.43)
---
## PHASE 1: COMMIT MESSAGE FORENSICS
**Step 1.1 — Subject line**
Record: `[driver core]` `[Replace]` — move `can_match` from a struct
bitfield to atomic flag accessors (`dev_can_match()` /
`dev_set_can_match()`).
**Step 1.2 — Tags**
Record:
- `Cc: Saravana Kannan <saravanak@kernel.org>`
- `Reviewed-by: Rafael J. Wysocki (Intel) <rafael@kernel.org>`
- `Reviewed-by: Danilo Krummrich <dakr@kernel.org>`
- `Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>`
- `Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>`
- `Signed-off-by: Douglas Anderson <dianders@chromium.org>`
- `Link: https://patch.msgid.link/20260406162231.v5.2.I54b3ae6311ff34ad3
0227659d91bb109911a4aea@changeid`
- `Signed-off-by: Danilo Krummrich <dakr@kernel.org>`
- No `Fixes:`, no `Reported-by:`, no `Cc: stable@vger.kernel.org`
- Notable: subsystem maintainer (Greg K-H) and PM/driver-core reviewers
acked/reviewed
**Step 1.3 — Body**
Record:
- **Bug:** In C, bitfields are not safe to modify from multiple threads
without locking.
- **Symptom:** Not spelled out; this is a concurrency-correctness fix,
not a crash report.
- **Root cause:** `can_match` was stored as a `bool` bitfield in `struct
device` while being read/written from concurrent probe paths.
- **Fix:** Move `can_match` into the existing `flags` bitmap (same
pattern as `DEV_FLAG_READY_TO_PROBE`) and use `dev_can_match()` /
`dev_set_can_match()` atomic accessors.
**Step 1.4 — Hidden bug fix?**
Record: **Yes.** Despite the neutral “Replace” wording, this fixes a
real data-race / undefined-behavior problem. The parent commit
`3e8fefd2997c8` explicitly avoided bitfields for `ready_to_probe` for
this exact reason, but left `can_match` as a bitfield — this patch
completes that design.
---
## PHASE 2: DIFF ANALYSIS
**Step 2.1 — Inventory**
Record:
- `include/linux/device.h`: +5 doc, +1 enum, −1 bitfield, +1 accessor
macro (~15 net lines)
- `drivers/base/core.c`: 6 sites, `dev->can_match` → `dev_can_match()` /
`dev_set_can_match()`
- `drivers/base/dd.c`: 4 sites, same replacement
- **Functions touched:** `dev_is_best_effort`,
`device_links_check_suppliers`, `device_links_driver_bound`,
`fw_devlink_no_driver`, `device_add`, `driver_deferred_probe_add`,
`__driver_probe_device`, `__device_attach_driver`, `__driver_attach`
- **Scope:** Single-subsystem, surgical mechanical refactor (~40 lines
changed)
**Step 2.2 — Code flow (per hunk)**
Record:
- **Before:** Direct read/write of `dev->can_match` bitfield (non-atomic
RMW on shared storage).
- **After:** `test_bit` / `set_bit` on `dev->flags[DEV_FLAG_CAN_MATCH]`
via inline accessors.
- **Paths affected:** Device probe attach, deferred probe, fw_devlink
supplier checks, `device_add()` tail.
**Step 2.3 — Bug mechanism**
Record: **Synchronization / data-race fix.** Category (b): concurrent
unsynchronized bitfield access. Adjacent bitfields in `struct device`
(`state_synced`, `offline`, `of_node_reused`, DMA flags) can be
corrupted by non-atomic RMW on `can_match`.
**Step 2.4 — Fix quality**
Record: Obviously correct — mirrors the already-merged `ready_to_probe`
pattern. Minimal risk; no API surface change for drivers (accessors are
static inline in `device.h`). Regression risk: very low.
---
## PHASE 3: GIT HISTORY INVESTIGATION
**Step 3.1 — Blame**
Record: `can_match` bitfield introduced in `3e8fefd2997c8` (“driver
core: Don't let a device probe until it's ready”), merged via
`5d324e5159d9e`, present since at least `v6.18.27` in this tree. Blame
on `include/linux/device.h:699` and `drivers/base/dd.c:868` points to
that introduction.
**Step 3.2 — Fixes: tag**
Record: N/A — no `Fixes:` tag. The logical bug-introducer is
`3e8fefd2997c8`, which **is** in this tree (`git merge-base --is-
ancestor` confirmed).
**Step 3.3 — Related file history**
Record: Recent driver-core commits in this tree (`0830287cc6cb7`,
`3880ee7c88d78`, etc.) do not touch `can_match`. No duplicate fix found.
This commit is **not** yet in the tree (`dev_can_match` grep returns
nothing).
**Step 3.4 — Author context**
Record: Douglas Anderson authored `3e8fefd2997c8` and `fa9a4c5e69aaa`
(similar fwnode flags thread-safety fix). Driver-core maintainer chain
reviewed both.
**Step 3.5 — Dependencies**
Record: Requires `3e8fefd2997c8` (adds `can_match`, `flags` bitmap,
`__create_dev_flag_accessors`). That prerequisite **exists** in
v6.18.43. Patch is standalone; no series dependency beyond that.
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
**Step 4.1 — Original discussion**
Record: `b4 dig -c <hash>` could not run — commit not in local tree.
Link fetch to patch.msgid.link and lore.kernel.org returned 403/bot-
block. **UNVERIFIED:** full review thread content.
**Step 4.2 — Reviewers**
Record: From commit message — Greg K-H (driver core maintainer), Rafael
Wysocki (PM/driver core), Danilo Krummrich (reviewer/committer), Marek
Szyprowski (Acked-by).
**Step 4.3 — Bug report**
Record: N/A — no external bug report linked.
**Step 4.4 — Series context**
Record: Link msgid contains `v5.2`, suggesting patch 2 of v5 of the
“ready to probe” series. This is a follow-up to `3e8fefd2997c8`, which
was `Cc: stable@vger.kernel.org`.
**Step 4.5 — Stable list**
Record: **UNVERIFIED** — could not search lore stable archive (403).
---
## PHASE 5: CODE SEMANTIC ANALYSIS
**Step 5.1 — Key functions**
Record: `dev_can_match`, `dev_set_can_match`, `dev_is_best_effort`,
`device_links_check_suppliers`, `device_links_driver_bound`,
`fw_devlink_no_driver`, `device_add`, `driver_deferred_probe_add`,
`__driver_probe_device`, `__device_attach_driver`, `__driver_attach`.
**Step 5.2 — Callers / concurrency**
Record:
- **Writes** to `can_match`: `__driver_probe_device` (device lock held
per `driver_probe_device` comment), `__device_attach_driver` (device
lock held in `__device_attach`), **`__driver_attach` (NO device_lock
when setting `can_match` at line 1258)**.
- **Reads**: `device_add()` at line 3778 **without** device lock;
`driver_deferred_probe_add()` without device lock;
`dev_is_best_effort()` during device-link walks under
`device_links_write_lock`; `fw_devlink_no_driver()` under
`device_links_write_lock`.
- Concurrent probe from module load (`driver_register` → `driver_attach`
→ `__driver_attach`) vs. `device_add()` is the documented race class
from `3e8fefd2997c8`.
**Step 5.3 — Callees**
Record: After fix, uses `test_bit`/`set_bit` on `dev->flags` — same as
`dev_ready_to_probe()`.
**Step 5.4 — Reachability**
Record: Reachable from `finit_module`/`modprobe`, `device_add()`,
deferred probe workqueue — common boot and hotplug paths. **Userspace-
reachable** via module loading.
**Step 5.5 — Similar patterns**
Record: `ready_to_probe` already uses atomic `flags`; `fa9a4c5e69aaa`
made fwnode flags thread-safe. `can_match` as bitfield is the
inconsistent outlier.
---
## PHASE 6: CROSS-REFERENCE AGAINST LOCAL TREE (v6.18.43)
**Step 6.1 — Buggy code present?**
Record: **Yes.** `bool can_match:1` at `include/linux/device.h:699`;
direct `dev->can_match` access in `drivers/base/core.c` and
`drivers/base/dd.c`. Introduced in `3e8fefd2997c8`, ancestor of HEAD.
**Step 6.2 — Backport complications**
Record: **Clean apply expected.** `DECLARE_BITMAP(flags,
DEV_FLAG_COUNT)` and `__create_dev_flag_accessors` macro already exist;
only need to add `DEV_FLAG_CAN_MATCH` and swap usages. No conflicting
local changes found.
**Step 6.3 — Fix already present?**
Record: **No.** `dev_can_match` / `DEV_FLAG_CAN_MATCH` absent from tree.
---
## PHASE 7: SUBSYSTEM CONTEXT
**Step 7.1 — Subsystem / criticality**
Record: **driver core** (`drivers/base/`) — **CORE** subsystem; affects
all device probe/bind on all platforms using the driver model.
**Step 7.2 — Activity**
Record: Actively maintained; recent probe/deferred-probe fixes in this
tree.
---
## PHASE 8: IMPACT AND RISK
**Step 8.1 — Who is affected**
Record: **Universal** for systems using driver core probe, especially
with fw_devlink and parallel/async module loading (Android, others).
**Step 8.2 — Trigger conditions**
Record: Concurrent device probe during `device_add()`, driver
registration, deferred probe, or async attach — timing-dependent but
realistic (documented in `3e8fefd2997c8` on Android parallel module
loading). Unprivileged users can trigger via `modprobe`/`finit_module`.
**Step 8.3 — Failure mode**
Record: Undefined behavior from concurrent bitfield RMW — possible
corruption of adjacent `struct device` bitfields (`state_synced`,
`offline`, `of_node_reused`, DMA flags), leading to incorrect fw_devlink
behavior, stuck deferred probe, or wrong best-effort probing.
**Severity: HIGH** (subtle but serious driver-core state corruption;
same class as the already-stable-nominated `3e8fefd` fix).
**Step 8.4 — Risk/benefit**
Record:
- **Benefit:** HIGH — eliminates UB in core probe path on a tree that
already carries the `can_match` feature.
- **Risk:** LOW — mechanical, pattern-proven, maintainer-reviewed.
- **Ratio:** Strong benefit, minimal risk.
---
## PHASE 9: FINAL SYNTHESIS
**Step 9.1 — Evidence**
| FOR | AGAINST |
|-----|---------|
| Fixes real concurrent bitfield UB | No user crash report for this
specific patch |
| Prerequisite `3e8fefd` already in v6.18.43 | Mailing-list stable
nomination unverified |
| Completes design of parent stable-nominated commit |
Theoretical/timing-dependent (no syzbot) |
| Small, surgical, maintainer-acked | |
| Core subsystem, userspace-triggerable via modprobe | |
| Same pattern as `ready_to_probe` (already atomic) | |
**Step 9.2 — Stable rules checklist**
1. Obviously correct and tested? **PASS** — mirrors existing
`ready_to_probe` accessors; reviewed by Greg K-H, Rafael, Danilo.
2. Fixes a real bug? **PASS** — concurrent bitfield access is C UB with
verified unsynchronized read/write paths.
3. Important issue? **PASS** — driver-core state corruption / probe
hangs (HIGH).
4. Small and contained? **PASS** — ~40 lines, 3 files.
5. No new features/APIs? **PASS** — internal refactor only.
6. Can apply to local tree? **PASS** — prerequisite present, no
conflicting fix.
**Step 9.3 — Exception category**
Record: N/A (not device ID/quirk/DT/build/doc); standard concurrency bug
fix.
**Step 9.4 — Decision rationale**
This tree (v6.18.43) contains `can_match` as an unsafe bitfield since
`3e8fefd2997c8`. That parent commit explicitly moved `ready_to_probe` to
atomic flags because bitfields are unsafe under concurrent probe, but
left `can_match` as a bitfield — an oversight this patch corrects.
Verified code paths show unsynchronized reads (`device_add`,
`driver_deferred_probe_add`) and writes (`__driver_attach` without
device lock) on the same field, with adjacent bitfields also modified
concurrently. The fix is small, obviously correct, maintainer-approved,
and directly supports the already-backported probe-race infrastructure
in this tree.
---
## Verification
- [Phase 1] Parsed subject, all tags from provided commit message
- [Phase 2] Diff analysis: 3 files, bitfield →
`dev_can_match()`/`dev_set_can_match()`, 10 call sites
- [Phase 3] `git describe HEAD` → `v6.18.43`
- [Phase 3] `git merge-base --is-ancestor 3e8fefd2997c8 HEAD` → yes
- [Phase 3] `git blame` on `device.h:699`, `dd.c:868` → introduced by
`3e8fefd2997c8`/`5d324e5159d9e`
- [Phase 3] `git log -S'can_match'` → single introduction commit
`3e8fefd2997c8`
- [Phase 3] `grep dev_can_match` → not in tree (fix not applied)
- [Phase 4] patch.msgid.link / lore.kernel.org → 403 blocked
- [Phase 4] `b4 dig` → cannot run without commit hash in tree
- [Phase 5] Read `dd.c:857-868`, `dd.c:1252-1258`, `core.c:3778`,
`device.h:718-740` — confirmed concurrent access patterns
- [Phase 5] `grep state_synced|offline` in `drivers/base/` — adjacent
bitfields modified concurrently
- [Phase 6] `grep dev->can_match` — 8 sites in `core.c`/`dd.c`; bitfield
at `device.h:699`
- [Phase 6] `DECLARE_BITMAP(flags)` and
`__create_dev_flag_accessors(ready_to_probe)` present at
`device.h:715-740`
- [Phase 8] Parent commit `3e8fefd2997c8` documents Android parallel
module-loading race; `Cc: stable@vger.kernel.org`
- **UNVERIFIED:** Lore review thread content, explicit stable-list
discussion for this specific patch
**YES**
drivers/base/core.c | 10 +++++-----
drivers/base/dd.c | 10 +++++-----
include/linux/device.h | 9 +++++----
3 files changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 5034d9b103642..2b0179096c73d 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1084,7 +1084,7 @@ static void device_links_missing_supplier(struct device *dev)
static bool dev_is_best_effort(struct device *dev)
{
- return (fw_devlink_best_effort && dev->can_match) ||
+ return (fw_devlink_best_effort && dev_can_match(dev)) ||
(dev->fwnode && fwnode_test_flag(dev->fwnode, FWNODE_FLAG_BEST_EFFORT));
}
@@ -1152,7 +1152,7 @@ int device_links_check_suppliers(struct device *dev)
if (dev_is_best_effort(dev) &&
device_link_test(link, DL_FLAG_INFERRED) &&
- !link->supplier->can_match) {
+ !dev_can_match(link->supplier)) {
ret = -EAGAIN;
continue;
}
@@ -1435,7 +1435,7 @@ void device_links_driver_bound(struct device *dev)
} else if (dev_is_best_effort(dev) &&
device_link_test(link, DL_FLAG_INFERRED) &&
link->status != DL_STATE_CONSUMER_PROBE &&
- !link->supplier->can_match) {
+ !dev_can_match(link->supplier)) {
/*
* When dev_is_best_effort() is true, we ignore device
* links to suppliers that don't have a driver. If the
@@ -1823,7 +1823,7 @@ static int fw_devlink_no_driver(struct device *dev, void *data)
{
struct device_link *link = to_devlink(dev);
- if (!link->supplier->can_match)
+ if (!dev_can_match(link->supplier))
fw_devlink_relax_link(link);
return 0;
@@ -3775,7 +3775,7 @@ int device_add(struct device *dev)
* match with any driver, don't block its consumers from probing in
* case the consumer device is able to operate without this supplier.
*/
- if (dev->fwnode && fw_devlink_drv_reg_done && !dev->can_match)
+ if (dev->fwnode && fw_devlink_drv_reg_done && !dev_can_match(dev))
fw_devlink_unblock_consumers(dev);
if (parent)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index dabdfc088f3f6..d019d0f98ad47 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -132,7 +132,7 @@ static DECLARE_WORK(deferred_probe_work, deferred_probe_work_func);
void driver_deferred_probe_add(struct device *dev)
{
- if (!dev->can_match)
+ if (!dev_can_match(dev))
return;
mutex_lock(&deferred_probe_mutex);
@@ -858,14 +858,14 @@ static int __driver_probe_device(const struct device_driver *drv, struct device
return dev_err_probe(dev, -EPROBE_DEFER, "Device not ready to probe\n");
/*
- * Set can_match = true after calling dev_ready_to_probe(), so
+ * Call dev_set_can_match() after calling dev_ready_to_probe(), so
* driver_deferred_probe_add() won't actually add the device to the
* deferred probe list when dev_ready_to_probe() returns false.
*
* When dev_ready_to_probe() returns false, it means that device_add()
* will do another probe() attempt for us.
*/
- dev->can_match = true;
+ dev_set_can_match(dev);
dev_dbg(dev, "bus: '%s': %s: matched device with driver %s\n",
drv->bus->name, __func__, drv->name);
@@ -1011,7 +1011,7 @@ static int __device_attach_driver(struct device_driver *drv, void *_data)
return 0;
} else if (ret == -EPROBE_DEFER) {
dev_dbg(dev, "Device match requests probe deferral\n");
- dev->can_match = true;
+ dev_set_can_match(dev);
driver_deferred_probe_add(dev);
/*
* Device can't match with a driver right now, so don't attempt
@@ -1255,7 +1255,7 @@ static int __driver_attach(struct device *dev, void *data)
return 0;
} else if (ret == -EPROBE_DEFER) {
dev_dbg(dev, "Device match requests probe deferral\n");
- dev->can_match = true;
+ dev_set_can_match(dev);
driver_deferred_probe_add(dev);
/*
* Driver could not match with device, but may match with
diff --git a/include/linux/device.h b/include/linux/device.h
index dc1252a06480c..56e5a9314367d 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -485,10 +485,14 @@ struct device_physical_location {
*
* @DEV_FLAG_READY_TO_PROBE: If set then device_add() has finished enough
* initialization that probe could be called.
+ * @DEV_FLAG_CAN_MATCH: The device has matched with a driver at least once or it
+ * is in a bus (like AMBA) which can't check for matching drivers
+ * until other devices probe successfully.
* @DEV_FLAG_COUNT: Number of defined struct_device_flags.
*/
enum struct_device_flags {
DEV_FLAG_READY_TO_PROBE = 0,
+ DEV_FLAG_CAN_MATCH = 1,
DEV_FLAG_COUNT
};
@@ -575,9 +579,6 @@ enum struct_device_flags {
* @state_synced: The hardware state of this device has been synced to match
* the software state of this device by calling the driver/bus
* sync_state() callback.
- * @can_match: The device has matched with a driver at least once or it is in
- * a bus (like AMBA) which can't check for matching drivers until
- * other devices probe successfully.
* @dma_coherent: this particular device is dma coherent, even if the
* architecture supports non-coherent devices.
* @dma_ops_bypass: If set to %true then the dma_ops are bypassed for the
@@ -696,7 +697,6 @@ struct device {
bool offline:1;
bool of_node_reused:1;
bool state_synced:1;
- bool can_match:1;
#if defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_DEVICE) || \
defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU) || \
defined(CONFIG_ARCH_HAS_SYNC_DMA_FOR_CPU_ALL)
@@ -738,6 +738,7 @@ static inline bool dev_test_and_set_##accessor_name(struct device *dev) \
}
__create_dev_flag_accessors(ready_to_probe, DEV_FLAG_READY_TO_PROBE);
+__create_dev_flag_accessors(can_match, DEV_FLAG_CAN_MATCH);
#undef __create_dev_flag_accessors
--
2.53.0
^ permalink raw reply related [flat|nested] 2+ messages in thread* [PATCH AUTOSEL 6.18] driver core: Avoid warning when removing a device while its supplier is unbinding
[not found] <20260831133314.4125787-1-sashal@kernel.org>
2026-08-31 13:26 ` [PATCH AUTOSEL 6.18] driver core: Replace dev->can_match with dev_can_match() Sasha Levin
@ 2026-08-31 13:30 ` Sasha Levin
1 sibling, 0 replies; 2+ messages in thread
From: Sasha Levin @ 2026-08-31 13:30 UTC (permalink / raw)
To: patches, stable
Cc: Herve Codina, Rafael J. Wysocki, Saravana Kannan,
Greg Kroah-Hartman, Sasha Levin, dakr, driver-core, linux-kernel
From: Herve Codina <herve.codina@bootlin.com>
[ Upstream commit 36d74f17e03f7e60e1b08fbe16cfad6e69cc3aa9 ]
During driver removal, the following warning can appear:
WARNING: CPU: 1 PID: 139 at drivers/base/core.c:1497 __device_links_no_driver+0xcc/0xfc
...
Call trace:
__device_links_no_driver+0xcc/0xfc (P)
device_links_driver_cleanup+0xa8/0xf0
device_release_driver_internal+0x208/0x23c
device_links_unbind_consumers+0xe0/0x108
device_release_driver_internal+0xec/0x23c
device_links_unbind_consumers+0xe0/0x108
device_release_driver_internal+0xec/0x23c
device_links_unbind_consumers+0xe0/0x108
device_release_driver_internal+0xec/0x23c
driver_detach+0xa0/0x12c
bus_remove_driver+0x6c/0xbc
driver_unregister+0x30/0x60
pci_unregister_driver+0x20/0x9c
lan966x_pci_driver_exit+0x18/0xa90 [lan966x_pci]
This warning is triggered when a consumer is removed because the links
status of its supplier is not DL_DEV_DRIVER_BOUND and the link flag
DL_FLAG_SYNC_STATE_ONLY is not set.
The topology in terms of consumers/suppliers used was the following
(consumer ---> supplier):
i2c -----------> OIC ----> PCI device
| ^
| |
+---> pinctrl ---+
When the PCI device is removed, the OIC (interrupt controller) has to be
removed. In order to remove the OIC, pinctrl and i2c need to be removed
and to remove pinctrl, i2c need to be removed. The removal order is:
1) i2c
2) pinctrl
3) OIC
4) PCI device
In details, the removal sequence is the following (with 0000:01:00.0 the
PCI device):
driver_detach: call device_release_driver_internal(0000:01:00.0)...
device_links_busy(0000:01:00.0):
links->status = DL_DEV_UNBINDING
device_links_unbind_consumers(0000:01:00.0):
0000:01:00.0--oic link->status = DL_STATE_SUPPLIER_UNBIND
call device_release_driver_internal(oic)...
device_links_busy(oic):
links->status = DL_DEV_UNBINDING
device_links_unbind_consumers(oic):
oic--pinctrl link->status = DL_STATE_SUPPLIER_UNBIND
call device_release_driver_internal(pinctrl)...
device_links_busy(pinctrl):
links->status = DL_DEV_UNBINDING
device_links_unbind_consumers(pinctrl):
pinctrl--i2c link->status = DL_STATE_SUPPLIER_UNBIND
call device_release_driver_internal(i2c)...
device_links_busy(i2c): links->status = DL_DEV_UNBINDING
__device_links_no_driver(i2c)...
pinctrl--i2c link->status is DL_STATE_SUPPLIER_UNBIND
oic--i2c link->status is DL_STATE_ACTIVE
oic--i2c link->supplier->links.status is DL_DEV_UNBINDING
The warning is triggered by the i2c removal because the OIC (supplier)
links status is not DL_DEV_DRIVER_BOUND. Its links status is indeed set
to DL_DEV_UNBINDING.
It is perfectly legit to have the links status set to DL_DEV_UNBINDING
in that case. Indeed we had started to unbind the OIC which triggered
the consumer unbinding and didn't finish yet when the i2c is unbound.
Avoid the warning when the supplier links status is set to
DL_DEV_UNBINDING and thus support this removal sequence without any
warnings.
Signed-off-by: Herve Codina <herve.codina@bootlin.com>
Reviewed-by: Rafael J. Wysocki <rafael@kernel.org>
Reviewed-by: Saravana Kannan <saravanak@google.com>
Link: https://patch.msgid.link/20260511155755.34428-4-herve.codina@bootlin.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
LLM Generated explanations, may be completely bogus:
# Stable Backport Analysis
**Local tree:** Linux 6.18.44 (`v6.18.44`, `make kernelversion` =
6.18.44)
**Commit under review:** `36d74f17e03f7` — *driver core: Avoid warning
when removing a device while its supplier is unbinding*
**Status in this tree:** Buggy code present; fix **not** yet applied
(`git apply --check` passes cleanly)
---
## PHASE 1: COMMIT MESSAGE FORENSICS
### Step 1.1: Parse the subject line
**Record:** `[driver core] [avoid] [false WARN_ON during cascading
device-link teardown when supplier is DL_DEV_UNBINDING]`
### Step 1.2: Parse all commit message tags
**Record:**
- **Fixes:** — not present (expected for manual review)
- **Reported-by:** — none
- **Tested-by:** — none on this commit
- **Reviewed-by:** Rafael J. Wysocki `<rafael@kernel.org>`, Saravana
Kannan `<saravanak@google.com>` (both driver-core/PM maintainers)
- **Acked-by:** — none
- **Link:** https://patch.msgid.link/20260511155755.34428-4-
herve.codina@bootlin.com
- **Cc: stable:** — absent (not a negative signal)
- **Signed-off-by:** Herve Codina (author), Greg Kroah-Hartman
(committer); ignore pipeline-added SOBs
**Notable:** Reviewed by both primary driver-core maintainers; no
syzbot/fuzzer report.
### Step 1.3: Analyze commit body
**Record:**
- **Bug:** `WARN_ON` fires in `__device_links_no_driver()` when a
consumer (i2c) is torn down while its supplier (OIC) is mid-unbind
(`DL_DEV_UNBINDING`), during PCI driver removal.
- **Symptom:** Kernel warning at `drivers/base/core.c:1497`, stack
through `device_links_driver_cleanup` →
`device_release_driver_internal` → `device_links_unbind_consumers` →
`pci_unregister_driver` → `lan966x_pci_driver_exit`.
- **Topology:** `i2c → OIC → PCI`, `i2c → pinctrl → OIC`.
- **Root cause:** `WARN_ON` only exempts `DL_FLAG_SYNC_STATE_ONLY` links
when supplier status ≠ `DL_DEV_DRIVER_BOUND`; `DL_DEV_UNBINDING` is
also legitimate during cascading unbind.
- **Version info:** None explicit; trigger hardware (`lan966x_pci`) is
in this tree since Oct 2024.
### Step 1.4: Detect hidden bug fixes
**Record:** Yes — disguised as “avoid warning,” but it corrects overly
strict validation in core driver-link teardown. Runtime behavior is
unchanged (`DL_STATE_DORMANT` still set); only a false-positive
`WARN_ON` is suppressed. With `panic_on_warn` or `CONFIG_BUG_ON_WARN`,
the spurious WARN can escalate to panic on module unload.
---
## PHASE 2: DIFF ANALYSIS
### Step 2.1: Inventory the changes
**Record:**
- **Files:** `drivers/base/core.c` (+2 / −1)
- **Function:** `__device_links_no_driver()`
- **Scope:** Single-file, surgical fix
### Step 2.2: Code flow change
**Record:**
- **Hunk (lines ~1500–1505):**
- **Before:** If supplier not `DL_DEV_DRIVER_BOUND`,
`WARN_ON(!DL_FLAG_SYNC_STATE_ONLY)` then set link
`DL_STATE_DORMANT`.
- **After:** Same, but skip WARN when supplier status is
`DL_DEV_UNBINDING`.
- **Path:** Driver removal cascade — `device_links_busy()` sets
`DL_DEV_UNBINDING`, consumers unbound recursively,
`device_links_driver_cleanup()` → `__device_links_no_driver()`.
### Step 2.3: Bug mechanism
**Record:** **Logic / correctness fix** — false-positive `WARN_ON`
during legitimate teardown. Category: incorrect validation in driver-
core device-link state machine (not UAF, leak, or race).
### Step 2.4: Fix quality
**Record:**
- Obviously correct: `DL_DEV_UNBINDING` is set in `device_links_busy()`
at line 1622 before consumer unbind begins.
- Minimal change; no API/struct changes.
- **Regression risk:** Very low — only suppresses WARN for an already-
handled state; link still goes to `DL_STATE_DORMANT`.
---
## PHASE 3: GIT HISTORY INVESTIGATION
### Step 3.1: Blame changed lines
**Record:**
- `WARN_ON` line: `b29929b819f35` (Jun 2025, Rafael) — refactor to
`device_link_test()`; no semantic change.
- Original `WARN_ON(!(link->flags & DL_FLAG_SYNC_STATE_ONLY))`:
`8c3e315d42964` (May 2020, Saravana Kannan).
- Surrounding logic: `8c3e315d429642` (May 2020).
- `DL_DEV_UNBINDING`: `9ed9895370aed` (2016).
- **Both buggy WARN and `DL_DEV_UNBINDING` are in 6.18.44.**
### Step 3.2: Follow Fixes: tag
**Record:** N/A — no `Fixes:` tag.
### Step 3.3: File history / related changes
**Record:**
- Related in-tree precedent: `74b84d1be0220` — *driver core: fw_devlink:
Don't warn about sync_state() pending* (reduced false driver-core
warnings).
- `b29929b819f35` — `device_link_test()` refactor; in tree.
- Fix commit `36d74f17e03f7` — on `master`, not in `HEAD`.
- Part of v7 lan966x series (patch 3/3), but this hunk is self-contained
in `core.c`.
### Step 3.4: Author context
**Record:** Herve Codina — lan966x_pci author (`185686beb4649`, Oct
2024); limited prior driver-core work (`0462c56c290a9`,
`3b62449da4445`).
### Step 3.5: Dependencies
**Record:** **Standalone.** No prerequisite commits; only adds
`DL_DEV_UNBINDING` exemption to existing WARN. Applies cleanly to
current `HEAD`.
---
## PHASE 4: MAILING LIST AND EXTERNAL RESEARCH
### Step 4.1: Original discussion
**Record:**
- `b4 dig -c 36d74f17e03f7`: https://patch.msgid.link/20260511155755.344
28-4-herve.codina@bootlin.com
- Series: v1–v7; committed version is v7 patch 3/3.
- Thread saved to `/tmp/driver-core-warn.mbox`.
### Step 4.2: Reviewers
**Record:** CC'd: Greg Kroah-Hartman, Rafael J. Wysocki, Saravana
Kannan, driver-core@lists.linux.dev, linux-kernel; appropriate
maintainers included.
### Step 4.3: Bug report
**Record:** Reproduced by author during `lan966x_pci`
`pci_unregister_driver()`; stack trace in commit message. No external
bugzilla/syzbot link.
### Step 4.4: Series context
**Record:** v7 cover is “lan966x pci device: Add support for SFPs, core
part”; patches 1–2 are lan966x/i2c-specific. **Patch 3/3 is independent
driver-core fix** — no dependency on other series patches for
correctness.
### Step 4.5: Stable list
**Record:** No `Cc: stable` or stable-list discussion found in mbox
(`grep -i stable` returned empty).
---
## PHASE 5: CODE SEMANTIC ANALYSIS
### Step 5.1: Key functions
**Record:** `__device_links_no_driver()` (modified); callers
`device_links_no_driver()`, `device_links_driver_cleanup()`.
### Step 5.2: Callers
**Record:**
- `device_links_driver_cleanup()` ← `__device_release_driver()` in
`drivers/base/dd.c:1359`
- Called during `device_release_driver_internal()` →
`device_links_unbind_consumers()` cascade
- Reachable from `pci_unregister_driver()` / module unload — confirmed
in commit stack trace
### Step 5.3: Callees
**Record:** `device_link_test()`, `WRITE_ONCE()` for link status; sets
`dev->links.status = DL_DEV_NO_DRIVER`.
### Step 5.4: Reachability
**Record:** Triggered on driver removal for devices with managed
supplier links in multi-level topologies. **Userspace-reachable** via
module unload / driver unbind. `lan966x_pci` in
`drivers/misc/lan966x_pci.c` is the documented trigger in this tree.
### Step 5.5: Similar patterns
**Record:** Same WARN pattern exists in
`device_links_missing_supplier()` (also from `8c3e315`); this fix
targets only `__device_links_no_driver()`. No other `DL_DEV_UNBINDING`
WARN exemptions in `core.c`.
---
## PHASE 6: CROSS-REFERENCE AGAINST LOCAL TREE (6.18.44)
### Step 6.1: Buggy code exists?
**Record:** **Yes.** Current `drivers/base/core.c:1503`:
```c
WARN_ON(!device_link_test(link, DL_FLAG_SYNC_STATE_ONLY));
```
Bug present since `8c3e315d42964` (2020); trigger topology possible
since `lan966x_pci` (`185686beb4649`, Oct 2024).
### Step 6.2: Backport complications
**Record:** **Clean apply** — `git show 36d74f17e03f7 --
drivers/base/core.c | git apply --check` succeeded.
### Step 6.3: Related fixes already present?
**Record:** Fix `36d74f17e03f7` **not** in tree. Related warn-reduction
`74b84d1be0220` is present. No duplicate fix for this specific case.
---
## PHASE 7: SUBSYSTEM CONTEXT
### Step 7.1: Subsystem criticality
**Record:** **driver core** (`drivers/base/`) — **CORE** subsystem;
affects all device link teardown.
### Step 7.2: Activity
**Record:** Active — recent commits include `3e8fefd2997c8`,
`74b84d1be0220`, `b29929b819f35` on `core.c`.
---
## PHASE 8: IMPACT AND RISK
### Step 8.1: Who is affected
**Record:** Users of hardware with multi-level managed device links
during driver removal. **In this tree:** `lan966x_pci` users
unloading/reloading the module. Broader applicability for similar
topologies.
### Step 8.2: Trigger conditions
**Record:** PCI (or other) driver unregister with supplier→consumer
chain where supplier is `DL_DEV_UNBINDING` while consumer still has
active supplier links. **Uncommon but real** — reproduced on lan966x.
Unprivileged users can trigger via module unload if module is loadable.
### Step 8.3: Failure mode severity
**Record:** Spurious `WARN_ON` in dmesg on every affected teardown.
Default: **MEDIUM** (noise, possible monitoring alerts). With
`panic_on_warn=y`: **HIGH** (panic on module unload). No corruption,
UAF, or deadlock.
### Step 8.4: Risk-benefit
**Record:**
- **Benefit:** Eliminates false warnings on real hardware in this tree;
corrects driver-core validation; very small diff.
- **Risk:** Very low — one additional legitimate state exempted from
WARN.
- **Ratio:** Favorable for 6.18.44, which ships `lan966x_pci`.
---
## PHASE 9: FINAL SYNTHESIS
### Step 9.1: Evidence summary
**FOR backport:**
- Real false-positive WARN on driver removal for hardware present in
6.18.44 (`lan966x_pci`)
- Core subsystem fix, 2-line change, applies cleanly
- Reviewed by Rafael Wysocki and Saravana Kannan
- Buggy code has been in tree since 2020; trigger hardware since Oct
2024
- Self-contained; no series dependencies
- Precedent: `74b84d1be0220` in same tree reduced driver-core false
warnings
**AGAINST backport:**
- No functional/runtime behavior change beyond suppressing WARN
- Default severity is log noise, not crash/corruption
- Narrow topology; not syzbot-reported
- No explicit stable nomination in lore thread
**Unresolved:** Whether production deployments use `panic_on_warn` for
lan966x systems (unverified).
### Step 9.2: Stable rules checklist
1. Obviously correct and tested? **PASS** — logic is clear; maintainer-
reviewed; author tested on lan966x unload path.
2. Fixes a real bug affecting users? **PASS** — false WARN on
`lan966x_pci` module unload in this tree.
3. Important issue? **PASS (borderline)** — MEDIUM by default; HIGH if
`panic_on_warn`; affects real hardware teardown in 6.18.44.
4. Small and contained? **PASS** — 2 lines, one function.
5. No new features/APIs? **PASS**
6. Can apply to local tree? **PASS** — clean apply verified.
### Step 9.3: Exception categories
**Record:** None (not device ID, quirk, DT, build, or docs).
### Step 9.4: Decision rationale
For **Linux 6.18.44 specifically**, this should be backported. The tree
contains both the longstanding overly strict WARN logic and the
`lan966x_pci` driver that reproduces it on every module unload. The fix
is minimal, maintainer-approved, self-contained, and applies cleanly.
While severity is moderate under default configs, it is a genuine
driver-core correctness fix for a real teardown path on shipped
hardware.
---
## Verification
- [Phase 1] Parsed commit message and tags from `git show 36d74f17e03f7`
- [Phase 2] Diff: 2-line WARN condition change in
`__device_links_no_driver()`
- [Phase 3] `git blame -L 1500,1505`: WARN from `b29929b`/`8c3e315`;
`git log -S`: original WARN in `8c3e315d42964`
- [Phase 3] `git merge-base --is-ancestor`: `8c3e315`, `b29929b`,
`185686beb4649` in tree; fix `36d74f17e03f7` NOT in tree
- [Phase 3] `git show 8c3e315d42964`: confirmed WARN introduction in
2020
- [Phase 4] `b4 dig -c 36d74f17e03f7`: lore URL found; v7 patch 3/3
- [Phase 4] `b4 dig -c 36d74f17e03f7 -a`: series v1–v7 listed
- [Phase 4] `b4 dig -c 36d74f17e03f7 -w`: maintainers CC'd
- [Phase 4] `/tmp/driver-core-warn.mbox`: no stable nomination found
- [Phase 4] WebFetch lore URL: blocked by Anubis (unverified via web;
mbox via b4 succeeded)
- [Phase 5] Call chain verified: `dd.c:1359` →
`device_links_driver_cleanup` → `__device_links_no_driver`
- [Phase 5] `device_links_busy()` sets `DL_DEV_UNBINDING` at
`core.c:1622`
- [Phase 6] `git describe HEAD` / `make kernelversion`: 6.18.44
- [Phase 6] `git apply --check`: patch applies cleanly
- [Phase 6] `grep lan966x_pci`: `drivers/misc/lan966x_pci.c` present
- [Phase 6] `git show 185686beb4649`: lan966x_pci added Oct 2024
- [Phase 7] `git log -20 -- drivers/base/core.c`: active subsystem
- [Phase 8] Failure mode assessed as MEDIUM (WARN), HIGH with
panic_on_warn (unverified in field)
- [UNVERIFIED] Exact panic_on_warn usage on lan966x deployments
**YES**Those background lookups finished and matched what the analysis
already used:
- **core.c history:** Recent driver-core commits are on this tree; the
fix (`36d74f17e03f7`) is not.
- **Related fixes:** Older device-link commits turned up, but nothing
that already covers this WARN case.
- **lan966x_pci:** Introduced in `185686beb4649` (Oct 2024) and present
in 6.18.44 — the hardware that triggers the spurious warning on module
unload.
Verdict from the full review: **YES** for backport to this 6.18.44 tree.
drivers/base/core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 2b0179096c73d..5ba783e5055ae 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -1500,7 +1500,8 @@ static void __device_links_no_driver(struct device *dev)
if (link->supplier->links.status == DL_DEV_DRIVER_BOUND) {
WRITE_ONCE(link->status, DL_STATE_AVAILABLE);
} else {
- WARN_ON(!device_link_test(link, DL_FLAG_SYNC_STATE_ONLY));
+ WARN_ON(link->supplier->links.status != DL_DEV_UNBINDING &&
+ !device_link_test(link, DL_FLAG_SYNC_STATE_ONLY));
WRITE_ONCE(link->status, DL_STATE_DORMANT);
}
}
--
2.53.0
^ permalink raw reply related [flat|nested] 2+ messages in thread