[Qemu-devel] [PATCH] xen-mapcache: don't unmap locked entry during mapcache invalidation

[Qemu-devel] [PATCH] xen-mapcache: don't unmap locked entry during mapcache invalidation

[Julien Grall]

When an IOREQ_TYPE_INVALIDATE is sent to QEMU, it invalidates all entry
of the map cache even if it's locked.

QEMU is not able to know that entry was invalidated, so when an IO
access is requested a segfault occured.

Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
  xen-mapcache.c |    3 +++
  1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/xen-mapcache.c b/xen-mapcache.c
index 585b559..6e7e5ab 100644
--- a/xen-mapcache.c
+++ b/xen-mapcache.c
@@ -370,6 +370,9 @@ void xen_invalidate_map_cache(void)
              continue;
          }
+	if (entry->lock > 0)
+	    continue;
+
          if (munmap(entry->vaddr_base, entry->size) != 0) {
              perror("unmap fails");
              exit(-1);
-- 
Julien Grall

[PATCH] xen-mapcache: don't unmap locked entry during mapcache invalidation

[Julien Grall]

When an IOREQ_TYPE_INVALIDATE is sent to QEMU, it invalidates all entry
of the map cache even if it's locked.

QEMU is not able to know that entry was invalidated, so when an IO
access is requested a segfault occured.

Signed-off-by: Julien Grall <julien.grall@citrix.com>
---
  xen-mapcache.c |    3 +++
  1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/xen-mapcache.c b/xen-mapcache.c
index 585b559..6e7e5ab 100644
--- a/xen-mapcache.c
+++ b/xen-mapcache.c
@@ -370,6 +370,9 @@ void xen_invalidate_map_cache(void)
              continue;
          }
