[Qemu-devel] [PATCH] virtio-9p: Fix a memory leak

[Qemu-devel] [PATCH] virtio-9p: Fix a memory leak

[Pedro Scarapicchia Junior]

At v9fs_walk_complete(), the memory allocated at v9fs_walk() is not being
released leading system to crash due out of memory.

This patch releases structure V9fsWalkState after v9fs_walk is complete.

Signed-off-by: Pedro Scarapicchia Junior <pedro.scarapiccha@br.flextronics.com>
---
 hw/9pfs/virtio-9p.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/hw/9pfs/virtio-9p.c b/hw/9pfs/virtio-9p.c
index b5fc52b..b1da2b7 100644
--- a/hw/9pfs/virtio-9p.c
+++ b/hw/9pfs/virtio-9p.c
@@ -1490,6 +1490,8 @@ static void v9fs_walk_complete(V9fsState *s, V9fsWalkState *vs, int err)
         qemu_free(vs->wnames);
         qemu_free(vs->qids);
     }
+
+    qemu_free(vs);
 }
 
 static void v9fs_walk_marshal(V9fsWalkState *vs)
-- 
1.7.1

Re: [Qemu-devel] [PATCH] virtio-9p: Fix a memory leak

[Stefan Hajnoczi]

On Fri, May 6, 2011 at 8:05 PM, Pedro Scarapicchia Junior
<pedrinho.rep51@gmail.com> wrote:
> At v9fs_walk_complete(), the memory allocated at v9fs_walk() is not being
> released leading system to crash due out of memory.
>
> This patch releases structure V9fsWalkState after v9fs_walk is complete.
>
> Signed-off-by: Pedro Scarapicchia Junior <pedro.scarapiccha@br.flextronics.com>
> ---
>  hw/9pfs/virtio-9p.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)

Thanks for this patch.  I suggest CCing Venkateswararao Jujjuri (JV)
<jvrao@linux.vnet.ibm.com>, the virtio-9p maintainer (see MAINTAINERS
file), on future patches so he can pick them up quickly.

Stefan

Re: [Qemu-devel] [PATCH] virtio-9p: Fix a memory leak

[Stefan Weil]

Am 07.05.2011 10:34, schrieb Stefan Hajnoczi:
> On Fri, May 6, 2011 at 8:05 PM, Pedro Scarapicchia Junior
> <pedrinho.rep51@gmail.com> wrote:
>> At v9fs_walk_complete(), the memory allocated at v9fs_walk() is not being
>> released leading system to crash due out of memory.
>>
>> This patch releases structure V9fsWalkState after v9fs_walk is complete.
>>
>> Signed-off-by: Pedro Scarapicchia Junior 
>> <pedro.scarapiccha@br.flextronics.com>
>> ---
>>  hw/9pfs/virtio-9p.c |    2 ++
>>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> Thanks for this patch. I suggest CCing Venkateswararao Jujjuri (JV)
> <jvrao@linux.vnet.ibm.com>, the virtio-9p maintainer (see MAINTAINERS
> file), on future patches so he can pick them up quickly.
>
> Stefan

Releasing the memory in v9fs_walk() were it was allocated
would be cleaner and easier to review. Is this not possible?

Stefan W.

Re: [Qemu-devel] [PATCH] virtio-9p: Fix a memory leak

[Pedro Scarapicchia Junior]

