Re: xc_map_foreign_bulk() memory leak in ARM version?

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Mon, May 20, 2013 at 11:55:48AM +0400, Nyashka Surovski wrote:
> Oh.. We have found out the root of the problem.
> In our version of code there was a mistake in condition inside
> privcmd_close().
> 
> It was:
>   if (!xen_feature(XENFEAT_auto_translated_physmap || !numpgs || !pages))
> 
> But the right one is:
>   if (!xen_feature(XENFEAT_auto_translated_physmap) || !numpgs || !pages)
> 
> Looks like typo.
> 
> git blame shows that this version of code was added in commit
> d71f513985c22f1050295d1a7e4327cf9fb060da on Oct 17 2012, so you can check
> it.

the fix for that is going to be sent to Linus shortly.
> 
> Thanks for your replies!
> 
> Best regards,
> Nyashka
> 
> 
> 2013/5/17 Mukesh Rathor <mukesh.rathor@oracle.com>
> 
> > On Fri, 17 May 2013 11:14:00 +0100
> > Ian Campbell <ian.campbell@citrix.com> wrote:
> >
> > > On Thu, 2013-05-16 at 19:36 +0400, Nyashka Surovski wrote:
> > > > Hi Xen folks!
> > > >
> > > >
> > > > I've faced with one strange thing in ARM version of Xen: when I use
> > > > xc_map_foreign_bulk() to map some memory from domU to dom0, after
> > > > unmap() for previous returned address - memory is not freed at all.
> > > >
> > > >
> > > > Let's look at call stack:
> > > >
> > > >
> > > > xc_map_foreign() ->
> > > >   linux_privcmd_map_foreign_bulk() ->
> > > >     {
> > > >     addr = mmap(fd);
> > > >     ioctl(fd, IOCTL_PRIVCMD_MMAPBATCH_V2 );
> > > >     }  ->
> > > >       alloc_empty_pages() ->
> > > >         alloc_xenballoned_pages();
> > > >
> > > > So, I think that unmap(addr) must call free_xenballoned_pages(), but
> > > > this doesn't happen. =(
> > > > Let me note, that mmap() knows about privcmd_close() function, and
> > > > it is the place where free_xenballoned_pages() is called, So we
> > > > have that unmap() doesn't call privcmd_close() at all. It's
> > > > something strange for me.
> > > >
> > > > Can somebody show me the place of my misunderstanding, or is it a
> > > > real bug?
> > >
> > > Do you mean munmap()?
> > >
> > > I think munmap will eventually end up calling close, when the
> > > references to the vma etc are gone. Since the code path is a bit
> > > twisty I'd be tempted to throw in a debug printk to confirm though.
> > >
> > > Can you share your usercode?
> >
> > I dealt with that a lot during PVH debug. Yes, munmap will call close.
> > If the process exits without calling munmap, then do_exit -> exit_mm will
> > result in call to privcmd_close.
> >
> > hope that helps.
> > Mukesh
> >
> >

> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel