[PATCH] PCI/VGA: Fix lost wakeup when waiting for a VGA resource

FAN YE via B4 Relay posted 1 patch 1 month, 1 week ago
drivers/pci/vgaarb.c | 29 +++++++++++++++++------------
1 file changed, 17 insertions(+), 12 deletions(-)
[PATCH] PCI/VGA: Fix lost wakeup when waiting for a VGA resource
Posted by FAN YE via B4 Relay 1 month, 1 week ago
From: FAN YE <fy15309206903@gmail.com>

A task waiting in vga_get() can sleep forever even though the conflicting
device has already released the resource.  Once __vga_tryget() reports a
conflict, vga_get() drops vga_lock and only then puts itself on
vga_wait_queue, while __vga_put() wakes that queue while holding vga_lock.
A wakeup landing in between finds the queue empty and is discarded, and as
the conflict is already gone no further wakeup is coming.  Callers passing
interruptible=0, such as the "lock" command of /dev/vga_arbiter, are then
unkillable and keep their lock counts forever.

Queue up before dropping vga_lock, the way prepare_to_wait() publishes a
waiter before the condition is re-tested.  The releasing side needs
vga_lock to reach the wakeup, so it can no longer pass an unqueued waiter.

Fixes: deb2d2ecd43d ("PCI/GPU: implement VGA arbitration on Linux")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5
Signed-off-by: FAN YE <fy15309206903@gmail.com>
---
Reproduced under QEMU on 818bebeb63dd with two VGA devices, the second one
behind a bridge that does not forward VGA so that it owns no legacy resources.
Two /dev/vga_arbiter clients each locking one device wedged the second client
in vga_get(), D state and unkillable, after 8538 rounds, while the conflicting
device already read back locks=none(0:0).  40000 rounds with this patch, none.
---
 drivers/pci/vgaarb.c | 29 +++++++++++++++++------------
 1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/pci/vgaarb.c b/drivers/pci/vgaarb.c
index 3de05aee78599..51c7d171c9558 100644
--- a/drivers/pci/vgaarb.c
+++ b/drivers/pci/vgaarb.c
@@ -459,6 +459,23 @@ int vga_get(struct pci_dev *pdev, unsigned int rsrc, int interruptible)
 			break;
 		}
 		conflict = __vga_tryget(vgadev, rsrc);
+		/*
+		 * We have a conflict; we wait until somebody kicks the
+		 * work queue. Currently we have one work queue that we
+		 * kick each time some resources are released, but it would
+		 * be fairly easy to have a per-device one so that we only
+		 * need to attach to the conflicting device.
+		 *
+		 * Queue up before dropping vga_lock: __vga_put() wakes the
+		 * queue while holding it, so a wakeup cannot slip past.
+		 */
+		if (!IS_ERR_OR_NULL(conflict)) {
+			init_waitqueue_entry(&wait, current);
+			add_wait_queue(&vga_wait_queue, &wait);
+			set_current_state(interruptible ?
+					  TASK_INTERRUPTIBLE :
+					  TASK_UNINTERRUPTIBLE);
+		}
 		spin_unlock_irqrestore(&vga_lock, flags);
 		if (IS_ERR(conflict)) {
 			rc = PTR_ERR(conflict);
@@ -467,18 +484,6 @@ int vga_get(struct pci_dev *pdev, unsigned int rsrc, int interruptible)
 		if (conflict == NULL)
 			break;
 
-		/*
-		 * We have a conflict; we wait until somebody kicks the
-		 * work queue. Currently we have one work queue that we
-		 * kick each time some resources are released, but it would
-		 * be fairly easy to have a per-device one so that we only
-		 * need to attach to the conflicting device.
-		 */
-		init_waitqueue_entry(&wait, current);
-		add_wait_queue(&vga_wait_queue, &wait);
-		set_current_state(interruptible ?
-				  TASK_INTERRUPTIBLE :
-				  TASK_UNINTERRUPTIBLE);
 		if (interruptible && signal_pending(current)) {
 			__set_current_state(TASK_RUNNING);
 			remove_wait_queue(&vga_wait_queue, &wait);

---
base-commit: 818bebeb63dd6bf5f4e07e145f6cdbace520a34c
change-id: 20260821-pci-vga-lost-wakeup-3b70658424e1

Best regards,
--  
FAN YE <fy15309206903@gmail.com>