+	if (entry->lock > 0)
+	    continue;
+
          if (munmap(entry->vaddr_base, entry->size) != 0) {
              perror("unmap fails");
              exit(-1);
-- 
Julien Grall

Re: [Qemu-devel] [PATCH] xen-mapcache: don't unmap locked entry during mapcache invalidation

[Stefano Stabellini]

On Thu, 15 Mar 2012, Julien Grall wrote:
> When an IOREQ_TYPE_INVALIDATE is sent to QEMU, it invalidates all entry
> of the map cache even if it's locked.
> 
> QEMU is not able to know that entry was invalidated, so when an IO
> access is requested a segfault occured.

The problem here is the long term mappings in QEMU that cannot easily be
re-created.
I am not sure whether this can cause any trouble to things like
xenpaging.


> Signed-off-by: Julien Grall <julien.grall@citrix.com>
> ---
>   xen-mapcache.c |    3 +++
>   1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/xen-mapcache.c b/xen-mapcache.c
> index 585b559..6e7e5ab 100644
> --- a/xen-mapcache.c
> +++ b/xen-mapcache.c
> @@ -370,6 +370,9 @@ void xen_invalidate_map_cache(void)
>               continue;
>           }
> +	if (entry->lock > 0)
> +	    continue;
> +
>           if (munmap(entry->vaddr_base, entry->size) != 0) {
>               perror("unmap fails");
>               exit(-1);
 
CODING_STYLE

Re: [PATCH] xen-mapcache: don't unmap locked entry during mapcache invalidation

[Stefano Stabellini]

On Thu, 15 Mar 2012, Julien Grall wrote:
> When an IOREQ_TYPE_INVALIDATE is sent to QEMU, it invalidates all entry
> of the map cache even if it's locked.
> 
> QEMU is not able to know that entry was invalidated, so when an IO
> access is requested a segfault occured.

The problem here is the long term mappings in QEMU that cannot easily be
re-created.
I am not sure whether this can cause any trouble to things like
xenpaging.


> Signed-off-by: Julien Grall <julien.grall@citrix.com>
> ---
>   xen-mapcache.c |    3 +++
>   1 files changed, 3 insertions(+), 0 deletions(-)
> 
> diff --git a/xen-mapcache.c b/xen-mapcache.c
> index 585b559..6e7e5ab 100644
> --- a/xen-mapcache.c
> +++ b/xen-mapcache.c
> @@ -370,6 +370,9 @@ void xen_invalidate_map_cache(void)
>               continue;
>           }
> +	if (entry->lock > 0)
> +	    continue;
> +
>           if (munmap(entry->vaddr_base, entry->size) != 0) {
>               perror("unmap fails");
>               exit(-1);
 
CODING_STYLE

Re: [Qemu-devel] [PATCH] xen-mapcache: don't unmap locked entry during mapcache invalidation

[Andres Lagar-Cavilla]

> On Thu, 15 Mar 2012, Julien Grall wrote:
>> When an IOREQ_TYPE_INVALIDATE is sent to QEMU, it invalidates all entry
>> of the map cache even if it's locked.
>>
>> QEMU is not able to know that entry was invalidated, so when an IO
>> access is requested a segfault occured.
>
> The problem here is the long term mappings in QEMU that cannot easily be
> re-created.
> I am not sure whether this can cause any trouble to things like
> xenpaging.

This is a performance optimization for xenpaging: it cannot page out pages
that are mapped by qemu, so it asks qemu to drop the mappings. If denied
(partially or fully) xenpaging won't crash, and the VM won't crash.
xenpaging will just have to keep trying to find other candidates for
paging out.

Andres

>
>
>> Signed-off-by: Julien Grall <julien.grall@citrix.com>
>> ---
>>   xen-mapcache.c |    3 +++
>>   1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/xen-mapcache.c b/xen-mapcache.c
>> index 585b559..6e7e5ab 100644
>> --- a/xen-mapcache.c
>> +++ b/xen-mapcache.c
>> @@ -370,6 +370,9 @@ void xen_invalidate_map_cache(void)
>>               continue;
>>           }
>> +	if (entry->lock > 0)
>> +	    continue;
>> +
>>           if (munmap(entry->vaddr_base, entry->size) != 0) {
>>               perror("unmap fails");
>>               exit(-1);
>
> CODING_STYLE
>

Re: [PATCH] xen-mapcache: don't unmap locked entry during mapcache invalidation

[Andres Lagar-Cavilla]

> On Thu, 15 Mar 2012, Julien Grall wrote:
>> When an IOREQ_TYPE_INVALIDATE is sent to QEMU, it invalidates all entry
>> of the map cache even if it's locked.
>>
>> QEMU is not able to know that entry was invalidated, so when an IO
>> access is requested a segfault occured.
>
> The problem here is the long term mappings in QEMU that cannot easily be
> re-created.
> I am not sure whether this can cause any trouble to things like
> xenpaging.

This is a performance optimization for xenpaging: it cannot page out pages
that are mapped by qemu, so it asks qemu to drop the mappings. If denied
(partially or fully) xenpaging won't crash, and the VM won't crash.
xenpaging will just have to keep trying to find other candidates for
paging out.

Andres

>
>
>> Signed-off-by: Julien Grall <julien.grall@citrix.com>
>> ---
>>   xen-mapcache.c |    3 +++
>>   1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/xen-mapcache.c b/xen-mapcache.c
>> index 585b559..6e7e5ab 100644
>> --- a/xen-mapcache.c
>> +++ b/xen-mapcache.c
>> @@ -370,6 +370,9 @@ void xen_invalidate_map_cache(void)
>>               continue;
>>           }
>> +	if (entry->lock > 0)
>> +	    continue;
>> +
>>           if (munmap(entry->vaddr_base, entry->size) != 0) {
>>               perror("unmap fails");
>>               exit(-1);
>
> CODING_STYLE
>

Re: [Qemu-devel] [PATCH] xen-mapcache: don't unmap locked entry during mapcache invalidation

[Olaf Hering]

On Thu, Mar 15, Stefano Stabellini wrote:

> On Thu, 15 Mar 2012, Julien Grall wrote:
> > When an IOREQ_TYPE_INVALIDATE is sent to QEMU, it invalidates all entry
> > of the map cache even if it's locked.
> > 
> > QEMU is not able to know that entry was invalidated, so when an IO
> > access is requested a segfault occured.
> 
> The problem here is the long term mappings in QEMU that cannot easily be
> re-created.
> I am not sure whether this can cause any trouble to things like
> xenpaging.

Yes, I was wondering about this as well. If the request is made, then
its expected that all mappings disappear because they can point to
ballooned gfns. IF that case is safe, then all is fine.

In case of xenpaging its not an issue because xenpaging just asks qemu
to release mappings so that the usage count of mfns drops to 1, so that
they can be nominated for eviction. If that fails, not a big deal as it
just means that no more pages can be evicted. xenpaging will retry.

Olaf

Re: [PATCH] xen-mapcache: don't unmap locked entry during mapcache invalidation

[Olaf Hering]

On Thu, Mar 15, Stefano Stabellini wrote:

> On Thu, 15 Mar 2012, Julien Grall wrote:
> > When an IOREQ_TYPE_INVALIDATE is sent to QEMU, it invalidates all entry
> > of the map cache even if it's locked.
> > 
> > QEMU is not able to know that entry was invalidated, so when an IO
> > access is requested a segfault occured.
> 
> The problem here is the long term mappings in QEMU that cannot easily be
> re-created.
> I am not sure whether this can cause any trouble to things like
> xenpaging.

Yes, I was wondering about this as well. If the request is made, then
its expected that all mappings disappear because they can point to
ballooned gfns. IF that case is safe, then all is fine.

In case of xenpaging its not an issue because xenpaging just asks qemu
to release mappings so that the usage count of mfns drops to 1, so that
they can be nominated for eviction. If that fails, not a big deal as it
just means that no more pages can be evicted. xenpaging will retry.

Olaf

Re: [Qemu-devel] [PATCH] xen-mapcache: don't unmap locked entry during mapcache invalidation

[Stefano Stabellini]

On Thu, 15 Mar 2012, Olaf Hering wrote:
> On Thu, Mar 15, Stefano Stabellini wrote:
> 
> > On Thu, 15 Mar 2012, Julien Grall wrote:
> > > When an IOREQ_TYPE_INVALIDATE is sent to QEMU, it invalidates all entry
> > > of the map cache even if it's locked.
> > > 
> > > QEMU is not able to know that entry was invalidated, so when an IO
> > > access is requested a segfault occured.
> > 
> > The problem here is the long term mappings in QEMU that cannot easily be
> > re-created.
> > I am not sure whether this can cause any trouble to things like
> > xenpaging.
> 
> Yes, I was wondering about this as well. If the request is made, then
> its expected that all mappings disappear because they can point to
> ballooned gfns. IF that case is safe, then all is fine.

The long lived mappings shouldn't involve any ballooned gfns, unless
something went terribly wrong.


> In case of xenpaging its not an issue because xenpaging just asks qemu
> to release mappings so that the usage count of mfns drops to 1, so that
> they can be nominated for eviction. If that fails, not a big deal as it
> just means that no more pages can be evicted. xenpaging will retry.

Great, sounds like there is no problem then.

Re: [PATCH] xen-mapcache: don't unmap locked entry during mapcache invalidation

[Stefano Stabellini]

On Thu, 15 Mar 2012, Olaf Hering wrote:
> On Thu, Mar 15, Stefano Stabellini wrote:
> 
> > On Thu, 15 Mar 2012, Julien Grall wrote:
> > > When an IOREQ_TYPE_INVALIDATE is sent to QEMU, it invalidates all entry
> > > of the map cache even if it's locked.
> > > 
> > > QEMU is not able to know that entry was invalidated, so when an IO
> > > access is requested a segfault occured.
> > 
> > The problem here is the long term mappings in QEMU that cannot easily be
> > re-created.
> > I am not sure whether this can cause any trouble to things like
> > xenpaging.
> 
> Yes, I was wondering about this as well. If the request is made, then
> its expected that all mappings disappear because they can point to
> ballooned gfns. IF that case is safe, then all is fine.

The long lived mappings shouldn't involve any ballooned gfns, unless
something went terribly wrong.


> In case of xenpaging its not an issue because xenpaging just asks qemu
> to release mappings so that the usage count of mfns drops to 1, so that
> they can be nominated for eviction. If that fails, not a big deal as it
> just means that no more pages can be evicted. xenpaging will retry.

Great, sounds like there is no problem then.

Re: [Qemu-devel] [Xen-devel] [PATCH] xen-mapcache: don't unmap locked entry during mapcache invalidation

[Tim Deegan]

At 17:14 +0000 on 15 Mar (1331831693), Stefano Stabellini wrote:
> On Thu, 15 Mar 2012, Julien Grall wrote:
> > When an IOREQ_TYPE_INVALIDATE is sent to QEMU, it invalidates all entry
> > of the map cache even if it's locked.
> > 
> > QEMU is not able to know that entry was invalidated, so when an IO
> > access is requested a segfault occured.
> 
> The problem here is the long term mappings in QEMU that cannot easily be
> re-created.
> I am not sure whether this can cause any trouble to things like
> xenpaging.

It causes some trouble to ballooning - a guest might try to return memory
to Xen only to find that Qemu won't let go of it.

If (as I hope is the case) qemu never has a locked mapping to something
that the guets ought to be ballooning, that's OK.  If this happens just
because the page was recently a DMA target, then it's not. 

Cheers,

Tim.

Re: [Xen-devel] [PATCH] xen-mapcache: don't unmap locked entry during mapcache invalidation

[Tim Deegan]

At 17:14 +0000 on 15 Mar (1331831693), Stefano Stabellini wrote:
> On Thu, 15 Mar 2012, Julien Grall wrote:
> > When an IOREQ_TYPE_INVALIDATE is sent to QEMU, it invalidates all entry
> > of the map cache even if it's locked.
> > 
> > QEMU is not able to know that entry was invalidated, so when an IO
> > access is requested a segfault occured.
> 
> The problem here is the long term mappings in QEMU that cannot easily be
> re-created.
> I am not sure whether this can cause any trouble to things like
> xenpaging.

It causes some trouble to ballooning - a guest might try to return memory
to Xen only to find that Qemu won't let go of it.

If (as I hope is the case) qemu never has a locked mapping to something
that the guets ought to be ballooning, that's OK.  If this happens just
because the page was recently a DMA target, then it's not. 

Cheers,

Tim.

Re: [Qemu-devel] [Xen-devel] [PATCH] xen-mapcache: don't unmap locked entry during mapcache invalidation

[Andres Lagar-Cavilla]

> At 17:14 +0000 on 15 Mar (1331831693), Stefano Stabellini wrote:
>> On Thu, 15 Mar 2012, Julien Grall wrote:
>> > When an IOREQ_TYPE_INVALIDATE is sent to QEMU, it invalidates all
>> entry
>> > of the map cache even if it's locked.
>> >
>> > QEMU is not able to know that entry was invalidated, so when an IO
>> > access is requested a segfault occured.
>>
>> The problem here is the long term mappings in QEMU that cannot easily be
>> re-created.
>> I am not sure whether this can cause any trouble to things like
>> xenpaging.
>
> It causes some trouble to ballooning - a guest might try to return memory
> to Xen only to find that Qemu won't let go of it.

That's the right causation (as opposed to invalidate cache being called
after balloon).

All that will happen is that the balloon request will be (partially)
failed. Up to the guest balloon driver to deal with it gracefully (and to
not choose weird pages to balloon away in the first place!).

Andres
>
> If (as I hope is the case) qemu never has a locked mapping to something
> that the guets ought to be ballooning, that's OK.  If this happens just
> because the page was recently a DMA target, then it's not.
>
> Cheers,
>
> Tim.
>

Re: [Xen-devel] [PATCH] xen-mapcache: don't unmap locked entry during mapcache invalidation

[Andres Lagar-Cavilla]

> At 17:14 +0000 on 15 Mar (1331831693), Stefano Stabellini wrote:
>> On Thu, 15 Mar 2012, Julien Grall wrote:
>> > When an IOREQ_TYPE_INVALIDATE is sent to QEMU, it invalidates all
>> entry
>> > of the map cache even if it's locked.
>> >
>> > QEMU is not able to know that entry was invalidated, so when an IO
>> > access is requested a segfault occured.
>>
>> The problem here is the long term mappings in QEMU that cannot easily be
>> re-created.
>> I am not sure whether this can cause any trouble to things like
>> xenpaging.
>
> It causes some trouble to ballooning - a guest might try to return memory
> to Xen only to find that Qemu won't let go of it.

That's the right causation (as opposed to invalidate cache being called
after balloon).

All that will happen is that the balloon request will be (partially)
failed. Up to the guest balloon driver to deal with it gracefully (and to
not choose weird pages to balloon away in the first place!).

Andres
>
> If (as I hope is the case) qemu never has a locked mapping to something
> that the guets ought to be ballooning, that's OK.  If this happens just
> because the page was recently a DMA target, then it's not.
>
> Cheers,
>
> Tim.
>

Re: [Qemu-devel] [Xen-devel] [PATCH] xen-mapcache: don't unmap locked entry during mapcache invalidation

[Tim Deegan]

At 10:32 -0700 on 15 Mar (1331807562), Andres Lagar-Cavilla wrote:
> > At 17:14 +0000 on 15 Mar (1331831693), Stefano Stabellini wrote:
> >> On Thu, 15 Mar 2012, Julien Grall wrote:
> >> > When an IOREQ_TYPE_INVALIDATE is sent to QEMU, it invalidates all
> >> entry
> >> > of the map cache even if it's locked.
> >> >
> >> > QEMU is not able to know that entry was invalidated, so when an IO
> >> > access is requested a segfault occured.
> >>
> >> The problem here is the long term mappings in QEMU that cannot easily be
> >> re-created.
> >> I am not sure whether this can cause any trouble to things like
> >> xenpaging.
> >
> > It causes some trouble to ballooning - a guest might try to return memory
> > to Xen only to find that Qemu won't let go of it.
> 
> That's the right causation (as opposed to invalidate cache being called
> after balloon).
> 
> All that will happen is that the balloon request will be (partially)
> failed. Up to the guest balloon driver to deal with it gracefully (and to
> not choose weird pages to balloon away in the first place!).

No - the balloon call will succeed, and the page will be removed from
the p2m and its ref dropped.  But the guest's memory usage won't drop
until qemu lets go of its mapping.

If qemu is liable to do this to a lot of memory, that's definitely a
problem.  But I suspect that in fact it won't do that.

Tim.

Re: [Xen-devel] [PATCH] xen-mapcache: don't unmap locked entry during mapcache invalidation

[Tim Deegan]

At 10:32 -0700 on 15 Mar (1331807562), Andres Lagar-Cavilla wrote:
> > At 17:14 +0000 on 15 Mar (1331831693), Stefano Stabellini wrote:
> >> On Thu, 15 Mar 2012, Julien Grall wrote:
> >> > When an IOREQ_TYPE_INVALIDATE is sent to QEMU, it invalidates all
> >> entry
> >> > of the map cache even if it's locked.
> >> >
> >> > QEMU is not able to know that entry was invalidated, so when an IO
> >> > access is requested a segfault occured.
> >>
> >> The problem here is the long term mappings in QEMU that cannot easily be
> >> re-created.
> >> I am not sure whether this can cause any trouble to things like
> >> xenpaging.
> >
> > It causes some trouble to ballooning - a guest might try to return memory
> > to Xen only to find that Qemu won't let go of it.
> 
> That's the right causation (as opposed to invalidate cache being called
> after balloon).
> 
> All that will happen is that the balloon request will be (partially)
> failed. Up to the guest balloon driver to deal with it gracefully (and to
> not choose weird pages to balloon away in the first place!).

No - the balloon call will succeed, and the page will be removed from
the p2m and its ref dropped.  But the guest's memory usage won't drop
until qemu lets go of its mapping.

If qemu is liable to do this to a lot of memory, that's definitely a
problem.  But I suspect that in fact it won't do that.

Tim.

Re: [Qemu-devel] [Xen-devel] [PATCH] xen-mapcache: don't unmap locked entry during mapcache invalidation

[Stefano Stabellini]

On Thu, 15 Mar 2012, Tim Deegan wrote:
> At 17:14 +0000 on 15 Mar (1331831693), Stefano Stabellini wrote:
> > On Thu, 15 Mar 2012, Julien Grall wrote:
> > > When an IOREQ_TYPE_INVALIDATE is sent to QEMU, it invalidates all entry
> > > of the map cache even if it's locked.
> > > 
> > > QEMU is not able to know that entry was invalidated, so when an IO
> > > access is requested a segfault occured.
> > 
> > The problem here is the long term mappings in QEMU that cannot easily be
> > re-created.
> > I am not sure whether this can cause any trouble to things like
> > xenpaging.
> 
> It causes some trouble to ballooning - a guest might try to return memory
> to Xen only to find that Qemu won't let go of it.
> 
> If (as I hope is the case) qemu never has a locked mapping to something
> that the guets ought to be ballooning, that's OK.

That should be the case.

> If this happens just
> because the page was recently a DMA target, then it's not. 

Only if the DMA is still in progress, in that case it is a bad idea to
balloon out that page.

Re: [Xen-devel] [PATCH] xen-mapcache: don't unmap locked entry during mapcache invalidation

[Stefano Stabellini]

On Thu, 15 Mar 2012, Tim Deegan wrote:
> At 17:14 +0000 on 15 Mar (1331831693), Stefano Stabellini wrote:
> > On Thu, 15 Mar 2012, Julien Grall wrote:
> > > When an IOREQ_TYPE_INVALIDATE is sent to QEMU, it invalidates all entry
> > > of the map cache even if it's locked.
> > > 
> > > QEMU is not able to know that entry was invalidated, so when an IO
> > > access is requested a segfault occured.
> > 
> > The problem here is the long term mappings in QEMU that cannot easily be
> > re-created.
> > I am not sure whether this can cause any trouble to things like
> > xenpaging.
> 
> It causes some trouble to ballooning - a guest might try to return memory
> to Xen only to find that Qemu won't let go of it.
> 
> If (as I hope is the case) qemu never has a locked mapping to something
> that the guets ought to be ballooning, that's OK.

That should be the case.

> If this happens just
> because the page was recently a DMA target, then it's not. 

Only if the DMA is still in progress, in that case it is a bad idea to
balloon out that page.