All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/1] Drivers: hv: balloon: Fix a memory leak
@ 2012-12-11 19:07 K. Y. Srinivasan
  2012-12-13  5:21 ` Jason Wang
  2012-12-13  9:37 ` Dan Carpenter
  0 siblings, 2 replies; 4+ messages in thread
From: K. Y. Srinivasan @ 2012-12-11 19:07 UTC (permalink / raw)
  To: gregkh, linux-kernel, devel, olaf, apw, jasowang; +Cc: K. Y. Srinivasan

The send buffer was being leaked; fix it.

Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
Reported-by: Jason Wang <jasowang@redhat.com>
---
 drivers/hv/hv_balloon.c |   23 +++++++++++++----------
 1 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
index 00c8447..c02a33e 100644
--- a/drivers/hv/hv_balloon.c
+++ b/drivers/hv/hv_balloon.c
@@ -882,7 +882,7 @@ static int balloon_probe(struct hv_device *dev,
 			balloon_onchannelcallback, dev);
 
 	if (ret)
-		return ret;
+		goto probe_error0;
 
 	dm_device.dev = dev;
 	dm_device.state = DM_INITIALIZING;
@@ -894,7 +894,7 @@ static int balloon_probe(struct hv_device *dev,
 		 kthread_run(dm_thread_func, &dm_device, "hv_balloon");
 	if (IS_ERR(dm_device.thread)) {
 		ret = PTR_ERR(dm_device.thread);
-		goto probe_error0;
+		goto probe_error1;
 	}
 
 	hv_set_drvdata(dev, &dm_device);
@@ -917,12 +917,12 @@ static int balloon_probe(struct hv_device *dev,
 				VM_PKT_DATA_INBAND,
 				VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
 	if (ret)
-		goto probe_error1;
+		goto probe_error2;
 
 	t = wait_for_completion_timeout(&dm_device.host_event, 5*HZ);
 	if (t == 0) {
 		ret = -ETIMEDOUT;
-		goto probe_error1;
+		goto probe_error2;
 	}
 
 	/*
@@ -931,7 +931,7 @@ static int balloon_probe(struct hv_device *dev,
 	 */
 	if (dm_device.state == DM_INIT_ERROR) {
 		ret = -ETIMEDOUT;
-		goto probe_error1;
+		goto probe_error2;
 	}
 	/*
 	 * Now submit our capabilities to the host.
@@ -964,12 +964,12 @@ static int balloon_probe(struct hv_device *dev,
 				VM_PKT_DATA_INBAND,
 				VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
 	if (ret)
-		goto probe_error1;
+		goto probe_error2;
 
 	t = wait_for_completion_timeout(&dm_device.host_event, 5*HZ);
 	if (t == 0) {
 		ret = -ETIMEDOUT;
-		goto probe_error1;
+		goto probe_error2;
 	}
 
 	/*
@@ -978,18 +978,20 @@ static int balloon_probe(struct hv_device *dev,
 	 */
 	if (dm_device.state == DM_INIT_ERROR) {
 		ret = -ETIMEDOUT;
-		goto probe_error1;
+		goto probe_error2;
 	}
 
 	dm_device.state = DM_INITIALIZED;
 
 	return 0;
 
-probe_error1:
+probe_error2:
 	kthread_stop(dm_device.thread);
 
-probe_error0:
+probe_error1:
 	vmbus_close(dev->channel);
+probe_error0:
+	kfree(send_buffer);
 	return ret;
 }
 
@@ -1002,6 +1004,7 @@ static int balloon_remove(struct hv_device *dev)
 
 	vmbus_close(dev->channel);
 	kthread_stop(dm->thread);
+	kfree(send_buffer);
 
 	return 0;
 }
-- 
1.7.4.1


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

* Re: [PATCH 1/1] Drivers: hv: balloon: Fix a memory leak
  2012-12-11 19:07 [PATCH 1/1] Drivers: hv: balloon: Fix a memory leak K. Y. Srinivasan
@ 2012-12-13  5:21 ` Jason Wang
  2012-12-13  9:37 ` Dan Carpenter
  1 sibling, 0 replies; 4+ messages in thread
From: Jason Wang @ 2012-12-13  5:21 UTC (permalink / raw)
  To: K. Y. Srinivasan; +Cc: gregkh, linux-kernel, devel, olaf, apw

On 12/12/2012 03:07 AM, K. Y. Srinivasan wrote:
> The send buffer was being leaked; fix it.
>
> Signed-off-by: K. Y. Srinivasan <kys@microsoft.com>
> Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
> Reported-by: Jason Wang <jasowang@redhat.com>
> ---
>  drivers/hv/hv_balloon.c |   23 +++++++++++++----------
>  1 files changed, 13 insertions(+), 10 deletions(-)

Thanks.

