From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Chentao(Boby)" Subject: Re: A memory leak problem in xen-blkback module Date: Mon, 15 Sep 2014 20:54:50 +0800 Message-ID: <5416E19A.4080209@huawei.com> References: <5412999C.6010600@huawei.com> <5416B78F.50208@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="windows-1252" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1XTVoZ-0008BB-TZ for xen-devel@lists.xenproject.org; Mon, 15 Sep 2014 12:55:32 +0000 In-Reply-To: <5416B78F.50208@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: =?windows-1252?Q?Roger_Pau_Monn=E9?= , konrad.wilk@oracle.com Cc: liujinlj.liu@huawei.com, xen-devel@lists.xenproject.org, Yanqiangjun , zhangmin , wu.wubin@huawei.com List-Id: xen-devel@lists.xenproject.org On 2014/9/15 17:55, Roger Pau Monn=E9 wrote: > 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 r= elease, and the newest 3.17-rc4 also has the same problem. The problem will= occur in below condition. >> >> In xen_blkbk_map function, first get_free_page from balloon or the l= ist of blkif free pages, then map this page. If get_free_page succeed, but = map failed, >> the grant handle corresponding to this page will be assigned to BLKBACK_= INVALID_HANDLE. Because map failed, it will execute xen_blkbk_unmap to retr= ieve resources. >> But in xen_blkbk_unmap function, if the grant handle of a page is BLKBAC= K_INVALID_HANDLE, it will continue to next loop to execute unmap and put_fr= ee_pages. >> Only executes put_free_pages, these pages will be returned to the list o= f blkif free pages and at last be returned to balloon. >> >> Make a summary, in the condition of get_free_page succeed but map fa= iled, the page will be leaked from balloon or the list of blkif free pages.= I have a immature thought, >> in xen_blkbk_unmap funtion, when judge the grant handle of a page is BLK= BACK_INVALID_HANDLE, can we execute put_free_pages to retrieve this one pag= e? >> >> 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. > = Thanks for pointing out my code mistake. And I'm very appreciated for your = reply. > 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. > = I think the funtion is xen_blkbk_unmap, not xen_blkbk_map_unmap. Am I right? > --- > 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. > = > diff --git a/drivers/block/xen-blkback/blkback.c b/drivers/block/xen-blkb= ack/blkback.c > index 64c60ed..63fc7f0 100644 > --- a/drivers/block/xen-blkback/blkback.c > +++ b/drivers/block/xen-blkback/blkback.c > @@ -763,6 +763,7 @@ again: > BUG_ON(new_map_idx >=3D segs_to_map); > if (unlikely(map[new_map_idx].status !=3D 0)) { > pr_debug(DRV_PFX "invalid buffer -- could not remap it\n"); > + put_free_pages(blkif, &pages[seg_idx]->page, 1); > pages[seg_idx]->handle =3D BLKBACK_INVALID_HANDLE; > ret |=3D 1; > goto next; > = > = > = > . >