| Message ID | 1304708747-3692-1-git-send-email-pedro.scarapiccha@br.flextronics.com |
|---|---|
| State | New |
| Headers | show |
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
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.
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. > > >
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. > > >
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)
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(-)