* xc_map_foreign_bulk() memory leak in ARM version?
@ 2013-05-16 15:36 Nyashka Surovski
2013-05-17 10:14 ` Ian Campbell
0 siblings, 1 reply; 6+ messages in thread
From: Nyashka Surovski @ 2013-05-16 15:36 UTC (permalink / raw)
To: xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 859 bytes --]
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?
Best regards,
Nyashka
[-- Attachment #1.2: Type: text/html, Size: 1073 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: xc_map_foreign_bulk() memory leak in ARM version?
2013-05-16 15:36 xc_map_foreign_bulk() memory leak in ARM version? Nyashka Surovski
@ 2013-05-17 10:14 ` Ian Campbell
2013-05-17 19:13 ` Mukesh Rathor
0 siblings, 1 reply; 6+ messages in thread
From: Ian Campbell @ 2013-05-17 10:14 UTC (permalink / raw)
To: Nyashka Surovski; +Cc: xen-devel
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?
Ian.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: xc_map_foreign_bulk() memory leak in ARM version?
2013-05-17 10:14 ` Ian Campbell
@ 2013-05-17 19:13 ` Mukesh Rathor
2013-05-20 7:55 ` Nyashka Surovski
0 siblings, 1 reply; 6+ messages in thread
From: Mukesh Rathor @ 2013-05-17 19:13 UTC (permalink / raw)
To: Ian Campbell; +Cc: Nyashka Surovski, xen-devel
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
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: xc_map_foreign_bulk() memory leak in ARM version?
2013-05-17 19:13 ` Mukesh Rathor
@ 2013-05-20 7:55 ` Nyashka Surovski
2013-05-20 9:19 ` Ian Campbell
2013-05-20 19:39 ` Konrad Rzeszutek Wilk
0 siblings, 2 replies; 6+ messages in thread
From: Nyashka Surovski @ 2013-05-20 7:55 UTC (permalink / raw)
To: xen-devel
[-- Attachment #1.1: Type: text/plain, Size: 2223 bytes --]
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.
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
>
>
[-- Attachment #1.2: Type: text/html, Size: 3364 bytes --]
[-- Attachment #2: Type: text/plain, Size: 126 bytes --]
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: xc_map_foreign_bulk() memory leak in ARM version?
2013-05-20 7:55 ` Nyashka Surovski
@ 2013-05-20 9:19 ` Ian Campbell
2013-05-20 19:39 ` Konrad Rzeszutek Wilk
1 sibling, 0 replies; 6+ messages in thread
From: Ian Campbell @ 2013-05-20 9:19 UTC (permalink / raw)
To: Nyashka Surovski; +Cc: Konrad Rzeszutek Wilk, xen-devel
On Mon, 2013-05-20 at 11:55 +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.
Absolutely!
This was picked up by Dan Carpenter with "xen/privcmd: fix condition in
privcmd_close()" ages ago but accidentally got dropped. Luckily he sent
a ping about it last week and I think it has now been picked up into
Konrad's tree and will no doubt be going to Linus soon.
Thanks for tracking this down.
Ian.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: xc_map_foreign_bulk() memory leak in ARM version?
2013-05-20 7:55 ` Nyashka Surovski
2013-05-20 9:19 ` Ian Campbell
@ 2013-05-20 19:39 ` Konrad Rzeszutek Wilk
1 sibling, 0 replies; 6+ messages in thread
From: Konrad Rzeszutek Wilk @ 2013-05-20 19:39 UTC (permalink / raw)
To: Nyashka Surovski; +Cc: xen-devel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-05-20 19:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-05-16 15:36 xc_map_foreign_bulk() memory leak in ARM version? Nyashka Surovski
2013-05-17 10:14 ` Ian Campbell
2013-05-17 19:13 ` Mukesh Rathor
2013-05-20 7:55 ` Nyashka Surovski
2013-05-20 9:19 ` Ian Campbell
2013-05-20 19:39 ` Konrad Rzeszutek Wilk
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).