linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCHi v2] mm: do not drop unused pages when userfaultd is running
@ 2018-07-02  7:50 Christian Borntraeger
  2018-07-02 21:06 ` Andrew Morton
  0 siblings, 1 reply; 3+ messages in thread
From: Christian Borntraeger @ 2018-07-02  7:50 UTC (permalink / raw)
  To: linux-mm, linux-s390, Andrew Morton
  Cc: kvm, Janosch Frank, David Hildenbrand, Cornelia Huck,
	linux-kernel, Christian Borntraeger, Martin Schwidefsky,
	Andrea Arcangeli, Mike Rapoport

KVM guests on s390 can notify the host of unused pages. This can result
in pte_unused callbacks to be true for KVM guest memory.

If a page is unused (checked with pte_unused) we might drop this page
instead of paging it. This can have side-effects on userfaultd, when the
page in question was already migrated:

The next access of that page will trigger a fault and a user fault
instead of faulting in a new and empty zero page. As QEMU does not
expect a userfault on an already migrated page this migration will fail.

The most straightforward solution is to ignore the pte_unused hint if a
userfault context is active for this VMA.

Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Andrea Arcangeli <aarcange@redhat.com>
Cc: Mike Rapoport <rppt@linux.vnet.ibm.com>
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
rfc->v2: use userfaultfd_armed
 mm/rmap.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/mm/rmap.c b/mm/rmap.c
index 6db729dc4c50..e8fa564676b6 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -64,6 +64,7 @@
 #include <linux/backing-dev.h>
 #include <linux/page_idle.h>
 #include <linux/memremap.h>
+#include <linux/userfaultfd_k.h>
 
 #include <asm/tlbflush.h>
 
@@ -1481,7 +1482,7 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
 				set_pte_at(mm, address, pvmw.pte, pteval);
 			}
 
-		} else if (pte_unused(pteval)) {
+		} else if (pte_unused(pteval) && !userfaultfd_armed(vma)) {
 			/*
 			 * The guest indicated that the page content is of no
 			 * interest anymore. Simply discard the pte, vmscan
-- 
2.17.0

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCHi v2] mm: do not drop unused pages when userfaultd is running
  2018-07-02  7:50 [PATCHi v2] mm: do not drop unused pages when userfaultd is running Christian Borntraeger
@ 2018-07-02 21:06 ` Andrew Morton
  2018-07-03  5:23   ` Christian Borntraeger
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Morton @ 2018-07-02 21:06 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: linux-mm, linux-s390, kvm, Janosch Frank, David Hildenbrand,
	Cornelia Huck, linux-kernel, Martin Schwidefsky,
	Andrea Arcangeli, Mike Rapoport

On Mon,  2 Jul 2018 09:50:49 +0200 Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> KVM guests on s390 can notify the host of unused pages. This can result
> in pte_unused callbacks to be true for KVM guest memory.
> 
> If a page is unused (checked with pte_unused) we might drop this page
> instead of paging it. This can have side-effects on userfaultd, when the
> page in question was already migrated:
> 
> The next access of that page will trigger a fault and a user fault
> instead of faulting in a new and empty zero page. As QEMU does not
> expect a userfault on an already migrated page this migration will fail.
> 
> The most straightforward solution is to ignore the pte_unused hint if a
> userfault context is active for this VMA.
> 
> ...
>
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -64,6 +64,7 @@
>  #include <linux/backing-dev.h>
>  #include <linux/page_idle.h>
>  #include <linux/memremap.h>
> +#include <linux/userfaultfd_k.h>
>  
>  #include <asm/tlbflush.h>
>  
> @@ -1481,7 +1482,7 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
>  				set_pte_at(mm, address, pvmw.pte, pteval);
>  			}
>  
> -		} else if (pte_unused(pteval)) {
> +		} else if (pte_unused(pteval) && !userfaultfd_armed(vma)) {
>  			/*
>  			 * The guest indicated that the page content is of no
>  			 * interest anymore. Simply discard the pte, vmscan

A reader of this code will wonder why we're checking
userfaultfd_armed().  So the writer of this code should add a comment
which explains this to them ;)  Please.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCHi v2] mm: do not drop unused pages when userfaultd is running
  2018-07-02 21:06 ` Andrew Morton
@ 2018-07-03  5:23   ` Christian Borntraeger
  0 siblings, 0 replies; 3+ messages in thread
From: Christian Borntraeger @ 2018-07-03  5:23 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, linux-s390, kvm, Janosch Frank, David Hildenbrand,
	Cornelia Huck, linux-kernel, Martin Schwidefsky,
	Andrea Arcangeli, Mike Rapoport



On 07/02/2018 11:06 PM, Andrew Morton wrote:
> On Mon,  2 Jul 2018 09:50:49 +0200 Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
>> KVM guests on s390 can notify the host of unused pages. This can result
>> in pte_unused callbacks to be true for KVM guest memory.
>>
>> If a page is unused (checked with pte_unused) we might drop this page
>> instead of paging it. This can have side-effects on userfaultd, when the
>> page in question was already migrated:
>>
>> The next access of that page will trigger a fault and a user fault
>> instead of faulting in a new and empty zero page. As QEMU does not
>> expect a userfault on an already migrated page this migration will fail.
>>
>> The most straightforward solution is to ignore the pte_unused hint if a
>> userfault context is active for this VMA.
>>
>> ...
>>
>> --- a/mm/rmap.c
>> +++ b/mm/rmap.c
>> @@ -64,6 +64,7 @@
>>  #include <linux/backing-dev.h>
>>  #include <linux/page_idle.h>
>>  #include <linux/memremap.h>
>> +#include <linux/userfaultfd_k.h>
>>  
>>  #include <asm/tlbflush.h>
>>  
>> @@ -1481,7 +1482,7 @@ static bool try_to_unmap_one(struct page *page, struct vm_area_struct *vma,
>>  				set_pte_at(mm, address, pvmw.pte, pteval);
>>  			}
>>  
>> -		} else if (pte_unused(pteval)) {
>> +		} else if (pte_unused(pteval) && !userfaultfd_armed(vma)) {


>>  			/*
>>  			 * The guest indicated that the page content is of no
>>  			 * interest anymore. Simply discard the pte, vmscan
> 
> A reader of this code will wonder why we're checking
> userfaultfd_armed().  So the writer of this code should add a comment
> which explains this to them ;)  Please.
> 
Something like:                    /*
                         * The guest indicated that the page content is of no
                         * interest anymore. Simply discard the pte, vmscan
                         * will take care of the rest.
			 * A future reference will then fault in a new zero
			 * page. When userfaultfd is active, we must not drop
			 * this page though, as its main user (postcopy
			 * migration) will not expect userfaults on already
			 * copied pages.
                         */

?

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2018-07-03  5:23 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-02  7:50 [PATCHi v2] mm: do not drop unused pages when userfaultd is running Christian Borntraeger
2018-07-02 21:06 ` Andrew Morton
2018-07-03  5:23   ` Christian Borntraeger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox