From mboxrd@z Thu Jan 1 00:00:00 1970 From: Konrad Rzeszutek Wilk Subject: Re: A memory leak problem in xen-blkback module Date: Wed, 1 Oct 2014 09:46:45 -0400 Message-ID: <20141001134645.GA11775@laptop.dumpdata.com> References: <5412999C.6010600@huawei.com> <5416B78F.50208@citrix.com> <542ACAE1.9080305@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XZKFD-0004zD-3v for xen-devel@lists.xenproject.org; Wed, 01 Oct 2014 13:47:03 +0000 Content-Disposition: inline In-Reply-To: <542ACAE1.9080305@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Roger Pau =?iso-8859-1?Q?Monn=E9?= Cc: xen-devel@lists.xenproject.org, David Vrabel , "Chentao(Boby)" List-Id: xen-devel@lists.xenproject.org On Tue, Sep 30, 2014 at 05:23:13PM +0200, Roger Pau Monn=E9 wrote: > El 15/09/14 a les 11.55, Roger Pau Monn=E9 ha escrit: > > El 12/09/14 a les 8.58, Chentao(Boby) ha escrit: > >> Hi Konrad, > >> > >> I find a memory leak problem in xen-blkback module of linux-3.14.4= release, and the newest 3.17-rc4 also has the same problem. The problem wi= ll occur in below condition. > >> > >> In xen_blkbk_map function, first get_free_page from balloon or the= list of blkif free pages, then map this page. If get_free_page succeed, bu= t map failed, > >> the grant handle corresponding to this page will be assigned to BLKBAC= K_INVALID_HANDLE. Because map failed, it will execute xen_blkbk_unmap to re= trieve resources. > >> But in xen_blkbk_unmap function, if the grant handle of a page is BLKB= ACK_INVALID_HANDLE, it will continue to next loop to execute unmap and put_= free_pages. > >> Only executes put_free_pages, these pages will be returned to the list= of blkif free pages and at last be returned to balloon. > >> > >> Make a summary, in the condition of get_free_page succeed but map = failed, the page will be leaked from balloon or the list of blkif free page= s. I have a immature thought, > >> in xen_blkbk_unmap funtion, when judge the grant handle of a page is B= LKBACK_INVALID_HANDLE, can we execute put_free_pages to retrieve this one p= age? > >> > >> Just like below: > >> if (pages[i]->handle =3D=3D BLKBACK_INVALID_HANDLE) { > >> put_free_pages(blkif, pages[i]->page, 1); > > = > > This is not correct, and will fail to compile AFAICT. put_free_pages = > > expects an array of pointers to page structs and you are passing a = > > pointer to a page struct. > > = > > I have the following patch, which I think solves the problem. I've = > > placed the free_pages call in xen_blkbk_map itself, but it could also = > > be done in xen_blkbk_map_unmap. > > = > > --- > > commit 879ebb502e2f72279553bb0a85cc885ec492a0c1 > > Author: Roger Pau Monne > > Date: Mon Sep 15 11:01:40 2014 +0200 > > = > > xen-blkback: fix leak on grant map error path > > = > > Fix leaking a page when a grant mapping has failed. > > = > > Signed-off-by: Roger Pau Monn=E9 > > Reported by: Tao Chen > > --- > > This patch should be backported to stable branches. > = > Ping? > = Reviewed. Testing it and once that is done will send it for Jens. Thanks!