Am 07.07.26 um 16:22 schrieb [email protected]:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] An AB-BA lock inversion is introduced between `vgasr_mutex` and 
`dev->clientlist_mutex`, leading to a potential deadlock.
--

--- Patch [5]: [PATCH 05/10] drm/amdgpu: Implement struct 
vga_switcheroo_client_ops.pre_switch ---
commit 31dd42c0f56cf03f20324c5c0e3557fa630afd13
Author: Thomas Zimmermann <[email protected]>

drm/amdgpu: Implement struct vga_switcheroo_client_ops.pre_switch

This commit implements the pre_switch callback for vga_switcheroo_client_ops
in the amdgpu driver. It calls drm_client_dev_acquire_outputs() to push fbcon
updates from vga_switcheroo into DRM's fbdev emulation.

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 211d30f03d25f..6c80c6f4b07ca 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1698,10 +1698,18 @@ static bool amdgpu_switcheroo_can_switch(struct pci_dev 
*pdev)
        return atomic_read(&dev->open_count) == 0;
  }
+static void amdgpu_switcheroo_pre_switch(struct pci_dev *pdev)
+{
+       struct drm_device *dev = pci_get_drvdata(pdev);
+
+       drm_client_dev_acquire_outputs(dev);
[Severity: High]
Can this introduce an AB-BA lock inversion between vgasr_mutex and
dev->clientlist_mutex?

This callback is invoked by vga_switcheroo while vgasr_mutex is held, and
drm_client_dev_acquire_outputs() then acquires dev->clientlist_mutex.

Meanwhile, the hotplug path acquires clientlist_mutex first:

drm_client_dev_hotplug()
   mutex_lock(&dev->clientlist_mutex);

and then eventually takes console_lock and vgasr_mutex (via
vga_switcheroo_client_fb_set() called from register_framebuffer()) deeper in
the call stack.

Patch 10 removes calls to vga_switcheroo_client_fb_set() entirely. I could try to reorganize the series to avoid the vulnerable intermediate states.


If a hotplug event and a VGA switcheroo switch execute concurrently, could this
lead to a deadlock?

+}
+
  static const struct vga_switcheroo_client_ops amdgpu_switcheroo_ops = {
        .set_gpu_state = amdgpu_switcheroo_set_state,
        .reprobe = NULL,
        .can_switch = amdgpu_switcheroo_can_switch,
+       .pre_switch = amdgpu_switcheroo_pre_switch,
  };

--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, (HRB 36809, AG Nürnberg)


Reply via email to