Skip to contentSkip to conversation

nouveau: unwind on load errors

PATCH Superseded Dan Carpenter 3 participants 5 messages Jul 29, 2010 – Aug 5, 2010 1 message not in the archive

Subsystems (4)
DRM drivers DRM misc Kernel janitors nouveau
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 nouveau: unwind on load errors

nouveau_load() just returned directly if there was an error instead of releasing resources.

Signed-off-by Dan Carpenter <[email protected]>
drivers/gpu/drm/nouveau/nouveau_state.c +31 -11
index e632339..e53253a 100644
@@ -688,10 +688,13 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 	struct drm_nouveau_private *dev_priv;
 	uint32_t reg0;
 	resource_size_t mmio_start_offs;
+	int ret;
 
 	dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL);
-	if (!dev_priv)
-		return -ENOMEM;
+	if (!dev_priv) {
+		ret = -ENOMEM;
+		goto err_out;
+	}
 	dev->dev_private = dev_priv;
 	dev_priv->dev = dev;
 

@@ -702,8 +705,10 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 		 dev->pci_vendor, dev->pci_device, dev->pdev->class);
 
 	dev_priv->wq = create_workqueue("nouveau");
-	if (!dev_priv->wq)
-		return -EINVAL;
+	if (!dev_priv->wq) {
+		ret = -EINVAL;
+		goto err_priv;
+	}
 
 	/* resource 0 is mmio regs */
 	/* resource 1 is linear FB */

@@ -716,7 +721,8 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 	if (!dev_priv->mmio) {
 		NV_ERROR(dev, "Unable to initialize the mmio mapping. "
 			 "Please report your setup to " DRIVER_EMAIL "\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_wq;
 	}
 	NV_DEBUG(dev, "regs mapped ok at 0x%llx\n",
 					(unsigned long long)mmio_start_offs);

@@ -764,16 +770,17 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 		break;
 	default:
 		NV_INFO(dev, "Unsupported chipset 0x%08x\n", reg0);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_mmio;
 	}
 
 	NV_INFO(dev, "Detected an NV%2x generation card (0x%08x)\n",
 		dev_priv->card_type, reg0);
 
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
-		int ret = nouveau_remove_conflicting_drivers(dev);
+		ret = nouveau_remove_conflicting_drivers(dev);
 		if (ret)
-			return ret;
+			goto err_mmio;
 	}
 
 	/* map larger RAMIN aperture on NV40 cards */

@@ -801,7 +808,8 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 							dev_priv->ramin_size);
 		if (!dev_priv->ramin) {
 			NV_ERROR(dev, "Failed to map BAR0 PRAMIN.\n");
-			return -ENOMEM;
+			ret = -ENOMEM;
+			goto err_mmio;
 		}
 	}
 

@@ -815,12 +823,24 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 
 	/* For kernel modesetting, init card now and bring up fbcon */
 	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
-		int ret = nouveau_card_init(dev);
+		ret = nouveau_card_init(dev);
 		if (ret)
-			return ret;
+			goto err_ramin;
 	}
 
 	return 0;
+
+err_ramin:
+	iounmap(dev_priv->ramin);
+err_mmio:
+	iounmap(dev_priv->mmio);
+err_wq:
+	destroy_workqueue(dev_priv->wq);
+err_priv:
+	kfree(dev_priv);
+	dev->dev_private = NULL;
+err_out:
+	return ret;
 }
 
 static void nouveau_close(struct drm_device *dev)
#1 lore raw
Quoted 4 lines · Dan Carpenter

nouveau_load() just returned directly if there was an error instead of releasing resources.

Signed-off-by Dan Carpenter <[email protected]>

Patch looks good, but could you base it on top of nouveau tree?

git://anongit.freedesktop.org/nouveau/linux-2.6

Marcin

A message that is not in the archive (sent off-list or not yet received): [email protected]

author
#3 lore raw

PATCH v2 1/1 nouveau: unwind on load errors

nuveau_load() just returned directly if there was an error instead of releasing resources.

Signed-off-by Dan Carpenter <[email protected]>

---
V2: updated to the nouveau git tree.

drivers/gpu/drm/nouveau/nouveau_state.c +30 -10
index ee3729e..cf16bfb 100644
@@ -739,8 +739,10 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 	int ret;
 
 	dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL);
-	if (!dev_priv)
-		return -ENOMEM;
+	if (!dev_priv) {
+		ret = -ENOMEM;
+		goto err_out;
+	}
 	dev->dev_private = dev_priv;
 	dev_priv->dev = dev;
 

@@ -750,8 +752,10 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 		 dev->pci_vendor, dev->pci_device, dev->pdev->class);
 
 	dev_priv->wq = create_workqueue("nouveau");
-	if (!dev_priv->wq)
-		return -EINVAL;
+	if (!dev_priv->wq) {
+		ret = -EINVAL;
+		goto err_priv;
+	}
 
 	/* resource 0 is mmio regs */
 	/* resource 1 is linear FB */

