kernel-janitors.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch] nouveau: unwind on load errors
@ 2010-07-29 18:02 Dan Carpenter
  2010-07-29 19:39 ` Marcin Slusarz
  0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2010-07-29 18:02 UTC (permalink / raw)
  To: David Airlie
  Cc: devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ben Skeggs,
	Dave Airlie, Francisco Jerez, Marcin Kościelnicki,
	Marcin Slusarz

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

Signed-off-by: Dan Carpenter <error27@gmail.com>

diff --git a/drivers/gpu/drm/nouveau/nouveau_state.c b/drivers/gpu/drm/nouveau/nouveau_state.c
index e632339..e53253a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_state.c
+++ b/drivers/gpu/drm/nouveau/nouveau_state.c
@@ -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)

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [patch] nouveau: unwind on load errors
  2010-07-29 18:02 [patch] nouveau: unwind on load errors Dan Carpenter
@ 2010-07-29 19:39 ` Marcin Slusarz
       [not found]   ` <20100729193948.GA4821-OI9uyE9O0yo@public.gmane.org>
  0 siblings, 1 reply; 5+ messages in thread
From: Marcin Slusarz @ 2010-07-29 19:39 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: nouveau, devicetree-discuss, kernel-janitors, dri-devel,
	Grant Likely, Ben Skeggs, Dave Airlie

On Thu, Jul 29, 2010 at 08:02:31PM +0200, Dan Carpenter wrote:
> nouveau_load() just returned directly if there was an error instead of
> releasing resources.
> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>

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

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

Marcin


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [patch v2] nouveau: unwind on load errors
       [not found]   ` <20100729193948.GA4821-OI9uyE9O0yo@public.gmane.org>
@ 2010-07-30 15:04     ` Dan Carpenter
  2010-07-30 20:20       ` Marcin Slusarz
  0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2010-07-30 15:04 UTC (permalink / raw)
  To: Marcin Slusarz
  Cc: David Airlie, nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	devicetree-discuss-uLR06cmDAlY/bJ5BZ2RsiQ,
	kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Ben Skeggs,
	Dave Airlie, Francisco Jerez, Marcin Kościelnicki

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

Signed-off-by: Dan Carpenter <error27@gmail.com>
---
V2: updated to the nouveau git tree.

diff --git a/drivers/gpu/drm/nouveau/nouveau_state.c b/drivers/gpu/drm/nouveau/nouveau_state.c
index ee3729e..cf16bfb 100644
--- a/drivers/gpu/drm/nouveau/nouveau_state.c
+++ b/drivers/gpu/drm/nouveau/nouveau_state.c
@@ -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)

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [patch v2] nouveau: unwind on load errors
  2010-07-30 15:04     ` [patch v2] " Dan Carpenter
@ 2010-07-30 20:20       ` Marcin Slusarz
  2010-08-05  0:27         ` Francisco Jerez
  0 siblings, 1 reply; 5+ messages in thread
From: Marcin Slusarz @ 2010-07-30 20:20 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: nouveau, devicetree-discuss, kernel-janitors, dri-devel,
	Grant Likely, Ben Skeggs, Dave Airlie

On Fri, Jul 30, 2010 at 05:04:32PM +0200, Dan Carpenter wrote:
> nuveau_load() just returned directly if there was an error instead of                                                       
  ^^^^^^ typo

> releasing resources.                                                                                                         
> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> ---
> V2: updated to the nouveau git tree.

Thanks.
Reviewed-by: Marcin Slusarz <marcin.slusarz@gmail.com>

> diff --git a/drivers/gpu/drm/nouveau/nouveau_state.c b/drivers/gpu/drm/nouveau/nouveau_state.c
> index ee3729e..cf16bfb 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_state.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_state.c
> @@ -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)

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [patch v2] nouveau: unwind on load errors
  2010-07-30 20:20       ` Marcin Slusarz
@ 2010-08-05  0:27         ` Francisco Jerez
  0 siblings, 0 replies; 5+ messages in thread
From: Francisco Jerez @ 2010-08-05  0:27 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: nouveau, devicetree-discuss, kernel-janitors, dri-devel,
	Grant Likely, Dave Airlie, Ben Skeggs


[-- Attachment #1.1: Type: text/plain, Size: 3900 bytes --]

Marcin Slusarz <marcin.slusarz@gmail.com> writes:

> On Fri, Jul 30, 2010 at 05:04:32PM +0200, Dan Carpenter wrote:
>> nuveau_load() just returned directly if there was an error instead of                                                       
>   ^^^^^^ typo
>
>> releasing resources.                                                                                                         
>> 
>> Signed-off-by: Dan Carpenter <error27@gmail.com>
>> ---
>> V2: updated to the nouveau git tree.
>
> Thanks.
> Reviewed-by: Marcin Slusarz <marcin.slusarz@gmail.com>

Thanks, pushed to the nouveau kernel tree.

>
>> diff --git a/drivers/gpu/drm/nouveau/nouveau_state.c b/drivers/gpu/drm/nouveau/nouveau_state.c
>> index ee3729e..cf16bfb 100644
>> --- a/drivers/gpu/drm/nouveau/nouveau_state.c
>> +++ b/drivers/gpu/drm/nouveau/nouveau_state.c
>> @@ -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)

[-- Attachment #2: Type: application/pgp-signature, Size: 229 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2010-08-05  0:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-29 18:02 [patch] nouveau: unwind on load errors Dan Carpenter
2010-07-29 19:39 ` Marcin Slusarz
     [not found]   ` <20100729193948.GA4821-OI9uyE9O0yo@public.gmane.org>
2010-07-30 15:04     ` [patch v2] " Dan Carpenter
2010-07-30 20:20       ` Marcin Slusarz
2010-08-05  0:27         ` Francisco Jerez

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).