accel/amdxdna: annotate PF/VF dev_lock nesting for lockdep - #1532
accel/amdxdna: annotate PF/VF dev_lock nesting for lockdep#1532NishadSaraf wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses a lockdep false-positive in the amdxdna PCI driver when SR-IOV is enabled: the PF holds its dev_lock while pci_enable_sriov() synchronously probes VFs, and each VF probe takes its own dev_lock. Since both locks share a lock class, lockdep can misreport this valid PF→VF nesting as recursive locking; the change annotates the VF lock as a deeper lockdep subclass.
Changes:
- Define a dedicated lockdep subclass value for VF
dev_lock. - In
amdxdna_probe(), detect VF devices and setdev_lockto the VF lockdep subclass to reflect valid PF→VF nesting.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| drivers/accel/amdxdna/amdxdna_pci_drv.h | Adds a VF lockdep subclass constant. |
| drivers/accel/amdxdna/amdxdna_pci_drv.c | Applies the lockdep subclass to VF dev_lock during probe. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
pci_enable_sriov() is called from amdxdna_sriov_configure() with the PF's dev_lock held and synchronously probes each VF, whose amdxdna_probe() takes that VF's own dev_lock. Both are initialized from the same drmm_mutex_init() call site, so they share one lock class and lockdep misreports the strictly hierarchical PF -> VF acquisition as "possible recursive locking". The PF and VF dev_locks are distinct instances and a VF never takes the PF's dev_lock, so there is no real deadlock. Place the VF instance in a deeper lockdep subclass to express the valid nesting. Signed-off-by: Nishad Saraf <nishad.saraf@amd.com>
xdavidz
left a comment
xdavidz
left a comment
There was a problem hiding this comment.
I don't understand what issue is fixed by this?
Can you please elaborate?
@houlz0507 do you have any thoughts on this? Basically it makes one of the locking checking tool happy, but it is not a real bug. |
pci_enable_sriov() is called from amdxdna_sriov_configure() with the PF's dev_lock held and synchronously probes each VF, whose amdxdna_probe() takes that VF's own dev_lock. Both are initialized from the same drmm_mutex_init() call site, so they share one lock class and lockdep misreports the strictly hierarchical PF -> VF acquisition as "possible recursive locking".
The PF and VF dev_locks are distinct instances and a VF never takes the PF's dev_lock, so there is no real deadlock. Place the VF instance in a deeper lockdep subclass to express the valid nesting.