@@ -764,7 +768,8 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 	if (!dev_priv->mmio) {
 		NV_ERROR(dev, "Unable to initialize the mmio mapping. "
 			 "Please report your setup to " DRIVER_EMAIL "\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_wq;
 	}
 	NV_DEBUG(dev, "regs mapped ok at 0x%llx\n",
 					(unsigned long long)mmio_start_offs);

@@ -812,7 +817,8 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 		break;
 	default:
 		NV_INFO(dev, "Unsupported chipset 0x%08x\n", reg0);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_mmio;
 	}
 
 	NV_INFO(dev, "Detected an NV%2x generation card (0x%08x)\n",

@@ -820,7 +826,7 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 
 	ret = nouveau_remove_conflicting_drivers(dev);
 	if (ret)
-		return ret;
+		goto err_mmio;
 
 	/* Map PRAMIN BAR, or on older cards, the aperture withing BAR0 */
 	if (dev_priv->card_type >= NV_40) {

@@ -834,7 +840,8 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 				dev_priv->ramin_size);
 		if (!dev_priv->ramin) {
 			NV_ERROR(dev, "Failed to PRAMIN BAR");
-			return -ENOMEM;
+			ret = -ENOMEM;
+			goto err_mmio;
 		}
 	} else {
 		dev_priv->ramin_size = 1 * 1024 * 1024;

@@ -842,7 +849,8 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 					  dev_priv->ramin_size);
 		if (!dev_priv->ramin) {
 			NV_ERROR(dev, "Failed to map BAR0 PRAMIN.\n");
-			return -ENOMEM;
+			ret = -ENOMEM;
+			goto err_mmio;
 		}
 	}
 

@@ -857,9 +865,21 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 	/* For kernel modesetting, init card now and bring up fbcon */
 	ret = nouveau_card_init(dev);
 	if (ret)
-		return ret;
+		goto err_ramin;
 
 	return 0;
+
+err_ramin:
+	iounmap(dev_priv->ramin);
+err_mmio:
+	iounmap(dev_priv->mmio);
+err_wq:
+	destroy_workqueue(dev_priv->wq);
+err_priv:
+	kfree(dev_priv);
+	dev->dev_private = NULL;
+err_out:
+	return ret;
 }
 
 void nouveau_lastclose(struct drm_device *dev)
Reviewed-by on v2 1/1
#4 lore raw
Quoted 1 line · Dan Carpenter

nuveau_load() just returned directly if there was an error instead of

^^^^^^ typo

Quoted 5 lines

releasing resources.

Signed-off-by Dan Carpenter <[email protected]>

---
V2: updated to the nouveau git tree.

Thanks.

Reviewed-by Marcin Slusarz <[email protected]>
Quoted 102 lines
drivers/gpu/drm/nouveau/nouveau_state.c +30 -10
index ee3729e..cf16bfb 100644
@@ -739,8 +739,10 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 	int ret;
 
 	dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL);
-	if (!dev_priv)
-		return -ENOMEM;
+	if (!dev_priv) {
+		ret = -ENOMEM;
+		goto err_out;
+	}
 	dev->dev_private = dev_priv;
 	dev_priv->dev = dev;
 

@@ -750,8 +752,10 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 		 dev->pci_vendor, dev->pci_device, dev->pdev->class);
 
 	dev_priv->wq = create_workqueue("nouveau");
-	if (!dev_priv->wq)
-		return -EINVAL;
+	if (!dev_priv->wq) {
+		ret = -EINVAL;
+		goto err_priv;
+	}
 
 	/* resource 0 is mmio regs */
 	/* resource 1 is linear FB */

@@ -764,7 +768,8 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 	if (!dev_priv->mmio) {
 		NV_ERROR(dev, "Unable to initialize the mmio mapping. "
 			 "Please report your setup to " DRIVER_EMAIL "\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_wq;
 	}
 	NV_DEBUG(dev, "regs mapped ok at 0x%llx\n",
 					(unsigned long long)mmio_start_offs);

@@ -812,7 +817,8 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 		break;
 	default:
 		NV_INFO(dev, "Unsupported chipset 0x%08x\n", reg0);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_mmio;
 	}
 
 	NV_INFO(dev, "Detected an NV%2x generation card (0x%08x)\n",

@@ -820,7 +826,7 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 
 	ret = nouveau_remove_conflicting_drivers(dev);
 	if (ret)
-		return ret;
+		goto err_mmio;
 
 	/* Map PRAMIN BAR, or on older cards, the aperture withing BAR0 */
 	if (dev_priv->card_type >= NV_40) {

@@ -834,7 +840,8 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 				dev_priv->ramin_size);
 		if (!dev_priv->ramin) {
 			NV_ERROR(dev, "Failed to PRAMIN BAR");
-			return -ENOMEM;
+			ret = -ENOMEM;
+			goto err_mmio;
 		}
 	} else {
 		dev_priv->ramin_size = 1 * 1024 * 1024;

@@ -842,7 +849,8 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 					  dev_priv->ramin_size);
 		if (!dev_priv->ramin) {
 			NV_ERROR(dev, "Failed to map BAR0 PRAMIN.\n");
-			return -ENOMEM;
+			ret = -ENOMEM;
+			goto err_mmio;
 		}
 	}
 

