* [PATCH v2] PCI: Init temporary pci device in new_id_store()
@ 2026-05-05 23:43 Samiullah Khawaja
2026-05-06 0:01 ` Danilo Krummrich
2026-05-06 22:01 ` Bjorn Helgaas
0 siblings, 2 replies; 3+ messages in thread
From: Samiullah Khawaja @ 2026-05-05 23:43 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: skhawaja, Danilo Krummrich, Bandan Das, Gui-Dong Han,
Greg Kroah-Hartman, Alex Williamson, linux-pci, linux-kernel
When setting new_id of a PCI device driver using sysfs a lockdep splat
occurs. This is because the new_id function new_id_store() checks for
driver_override by creating temporary pci_dev structs.
Since the newly added driver_override spinlock is not init for temporary
pci_dev structs, the lockdep splat complains about it.
Initialize the temporary pci device to fix this.
[ 4.464296] INFO: trying to register non-static key.
[ 4.466207] The code is fine but needs lockdep annotation, or maybe
[ 4.468487] you didn't initialize this object before use?
[ 4.470486] turning off the locking correctness validator.
[ 4.472494] CPU: 2 UID: 0 PID: 177 Comm: liveupdate-iomm Not tainted 7.0.0+ #9 PREEMPT(full)
[ 4.472497] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
[ 4.472498] Call Trace:
[ 4.472501] <TASK>
[ 4.472503] dump_stack_lvl+0x5d/0x80
[ 4.472508] register_lock_class+0x77e/0x790
[ 4.472512] ? check_prev_add+0xf4/0xd30
[ 4.472513] ? check_prev_add+0xf4/0xd30
[ 4.472515] __lock_acquire+0x3b4/0x1b80
[ 4.472518] ? raw_irqentry_exit_cond_resched+0x20/0x50
[ 4.472556] lock_acquire+0xbf/0x2e0
[ 4.472558] ? pci_match_device+0x24/0x180
[ 4.472563] _raw_spin_lock+0x30/0x40
[ 4.472567] ? pci_match_device+0x24/0x180
[ 4.472569] pci_match_device+0x24/0x180
[ 4.472571] new_id_store+0x189/0x1d0
[ 4.472574] kernfs_fop_write_iter+0x14f/0x210
[ 4.472578] vfs_write+0x263/0x5e0
[ 4.472583] ksys_write+0x79/0xf0
[ 4.472585] do_syscall_64+0x117/0xf80
[ 4.472587] ? clear_bhb_loop+0x40/0x90
[ 4.472590] entry_SYSCALL_64_after_hwframe+0x77/0x7f
[ 4.472592] RIP: 0033:0x7fe7bc0beeb2
[ 4.472595] Code: 18 41 8b 93 08 03 00 00 59 5e 48 83 f8 fc 75 1a 83 e2 39 83 fa 08 75 12 e8 2b ff ff ff 0f 1f 00 49 89 ca 48 8b 44 24 20 0f 05 <48> 83 c4 18 c3 66 0f 1f 84 00 00 00 00 00 48 83 ec 10 ff 74 24 18
[ 4.472596] RSP: 002b:00007ffcdbb1ed80 EFLAGS: 00000202 ORIG_RAX: 0000000000000001
[ 4.472598] RAX: ffffffffffffffda RBX: 00007fe7bc2415c0 RCX: 00007fe7bc0beeb2
[ 4.472599] RDX: 000000000000000a RSI: 00005580955433c0 RDI: 0000000000000001
[ 4.472600] RBP: 000000000000000a R08: 0000000000000000 R09: 0000000000000000
[ 4.472601] R10: 0000000000000000 R11: 0000000000000202 R12: 000000000000000a
[ 4.472602] R13: 00005580955433c0 R14: 000055805932493e R15: 0000558095545490
[ 4.472605] </TASK>
Fixes: 10a4206a2401 ("PCI: use generic driver_override infrastructure")
Fixes: 8895d3bcb8ba ("PCI: Fail new_id for vendor/device values already built into driver")
Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
---
v2:
- Use device_initialize instead of spinlock init
- Set release cb and call put_device to release
- Add Fixes tag for the commit that added temporary pci device without
initialize.
---
drivers/pci/pci-driver.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index d10ece0889f0..e3f59001785a 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -179,6 +179,11 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
return NULL;
}
+static void _pci_free_device(struct device *dev)
+{
+ kfree(to_pci_dev(dev));
+}
+
/**
* new_id_store - sysfs frontend to pci_add_dynid()
* @driver: target device driver
@@ -214,11 +219,13 @@ static ssize_t new_id_store(struct device_driver *driver, const char *buf,
pdev->subsystem_vendor = subvendor;
pdev->subsystem_device = subdevice;
pdev->class = class;
+ pdev->dev.release = _pci_free_device;
+ device_initialize(&pdev->dev);
if (pci_match_device(pdrv, pdev))
retval = -EEXIST;
- kfree(pdev);
+ put_device(&pdev->dev);
if (retval)
return retval;
base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
--
2.54.0.545.g6539524ca2-goog
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] PCI: Init temporary pci device in new_id_store()
2026-05-05 23:43 [PATCH v2] PCI: Init temporary pci device in new_id_store() Samiullah Khawaja
@ 2026-05-06 0:01 ` Danilo Krummrich
2026-05-06 22:01 ` Bjorn Helgaas
1 sibling, 0 replies; 3+ messages in thread
From: Danilo Krummrich @ 2026-05-06 0:01 UTC (permalink / raw)
To: Samiullah Khawaja
Cc: Bjorn Helgaas, Bandan Das, Gui-Dong Han, Greg Kroah-Hartman,
Alex Williamson, linux-pci, linux-kernel
On Wed May 6, 2026 at 1:43 AM CEST, Samiullah Khawaja wrote:
> When setting new_id of a PCI device driver using sysfs a lockdep splat
> occurs. This is because the new_id function new_id_store() checks for
> driver_override by creating temporary pci_dev structs.
>
> Since the newly added driver_override spinlock is not init for temporary
> pci_dev structs, the lockdep splat complains about it.
>
> Initialize the temporary pci device to fix this.
>
> [ 4.464296] INFO: trying to register non-static key.
> [ 4.466207] The code is fine but needs lockdep annotation, or maybe
> [ 4.468487] you didn't initialize this object before use?
> [ 4.470486] turning off the locking correctness validator.
> [ 4.472494] CPU: 2 UID: 0 PID: 177 Comm: liveupdate-iomm Not tainted 7.0.0+ #9 PREEMPT(full)
> [ 4.472497] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
> [ 4.472498] Call Trace:
> [ 4.472501] <TASK>
> [ 4.472503] dump_stack_lvl+0x5d/0x80
> [ 4.472508] register_lock_class+0x77e/0x790
> [ 4.472512] ? check_prev_add+0xf4/0xd30
> [ 4.472513] ? check_prev_add+0xf4/0xd30
> [ 4.472515] __lock_acquire+0x3b4/0x1b80
> [ 4.472518] ? raw_irqentry_exit_cond_resched+0x20/0x50
> [ 4.472556] lock_acquire+0xbf/0x2e0
> [ 4.472558] ? pci_match_device+0x24/0x180
> [ 4.472563] _raw_spin_lock+0x30/0x40
> [ 4.472567] ? pci_match_device+0x24/0x180
> [ 4.472569] pci_match_device+0x24/0x180
> [ 4.472571] new_id_store+0x189/0x1d0
> [ 4.472574] kernfs_fop_write_iter+0x14f/0x210
> [ 4.472578] vfs_write+0x263/0x5e0
> [ 4.472583] ksys_write+0x79/0xf0
> [ 4.472585] do_syscall_64+0x117/0xf80
> [ 4.472587] ? clear_bhb_loop+0x40/0x90
> [ 4.472590] entry_SYSCALL_64_after_hwframe+0x77/0x7f
> [ 4.472592] RIP: 0033:0x7fe7bc0beeb2
> [ 4.472595] Code: 18 41 8b 93 08 03 00 00 59 5e 48 83 f8 fc 75 1a 83 e2 39 83 fa 08 75 12 e8 2b ff ff ff 0f 1f 00 49 89 ca 48 8b 44 24 20 0f 05 <48> 83 c4 18 c3 66 0f 1f 84 00 00 00 00 00 48 83 ec 10 ff 74 24 18
> [ 4.472596] RSP: 002b:00007ffcdbb1ed80 EFLAGS: 00000202 ORIG_RAX: 0000000000000001
> [ 4.472598] RAX: ffffffffffffffda RBX: 00007fe7bc2415c0 RCX: 00007fe7bc0beeb2
> [ 4.472599] RDX: 000000000000000a RSI: 00005580955433c0 RDI: 0000000000000001
> [ 4.472600] RBP: 000000000000000a R08: 0000000000000000 R09: 0000000000000000
> [ 4.472601] R10: 0000000000000000 R11: 0000000000000202 R12: 000000000000000a
> [ 4.472602] R13: 00005580955433c0 R14: 000055805932493e R15: 0000558095545490
> [ 4.472605] </TASK>
>
> Fixes: 10a4206a2401 ("PCI: use generic driver_override infrastructure")
> Fixes: 8895d3bcb8ba ("PCI: Fail new_id for vendor/device values already built into driver")
> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
>
> ---
>
> v2:
> - Use device_initialize instead of spinlock init
> - Set release cb and call put_device to release
> - Add Fixes tag for the commit that added temporary pci device without
> initialize.
Reviewed-by: Danilo Krummrich <dakr@kernel.org>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] PCI: Init temporary pci device in new_id_store()
2026-05-05 23:43 [PATCH v2] PCI: Init temporary pci device in new_id_store() Samiullah Khawaja
2026-05-06 0:01 ` Danilo Krummrich
@ 2026-05-06 22:01 ` Bjorn Helgaas
1 sibling, 0 replies; 3+ messages in thread
From: Bjorn Helgaas @ 2026-05-06 22:01 UTC (permalink / raw)
To: Samiullah Khawaja
Cc: Bjorn Helgaas, Danilo Krummrich, Bandan Das, Gui-Dong Han,
Greg Kroah-Hartman, Alex Williamson, linux-pci, linux-kernel
On Tue, May 05, 2026 at 11:43:27PM +0000, Samiullah Khawaja wrote:
> When setting new_id of a PCI device driver using sysfs a lockdep splat
> occurs. This is because the new_id function new_id_store() checks for
> driver_override by creating temporary pci_dev structs.
>
> Since the newly added driver_override spinlock is not init for temporary
> pci_dev structs, the lockdep splat complains about it.
>
> Initialize the temporary pci device to fix this.
>
> [ 4.464296] INFO: trying to register non-static key.
> [ 4.466207] The code is fine but needs lockdep annotation, or maybe
> [ 4.468487] you didn't initialize this object before use?
> [ 4.470486] turning off the locking correctness validator.
> [ 4.472494] CPU: 2 UID: 0 PID: 177 Comm: liveupdate-iomm Not tainted 7.0.0+ #9 PREEMPT(full)
> [ 4.472497] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.16.3-0-ga6ed6b701f0a-prebuilt.qemu.org 04/01/2014
> [ 4.472498] Call Trace:
> [ 4.472501] <TASK>
> [ 4.472503] dump_stack_lvl+0x5d/0x80
> [ 4.472508] register_lock_class+0x77e/0x790
> [ 4.472512] ? check_prev_add+0xf4/0xd30
> [ 4.472513] ? check_prev_add+0xf4/0xd30
> [ 4.472515] __lock_acquire+0x3b4/0x1b80
> [ 4.472518] ? raw_irqentry_exit_cond_resched+0x20/0x50
> [ 4.472556] lock_acquire+0xbf/0x2e0
> [ 4.472558] ? pci_match_device+0x24/0x180
> [ 4.472563] _raw_spin_lock+0x30/0x40
> [ 4.472567] ? pci_match_device+0x24/0x180
> [ 4.472569] pci_match_device+0x24/0x180
> [ 4.472571] new_id_store+0x189/0x1d0
> [ 4.472574] kernfs_fop_write_iter+0x14f/0x210
> [ 4.472578] vfs_write+0x263/0x5e0
> [ 4.472583] ksys_write+0x79/0xf0
> [ 4.472585] do_syscall_64+0x117/0xf80
> [ 4.472587] ? clear_bhb_loop+0x40/0x90
> [ 4.472590] entry_SYSCALL_64_after_hwframe+0x77/0x7f
> [ 4.472592] RIP: 0033:0x7fe7bc0beeb2
> [ 4.472595] Code: 18 41 8b 93 08 03 00 00 59 5e 48 83 f8 fc 75 1a 83 e2 39 83 fa 08 75 12 e8 2b ff ff ff 0f 1f 00 49 89 ca 48 8b 44 24 20 0f 05 <48> 83 c4 18 c3 66 0f 1f 84 00 00 00 00 00 48 83 ec 10 ff 74 24 18
> [ 4.472596] RSP: 002b:00007ffcdbb1ed80 EFLAGS: 00000202 ORIG_RAX: 0000000000000001
> [ 4.472598] RAX: ffffffffffffffda RBX: 00007fe7bc2415c0 RCX: 00007fe7bc0beeb2
> [ 4.472599] RDX: 000000000000000a RSI: 00005580955433c0 RDI: 0000000000000001
> [ 4.472600] RBP: 000000000000000a R08: 0000000000000000 R09: 0000000000000000
> [ 4.472601] R10: 0000000000000000 R11: 0000000000000202 R12: 000000000000000a
> [ 4.472602] R13: 00005580955433c0 R14: 000055805932493e R15: 0000558095545490
> [ 4.472605] </TASK>
>
> Fixes: 10a4206a2401 ("PCI: use generic driver_override infrastructure")
> Fixes: 8895d3bcb8ba ("PCI: Fail new_id for vendor/device values already built into driver")
> Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
>
> ---
>
> v2:
> - Use device_initialize instead of spinlock init
> - Set release cb and call put_device to release
> - Add Fixes tag for the commit that added temporary pci device without
> initialize.
> ---
> drivers/pci/pci-driver.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index d10ece0889f0..e3f59001785a 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -179,6 +179,11 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
> return NULL;
> }
>
> +static void _pci_free_device(struct device *dev)
> +{
> + kfree(to_pci_dev(dev));
> +}
> +
> /**
> * new_id_store - sysfs frontend to pci_add_dynid()
> * @driver: target device driver
> @@ -214,11 +219,13 @@ static ssize_t new_id_store(struct device_driver *driver, const char *buf,
> pdev->subsystem_vendor = subvendor;
> pdev->subsystem_device = subdevice;
> pdev->class = class;
> + pdev->dev.release = _pci_free_device;
>
> + device_initialize(&pdev->dev);
> if (pci_match_device(pdrv, pdev))
> retval = -EEXIST;
>
> - kfree(pdev);
> + put_device(&pdev->dev);
The commit log says new_id_store() checks for driver_override, which
is technically true because it uses pci_match_device(), which calls
device_match_driver_override(). But I don't think it's relevant in
this path because dev->driver_override.name is not set in the
temporary device.
I wish we had a lower-level function to match a simple struct
pci_device_id against the driver dynids list. Then we could use that
here instead of faking up this temporary pci_dev and using
pci_match_device(), and we wouldn't need to mess with
device_initialize() and put_device().
And maybe pci_match_device() could be built on top of it for the probe
and bus match paths that already have a struct pci_dev for a real
device.
But I don't know whether that would all be feasible, and it would be
far out of scope for this patch anyway.
Applied this on pci/for-linus for v7.1, since 10a4206a2401 appeared in
v7.1-rc1.
> if (retval)
> return retval;
>
> base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
> --
> 2.54.0.545.g6539524ca2-goog
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-05-06 22:01 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-05 23:43 [PATCH v2] PCI: Init temporary pci device in new_id_store() Samiullah Khawaja
2026-05-06 0:01 ` Danilo Krummrich
2026-05-06 22:01 ` Bjorn Helgaas
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox