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.

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,
>  };

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=5

Reply via email to