@@ -857,9 +865,21 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 	/* For kernel modesetting, init card now and bring up fbcon */
 	ret = nouveau_card_init(dev);
 	if (ret)
-		return ret;
+		goto err_ramin;
 
 	return 0;
+
+err_ramin:
+	iounmap(dev_priv->ramin);
+err_mmio:
+	iounmap(dev_priv->mmio);
+err_wq:
+	destroy_workqueue(dev_priv->wq);
+err_priv:
+	kfree(dev_priv);
+	dev->dev_private = NULL;
+err_out:
+	return ret;
 }
 
 void nouveau_lastclose(struct drm_device *dev)
#5 lore raw
Quoted 4 lines · Marcin Slusarz
Quoted 1 line · Dan Carpenter

nuveau_load() just returned directly if there was an error instead of

^^^^^^ typo

Quoted 8 lines
Quoted 5 lines

releasing resources.

Signed-off-by Dan Carpenter <[email protected]>

---
V2: updated to the nouveau git tree.

Thanks.

Reviewed-by Marcin Slusarz <[email protected]>

Thanks, pushed to the nouveau kernel tree.

Quoted 102 lines
Quoted 102 lines
drivers/gpu/drm/nouveau/nouveau_state.c +30 -10
index ee3729e..cf16bfb 100644
@@ -739,8 +739,10 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 	int ret;
 
 	dev_priv = kzalloc(sizeof(*dev_priv), GFP_KERNEL);
-	if (!dev_priv)
-		return -ENOMEM;
+	if (!dev_priv) {
+		ret = -ENOMEM;
+		goto err_out;
+	}
 	dev->dev_private = dev_priv;
 	dev_priv->dev = dev;
 

@@ -750,8 +752,10 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 		 dev->pci_vendor, dev->pci_device, dev->pdev->class);
 
 	dev_priv->wq = create_workqueue("nouveau");
-	if (!dev_priv->wq)
-		return -EINVAL;
+	if (!dev_priv->wq) {
+		ret = -EINVAL;
+		goto err_priv;
+	}
 
 	/* resource 0 is mmio regs */
 	/* resource 1 is linear FB */

@@ -764,7 +768,8 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 	if (!dev_priv->mmio) {
 		NV_ERROR(dev, "Unable to initialize the mmio mapping. "
 			 "Please report your setup to " DRIVER_EMAIL "\n");
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_wq;
 	}
 	NV_DEBUG(dev, "regs mapped ok at 0x%llx\n",
 					(unsigned long long)mmio_start_offs);

@@ -812,7 +817,8 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 		break;
 	default:
 		NV_INFO(dev, "Unsupported chipset 0x%08x\n", reg0);
-		return -EINVAL;
+		ret = -EINVAL;
+		goto err_mmio;
 	}
 
 	NV_INFO(dev, "Detected an NV%2x generation card (0x%08x)\n",

@@ -820,7 +826,7 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 
 	ret = nouveau_remove_conflicting_drivers(dev);
 	if (ret)
-		return ret;
+		goto err_mmio;
 
 	/* Map PRAMIN BAR, or on older cards, the aperture withing BAR0 */
 	if (dev_priv->card_type >= NV_40) {

@@ -834,7 +840,8 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 				dev_priv->ramin_size);
 		if (!dev_priv->ramin) {
 			NV_ERROR(dev, "Failed to PRAMIN BAR");
-			return -ENOMEM;
+			ret = -ENOMEM;
+			goto err_mmio;
 		}
 	} else {
 		dev_priv->ramin_size = 1 * 1024 * 1024;

@@ -842,7 +849,8 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 					  dev_priv->ramin_size);
 		if (!dev_priv->ramin) {
 			NV_ERROR(dev, "Failed to map BAR0 PRAMIN.\n");
-			return -ENOMEM;
+			ret = -ENOMEM;
+			goto err_mmio;
 		}
 	}
 

@@ -857,9 +865,21 @@ int nouveau_load(struct drm_device *dev, unsigned long flags)
 	/* For kernel modesetting, init card now and bring up fbcon */
 	ret = nouveau_card_init(dev);
 	if (ret)
-		return ret;
+		goto err_ramin;
 
 	return 0;
+
+err_ramin:
+	iounmap(dev_priv->ramin);
+err_mmio:
+	iounmap(dev_priv->mmio);
+err_wq:
+	destroy_workqueue(dev_priv->wq);
+err_priv:
+	kfree(dev_priv);
+	dev->dev_private = NULL;
+err_out:
+	return ret;
 }
Quoted 1 line

drivers/gpu/drm/nouveau/nouveau_state.c int nouveau_load(struct drm_device *dev, unsigned long flags) · line 885

Quoted 1 line
drivers/gpu/drm/nouveau/nouveau_state.c +0 -0
 void nouveau_lastclose(struct drm_device *dev)

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel