From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:60606) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QJ7yc-0001ri-T9 for qemu-devel@nongnu.org; Sun, 08 May 2011 13:41:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QJ7yb-0006kU-BQ for qemu-devel@nongnu.org; Sun, 08 May 2011 13:41:06 -0400 Received: from e32.co.us.ibm.com ([32.97.110.150]:56334) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QJ7ya-0006kG-Uu for qemu-devel@nongnu.org; Sun, 08 May 2011 13:41:05 -0400 Received: from d03relay05.boulder.ibm.com (d03relay05.boulder.ibm.com [9.17.195.107]) by e32.co.us.ibm.com (8.14.4/8.13.1) with ESMTP id p48HTh7d003329 for ; Sun, 8 May 2011 11:29:43 -0600 Received: from d03av02.boulder.ibm.com (d03av02.boulder.ibm.com [9.17.195.168]) by d03relay05.boulder.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id p48Hexbq343396 for ; Sun, 8 May 2011 11:40:59 -0600 Received: from d03av02.boulder.ibm.com (loopback [127.0.0.1]) by d03av02.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id p48BeWsl006311 for ; Sun, 8 May 2011 05:40:32 -0600 Received: from oc6675851006.ibm.com (sig-9-65-198-18.mts.ibm.com [9.65.198.18]) by d03av02.boulder.ibm.com (8.14.4/8.13.1/NCO v10.0 AVin) with ESMTP id p48BeUdV006247 for ; Sun, 8 May 2011 05:40:30 -0600 Message-ID: <4DC6D5A8.5010802@linux.vnet.ibm.com> Date: Sun, 08 May 2011 10:40:56 -0700 From: Venkateswararao Jujjuri MIME-Version: 1.0 References: <1304708747-3692-1-git-send-email-pedro.scarapiccha@br.flextronics.com> <4DC50954.4060601@mail.berlios.de> In-Reply-To: Content-Type: multipart/alternative; boundary="------------070902010104040305080707" Subject: Re: [Qemu-devel] [PATCH] virtio-9p: Fix a memory leak List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org This is a multi-part message in MIME format. --------------070902010104040305080707 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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 > wrote: > > Am 07.05.2011 10:34, schrieb Stefan Hajnoczi: > > On Fri, May 6, 2011 at 8:05 PM, Pedro Scarapicchia Junior > > > 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 > > > --- > hw/9pfs/virtio-9p.c | 2 ++ > 1 files changed, 2 insertions(+), 0 deletions(-) > > > Thanks for this patch. I suggest CCing Venkateswararao Jujjuri > (JV) > >, > 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. > > > --------------070902010104040305080707 Content-Type: text/html; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit 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> 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.




--------------070902010104040305080707--