[-- Attachment #1: Type: text/plain, Size: 1384 bytes --]

Hi Stefan,

Thanks for the comment.

I believe that it is possible to release the memory at v9fs_walk. However
v9fs_walk_complete() is called from two another
functions: v9fs_walk_post_newfid_lstat()  and  v9fs_walk_post_oldfid_lstat().
Placing qemu_free at end of v9fs_walk_complete() solve memory leak in both
cases.

Venkateswararao, what's your opinion?

Best regards,

Pedro

On Sat, May 7, 2011 at 5:56 AM, Stefan Weil <weil@mail.berlios.de> wrote:

> Am 07.05.2011 10:34, schrieb Stefan Hajnoczi:
>
>  On Fri, May 6, 2011 at 8:05 PM, Pedro Scarapicchia Junior
>> <pedrinho.rep51@gmail.com> wrote:
>>
>>> At v9fs_walk_complete(), the memory allocated at v9fs_walk() is not being
>>> released leading system to crash due out of memory.
>>>
>>> This patch releases structure V9fsWalkState after v9fs_walk is complete.
>>>
>>> Signed-off-by: Pedro Scarapicchia Junior <
>>> pedro.scarapiccha@br.flextronics.com>
>>> ---
>>>  hw/9pfs/virtio-9p.c |    2 ++
>>>  1 files changed, 2 insertions(+), 0 deletions(-)
>>>
>>
>> Thanks for this patch. I suggest CCing Venkateswararao Jujjuri (JV)
>> <jvrao@linux.vnet.ibm.com>, the virtio-9p maintainer (see MAINTAINERS
>> file), on future patches so he can pick them up quickly.
>>
>> Stefan
>>
>
> Releasing the memory in v9fs_walk() were it was allocated
> would be cleaner and easier to review. Is this not possible?
>
> Stefan W.
>
>
>

[-- Attachment #2: Type: text/html, Size: 2348 bytes --]

Re: [Qemu-devel] [PATCH] virtio-9p: Fix a memory leak

[Venkateswararao Jujjuri]

[-- Attachment #1: Type: text/plain, Size: 2163 bytes --]

On 05/07/2011 01:27 PM, Pedro Scarapicchia Junior wrote:
> Hi Stefan,
>
> Thanks for the comment.
>
> I believe that it is possible to release the memory at v9fs_walk. 
> However v9fs_walk_complete() is called from two another 
> functions: v9fs_walk_post_newfid_lstat() 
>  and  v9fs_walk_post_oldfid_lstat(). Placing qemu_free at end 
> of v9fs_walk_complete() solve memory leak in both cases.
>
> Venkateswararao, what's your opinion?
I agree with Pedro. Given the state machine model the v9fs_walk() 
returns from multiple places.
We can take this patch for now. A new patch set is getting brewed 
converting this whole method with
coroutines + glib thread pools. That will replace all these *post* 
functions with more of a
readable format. Again thanks for the patch.

- JV
>
> Best regards,
>
> Pedro
>
> On Sat, May 7, 2011 at 5:56 AM, Stefan Weil <weil@mail.berlios.de 
> <mailto:weil@mail.berlios.de>> wrote:
>
>     Am 07.05.2011 10:34, schrieb Stefan Hajnoczi:
>
>         On Fri, May 6, 2011 at 8:05 PM, Pedro Scarapicchia Junior
>         <pedrinho.rep51@gmail.com <mailto:pedrinho.rep51@gmail.com>>
>         wrote:
>
>             At v9fs_walk_complete(), the memory allocated at
>             v9fs_walk() is not being
>             released leading system to crash due out of memory.
>
>             This patch releases structure V9fsWalkState after
>             v9fs_walk is complete.
>
>             Signed-off-by: Pedro Scarapicchia Junior
>             <pedro.scarapiccha@br.flextronics.com
>             <mailto:pedro.scarapiccha@br.flextronics.com>>
>             ---
>              hw/9pfs/virtio-9p.c |    2 ++
>              1 files changed, 2 insertions(+), 0 deletions(-)
>
>
>         Thanks for this patch. I suggest CCing Venkateswararao Jujjuri
>         (JV)
>         <jvrao@linux.vnet.ibm.com <mailto:jvrao@linux.vnet.ibm.com>>,
>         the virtio-9p maintainer (see MAINTAINERS
>         file), on future patches so he can pick them up quickly.
>
>         Stefan
>
>
>     Releasing the memory in v9fs_walk() were it was allocated
>     would be cleaner and easier to review. Is this not possible?
>
>     Stefan W.
>
>
>


[-- Attachment #2: Type: text/html, Size: 4654 bytes --]