Linux driver-core infrastructure
 help / color / mirror / Atom feed
* [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

end of thread, other threads:[~2026-08-31 13:51 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [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 ` [PATCH AUTOSEL 6.18] driver core: Avoid warning when removing a device while its supplier is unbinding Sasha Levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox