[04/15] vga_switcheroo: Add missing locking

Commit Message

Lukas Wunner Aug. 23, 2015, 9:23 p.m. UTC

The following functions iterate over the client list, invoke client
callbacks or invoke handler callbacks without locking anything at all:

- Introduced by c8e9cf7bb240 ("vga_switcheroo: Add a helper function to
  get the client state"):
  vga_switcheroo_get_client_state()

- Introduced by 0d69704ae348 ("gpu/vga_switcheroo: add driver control
  power feature. (v3)"):
  vga_switcheroo_set_dynamic_switch()
  vga_switcheroo_runtime_suspend()
  vga_switcheroo_runtime_resume()
  vga_switcheroo_runtime_resume_hdmi_audio()

Refactor vga_switcheroo_runtime_resume_hdmi_audio() a bit to be able to
release vgasr_mutex immediately after iterating over the client list.

Signed-off-by: Lukas Wunner <[e-mail]>
---
 drivers/gpu/vga/vga_switcheroo.c | 50 +++++++++++++++++++++++++---------------
 1 file changed, 32 insertions(+), 18 deletions(-)

Patch

diff --git a/drivers/gpu/vga/vga_switcheroo.c b/drivers/gpu/vga/vga_switcheroo.c
index fe32536..2138162 100644
--- a/drivers/gpu/vga/vga_switcheroo.c
+++ b/drivers/gpu/vga/vga_switcheroo.c
@@ -349,13 +349,18 @@  find_active_client(struct list_head *head)
 int vga_switcheroo_get_client_state(struct pci_dev *pdev)
 {
 	struct vga_switcheroo_client *client;
+	enum vga_switcheroo_state ret;
 
+	mutex_lock(&vgasr_mutex);
 	client = find_client_from_pci(&vgasr_priv.clients, pdev);
 	if (!client)
-		return VGA_SWITCHEROO_NOT_FOUND;
-	if (!vgasr_priv.active)
-		return VGA_SWITCHEROO_INIT;
-	return client->pwr_state;
+		ret = VGA_SWITCHEROO_NOT_FOUND;
+	else if (!vgasr_priv.active)
+		ret = VGA_SWITCHEROO_INIT;
+	else
+		ret = client->pwr_state;
+	mutex_unlock(&vgasr_mutex);
+	return ret;
 }
 EXPORT_SYMBOL(vga_switcheroo_get_client_state);
 
@@ -847,15 +852,16 @@  void vga_switcheroo_set_dynamic_switch(struct pci_dev *pdev,
 {
 	struct vga_switcheroo_client *client;
 
+	mutex_lock(&vgasr_mutex);
 	client = find_client_from_pci(&vgasr_priv.clients, pdev);
-	if (!client)
-		return;
-
-	if (!client->driver_power_control)
+	if (!client || !client->driver_power_control) {
+		mutex_unlock(&vgasr_mutex);
 		return;
+	}
 
 	client->pwr_state = dynamic;
 	set_audio_state(client->id, dynamic);
+	mutex_unlock(&vgasr_mutex);
 }
 EXPORT_SYMBOL(vga_switcheroo_set_dynamic_switch);
 
@@ -868,9 +874,11 @@  static int vga_switcheroo_runtime_suspend(struct device *dev)
 	ret = dev->bus->pm->runtime_suspend(dev);
 	if (ret)
 		return ret;
+	mutex_lock(&vgasr_mutex);
 	if (vgasr_priv.handler->switchto)
 		vgasr_priv.handler->switchto(VGA_SWITCHEROO_IGD);
 	vga_switcheroo_power_switch(pdev, VGA_SWITCHEROO_OFF);
+	mutex_unlock(&vgasr_mutex);
 	return 0;
 }
 
@@ -879,7 +887,9 @@  static int vga_switcheroo_runtime_resume(struct device *dev)
 	struct pci_dev *pdev = to_pci_dev(dev);
 	int ret;
 
+	mutex_lock(&vgasr_mutex);
 	vga_switcheroo_power_switch(pdev, VGA_SWITCHEROO_ON);
+	mutex_unlock(&vgasr_mutex);
 	ret = dev->bus->pm->runtime_resume(dev);
 	if (ret)
 		return ret;
@@ -925,29 +935,33 @@  EXPORT_SYMBOL(vga_switcheroo_fini_domain_pm_ops);
 static int vga_switcheroo_runtime_resume_hdmi_audio(struct device *dev)
 {
 	struct pci_dev *pdev = to_pci_dev(dev);
+	struct vga_switcheroo_client *client;
+	struct device *video_dev = NULL;
 	int ret;
-	struct vga_switcheroo_client *client, *found = NULL;
 
 	/* we need to check if we have to switch back on the video
 	   device so the audio device can come back */
+	mutex_lock(&vgasr_mutex);
 	list_for_each_entry(client, &vgasr_priv.clients, list) {
 		if (PCI_SLOT(client->pdev->devfn) == PCI_SLOT(pdev->devfn) &&
 		    client_is_vga(client)) {
-			found = client;
-			ret = pm_runtime_get_sync(&client->pdev->dev);
-			if (ret) {
-				if (ret != 1)
-					return ret;
-			}
+			video_dev = &client->pdev->dev;
 			break;
 		}
 	}
+	mutex_unlock(&vgasr_mutex);
+
+	if (video_dev) {
+		ret = pm_runtime_get_sync(video_dev);
+		if (ret && ret != 1)
+			return ret;
+	}
 	ret = dev->bus->pm->runtime_resume(dev);
 
 	/* put the reference for the gpu */
-	if (found) {
-		pm_runtime_mark_last_busy(&found->pdev->dev);
-		pm_runtime_put_autosuspend(&found->pdev->dev);
+	if (video_dev) {
+		pm_runtime_mark_last_busy(video_dev);
+		pm_runtime_put_autosuspend(video_dev);
 	}
 	return ret;
 }