Re: [PATCH 07/10] drm/nouveau: Implement vga_switcheroo_client_ops.pre_switch

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] New AB-BA deadlock introduced between vgasr_mutex and clientlist_mutex.

Pre-existing issues:
- [High] Preexisting AB-BA deadlock between vgasr_mutex and console_lock.
--

--- Patch [7]: [PATCH 07/10] drm/nouveau: Implement vga_switcheroo_client_ops.pre_switch ---
commit a0ca13a7165f71c977582a84acf58b2f2681a805
Author: Thomas Zimmermann <tzimmermann@suse.de>

drm/nouveau: Implement vga_switcheroo_client_ops.pre_switch

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

> diff --git a/drivers/gpu/drm/nouveau/nouveau_vga.c b/drivers/gpu/drm/nouveau/nouveau_vga.c
> index a6c375a241542..2d2d08be8fbe2 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_vga.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_vga.c
> @@ -76,11 +76,20 @@ nouveau_switcheroo_can_switch(struct pci_dev *pdev)
>  	return atomic_read(&drm->dev->open_count) == 0;
>  }
>  
> +static void
> +nouveau_switcheroo_pre_switch(struct pci_dev *pdev)
> +{
> +	struct nouveau_drm *drm = pci_get_drvdata(pdev);
> +
> +	drm_client_dev_acquire_outputs(drm->dev);
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
Does this introduce an AB-BA deadlock between vgasr_mutex and
clientlist_mutex?

When drm_client_dev_hotplug() is called, it acquires dev->clientlist_mutex
and eventually calls register_framebuffer(). This takes the console_lock()
and then calls vga_switcheroo_client_fb_set(), taking vgasr_mutex. This
establishes the lock order:
clientlist_mutex -> console_lock -> vgasr_mutex

However, vga_switcheroo_debugfs_write() acquires vgasr_mutex and calls
vga_switchto_stage2(), which invokes this new pre_switch callback.
Calling drm_client_dev_acquire_outputs() here attempts to acquire
dev->clientlist_mutex, establishing the reverse lock order:
vgasr_mutex -> clientlist_mutex

[Severity: High]
This is a pre-existing issue, but does this path also trigger a deadlock
between vgasr_mutex and console_lock?

When register_framebuffer() is called (e.g., during driver probe or
hotplug), it calls fbcon_fb_registered(), which acquires console_lock()
and then calls vga_switcheroo_client_fb_set(), taking vgasr_mutex.

Conversely, vga_switcheroo_debugfs_write() acquires vgasr_mutex and calls
vga_switchto_stage2(). This invokes pre_switch(), which leads to
drm_client_dev_acquire_outputs() and then fb_switch_outputs(), attempting
to acquire console_lock(). This creates an exact inversion:
vgasr_mutex -> console_lock

> +}
> +
>  static const struct vga_switcheroo_client_ops
>  nouveau_switcheroo_ops = {
>  	.set_gpu_state = nouveau_switcheroo_set_state,
>  	.reprobe = nouveau_switcheroo_reprobe,
>  	.can_switch = nouveau_switcheroo_can_switch,
> +	.pre_switch = nouveau_switcheroo_pre_switch,
>  };
>  
>  void

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260707135724.247562-1-tzimmermann@suse.de?part=7