diff mbox series

[v12,23/31] mm: don't do swap readahead during speculative page fault

Message ID 20190416134522.17540-24-ldufour@linux.ibm.com (mailing list archive)
State New, archived
Headers show
Series Speculative page faults | expand

Commit Message

Laurent Dufour April 16, 2019, 1:45 p.m. UTC
Vinayak Menon faced a panic because one thread was page faulting a page in
swap, while another one was mprotecting a part of the VMA leading to a VMA
split.
This raise a panic in swap_vma_readahead() because the VMA's boundaries
were not more matching the faulting address.

To avoid this, if the page is not found in the swap, the speculative page
fault is aborted to retry a regular page fault.

Reported-by: Vinayak Menon <vinmenon@codeaurora.org>
Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
---
 mm/memory.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

Comments

Jerome Glisse April 22, 2019, 9:36 p.m. UTC | #1
On Tue, Apr 16, 2019 at 03:45:14PM +0200, Laurent Dufour wrote:
> Vinayak Menon faced a panic because one thread was page faulting a page in
> swap, while another one was mprotecting a part of the VMA leading to a VMA
> split.
> This raise a panic in swap_vma_readahead() because the VMA's boundaries
> were not more matching the faulting address.
> 
> To avoid this, if the page is not found in the swap, the speculative page
> fault is aborted to retry a regular page fault.
> 
> Reported-by: Vinayak Menon <vinmenon@codeaurora.org>
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>

Reviewed-by: Jérôme Glisse <jglisse@redhat.com>

Note that you should also skip non swap entry in do_swap_page() when doing
speculative page fault at very least you need to is_device_private_entry()
case.

But this should either be part of patch 22 or another patch to fix swap
case.

> ---
>  mm/memory.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index 6e6bf61c0e5c..1991da97e2db 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -2900,6 +2900,17 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
>  				lru_cache_add_anon(page);
>  				swap_readpage(page, true);
>  			}
> +		} else if (vmf->flags & FAULT_FLAG_SPECULATIVE) {
> +			/*
> +			 * Don't try readahead during a speculative page fault
> +			 * as the VMA's boundaries may change in our back.
> +			 * If the page is not in the swap cache and synchronous
> +			 * read is disabled, fall back to the regular page
> +			 * fault mechanism.
> +			 */
> +			delayacct_clear_flag(DELAYACCT_PF_SWAPIN);
> +			ret = VM_FAULT_RETRY;
> +			goto out;
>  		} else {
>  			page = swapin_readahead(entry, GFP_HIGHUSER_MOVABLE,
>  						vmf);
> -- 
> 2.21.0
>
Laurent Dufour April 24, 2019, 2:57 p.m. UTC | #2
Le 22/04/2019 à 23:36, Jerome Glisse a écrit :
> On Tue, Apr 16, 2019 at 03:45:14PM +0200, Laurent Dufour wrote:
>> Vinayak Menon faced a panic because one thread was page faulting a page in
>> swap, while another one was mprotecting a part of the VMA leading to a VMA
>> split.
>> This raise a panic in swap_vma_readahead() because the VMA's boundaries
>> were not more matching the faulting address.
>>
>> To avoid this, if the page is not found in the swap, the speculative page
>> fault is aborted to retry a regular page fault.
>>
>> Reported-by: Vinayak Menon <vinmenon@codeaurora.org>
>> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> 
> Reviewed-by: Jérôme Glisse <jglisse@redhat.com>
> 
> Note that you should also skip non swap entry in do_swap_page() when doing
> speculative page fault at very least you need to is_device_private_entry()
> case.
> 
> But this should either be part of patch 22 or another patch to fix swap
> case.

Thanks Jérôme,

Yes I missed that, I guess the best option would be to abort on non swap 
entry. I'll add that in the patch 22.

>> ---
>>   mm/memory.c | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/mm/memory.c b/mm/memory.c
>> index 6e6bf61c0e5c..1991da97e2db 100644
>> --- a/mm/memory.c
>> +++ b/mm/memory.c
>> @@ -2900,6 +2900,17 @@ vm_fault_t do_swap_page(struct vm_fault *vmf)
>>   				lru_cache_add_anon(page);
>>   				swap_readpage(page, true);
>>   			}
>> +		} else if (vmf->flags & FAULT_FLAG_SPECULATIVE) {
>> +			/*
>> +			 * Don't try readahead during a speculative page fault
>> +			 * as the VMA's boundaries may change in our back.
>> +			 * If the page is not in the swap cache and synchronous
>> +			 * read is disabled, fall back to the regular page
>> +			 * fault mechanism.
>> +			 */
>> +			delayacct_clear_flag(DELAYACCT_PF_SWAPIN);
>> +			ret = VM_FAULT_RETRY;
>> +			goto out;
>>   		} else {
>>   			page = swapin_readahead(entry, GFP_HIGHUSER_MOVABLE,
>>   						vmf);
>> -- 
>> 2.21.0
>>
>
diff mbox series

Patch

diff --git a/mm/memory.c b/mm/memory.c
index 6e6bf61c0e5c..1991da97e2db 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2900,6 +2900,17 @@  vm_fault_t do_swap_page(struct vm_fault *vmf)
 				lru_cache_add_anon(page);
 				swap_readpage(page, true);
 			}
+		} else if (vmf->flags & FAULT_FLAG_SPECULATIVE) {
+			/*
+			 * Don't try readahead during a speculative page fault
+			 * as the VMA's boundaries may change in our back.
+			 * If the page is not in the swap cache and synchronous
+			 * read is disabled, fall back to the regular page
+			 * fault mechanism.
+			 */
+			delayacct_clear_flag(DELAYACCT_PF_SWAPIN);
+			ret = VM_FAULT_RETRY;
+			goto out;
 		} else {
 			page = swapin_readahead(entry, GFP_HIGHUSER_MOVABLE,
 						vmf);