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

FAN YE <[email protected]>
Newsgroups org.kernel.feeds.b4-sent,org.kernel.vger.linux-kernel,org.kernel.vger.linux-pci
Message-ID <[email protected]>
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: [email protected]
Assisted-by: Claude:claude-opus-5
Signed-off-by: FAN YE <[email protected]>
---
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 <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.