Acked-by: Jason Wang <jasowang@redhat.com>
> diff --git a/drivers/hv/hv_balloon.c b/drivers/hv/hv_balloon.c
> index 00c8447..c02a33e 100644
> --- a/drivers/hv/hv_balloon.c
> +++ b/drivers/hv/hv_balloon.c
> @@ -882,7 +882,7 @@ static int balloon_probe(struct hv_device *dev,
>  			balloon_onchannelcallback, dev);
>  
>  	if (ret)
> -		return ret;
> +		goto probe_error0;
>  
>  	dm_device.dev = dev;
>  	dm_device.state = DM_INITIALIZING;
> @@ -894,7 +894,7 @@ static int balloon_probe(struct hv_device *dev,
>  		 kthread_run(dm_thread_func, &dm_device, "hv_balloon");
>  	if (IS_ERR(dm_device.thread)) {
>  		ret = PTR_ERR(dm_device.thread);
> -		goto probe_error0;
> +		goto probe_error1;
>  	}
>  
>  	hv_set_drvdata(dev, &dm_device);
> @@ -917,12 +917,12 @@ static int balloon_probe(struct hv_device *dev,
>  				VM_PKT_DATA_INBAND,
>  				VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
>  	if (ret)
> -		goto probe_error1;
> +		goto probe_error2;
>  
>  	t = wait_for_completion_timeout(&dm_device.host_event, 5*HZ);
>  	if (t == 0) {
>  		ret = -ETIMEDOUT;
> -		goto probe_error1;
> +		goto probe_error2;
>  	}
>  
>  	/*
> @@ -931,7 +931,7 @@ static int balloon_probe(struct hv_device *dev,
>  	 */
>  	if (dm_device.state == DM_INIT_ERROR) {
>  		ret = -ETIMEDOUT;
> -		goto probe_error1;
> +		goto probe_error2;
>  	}
>  	/*
>  	 * Now submit our capabilities to the host.
> @@ -964,12 +964,12 @@ static int balloon_probe(struct hv_device *dev,
>  				VM_PKT_DATA_INBAND,
>  				VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED);
>  	if (ret)
> -		goto probe_error1;
> +		goto probe_error2;
>  
>  	t = wait_for_completion_timeout(&dm_device.host_event, 5*HZ);
>  	if (t == 0) {
>  		ret = -ETIMEDOUT;
> -		goto probe_error1;
> +		goto probe_error2;
>  	}
>  
>  	/*
> @@ -978,18 +978,20 @@ static int balloon_probe(struct hv_device *dev,
>  	 */
>  	if (dm_device.state == DM_INIT_ERROR) {
>  		ret = -ETIMEDOUT;
> -		goto probe_error1;
> +		goto probe_error2;
>  	}
>  
>  	dm_device.state = DM_INITIALIZED;
>  
>  	return 0;
>  
> -probe_error1:
> +probe_error2:
>  	kthread_stop(dm_device.thread);
>  
> -probe_error0:
> +probe_error1:
>  	vmbus_close(dev->channel);
> +probe_error0:
> +	kfree(send_buffer);
>  	return ret;
>  }
>  
> @@ -1002,6 +1004,7 @@ static int balloon_remove(struct hv_device *dev)
>  
>  	vmbus_close(dev->channel);
>  	kthread_stop(dm->thread);
> +	kfree(send_buffer);
>  
>  	return 0;
>  }


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

* Re: [PATCH 1/1] Drivers: hv: balloon: Fix a memory leak
  2012-12-11 19:07 [PATCH 1/1] Drivers: hv: balloon: Fix a memory leak K. Y. Srinivasan
  2012-12-13  5:21 ` Jason Wang
@ 2012-12-13  9:37 ` Dan Carpenter
  2012-12-13 15:29   ` KY Srinivasan
  1 sibling, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2012-12-13  9:37 UTC (permalink / raw)
  To: K. Y. Srinivasan; +Cc: gregkh, linux-kernel, devel, olaf, apw, jasowang

On Tue, Dec 11, 2012 at 11:07:17AM -0800, K. Y. Srinivasan wrote:
> -probe_error1:
> +probe_error2:
>  	kthread_stop(dm_device.thread);
>  
> -probe_error0:
> +probe_error1:
>  	vmbus_close(dev->channel);
> +probe_error0:
> +	kfree(send_buffer);

If you used labels that describe what the code does instead of
GW-BASIC labels then you wouldn't need to rename them.

err_stop_thread:
	kthread_stop(dm_device.thread);
err_close_vmbus:
	vmbus_close(dev->channel);
err_free_send_buf:
	kfree(send_buffer);

regards,
dan carpenter


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

* RE: [PATCH 1/1] Drivers: hv: balloon: Fix a memory leak
  2012-12-13  9:37 ` Dan Carpenter
@ 2012-12-13 15:29   ` KY Srinivasan
  0 siblings, 0 replies; 4+ messages in thread
From: KY Srinivasan @ 2012-12-13 15:29 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: gregkh, linux-kernel, devel, olaf, apw, jasowang



> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> Sent: Thursday, December 13, 2012 4:38 AM
> To: KY Srinivasan
> Cc: gregkh@linuxfoundation.org; linux-kernel@vger.kernel.org;
> devel@linuxdriverproject.org; olaf@aepfle.de; apw@canonical.com;
> jasowang@redhat.com
> Subject: Re: [PATCH 1/1] Drivers: hv: balloon: Fix a memory leak
> 
> On Tue, Dec 11, 2012 at 11:07:17AM -0800, K. Y. Srinivasan wrote:
> > -probe_error1:
> > +probe_error2:
> >  	kthread_stop(dm_device.thread);
> >
> > -probe_error0:
> > +probe_error1:
> >  	vmbus_close(dev->channel);
> > +probe_error0:
> > +	kfree(send_buffer);
> 
> If you used labels that describe what the code does instead of
> GW-BASIC labels then you wouldn't need to rename them.
> 
> err_stop_thread:
> 	kthread_stop(dm_device.thread);
> err_close_vmbus:
> 	vmbus_close(dev->channel);
> err_free_send_buf:
> 	kfree(send_buffer);

Thanks Dan. I am not sure how this labeling scheme would work in general where each error
recovery block is a collection of rollback operation. I see your point though.

Regards,

K. Y 
 



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

end of thread, other threads:[~2012-12-13 15:33 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-12-11 19:07 [PATCH 1/1] Drivers: hv: balloon: Fix a memory leak K. Y. Srinivasan
2012-12-13  5:21 ` Jason Wang
2012-12-13  9:37 ` Dan Carpenter
2012-12-13 15:29   ` KY Srinivasan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.