Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1714438 > unrolled thread
| Started by | Laurent Dufour <ldufour@linux.vnet.ibm.com> |
|---|---|
| First post | 2017-08-18 00:10 +0200 |
| Last post | 2017-08-25 11:50 +0200 |
| Articles | 17 on this page of 37 — 9 participants |
Back to article view | Back to linux.kernel
[PATCH v2 00/20] Speculative page faults Laurent Dufour <ldufour@linux.vnet.ibm.com> - 2017-08-18 00:10 +0200
[PATCH v2 10/20] mm: Introduce __lru_cache_add_active_or_unevictable Laurent Dufour <ldufour@linux.vnet.ibm.com> - 2017-08-18 00:20 +0200
[PATCH v2 12/20] mm: Introduce __vm_normal_page() Laurent Dufour <ldufour@linux.vnet.ibm.com> - 2017-08-18 00:20 +0200
[PATCH v2 09/20] mm/migrate: Pass vm_fault pointer to migrate_misplaced_page() Laurent Dufour <ldufour@linux.vnet.ibm.com> - 2017-08-18 00:20 +0200
[PATCH v2 14/20] mm: Provide speculative fault infrastructure Laurent Dufour <ldufour@linux.vnet.ibm.com> - 2017-08-18 00:20 +0200
Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure Sergey Senozhatsky <sergey.senozhatsky@gmail.com> - 2017-08-20 14:20 +0200
Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure Laurent Dufour <ldufour@linux.vnet.ibm.com> - 2017-08-25 11:00 +0200
Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure "Kirill A. Shutemov" <kirill@shutemov.name> - 2017-08-27 02:20 +0200
Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure Peter Zijlstra <peterz@infradead.org> - 2017-08-28 11:40 +0200
Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-08-28 23:30 +0200
Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure Andi Kleen <ak@linux.intel.com> - 2017-08-29 00:40 +0200
Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure Peter Zijlstra <peterz@infradead.org> - 2017-08-29 10:20 +0200
Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure Peter Zijlstra <peterz@infradead.org> - 2017-08-29 10:40 +0200
Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure Peter Zijlstra <peterz@infradead.org> - 2017-08-29 13:30 +0200
Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure Benjamin Herrenschmidt <benh@kernel.crashing.org> - 2017-08-30 00:00 +0200
Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure Peter Zijlstra <peterz@infradead.org> - 2017-08-30 08:20 +0200
Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure Laurent Dufour <ldufour@linux.vnet.ibm.com> - 2017-08-29 10:00 +0200
Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure Peter Zijlstra <peterz@infradead.org> - 2017-08-29 14:10 +0200
Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure Laurent Dufour <ldufour@linux.vnet.ibm.com> - 2017-08-29 15:20 +0200
Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure Peter Zijlstra <peterz@infradead.org> - 2017-08-29 15:50 +0200
Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2017-08-30 07:10 +0200
Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure Peter Zijlstra <peterz@infradead.org> - 2017-08-30 08:00 +0200
Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure Laurent Dufour <ldufour@linux.vnet.ibm.com> - 2017-08-30 11:40 +0200
Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2017-08-31 09:00 +0200
Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure Peter Zijlstra <peterz@infradead.org> - 2017-08-31 09:40 +0200
Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure Laurent Dufour <ldufour@linux.vnet.ibm.com> - 2017-08-30 12:00 +0200
Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2017-08-30 05:50 +0200
Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2017-08-30 07:30 +0200
Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure Laurent Dufour <ldufour@linux.vnet.ibm.com> - 2017-08-30 11:00 +0200
[PATCH v2 02/20] mm: Prepare for FAULT_FLAG_SPECULATIVE Laurent Dufour <ldufour@linux.vnet.ibm.com> - 2017-08-18 00:20 +0200
[PATCH v2 06/20] mm: RCU free VMAs Laurent Dufour <ldufour@linux.vnet.ibm.com> - 2017-08-18 00:20 +0200
[PATCH v2 08/20] mm: Protect SPF handler against anon_vma changes Laurent Dufour <ldufour@linux.vnet.ibm.com> - 2017-08-18 00:20 +0200
Re: [PATCH v2 00/20] Speculative page faults Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> - 2017-08-21 04:30 +0200
Re: [PATCH v2 00/20] Speculative page faults Laurent Dufour <ldufour@linux.vnet.ibm.com> - 2017-09-08 11:30 +0200
Re: [PATCH v2 00/20] Speculative page faults Anshuman Khandual <khandual@linux.vnet.ibm.com> - 2017-08-21 08:30 +0200
Re: [PATCH v2 00/20] Speculative page faults "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> - 2017-08-22 02:50 +0200
Re: [PATCH v2 00/20] Speculative page faults Laurent Dufour <ldufour@linux.vnet.ibm.com> - 2017-08-25 11:50 +0200
Page 2 of 2 — ← Prev page 1 [2]
| From | Anshuman Khandual <khandual@linux.vnet.ibm.com> |
|---|---|
| Date | 2017-08-30 07:10 +0200 |
| Subject | Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure |
| Message-ID | <uk3qh-44H-3@gated-at.bofh.it> |
| In reply to | #1722500 |
On 08/29/2017 07:15 PM, Peter Zijlstra wrote:
> On Tue, Aug 29, 2017 at 03:18:25PM +0200, Laurent Dufour wrote:
>> On 29/08/2017 14:04, Peter Zijlstra wrote:
>>> On Tue, Aug 29, 2017 at 09:59:30AM +0200, Laurent Dufour wrote:
>>>> On 27/08/2017 02:18, Kirill A. Shutemov wrote:
>>>>>> +
>>>>>> + if (unlikely(!vma->anon_vma))
>>>>>> + goto unlock;
>>>>>
>>>>> It deserves a comment.
>>>>
>>>> You're right I'll add it in the next version.
>>>> For the record, the root cause is that __anon_vma_prepare() requires the
>>>> mmap_sem to be held because vm_next and vm_prev must be safe.
>>>
>>> But should that test not be:
>>>
>>> if (unlikely(vma_is_anonymous(vma) && !vma->anon_vma))
>>> goto unlock;
>>>
>>> Because !anon vmas will never have ->anon_vma set and you don't want to
>>> exclude those.
>>
>> Yes in the case we later allow non anonymous vmas to be handled.
>> Currently only anonymous vmas are supported so the check is good enough,
>> isn't it ?
>
> That wasn't at all clear from reading the code. This makes it clear
> ->anon_vma is only ever looked at for anonymous.
>
> And like Kirill says, we _really_ should start allowing some (if not
> all) vm_ops. Large file based mappings aren't particularly rare.
>
> I'm not sure we want to introduce a white-list or just bite the bullet
> and audit all ->fault() implementations. But either works and isn't
> terribly difficult, auditing all is more work though.
filemap_fault() is used as vma-vm_ops->fault() for most of the file
systems. Changing it can enable speculative fault support for all of
them. It will still exclude other driver based vma-vm_ops->fault()
implementation. AFAICS, __lock_page_or_retry() function can drop
mm->mmap_sem if the page could not be locked right away. As suggested
by Peterz, making it understand FAULT_FLAG_SPECULATIVE should be good
enough. The patch is lightly tested for file mappings on top of this
series.
diff --git a/mm/filemap.c b/mm/filemap.c
index a497024..08f3042 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1181,6 +1181,18 @@ int __lock_page_killable(struct page *__page)
int __lock_page_or_retry(struct page *page, struct mm_struct *mm,
unsigned int flags)
{
+ if (flags & FAULT_FLAG_SPECULATIVE) {
+ if (flags & FAULT_FLAG_KILLABLE) {
+ int ret;
+
+ ret = __lock_page_killable(page);
+ if (ret)
+ return 0;
+ } else
+ __lock_page(page);
+ return 1;
+ }
+
if (flags & FAULT_FLAG_ALLOW_RETRY) {
/*
* CAUTION! In this case, mmap_sem is not released
diff --git a/mm/memory.c b/mm/memory.c
index 549d235..02347f3 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3836,8 +3836,6 @@ static int handle_pte_fault(struct vm_fault *vmf)
if (!vmf->pte) {
if (vma_is_anonymous(vmf->vma))
return do_anonymous_page(vmf);
- else if (vmf->flags & FAULT_FLAG_SPECULATIVE)
- return VM_FAULT_RETRY;
else
return do_fault(vmf);
}
@@ -4012,17 +4010,7 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,
goto unlock;
}
- /*
- * Can't call vm_ops service has we don't know what they would do
- * with the VMA.
- * This include huge page from hugetlbfs.
- */
- if (vma->vm_ops) {
- trace_spf_vma_notsup(_RET_IP_, vma, address);
- goto unlock;
- }
-
- if (unlikely(!vma->anon_vma)) {
+ if (unlikely(vma_is_anonymous(vma) && !vma->anon_vma)) {
trace_spf_vma_notsup(_RET_IP_, vma, address);
goto unlock;
}
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-08-30 08:00 +0200 |
| Subject | Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure |
| Message-ID | <uk4cH-4mT-11@gated-at.bofh.it> |
| In reply to | #1723022 |
On Wed, Aug 30, 2017 at 10:33:50AM +0530, Anshuman Khandual wrote:
> diff --git a/mm/filemap.c b/mm/filemap.c
> index a497024..08f3042 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -1181,6 +1181,18 @@ int __lock_page_killable(struct page *__page)
> int __lock_page_or_retry(struct page *page, struct mm_struct *mm,
> unsigned int flags)
> {
> + if (flags & FAULT_FLAG_SPECULATIVE) {
> + if (flags & FAULT_FLAG_KILLABLE) {
> + int ret;
> +
> + ret = __lock_page_killable(page);
> + if (ret)
> + return 0;
> + } else
> + __lock_page(page);
> + return 1;
> + }
> +
> if (flags & FAULT_FLAG_ALLOW_RETRY) {
> /*
> * CAUTION! In this case, mmap_sem is not released
Yeah, that looks right.
> @@ -4012,17 +4010,7 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,
> goto unlock;
> }
>
> + if (unlikely(vma_is_anonymous(vma) && !vma->anon_vma)) {
> trace_spf_vma_notsup(_RET_IP_, vma, address);
> goto unlock;
> }
As riel pointed out on IRC slightly later, private file maps also need
->anon_vma and those actually have ->vm_ops IIRC so the condition needs
to be slightly more complicated.
[toc] | [prev] | [next] | [standalone]
| From | Laurent Dufour <ldufour@linux.vnet.ibm.com> |
|---|---|
| Date | 2017-08-30 11:40 +0200 |
| Subject | Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure |
| Message-ID | <uk7Dz-6yY-5@gated-at.bofh.it> |
| In reply to | #1723043 |
On 30/08/2017 07:58, Peter Zijlstra wrote:
> On Wed, Aug 30, 2017 at 10:33:50AM +0530, Anshuman Khandual wrote:
>> diff --git a/mm/filemap.c b/mm/filemap.c
>> index a497024..08f3042 100644
>> --- a/mm/filemap.c
>> +++ b/mm/filemap.c
>> @@ -1181,6 +1181,18 @@ int __lock_page_killable(struct page *__page)
>> int __lock_page_or_retry(struct page *page, struct mm_struct *mm,
>> unsigned int flags)
>> {
>> + if (flags & FAULT_FLAG_SPECULATIVE) {
>> + if (flags & FAULT_FLAG_KILLABLE) {
>> + int ret;
>> +
>> + ret = __lock_page_killable(page);
>> + if (ret)
>> + return 0;
>> + } else
>> + __lock_page(page);
>> + return 1;
>> + }
>> +
>> if (flags & FAULT_FLAG_ALLOW_RETRY) {
>> /*
>> * CAUTION! In this case, mmap_sem is not released
>
> Yeah, that looks right.
Hum, I'm wondering if FAULT_FLAG_RETRY_NOWAIT should be forced in the
speculative path in that case to match the semantics of
__lock_page_or_retry().
>
>> @@ -4012,17 +4010,7 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,
>> goto unlock;
>> }
>>
>> + if (unlikely(vma_is_anonymous(vma) && !vma->anon_vma)) {
>> trace_spf_vma_notsup(_RET_IP_, vma, address);
>> goto unlock;
>> }
>
> As riel pointed out on IRC slightly later, private file maps also need
> ->anon_vma and those actually have ->vm_ops IIRC so the condition needs
> to be slightly more complicated.
Yes I read again the code and lead to the same conclusion.
[toc] | [prev] | [next] | [standalone]
| From | Anshuman Khandual <khandual@linux.vnet.ibm.com> |
|---|---|
| Date | 2017-08-31 09:00 +0200 |
| Subject | Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure |
| Message-ID | <ukrCi-2fq-7@gated-at.bofh.it> |
| In reply to | #1723156 |
On 08/30/2017 03:02 PM, Laurent Dufour wrote:
> On 30/08/2017 07:58, Peter Zijlstra wrote:
>> On Wed, Aug 30, 2017 at 10:33:50AM +0530, Anshuman Khandual wrote:
>>> diff --git a/mm/filemap.c b/mm/filemap.c
>>> index a497024..08f3042 100644
>>> --- a/mm/filemap.c
>>> +++ b/mm/filemap.c
>>> @@ -1181,6 +1181,18 @@ int __lock_page_killable(struct page *__page)
>>> int __lock_page_or_retry(struct page *page, struct mm_struct *mm,
>>> unsigned int flags)
>>> {
>>> + if (flags & FAULT_FLAG_SPECULATIVE) {
>>> + if (flags & FAULT_FLAG_KILLABLE) {
>>> + int ret;
>>> +
>>> + ret = __lock_page_killable(page);
>>> + if (ret)
>>> + return 0;
>>> + } else
>>> + __lock_page(page);
>>> + return 1;
>>> + }
>>> +
>>> if (flags & FAULT_FLAG_ALLOW_RETRY) {
>>> /*
>>> * CAUTION! In this case, mmap_sem is not released
>>
>> Yeah, that looks right.
>
> Hum, I'm wondering if FAULT_FLAG_RETRY_NOWAIT should be forced in the
> speculative path in that case to match the semantics of
> __lock_page_or_retry().
Doing that would force us to have another retry through classic fault
path wasting all the work done till now through SPF. Hence it may be
better to just wait, get the lock here and complete the fault. Peterz,
would you agree ? Or we should do as suggested by Laurent. More over,
forcing FAULT_FLAG_RETRY_NOWAIT on FAULT_FLAG_SPECULTIVE at this point
would look like a hack.
[toc] | [prev] | [next] | [standalone]
| From | Peter Zijlstra <peterz@infradead.org> |
|---|---|
| Date | 2017-08-31 09:40 +0200 |
| Subject | Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure |
| Message-ID | <uksf2-2Jn-49@gated-at.bofh.it> |
| In reply to | #1723876 |
On Thu, Aug 31, 2017 at 12:25:16PM +0530, Anshuman Khandual wrote:
> On 08/30/2017 03:02 PM, Laurent Dufour wrote:
> > On 30/08/2017 07:58, Peter Zijlstra wrote:
> >> On Wed, Aug 30, 2017 at 10:33:50AM +0530, Anshuman Khandual wrote:
> >>> diff --git a/mm/filemap.c b/mm/filemap.c
> >>> index a497024..08f3042 100644
> >>> --- a/mm/filemap.c
> >>> +++ b/mm/filemap.c
> >>> @@ -1181,6 +1181,18 @@ int __lock_page_killable(struct page *__page)
> >>> int __lock_page_or_retry(struct page *page, struct mm_struct *mm,
> >>> unsigned int flags)
> >>> {
> >>> + if (flags & FAULT_FLAG_SPECULATIVE) {
> >>> + if (flags & FAULT_FLAG_KILLABLE) {
> >>> + int ret;
> >>> +
> >>> + ret = __lock_page_killable(page);
> >>> + if (ret)
> >>> + return 0;
> >>> + } else
> >>> + __lock_page(page);
> >>> + return 1;
> >>> + }
> >>> +
> >>> if (flags & FAULT_FLAG_ALLOW_RETRY) {
> >>> /*
> >>> * CAUTION! In this case, mmap_sem is not released
> >>
> >> Yeah, that looks right.
> >
> > Hum, I'm wondering if FAULT_FLAG_RETRY_NOWAIT should be forced in the
> > speculative path in that case to match the semantics of
> > __lock_page_or_retry().
>
> Doing that would force us to have another retry through classic fault
> path wasting all the work done till now through SPF. Hence it may be
> better to just wait, get the lock here and complete the fault. Peterz,
> would you agree ? Or we should do as suggested by Laurent. More over,
> forcing FAULT_FLAG_RETRY_NOWAIT on FAULT_FLAG_SPECULTIVE at this point
> would look like a hack.
Is there ever a situation where SPECULATIVE and NOWAIT are used
together? That seems like something to avoid.
A git-grep seems to suggest gup() can set it, but gup() will not be
doing speculative faults. s390 also sets it, but then again, they don't
have speculative fault support yet and when they do they can avoid
setting them together.
So maybe put in a WARN_ON_ONCE() on having both of them, it is not
something that makes sense to me, but maybe someone sees a rationale for
it?
[toc] | [prev] | [next] | [standalone]
| From | Laurent Dufour <ldufour@linux.vnet.ibm.com> |
|---|---|
| Date | 2017-08-30 12:00 +0200 |
| Subject | Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure |
| Message-ID | <uk7WW-6Hb-19@gated-at.bofh.it> |
| In reply to | #1723022 |
On 30/08/2017 07:03, Anshuman Khandual wrote:
> On 08/29/2017 07:15 PM, Peter Zijlstra wrote:
>> On Tue, Aug 29, 2017 at 03:18:25PM +0200, Laurent Dufour wrote:
>>> On 29/08/2017 14:04, Peter Zijlstra wrote:
>>>> On Tue, Aug 29, 2017 at 09:59:30AM +0200, Laurent Dufour wrote:
>>>>> On 27/08/2017 02:18, Kirill A. Shutemov wrote:
>>>>>>> +
>>>>>>> + if (unlikely(!vma->anon_vma))
>>>>>>> + goto unlock;
>>>>>>
>>>>>> It deserves a comment.
>>>>>
>>>>> You're right I'll add it in the next version.
>>>>> For the record, the root cause is that __anon_vma_prepare() requires the
>>>>> mmap_sem to be held because vm_next and vm_prev must be safe.
>>>>
>>>> But should that test not be:
>>>>
>>>> if (unlikely(vma_is_anonymous(vma) && !vma->anon_vma))
>>>> goto unlock;
>>>>
>>>> Because !anon vmas will never have ->anon_vma set and you don't want to
>>>> exclude those.
>>>
>>> Yes in the case we later allow non anonymous vmas to be handled.
>>> Currently only anonymous vmas are supported so the check is good enough,
>>> isn't it ?
>>
>> That wasn't at all clear from reading the code. This makes it clear
>> ->anon_vma is only ever looked at for anonymous.
>>
>> And like Kirill says, we _really_ should start allowing some (if not
>> all) vm_ops. Large file based mappings aren't particularly rare.
>>
>> I'm not sure we want to introduce a white-list or just bite the bullet
>> and audit all ->fault() implementations. But either works and isn't
>> terribly difficult, auditing all is more work though.
>
> filemap_fault() is used as vma-vm_ops->fault() for most of the file
> systems. Changing it can enable speculative fault support for all of
> them. It will still exclude other driver based vma-vm_ops->fault()
> implementation. AFAICS, __lock_page_or_retry() function can drop
> mm->mmap_sem if the page could not be locked right away. As suggested
> by Peterz, making it understand FAULT_FLAG_SPECULATIVE should be good
> enough. The patch is lightly tested for file mappings on top of this
> series.
Hi Anshuman,
This sounds pretty good, except for the FAULT_FLAG_RETRY_NOWAIT's case I
mentioned in another mail.
The next step would be to find a way to discriminate between the vm_fault()
functions. Any idea ?
Thanks,
Laurent.
>
> diff --git a/mm/filemap.c b/mm/filemap.c
> index a497024..08f3042 100644
> --- a/mm/filemap.c
> +++ b/mm/filemap.c
> @@ -1181,6 +1181,18 @@ int __lock_page_killable(struct page *__page)
> int __lock_page_or_retry(struct page *page, struct mm_struct *mm,
> unsigned int flags)
> {
> + if (flags & FAULT_FLAG_SPECULATIVE) {
> + if (flags & FAULT_FLAG_KILLABLE) {
> + int ret;
> +
> + ret = __lock_page_killable(page);
> + if (ret)
> + return 0;
> + } else
> + __lock_page(page);
> + return 1;
> + }
> +
> if (flags & FAULT_FLAG_ALLOW_RETRY) {
> /*
> * CAUTION! In this case, mmap_sem is not released
> diff --git a/mm/memory.c b/mm/memory.c
> index 549d235..02347f3 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -3836,8 +3836,6 @@ static int handle_pte_fault(struct vm_fault *vmf)
> if (!vmf->pte) {
> if (vma_is_anonymous(vmf->vma))
> return do_anonymous_page(vmf);
> - else if (vmf->flags & FAULT_FLAG_SPECULATIVE)
> - return VM_FAULT_RETRY;
> else
> return do_fault(vmf);
> }
> @@ -4012,17 +4010,7 @@ int handle_speculative_fault(struct mm_struct *mm, unsigned long address,
> goto unlock;
> }
>
> - /*
> - * Can't call vm_ops service has we don't know what they would do
> - * with the VMA.
> - * This include huge page from hugetlbfs.
> - */
> - if (vma->vm_ops) {
> - trace_spf_vma_notsup(_RET_IP_, vma, address);
> - goto unlock;
> - }
> -
> - if (unlikely(!vma->anon_vma)) {
> + if (unlikely(vma_is_anonymous(vma) && !vma->anon_vma)) {
> trace_spf_vma_notsup(_RET_IP_, vma, address);
> goto unlock;
> }
>
[toc] | [prev] | [next] | [standalone]
| From | Anshuman Khandual <khandual@linux.vnet.ibm.com> |
|---|---|
| Date | 2017-08-30 05:50 +0200 |
| Subject | Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure |
| Message-ID | <uk2aR-38o-3@gated-at.bofh.it> |
| In reply to | #1722422 |
On 08/29/2017 05:34 PM, Peter Zijlstra wrote: > On Tue, Aug 29, 2017 at 09:59:30AM +0200, Laurent Dufour wrote: >> On 27/08/2017 02:18, Kirill A. Shutemov wrote: >>>> + >>>> + if (unlikely(!vma->anon_vma)) >>>> + goto unlock; >>> It deserves a comment. >> You're right I'll add it in the next version. >> For the record, the root cause is that __anon_vma_prepare() requires the >> mmap_sem to be held because vm_next and vm_prev must be safe. > But should that test not be: > > if (unlikely(vma_is_anonymous(vma) && !vma->anon_vma)) > goto unlock; This makes more sense. We are backing off from speculative path because struct anon_vma has not been created for this anonymous vma and we cannot do that without holding mmap_sem. This should have nothing to do with vma->vm_ops availability.
[toc] | [prev] | [next] | [standalone]
| From | Anshuman Khandual <khandual@linux.vnet.ibm.com> |
|---|---|
| Date | 2017-08-30 07:30 +0200 |
| Subject | Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure |
| Message-ID | <uk3JD-4bg-1@gated-at.bofh.it> |
| In reply to | #1720719 |
On 08/27/2017 05:48 AM, Kirill A. Shutemov wrote: >> + /* Transparent huge pages are not supported. */ >> + if (unlikely(pmd_trans_huge(*pmd))) >> + goto out_walk; > That's looks like a blocker to me. > > Is there any problem with making it supported (besides plain coding)? IIUC we would have to reattempt once for each PMD level fault because of the lack of a page table entry there. Besides do we want to support huge pages in general as part of speculative page fault path ? The number of faults will be very less (256 times lower on POWER and 512 times lower on X86). So is it worth it ? BTW calling hugetlb_fault() after figuring out the VMA, works correctly inside handle_speculative _fault() last time I checked.
[toc] | [prev] | [next] | [standalone]
| From | Laurent Dufour <ldufour@linux.vnet.ibm.com> |
|---|---|
| Date | 2017-08-30 11:00 +0200 |
| Subject | Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure |
| Message-ID | <uk70S-66H-37@gated-at.bofh.it> |
| In reply to | #1720719 |
On 27/08/2017 02:18, Kirill A. Shutemov wrote:
> On Fri, Aug 18, 2017 at 12:05:13AM +0200, Laurent Dufour wrote:
>> +/*
>> + * vm_normal_page() adds some processing which should be done while
>> + * hodling the mmap_sem.
>> + */
>> +int handle_speculative_fault(struct mm_struct *mm, unsigned long address,
>> + unsigned int flags)
>> +{
>> + struct vm_fault vmf = {
>> + .address = address,
>> + };
>> + pgd_t *pgd;
>> + p4d_t *p4d;
>> + pud_t *pud;
>> + pmd_t *pmd;
>> + int dead, seq, idx, ret = VM_FAULT_RETRY;
>> + struct vm_area_struct *vma;
>> + struct mempolicy *pol;
>> +
>> + /* Clear flags that may lead to release the mmap_sem to retry */
>> + flags &= ~(FAULT_FLAG_ALLOW_RETRY|FAULT_FLAG_KILLABLE);
>> + flags |= FAULT_FLAG_SPECULATIVE;
>> +
>> + idx = srcu_read_lock(&vma_srcu);
>> + vma = find_vma_srcu(mm, address);
>> + if (!vma)
>> + goto unlock;
>> +
>> + /*
>> + * Validate the VMA found by the lockless lookup.
>> + */
>> + dead = RB_EMPTY_NODE(&vma->vm_rb);
>> + seq = raw_read_seqcount(&vma->vm_sequence); /* rmb <-> seqlock,vma_rb_erase() */
>> + if ((seq & 1) || dead)
>> + goto unlock;
>> +
>> + /*
>> + * Can't call vm_ops service has we don't know what they would do
>> + * with the VMA.
>> + * This include huge page from hugetlbfs.
>> + */
>> + if (vma->vm_ops)
>> + goto unlock;
>
> I think we need to have a way to white-list safe ->vm_ops.
>
>> +
>> + if (unlikely(!vma->anon_vma))
>> + goto unlock;
>
> It deserves a comment.
>
>> +
>> + vmf.vma_flags = READ_ONCE(vma->vm_flags);
>> + vmf.vma_page_prot = READ_ONCE(vma->vm_page_prot);
>> +
>> + /* Can't call userland page fault handler in the speculative path */
>> + if (unlikely(vmf.vma_flags & VM_UFFD_MISSING))
>> + goto unlock;
>> +
>> + /*
>> + * MPOL_INTERLEAVE implies additional check in mpol_misplaced() which
>> + * are not compatible with the speculative page fault processing.
>> + */
>> + pol = __get_vma_policy(vma, address);
>> + if (!pol)
>> + pol = get_task_policy(current);
>> + if (pol && pol->mode == MPOL_INTERLEAVE)
>> + goto unlock;
>> +
>> + if (vmf.vma_flags & VM_GROWSDOWN || vmf.vma_flags & VM_GROWSUP)
>> + /*
>> + * This could be detected by the check address against VMA's
>> + * boundaries but we want to trace it as not supported instead
>> + * of changed.
>> + */
>> + goto unlock;
>> +
>> + if (address < READ_ONCE(vma->vm_start)
>> + || READ_ONCE(vma->vm_end) <= address)
>> + goto unlock;
>> +
>> + /*
>> + * The three following checks are copied from access_error from
>> + * arch/x86/mm/fault.c
>> + */
>> + if (!arch_vma_access_permitted(vma, flags & FAULT_FLAG_WRITE,
>> + flags & FAULT_FLAG_INSTRUCTION,
>> + flags & FAULT_FLAG_REMOTE))
>> + goto unlock;
>> +
>> + /* This is one is required to check that the VMA has write access set */
>> + if (flags & FAULT_FLAG_WRITE) {
>> + if (unlikely(!(vmf.vma_flags & VM_WRITE)))
>> + goto unlock;
>> + } else {
>> + if (unlikely(!(vmf.vma_flags & (VM_READ | VM_EXEC | VM_WRITE))))
>> + goto unlock;
>> + }
>> +
>> + /*
>> + * Do a speculative lookup of the PTE entry.
>> + */
>> + local_irq_disable();
>> + pgd = pgd_offset(mm, address);
>> + if (pgd_none(*pgd) || unlikely(pgd_bad(*pgd)))
>> + goto out_walk;
>> +
>> + p4d = p4d_alloc(mm, pgd, address);
>> + if (p4d_none(*p4d) || unlikely(p4d_bad(*p4d)))
>> + goto out_walk;
>> +
>> + pud = pud_alloc(mm, p4d, address);
>> + if (pud_none(*pud) || unlikely(pud_bad(*pud)))
>> + goto out_walk;
>> +
>> + pmd = pmd_offset(pud, address);
>> + if (pmd_none(*pmd) || unlikely(pmd_bad(*pmd)))
>> + goto out_walk;
>> +
>> + /*
>> + * The above does not allocate/instantiate page-tables because doing so
>> + * would lead to the possibility of instantiating page-tables after
>> + * free_pgtables() -- and consequently leaking them.
>> + *
>> + * The result is that we take at least one !speculative fault per PMD
>> + * in order to instantiate it.
>> + */
>
>
> Doing all this job and just give up because we cannot allocate page tables
> looks very wasteful to me.
>
> Have you considered to look how we can hand over from speculative to
> non-speculative path without starting from scratch (when possible)?
>
>> + /* Transparent huge pages are not supported. */
>> + if (unlikely(pmd_trans_huge(*pmd)))
>> + goto out_walk;
>
> That's looks like a blocker to me.
>
> Is there any problem with making it supported (besides plain coding)?
This is not straight forward, as the THP are mainly handled in
__handle_mm_fault() and it is not called during the speculative path.
Having THP handled in the speculative path sounds doable but I'd have to
double check all the callees deeper, and this will required either
redesigning __handle_mm_fault() or doing the job in a dedicated way in
handle_speculative_fault() .
Furthermore, we should handle both PUD and PMD's level huge pages.
This being said, I can't see any blocking issue at this time except plain
coding but I'd prefer to get it done in a next step, as an optimization,
since huge page's faults are far less frequent per design.
Having _standard_ page's fault handled in a speculative way is already
providing good performance improvement, we should consider having it
upstreamed and then adding support for THP as well as other compatible
vm_ops like hugetlb, isn't it ?
Cheers,
Laurent.
>> +
>> + vmf.vma = vma;
>> + vmf.pmd = pmd;
>> + vmf.pgoff = linear_page_index(vma, address);
>> + vmf.gfp_mask = __get_fault_gfp_mask(vma);
>> + vmf.sequence = seq;
>> + vmf.flags = flags;
>> +
>> + local_irq_enable();
>> +
>> + /*
>> + * We need to re-validate the VMA after checking the bounds, otherwise
>> + * we might have a false positive on the bounds.
>> + */
>> + if (read_seqcount_retry(&vma->vm_sequence, seq))
>> + goto unlock;
>> +
>> + ret = handle_pte_fault(&vmf);
>> +
>> +unlock:
>> + srcu_read_unlock(&vma_srcu, idx);
>> + return ret;
>> +
>> +out_walk:
>> + local_irq_enable();
>> + goto unlock;
>> +}
>> +#endif /* __HAVE_ARCH_CALL_SPF */
>> +
>> /*
>> * By the time we get here, we already hold the mm semaphore
>> *
>> --
>> 2.7.4
>>
>
[toc] | [prev] | [next] | [standalone]
| From | Laurent Dufour <ldufour@linux.vnet.ibm.com> |
|---|---|
| Date | 2017-08-18 00:20 +0200 |
| Subject | [PATCH v2 02/20] mm: Prepare for FAULT_FLAG_SPECULATIVE |
| Message-ID | <ufBiX-2Hx-37@gated-at.bofh.it> |
| In reply to | #1714438 |
From: Peter Zijlstra <peterz@infradead.org>
When speculating faults (without holding mmap_sem) we need to validate
that the vma against which we loaded pages is still valid when we're
ready to install the new PTE.
Therefore, replace the pte_offset_map_lock() calls that (re)take the
PTL with pte_map_lock() which can fail in case we find the VMA changed
since we started the fault.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
[Port to 4.12 kernel]
[Remove the comment about the fault_env structure which has been
implemented as the vm_fault structure in the kernel]
Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
include/linux/mm.h | 1 +
mm/memory.c | 55 ++++++++++++++++++++++++++++++++++++++----------------
2 files changed, 40 insertions(+), 16 deletions(-)
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 46b9ac5e8569..8763ec96dc78 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -286,6 +286,7 @@ extern pgprot_t protection_map[16];
#define FAULT_FLAG_USER 0x40 /* The fault originated in userspace */
#define FAULT_FLAG_REMOTE 0x80 /* faulting for non current tsk/mm */
#define FAULT_FLAG_INSTRUCTION 0x100 /* The fault was during an instruction fetch */
+#define FAULT_FLAG_SPECULATIVE 0x200 /* Speculative fault, not holding mmap_sem */
#define FAULT_FLAG_TRACE \
{ FAULT_FLAG_WRITE, "WRITE" }, \
diff --git a/mm/memory.c b/mm/memory.c
index 36609c082256..3ed1b00ca841 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2269,6 +2269,12 @@ static inline void wp_page_reuse(struct vm_fault *vmf)
pte_unmap_unlock(vmf->pte, vmf->ptl);
}
+static bool pte_map_lock(struct vm_fault *vmf)
+{
+ vmf->pte = pte_offset_map_lock(vmf->vma->vm_mm, vmf->pmd, vmf->address, &vmf->ptl);
+ return true;
+}
+
/*
* Handle the case of a page which we actually need to copy to a new page.
*
@@ -2296,6 +2302,7 @@ static int wp_page_copy(struct vm_fault *vmf)
const unsigned long mmun_start = vmf->address & PAGE_MASK;
const unsigned long mmun_end = mmun_start + PAGE_SIZE;
struct mem_cgroup *memcg;
+ int ret = VM_FAULT_OOM;
if (unlikely(anon_vma_prepare(vma)))
goto oom;
@@ -2323,7 +2330,11 @@ static int wp_page_copy(struct vm_fault *vmf)
/*
* Re-check the pte - we dropped the lock
*/
- vmf->pte = pte_offset_map_lock(mm, vmf->pmd, vmf->address, &vmf->ptl);
+ if (!pte_map_lock(vmf)) {
+ mem_cgroup_cancel_charge(new_page, memcg, false);
+ ret = VM_FAULT_RETRY;
+ goto oom_free_new;
+ }
if (likely(pte_same(*vmf->pte, vmf->orig_pte))) {
if (old_page) {
if (!PageAnon(old_page)) {
@@ -2411,7 +2422,7 @@ static int wp_page_copy(struct vm_fault *vmf)
oom:
if (old_page)
put_page(old_page);
- return VM_FAULT_OOM;
+ return ret;
}
/**
@@ -2432,8 +2443,8 @@ static int wp_page_copy(struct vm_fault *vmf)
int finish_mkwrite_fault(struct vm_fault *vmf)
{
WARN_ON_ONCE(!(vmf->vma->vm_flags & VM_SHARED));
- vmf->pte = pte_offset_map_lock(vmf->vma->vm_mm, vmf->pmd, vmf->address,
- &vmf->ptl);
+ if (!pte_map_lock(vmf))
+ return VM_FAULT_RETRY;
/*
* We might have raced with another page fault while we released the
* pte_offset_map_lock.
@@ -2551,8 +2562,11 @@ static int do_wp_page(struct vm_fault *vmf)
get_page(vmf->page);
pte_unmap_unlock(vmf->pte, vmf->ptl);
lock_page(vmf->page);
- vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
- vmf->address, &vmf->ptl);
+ if (!pte_map_lock(vmf)) {
+ unlock_page(vmf->page);
+ put_page(vmf->page);
+ return VM_FAULT_RETRY;
+ }
if (!pte_same(*vmf->pte, vmf->orig_pte)) {
unlock_page(vmf->page);
pte_unmap_unlock(vmf->pte, vmf->ptl);
@@ -2710,8 +2724,10 @@ int do_swap_page(struct vm_fault *vmf)
* Back out if somebody else faulted in this pte
* while we released the pte lock.
*/
- vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
- vmf->address, &vmf->ptl);
+ if (!pte_map_lock(vmf)) {
+ delayacct_clear_flag(DELAYACCT_PF_SWAPIN);
+ return VM_FAULT_RETRY;
+ }
if (likely(pte_same(*vmf->pte, vmf->orig_pte)))
ret = VM_FAULT_OOM;
delayacct_clear_flag(DELAYACCT_PF_SWAPIN);
@@ -2767,8 +2783,11 @@ int do_swap_page(struct vm_fault *vmf)
/*
* Back out if somebody else already faulted in this pte.
*/
- vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd, vmf->address,
- &vmf->ptl);
+ if (!pte_map_lock(vmf)) {
+ ret = VM_FAULT_RETRY;
+ mem_cgroup_cancel_charge(page, memcg, false);
+ goto out_page;
+ }
if (unlikely(!pte_same(*vmf->pte, vmf->orig_pte)))
goto out_nomap;
@@ -2894,8 +2913,8 @@ static int do_anonymous_page(struct vm_fault *vmf)
!mm_forbids_zeropage(vma->vm_mm)) {
entry = pte_mkspecial(pfn_pte(my_zero_pfn(vmf->address),
vma->vm_page_prot));
- vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd,
- vmf->address, &vmf->ptl);
+ if (!pte_map_lock(vmf))
+ return VM_FAULT_RETRY;
if (!pte_none(*vmf->pte))
goto unlock;
/* Deliver the page fault to userland, check inside PT lock */
@@ -2927,8 +2946,11 @@ static int do_anonymous_page(struct vm_fault *vmf)
if (vma->vm_flags & VM_WRITE)
entry = pte_mkwrite(pte_mkdirty(entry));
- vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd, vmf->address,
- &vmf->ptl);
+ if (!pte_map_lock(vmf)) {
+ mem_cgroup_cancel_charge(page, memcg, false);
+ put_page(page);
+ return VM_FAULT_RETRY;
+ }
if (!pte_none(*vmf->pte))
goto release;
@@ -3048,8 +3070,9 @@ static int pte_alloc_one_map(struct vm_fault *vmf)
* pte_none() under vmf->ptl protection when we return to
* alloc_set_pte().
*/
- vmf->pte = pte_offset_map_lock(vma->vm_mm, vmf->pmd, vmf->address,
- &vmf->ptl);
+ if (!pte_map_lock(vmf))
+ return VM_FAULT_RETRY;
+
return 0;
}
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Laurent Dufour <ldufour@linux.vnet.ibm.com> |
|---|---|
| Date | 2017-08-18 00:20 +0200 |
| Subject | [PATCH v2 06/20] mm: RCU free VMAs |
| Message-ID | <ufBiX-2Hx-43@gated-at.bofh.it> |
| In reply to | #1714438 |
From: Peter Zijlstra <peterz@infradead.org>
Manage the VMAs with SRCU such that we can do a lockless VMA lookup.
We put the fput(vma->vm_file) in the SRCU callback, this keeps files
valid during speculative faults, this is possible due to the delayed
fput work by Al Viro -- do we need srcu_barrier() in unmount
someplace?
We guard the mm_rb tree with a seqlock (this could be a seqcount but
we'd have to disable preemption around the write side in order to make
the retry loop in __read_seqcount_begin() work) such that we can know
if the rb tree walk was correct. We cannot trust the restult of a
lockless tree walk in the face of concurrent tree rotations; although
we can trust on the termination of such walks -- tree rotations
guarantee the end result is a tree again after all.
Furthermore, we rely on the WMB implied by the
write_seqlock/count_begin() to separate the VMA initialization and the
publishing stores, analogous to the RELEASE in rcu_assign_pointer().
We also rely on the RMB from read_seqretry() to separate the vma load
from further loads like the smp_read_barrier_depends() in regular
RCU.
We must not touch the vmacache while doing SRCU lookups as that is not
properly serialized against changes. We update gap information after
publishing the VMA, but A) we don't use that and B) the seqlock
read side would fix that anyhow.
We clear vma->vm_rb for nodes removed from the vma tree such that we
can easily detect such 'dead' nodes, we rely on the WMB from
write_sequnlock() to separate the tree removal and clearing the node.
Provide find_vma_srcu() which wraps the required magic.
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
[Remove the warnings in description about the SRCU global lock which
has been removed now]
[Rename vma_is_dead() to vma_has_changed() and move its adding to the next
patch]
Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
include/linux/mm_types.h | 2 +
kernel/fork.c | 1 +
mm/init-mm.c | 1 +
mm/internal.h | 5 +++
mm/mmap.c | 100 +++++++++++++++++++++++++++++++++++------------
5 files changed, 83 insertions(+), 26 deletions(-)
diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 642aad26b32f..f3851b250fde 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -343,6 +343,7 @@ struct vm_area_struct {
#endif
struct vm_userfaultfd_ctx vm_userfaultfd_ctx;
seqcount_t vm_sequence;
+ struct rcu_head vm_rcu_head;
} __randomize_layout;
struct core_thread {
@@ -360,6 +361,7 @@ struct kioctx_table;
struct mm_struct {
struct vm_area_struct *mmap; /* list of VMAs */
struct rb_root mm_rb;
+ seqlock_t mm_seq;
u32 vmacache_seqnum; /* per-thread vmacache */
#ifdef CONFIG_MMU
unsigned long (*get_unmapped_area) (struct file *filp,
diff --git a/kernel/fork.c b/kernel/fork.c
index e075b7780421..f28aa54c668c 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -791,6 +791,7 @@ static struct mm_struct *mm_init(struct mm_struct *mm, struct task_struct *p,
mm->mmap = NULL;
mm->mm_rb = RB_ROOT;
mm->vmacache_seqnum = 0;
+ seqlock_init(&mm->mm_seq);
atomic_set(&mm->mm_users, 1);
atomic_set(&mm->mm_count, 1);
init_rwsem(&mm->mmap_sem);
diff --git a/mm/init-mm.c b/mm/init-mm.c
index 975e49f00f34..2b1fa061684f 100644
--- a/mm/init-mm.c
+++ b/mm/init-mm.c
@@ -16,6 +16,7 @@
struct mm_struct init_mm = {
.mm_rb = RB_ROOT,
+ .mm_seq = __SEQLOCK_UNLOCKED(init_mm.mm_seq),
.pgd = swapper_pg_dir,
.mm_users = ATOMIC_INIT(2),
.mm_count = ATOMIC_INIT(1),
diff --git a/mm/internal.h b/mm/internal.h
index 4ef49fc55e58..736540f15936 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -40,6 +40,11 @@ void page_writeback_init(void);
int do_swap_page(struct vm_fault *vmf);
+extern struct srcu_struct vma_srcu;
+
+extern struct vm_area_struct *find_vma_srcu(struct mm_struct *mm,
+ unsigned long addr);
+
void free_pgtables(struct mmu_gather *tlb, struct vm_area_struct *start_vma,
unsigned long floor, unsigned long ceiling);
diff --git a/mm/mmap.c b/mm/mmap.c
index b480043e38fb..34a7f1bdffe4 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -159,6 +159,23 @@ void unlink_file_vma(struct vm_area_struct *vma)
}
}
+DEFINE_SRCU(vma_srcu);
+
+static void __free_vma(struct rcu_head *head)
+{
+ struct vm_area_struct *vma =
+ container_of(head, struct vm_area_struct, vm_rcu_head);
+
+ if (vma->vm_file)
+ fput(vma->vm_file);
+ kmem_cache_free(vm_area_cachep, vma);
+}
+
+static void free_vma(struct vm_area_struct *vma)
+{
+ call_srcu(&vma_srcu, &vma->vm_rcu_head, __free_vma);
+}
+
/*
* Close a vm structure and free it, returning the next.
*/
@@ -169,10 +186,8 @@ static struct vm_area_struct *remove_vma(struct vm_area_struct *vma)
might_sleep();
if (vma->vm_ops && vma->vm_ops->close)
vma->vm_ops->close(vma);
- if (vma->vm_file)
- fput(vma->vm_file);
mpol_put(vma_policy(vma));
- kmem_cache_free(vm_area_cachep, vma);
+ free_vma(vma);
return next;
}
@@ -410,26 +425,37 @@ static void vma_gap_update(struct vm_area_struct *vma)
}
static inline void vma_rb_insert(struct vm_area_struct *vma,
- struct rb_root *root)
+ struct mm_struct *mm)
{
+ struct rb_root *root = &mm->mm_rb;
+
/* All rb_subtree_gap values must be consistent prior to insertion */
validate_mm_rb(root, NULL);
rb_insert_augmented(&vma->vm_rb, root, &vma_gap_callbacks);
}
-static void __vma_rb_erase(struct vm_area_struct *vma, struct rb_root *root)
+static void __vma_rb_erase(struct vm_area_struct *vma, struct mm_struct *mm)
{
+ struct rb_root *root = &mm->mm_rb;
/*
* Note rb_erase_augmented is a fairly large inline function,
* so make sure we instantiate it only once with our desired
* augmented rbtree callbacks.
*/
+ write_seqlock(&mm->mm_seq);
rb_erase_augmented(&vma->vm_rb, root, &vma_gap_callbacks);
+ write_sequnlock(&mm->mm_seq); /* wmb */
+
+ /*
+ * Ensure the removal is complete before clearing the node.
+ * Matched by vma_has_changed()/handle_speculative_fault().
+ */
+ RB_CLEAR_NODE(&vma->vm_rb);
}
static __always_inline void vma_rb_erase_ignore(struct vm_area_struct *vma,
- struct rb_root *root,
+ struct mm_struct *mm,
struct vm_area_struct *ignore)
{
/*
@@ -437,21 +463,21 @@ static __always_inline void vma_rb_erase_ignore(struct vm_area_struct *vma,
* with the possible exception of the "next" vma being erased if
* next->vm_start was reduced.
*/
- validate_mm_rb(root, ignore);
+ validate_mm_rb(&mm->mm_rb, ignore);
- __vma_rb_erase(vma, root);
+ __vma_rb_erase(vma, mm);
}
static __always_inline void vma_rb_erase(struct vm_area_struct *vma,
- struct rb_root *root)
+ struct mm_struct *mm)
{
/*
* All rb_subtree_gap values must be consistent prior to erase,
* with the possible exception of the vma being erased.
*/
- validate_mm_rb(root, vma);
+ validate_mm_rb(&mm->mm_rb, vma);
- __vma_rb_erase(vma, root);
+ __vma_rb_erase(vma, mm);
}
/*
@@ -568,10 +594,12 @@ void __vma_link_rb(struct mm_struct *mm, struct vm_area_struct *vma,
* immediately update the gap to the correct value. Finally we
* rebalance the rbtree after all augmented values have been set.
*/
+ write_seqlock(&mm->mm_seq);
rb_link_node(&vma->vm_rb, rb_parent, rb_link);
vma->rb_subtree_gap = 0;
vma_gap_update(vma);
- vma_rb_insert(vma, &mm->mm_rb);
+ vma_rb_insert(vma, mm);
+ write_sequnlock(&mm->mm_seq);
}
static void __vma_link_file(struct vm_area_struct *vma)
@@ -647,7 +675,7 @@ static __always_inline void __vma_unlink_common(struct mm_struct *mm,
{
struct vm_area_struct *next;
- vma_rb_erase_ignore(vma, &mm->mm_rb, ignore);
+ vma_rb_erase_ignore(vma, mm, ignore);
next = vma->vm_next;
if (has_prev)
prev->vm_next = next;
@@ -901,15 +929,13 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start,
}
if (remove_next) {
- if (file) {
+ if (file)
uprobe_munmap(next, next->vm_start, next->vm_end);
- fput(file);
- }
if (next->anon_vma)
anon_vma_merge(vma, next);
mm->map_count--;
mpol_put(vma_policy(next));
- kmem_cache_free(vm_area_cachep, next);
+ free_vma(next);
write_seqcount_end(&next->vm_sequence);
/*
* In mprotect's case 6 (see comments on vma_merge),
@@ -2130,15 +2156,10 @@ get_unmapped_area(struct file *file, unsigned long addr, unsigned long len,
EXPORT_SYMBOL(get_unmapped_area);
/* Look up the first VMA which satisfies addr < vm_end, NULL if none. */
-struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr)
+static struct vm_area_struct *__find_vma(struct mm_struct *mm, unsigned long addr)
{
struct rb_node *rb_node;
- struct vm_area_struct *vma;
-
- /* Check the cache first. */
- vma = vmacache_find(mm, addr);
- if (likely(vma))
- return vma;
+ struct vm_area_struct *vma = NULL;
rb_node = mm->mm_rb.rb_node;
@@ -2156,13 +2177,40 @@ struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr)
rb_node = rb_node->rb_right;
}
+ return vma;
+}
+
+struct vm_area_struct *find_vma(struct mm_struct *mm, unsigned long addr)
+{
+ struct vm_area_struct *vma;
+
+ /* Check the cache first. */
+ vma = vmacache_find(mm, addr);
+ if (likely(vma))
+ return vma;
+
+ vma = __find_vma(mm, addr);
if (vma)
vmacache_update(addr, vma);
return vma;
}
-
EXPORT_SYMBOL(find_vma);
+struct vm_area_struct *find_vma_srcu(struct mm_struct *mm, unsigned long addr)
+{
+ struct vm_area_struct *vma;
+ unsigned int seq;
+
+ WARN_ON_ONCE(!srcu_read_lock_held(&vma_srcu));
+
+ do {
+ seq = read_seqbegin(&mm->mm_seq);
+ vma = __find_vma(mm, addr);
+ } while (read_seqretry(&mm->mm_seq, seq));
+
+ return vma;
+}
+
/*
* Same as find_vma, but also return a pointer to the previous VMA in *pprev.
*/
@@ -2530,7 +2578,7 @@ detach_vmas_to_be_unmapped(struct mm_struct *mm, struct vm_area_struct *vma,
insertion_point = (prev ? &prev->vm_next : &mm->mmap);
vma->vm_prev = NULL;
do {
- vma_rb_erase(vma, &mm->mm_rb);
+ vma_rb_erase(vma, mm);
mm->map_count--;
tail_vma = vma;
vma = vma->vm_next;
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Laurent Dufour <ldufour@linux.vnet.ibm.com> |
|---|---|
| Date | 2017-08-18 00:20 +0200 |
| Subject | [PATCH v2 08/20] mm: Protect SPF handler against anon_vma changes |
| Message-ID | <ufBiX-2Hx-45@gated-at.bofh.it> |
| In reply to | #1714438 |
The speculative page fault handler must be protected against anon_vma
changes. This is because page_add_new_anon_rmap() is called during the
speculative path.
In addition, don't try speculative page fault if the VMA don't have an
anon_vma structure allocated because its allocation should be
protected by the mmap_sem.
In __vma_adjust() when importer->anon_vma is set, there is no need to
protect against speculative page faults since speculative page fault
is aborted if the vma->anon_vma is not set.
When calling page_add_new_anon_rmap() vma->anon_vma is necessarily
valid since we checked for it when locking the pte and the anon_vma is
removed once the pte is unlocked. So even if the speculative page
fault handler is running concurrently with do_unmap(), as the pte is
locked in unmap_region() - through unmap_vmas() - and the anon_vma
unlinked later, because we check for the vma sequence counter which is
updated in unmap_page_range() before locking the pte, and then in
free_pgtables() so when locking the pte the change will be detected.
Signed-off-by: Laurent Dufour <ldufour@linux.vnet.ibm.com>
---
mm/memory.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/mm/memory.c b/mm/memory.c
index da3bd07bb052..68e4fdcce692 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -615,7 +615,9 @@ void free_pgtables(struct mmu_gather *tlb, struct vm_area_struct *vma,
* Hide vma from rmap and truncate_pagecache before freeing
* pgtables
*/
+ write_seqcount_begin(&vma->vm_sequence);
unlink_anon_vmas(vma);
+ write_seqcount_end(&vma->vm_sequence);
unlink_file_vma(vma);
if (is_vm_hugetlb_page(vma)) {
@@ -629,7 +631,9 @@ void free_pgtables(struct mmu_gather *tlb, struct vm_area_struct *vma,
&& !is_vm_hugetlb_page(next)) {
vma = next;
next = vma->vm_next;
+ write_seqcount_begin(&vma->vm_sequence);
unlink_anon_vmas(vma);
+ write_seqcount_end(&vma->vm_sequence);
unlink_file_vma(vma);
}
free_pgd_range(tlb, addr, vma->vm_end,
--
2.7.4
[toc] | [prev] | [next] | [standalone]
| From | Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com> |
|---|---|
| Date | 2017-08-21 04:30 +0200 |
| Message-ID | <ugKDv-6Kr-9@gated-at.bofh.it> |
| In reply to | #1714438 |
Hello,
On (08/18/17 00:04), Laurent Dufour wrote:
> This is a port on kernel 4.13 of the work done by Peter Zijlstra to
> handle page fault without holding the mm semaphore [1].
>
> The idea is to try to handle user space page faults without holding the
> mmap_sem. This should allow better concurrency for massively threaded
> process since the page fault handler will not wait for other threads memory
> layout change to be done, assuming that this change is done in another part
> of the process's memory space. This type page fault is named speculative
> page fault. If the speculative page fault fails because of a concurrency is
> detected or because underlying PMD or PTE tables are not yet allocating, it
> is failing its processing and a classic page fault is then tried.
>
> The speculative page fault (SPF) has to look for the VMA matching the fault
> address without holding the mmap_sem, so the VMA list is now managed using
> SRCU allowing lockless walking. The only impact would be the deferred file
> derefencing in the case of a file mapping, since the file pointer is
> released once the SRCU cleaning is done. This patch relies on the change
> done recently by Paul McKenney in SRCU which now runs a callback per CPU
> instead of per SRCU structure [1].
>
> The VMA's attributes checked during the speculative page fault processing
> have to be protected against parallel changes. This is done by using a per
> VMA sequence lock. This sequence lock allows the speculative page fault
> handler to fast check for parallel changes in progress and to abort the
> speculative page fault in that case.
>
> Once the VMA is found, the speculative page fault handler would check for
> the VMA's attributes to verify that the page fault has to be handled
> correctly or not. Thus the VMA is protected through a sequence lock which
> allows fast detection of concurrent VMA changes. If such a change is
> detected, the speculative page fault is aborted and a *classic* page fault
> is tried. VMA sequence locks are added when VMA attributes which are
> checked during the page fault are modified.
>
> When the PTE is fetched, the VMA is checked to see if it has been changed,
> so once the page table is locked, the VMA is valid, so any other changes
> leading to touching this PTE will need to lock the page table, so no
> parallel change is possible at this time.
[ 2311.315400] ======================================================
[ 2311.315401] WARNING: possible circular locking dependency detected
[ 2311.315403] 4.13.0-rc5-next-20170817-dbg-00039-gaf11d7500492-dirty #1743 Not tainted
[ 2311.315404] ------------------------------------------------------
[ 2311.315406] khugepaged/43 is trying to acquire lock:
[ 2311.315407] (&mapping->i_mmap_rwsem){++++}, at: [<ffffffff8111b339>] rmap_walk_file+0x5a/0x147
[ 2311.315415]
but task is already holding lock:
[ 2311.315416] (fs_reclaim){+.+.}, at: [<ffffffff810ebd80>] fs_reclaim_acquire+0x12/0x35
[ 2311.315420]
which lock already depends on the new lock.
[ 2311.315422]
the existing dependency chain (in reverse order) is:
[ 2311.315423]
-> #3 (fs_reclaim){+.+.}:
[ 2311.315427] fs_reclaim_acquire+0x32/0x35
[ 2311.315429] __alloc_pages_nodemask+0x8d/0x217
[ 2311.315432] pte_alloc_one+0x13/0x5e
[ 2311.315434] __pte_alloc+0x1f/0x83
[ 2311.315436] move_page_tables+0x2c9/0x5ac
[ 2311.315438] move_vma.isra.25+0xff/0x2a2
[ 2311.315439] SyS_mremap+0x41b/0x49e
[ 2311.315442] entry_SYSCALL_64_fastpath+0x18/0xad
[ 2311.315443]
-> #2 (&vma->vm_sequence/1){+.+.}:
[ 2311.315449] write_seqcount_begin_nested+0x1b/0x1d
[ 2311.315451] __vma_adjust+0x1b7/0x5d6
[ 2311.315453] __split_vma+0x142/0x1a3
[ 2311.315454] do_munmap+0x128/0x2af
[ 2311.315455] vm_munmap+0x5a/0x73
[ 2311.315458] elf_map+0xb1/0xce
[ 2311.315459] load_elf_binary+0x8e0/0x1348
[ 2311.315462] search_binary_handler+0x70/0x1f3
[ 2311.315464] load_script+0x1a6/0x1b5
[ 2311.315466] search_binary_handler+0x70/0x1f3
[ 2311.315468] do_execveat_common+0x461/0x691
[ 2311.315471] kernel_init+0x5a/0xf0
[ 2311.315472] ret_from_fork+0x27/0x40
[ 2311.315473]
-> #1 (&vma->vm_sequence){+.+.}:
[ 2311.315478] write_seqcount_begin_nested+0x1b/0x1d
[ 2311.315480] __vma_adjust+0x19c/0x5d6
[ 2311.315481] __split_vma+0x142/0x1a3
[ 2311.315482] do_munmap+0x128/0x2af
[ 2311.315484] vm_munmap+0x5a/0x73
[ 2311.315485] elf_map+0xb1/0xce
[ 2311.315487] load_elf_binary+0x8e0/0x1348
[ 2311.315489] search_binary_handler+0x70/0x1f3
[ 2311.315490] load_script+0x1a6/0x1b5
[ 2311.315492] search_binary_handler+0x70/0x1f3
[ 2311.315494] do_execveat_common+0x461/0x691
[ 2311.315496] kernel_init+0x5a/0xf0
[ 2311.315497] ret_from_fork+0x27/0x40
[ 2311.315498]
-> #0 (&mapping->i_mmap_rwsem){++++}:
[ 2311.315503] lock_acquire+0x176/0x19e
[ 2311.315505] down_read+0x3b/0x55
[ 2311.315507] rmap_walk_file+0x5a/0x147
[ 2311.315508] page_referenced+0x11c/0x134
[ 2311.315511] shrink_page_list+0x36b/0xb80
[ 2311.315512] shrink_inactive_list+0x1d9/0x437
[ 2311.315514] shrink_node_memcg.constprop.71+0x3e7/0x571
[ 2311.315515] shrink_node+0x3f/0x149
[ 2311.315517] try_to_free_pages+0x270/0x45f
[ 2311.315518] __alloc_pages_slowpath+0x34a/0xaa2
[ 2311.315520] __alloc_pages_nodemask+0x111/0x217
[ 2311.315523] khugepaged_alloc_page+0x17/0x45
[ 2311.315524] khugepaged+0xa29/0x16b5
[ 2311.315527] kthread+0xfb/0x103
[ 2311.315529] ret_from_fork+0x27/0x40
[ 2311.315530]
other info that might help us debug this:
[ 2311.315531] Chain exists of:
&mapping->i_mmap_rwsem --> &vma->vm_sequence/1 --> fs_reclaim
[ 2311.315537] Possible unsafe locking scenario:
[ 2311.315538] CPU0 CPU1
[ 2311.315539] ---- ----
[ 2311.315540] lock(fs_reclaim);
[ 2311.315542] lock(&vma->vm_sequence/1);
[ 2311.315545] lock(fs_reclaim);
[ 2311.315547] lock(&mapping->i_mmap_rwsem);
[ 2311.315549]
*** DEADLOCK ***
[ 2311.315551] 1 lock held by khugepaged/43:
[ 2311.315552] #0: (fs_reclaim){+.+.}, at: [<ffffffff810ebd80>] fs_reclaim_acquire+0x12/0x35
[ 2311.315556]
stack backtrace:
[ 2311.315559] CPU: 0 PID: 43 Comm: khugepaged Not tainted 4.13.0-rc5-next-20170817-dbg-00039-gaf11d7500492-dirty #1743
[ 2311.315560] Call Trace:
[ 2311.315564] dump_stack+0x67/0x8e
[ 2311.315568] print_circular_bug.isra.39+0x1c7/0x1d4
[ 2311.315570] __lock_acquire+0xb1a/0xe06
[ 2311.315572] ? graph_unlock+0x69/0x69
[ 2311.315575] lock_acquire+0x176/0x19e
[ 2311.315577] ? rmap_walk_file+0x5a/0x147
[ 2311.315579] down_read+0x3b/0x55
[ 2311.315581] ? rmap_walk_file+0x5a/0x147
[ 2311.315583] rmap_walk_file+0x5a/0x147
[ 2311.315585] page_referenced+0x11c/0x134
[ 2311.315587] ? page_vma_mapped_walk_done.isra.15+0xb/0xb
[ 2311.315589] ? page_get_anon_vma+0x6d/0x6d
[ 2311.315591] shrink_page_list+0x36b/0xb80
[ 2311.315593] ? _raw_spin_unlock_irq+0x29/0x46
[ 2311.315595] shrink_inactive_list+0x1d9/0x437
[ 2311.315597] shrink_node_memcg.constprop.71+0x3e7/0x571
[ 2311.315600] shrink_node+0x3f/0x149
[ 2311.315602] try_to_free_pages+0x270/0x45f
[ 2311.315604] __alloc_pages_slowpath+0x34a/0xaa2
[ 2311.315608] ? ___might_sleep+0xd5/0x234
[ 2311.315609] __alloc_pages_nodemask+0x111/0x217
[ 2311.315612] khugepaged_alloc_page+0x17/0x45
[ 2311.315613] khugepaged+0xa29/0x16b5
[ 2311.315616] ? remove_wait_queue+0x47/0x47
[ 2311.315618] ? collapse_shmem.isra.43+0x882/0x882
[ 2311.315620] kthread+0xfb/0x103
[ 2311.315622] ? __list_del_entry+0x1d/0x1d
[ 2311.315624] ret_from_fork+0x27/0x40
-ss
[toc] | [prev] | [next] | [standalone]
| From | Laurent Dufour <ldufour@linux.vnet.ibm.com> |
|---|---|
| Date | 2017-09-08 11:30 +0200 |
| Message-ID | <unnLQ-4OG-9@gated-at.bofh.it> |
| In reply to | #1716071 |
On 21/08/2017 04:26, Sergey Senozhatsky wrote:
> Hello,
>
> On (08/18/17 00:04), Laurent Dufour wrote:
>> This is a port on kernel 4.13 of the work done by Peter Zijlstra to
>> handle page fault without holding the mm semaphore [1].
>>
>> The idea is to try to handle user space page faults without holding the
>> mmap_sem. This should allow better concurrency for massively threaded
>> process since the page fault handler will not wait for other threads memory
>> layout change to be done, assuming that this change is done in another part
>> of the process's memory space. This type page fault is named speculative
>> page fault. If the speculative page fault fails because of a concurrency is
>> detected or because underlying PMD or PTE tables are not yet allocating, it
>> is failing its processing and a classic page fault is then tried.
>>
>> The speculative page fault (SPF) has to look for the VMA matching the fault
>> address without holding the mmap_sem, so the VMA list is now managed using
>> SRCU allowing lockless walking. The only impact would be the deferred file
>> derefencing in the case of a file mapping, since the file pointer is
>> released once the SRCU cleaning is done. This patch relies on the change
>> done recently by Paul McKenney in SRCU which now runs a callback per CPU
>> instead of per SRCU structure [1].
>>
>> The VMA's attributes checked during the speculative page fault processing
>> have to be protected against parallel changes. This is done by using a per
>> VMA sequence lock. This sequence lock allows the speculative page fault
>> handler to fast check for parallel changes in progress and to abort the
>> speculative page fault in that case.
>>
>> Once the VMA is found, the speculative page fault handler would check for
>> the VMA's attributes to verify that the page fault has to be handled
>> correctly or not. Thus the VMA is protected through a sequence lock which
>> allows fast detection of concurrent VMA changes. If such a change is
>> detected, the speculative page fault is aborted and a *classic* page fault
>> is tried. VMA sequence locks are added when VMA attributes which are
>> checked during the page fault are modified.
>>
>> When the PTE is fetched, the VMA is checked to see if it has been changed,
>> so once the page table is locked, the VMA is valid, so any other changes
>> leading to touching this PTE will need to lock the page table, so no
>> parallel change is possible at this time.
>
> [ 2311.315400] ======================================================
> [ 2311.315401] WARNING: possible circular locking dependency detected
> [ 2311.315403] 4.13.0-rc5-next-20170817-dbg-00039-gaf11d7500492-dirty #1743 Not tainted
> [ 2311.315404] ------------------------------------------------------
> [ 2311.315406] khugepaged/43 is trying to acquire lock:
> [ 2311.315407] (&mapping->i_mmap_rwsem){++++}, at: [<ffffffff8111b339>] rmap_walk_file+0x5a/0x147
> [ 2311.315415]
> but task is already holding lock:
> [ 2311.315416] (fs_reclaim){+.+.}, at: [<ffffffff810ebd80>] fs_reclaim_acquire+0x12/0x35
> [ 2311.315420]
> which lock already depends on the new lock.
>
> [ 2311.315422]
> the existing dependency chain (in reverse order) is:
> [ 2311.315423]
> -> #3 (fs_reclaim){+.+.}:
> [ 2311.315427] fs_reclaim_acquire+0x32/0x35
> [ 2311.315429] __alloc_pages_nodemask+0x8d/0x217
> [ 2311.315432] pte_alloc_one+0x13/0x5e
> [ 2311.315434] __pte_alloc+0x1f/0x83
> [ 2311.315436] move_page_tables+0x2c9/0x5ac
> [ 2311.315438] move_vma.isra.25+0xff/0x2a2
> [ 2311.315439] SyS_mremap+0x41b/0x49e
> [ 2311.315442] entry_SYSCALL_64_fastpath+0x18/0xad
> [ 2311.315443]
> -> #2 (&vma->vm_sequence/1){+.+.}:
> [ 2311.315449] write_seqcount_begin_nested+0x1b/0x1d
> [ 2311.315451] __vma_adjust+0x1b7/0x5d6
> [ 2311.315453] __split_vma+0x142/0x1a3
> [ 2311.315454] do_munmap+0x128/0x2af
> [ 2311.315455] vm_munmap+0x5a/0x73
> [ 2311.315458] elf_map+0xb1/0xce
> [ 2311.315459] load_elf_binary+0x8e0/0x1348
> [ 2311.315462] search_binary_handler+0x70/0x1f3
> [ 2311.315464] load_script+0x1a6/0x1b5
> [ 2311.315466] search_binary_handler+0x70/0x1f3
> [ 2311.315468] do_execveat_common+0x461/0x691
> [ 2311.315471] kernel_init+0x5a/0xf0
> [ 2311.315472] ret_from_fork+0x27/0x40
> [ 2311.315473]
> -> #1 (&vma->vm_sequence){+.+.}:
> [ 2311.315478] write_seqcount_begin_nested+0x1b/0x1d
> [ 2311.315480] __vma_adjust+0x19c/0x5d6
> [ 2311.315481] __split_vma+0x142/0x1a3
> [ 2311.315482] do_munmap+0x128/0x2af
> [ 2311.315484] vm_munmap+0x5a/0x73
> [ 2311.315485] elf_map+0xb1/0xce
> [ 2311.315487] load_elf_binary+0x8e0/0x1348
> [ 2311.315489] search_binary_handler+0x70/0x1f3
> [ 2311.315490] load_script+0x1a6/0x1b5
> [ 2311.315492] search_binary_handler+0x70/0x1f3
> [ 2311.315494] do_execveat_common+0x461/0x691
> [ 2311.315496] kernel_init+0x5a/0xf0
> [ 2311.315497] ret_from_fork+0x27/0x40
> [ 2311.315498]
> -> #0 (&mapping->i_mmap_rwsem){++++}:
> [ 2311.315503] lock_acquire+0x176/0x19e
> [ 2311.315505] down_read+0x3b/0x55
> [ 2311.315507] rmap_walk_file+0x5a/0x147
> [ 2311.315508] page_referenced+0x11c/0x134
> [ 2311.315511] shrink_page_list+0x36b/0xb80
> [ 2311.315512] shrink_inactive_list+0x1d9/0x437
> [ 2311.315514] shrink_node_memcg.constprop.71+0x3e7/0x571
> [ 2311.315515] shrink_node+0x3f/0x149
> [ 2311.315517] try_to_free_pages+0x270/0x45f
> [ 2311.315518] __alloc_pages_slowpath+0x34a/0xaa2
> [ 2311.315520] __alloc_pages_nodemask+0x111/0x217
> [ 2311.315523] khugepaged_alloc_page+0x17/0x45
> [ 2311.315524] khugepaged+0xa29/0x16b5
> [ 2311.315527] kthread+0xfb/0x103
> [ 2311.315529] ret_from_fork+0x27/0x40
> [ 2311.315530]
> other info that might help us debug this:
>
> [ 2311.315531] Chain exists of:
> &mapping->i_mmap_rwsem --> &vma->vm_sequence/1 --> fs_reclaim
Hi Sergey,
I can't see where such a chain could happen.
I tried to recreate it on top of the latest mm tree, to latest stack output
but I can't get it.
How did you raised this one ?
Thanks,
Laurent.
>
> [ 2311.315537] Possible unsafe locking scenario:
>
> [ 2311.315538] CPU0 CPU1
> [ 2311.315539] ---- ----
> [ 2311.315540] lock(fs_reclaim);
> [ 2311.315542] lock(&vma->vm_sequence/1);
> [ 2311.315545] lock(fs_reclaim);
> [ 2311.315547] lock(&mapping->i_mmap_rwsem);
> [ 2311.315549]
> *** DEADLOCK ***
>
> [ 2311.315551] 1 lock held by khugepaged/43:
> [ 2311.315552] #0: (fs_reclaim){+.+.}, at: [<ffffffff810ebd80>] fs_reclaim_acquire+0x12/0x35
> [ 2311.315556]
> stack backtrace:
> [ 2311.315559] CPU: 0 PID: 43 Comm: khugepaged Not tainted 4.13.0-rc5-next-20170817-dbg-00039-gaf11d7500492-dirty #1743
> [ 2311.315560] Call Trace:
> [ 2311.315564] dump_stack+0x67/0x8e
> [ 2311.315568] print_circular_bug.isra.39+0x1c7/0x1d4
> [ 2311.315570] __lock_acquire+0xb1a/0xe06
> [ 2311.315572] ? graph_unlock+0x69/0x69
> [ 2311.315575] lock_acquire+0x176/0x19e
> [ 2311.315577] ? rmap_walk_file+0x5a/0x147
> [ 2311.315579] down_read+0x3b/0x55
> [ 2311.315581] ? rmap_walk_file+0x5a/0x147
> [ 2311.315583] rmap_walk_file+0x5a/0x147
> [ 2311.315585] page_referenced+0x11c/0x134
> [ 2311.315587] ? page_vma_mapped_walk_done.isra.15+0xb/0xb
> [ 2311.315589] ? page_get_anon_vma+0x6d/0x6d
> [ 2311.315591] shrink_page_list+0x36b/0xb80
> [ 2311.315593] ? _raw_spin_unlock_irq+0x29/0x46
> [ 2311.315595] shrink_inactive_list+0x1d9/0x437
> [ 2311.315597] shrink_node_memcg.constprop.71+0x3e7/0x571
> [ 2311.315600] shrink_node+0x3f/0x149
> [ 2311.315602] try_to_free_pages+0x270/0x45f
> [ 2311.315604] __alloc_pages_slowpath+0x34a/0xaa2
> [ 2311.315608] ? ___might_sleep+0xd5/0x234
> [ 2311.315609] __alloc_pages_nodemask+0x111/0x217
> [ 2311.315612] khugepaged_alloc_page+0x17/0x45
> [ 2311.315613] khugepaged+0xa29/0x16b5
> [ 2311.315616] ? remove_wait_queue+0x47/0x47
> [ 2311.315618] ? collapse_shmem.isra.43+0x882/0x882
> [ 2311.315620] kthread+0xfb/0x103
> [ 2311.315622] ? __list_del_entry+0x1d/0x1d
> [ 2311.315624] ret_from_fork+0x27/0x40
>
> -ss
>
[toc] | [prev] | [next] | [standalone]
| From | Anshuman Khandual <khandual@linux.vnet.ibm.com> |
|---|---|
| Date | 2017-08-21 08:30 +0200 |
| Message-ID | <ugOnL-Gt-1@gated-at.bofh.it> |
| In reply to | #1714438 |
On 08/18/2017 03:34 AM, Laurent Dufour wrote: > This is a port on kernel 4.13 of the work done by Peter Zijlstra to > handle page fault without holding the mm semaphore [1]. > > The idea is to try to handle user space page faults without holding the > mmap_sem. This should allow better concurrency for massively threaded > process since the page fault handler will not wait for other threads memory > layout change to be done, assuming that this change is done in another part > of the process's memory space. This type page fault is named speculative > page fault. If the speculative page fault fails because of a concurrency is > detected or because underlying PMD or PTE tables are not yet allocating, it > is failing its processing and a classic page fault is then tried. > > The speculative page fault (SPF) has to look for the VMA matching the fault > address without holding the mmap_sem, so the VMA list is now managed using > SRCU allowing lockless walking. The only impact would be the deferred file > derefencing in the case of a file mapping, since the file pointer is > released once the SRCU cleaning is done. This patch relies on the change > done recently by Paul McKenney in SRCU which now runs a callback per CPU > instead of per SRCU structure [1]. > > The VMA's attributes checked during the speculative page fault processing > have to be protected against parallel changes. This is done by using a per > VMA sequence lock. This sequence lock allows the speculative page fault > handler to fast check for parallel changes in progress and to abort the > speculative page fault in that case. > > Once the VMA is found, the speculative page fault handler would check for > the VMA's attributes to verify that the page fault has to be handled > correctly or not. Thus the VMA is protected through a sequence lock which > allows fast detection of concurrent VMA changes. If such a change is > detected, the speculative page fault is aborted and a *classic* page fault > is tried. VMA sequence locks are added when VMA attributes which are > checked during the page fault are modified. > > When the PTE is fetched, the VMA is checked to see if it has been changed, > so once the page table is locked, the VMA is valid, so any other changes > leading to touching this PTE will need to lock the page table, so no > parallel change is possible at this time. > > Compared to the Peter's initial work, this series introduces a spin_trylock > when dealing with speculative page fault. This is required to avoid dead > lock when handling a page fault while a TLB invalidate is requested by an > other CPU holding the PTE. Another change due to a lock dependency issue > with mapping->i_mmap_rwsem. > > In addition some VMA field values which are used once the PTE is unlocked > at the end the page fault path are saved into the vm_fault structure to > used the values matching the VMA at the time the PTE was locked. > > This series builds on top of v4.13-rc5 and is functional on x86 and > PowerPC. > > Tests have been made using a large commercial in-memory database on a > PowerPC system with 752 CPU using RFC v5. The results are very encouraging > since the loading of the 2TB database was faster by 14% with the > speculative page fault. > You specifically mention loading as most of the page faults will happen at that time and then the working set will settle down with very less page faults there after ? That means unless there is another wave of page faults we wont notice performance improvement during the runtime. > Using ebizzy test [3], which spreads a lot of threads, the result are good > when running on both a large or a small system. When using kernbench, the The performance improvements are greater as there is a lot of creation and destruction of anon mappings which generates constant flow of page faults to be handled. > result are quite similar which expected as not so much multi threaded > processes are involved. But there is no performance degradation neither > which is good. If we compile with 'make -j N' there would be a lot of threads but I guess the problem is SPF does not support handling file mapping IIUC which limits the performance improvement for some workloads. > > ------------------ > Benchmarks results > > Note these test have been made on top of 4.13-rc3 with the following patch > from Paul McKenney applied: > "srcu: Provide ordering for CPU not involved in grace period" [5] Is this patch an improvement for SRCU which we are using for walking VMAs. > > Ebizzy: > ------- > The test is counting the number of records per second it can manage, the > higher is the best. I run it like this 'ebizzy -mTRp'. To get consistent > result I repeated the test 100 times and measure the average result, mean > deviation, max and min. > > - 16 CPUs x86 VM > Records/s 4.13-rc5 4.13-rc5-spf > Average 11350.29 21760.36 > Mean deviation 396.56 881.40 > Max 13773 26194 > Min 10567 19223 > > - 80 CPUs Power 8 node: > Records/s 4.13-rc5 4.13-rc5-spf > Average 33904.67 58847.91 > Mean deviation 789.40 1753.19 > Max 36703 68958 > Min 31759 55125 > Can you also mention % improvement or degradation in a new column. > The number of record per second is far better with the speculative page > fault. > The mean deviation is higher with the speculative page fault, may be > because sometime the fault are not handled in a speculative way leading to > more variation. we need to analyze that. Why speculative page faults failed on those occasions for exact same workload. > > > Kernbench: > ---------- > This test is building a 4.12 kernel using platform default config. The > build has been run 5 times each time. > > - 16 CPUs x86 VM > Average Half load -j 8 Run (std deviation) > 4.13.0-rc5 4.13.0-rc5-spf > Elapsed Time 166.574 (0.340779) 145.754 (0.776325) > User Time 1080.77 (2.05871) 999.272 (4.12142) > System Time 204.594 (1.02449) 116.362 (1.22974) > Percent CPU 771.2 (1.30384) 765 (0.707107) > Context Switches 46590.6 (935.591) 66316.4 (744.64) > Sleeps 84421.2 (596.612) 85186 (523.041) > > Average Optimal load -j 16 Run (std deviation) > 4.13.0-rc5 4.13.0-rc5-spf > Elapsed Time 85.422 (0.42293) 74.81 (0.419345) > User Time 1031.79 (51.6557) 954.912 (46.8439) > System Time 186.528 (19.0575) 107.514 (9.36902) > Percent CPU 1059.2 (303.607) 1056.8 (307.624) > Context Switches 67240.3 (21788.9) 89360.6 (24299.9) > Sleeps 89607.8 (5511.22) 90372.5 (5490.16) > > The elapsed time is a bit shorter in the case of the SPF release, but the > impact less important since there are less multithreaded processes involved > here. > > - 80 CPUs Power 8 node: > Average Half load -j 40 Run (std deviation) > 4.13.0-rc5 4.13.0-rc5-spf > Elapsed Time 117.176 (0.824093) 116.792 (0.695392) > User Time 4412.34 (24.29) 4396.02 (24.4819) > System Time 131.106 (1.28343) 133.452 (0.708851) > Percent CPU 3876.8 (18.1439) 3877.6 (21.9955) > Context Switches 72470.2 (466.181) 72971 (673.624) > Sleeps 161294 (2284.85) 161946 (2217.9) > > Average Optimal load -j 80 Run (std deviation) > 4.13.0-rc5 4.13.0-rc5-spf > Elapsed Time 111.176 (1.11123) 111.242 (0.801542) > User Time 5930.03 (1600.07) 5929.89 (1617) > System Time 166.258 (37.0662) 169.337 (37.8419) > Percent CPU 5378.5 (1584.16) 5385.6 (1590.24) > Context Switches 117389 (47350.1) 130132 (60256.3) > Sleeps 163354 (4153.9) 163219 (2251.27) > Can you also mention % improvement or degradation in a new column. > Here the elapsed time is a bit shorter using the spf release, but we > remain in the error margin. It has to be noted that this system is not > correctly balanced on the NUMA point of view as all the available memory is > attached to one core. Why different NUMA configuration would have changed the outcome ? > > ------------------------ > Changes since v1: > - Remove PERF_COUNT_SW_SPF_FAILED perf event. > - Add tracing events to details speculative page fault failures. > - Cache VMA fields values which are used once the PTE is unlocked at the > end of the page fault events. Why is this required ?
[toc] | [prev] | [next] | [standalone]
| From | "Paul E. McKenney" <paulmck@linux.vnet.ibm.com> |
|---|---|
| Date | 2017-08-22 02:50 +0200 |
| Message-ID | <uh5yi-37f-5@gated-at.bofh.it> |
| In reply to | #1716110 |
On Mon, Aug 21, 2017 at 11:58:03AM +0530, Anshuman Khandual wrote: > On 08/18/2017 03:34 AM, Laurent Dufour wrote: > > This is a port on kernel 4.13 of the work done by Peter Zijlstra to > > handle page fault without holding the mm semaphore [1]. > > > > The idea is to try to handle user space page faults without holding the > > mmap_sem. This should allow better concurrency for massively threaded > > process since the page fault handler will not wait for other threads memory > > layout change to be done, assuming that this change is done in another part > > of the process's memory space. This type page fault is named speculative > > page fault. If the speculative page fault fails because of a concurrency is > > detected or because underlying PMD or PTE tables are not yet allocating, it > > is failing its processing and a classic page fault is then tried. > > > > The speculative page fault (SPF) has to look for the VMA matching the fault > > address without holding the mmap_sem, so the VMA list is now managed using > > SRCU allowing lockless walking. The only impact would be the deferred file > > derefencing in the case of a file mapping, since the file pointer is > > released once the SRCU cleaning is done. This patch relies on the change > > done recently by Paul McKenney in SRCU which now runs a callback per CPU > > instead of per SRCU structure [1]. > > > > The VMA's attributes checked during the speculative page fault processing > > have to be protected against parallel changes. This is done by using a per > > VMA sequence lock. This sequence lock allows the speculative page fault > > handler to fast check for parallel changes in progress and to abort the > > speculative page fault in that case. > > > > Once the VMA is found, the speculative page fault handler would check for > > the VMA's attributes to verify that the page fault has to be handled > > correctly or not. Thus the VMA is protected through a sequence lock which > > allows fast detection of concurrent VMA changes. If such a change is > > detected, the speculative page fault is aborted and a *classic* page fault > > is tried. VMA sequence locks are added when VMA attributes which are > > checked during the page fault are modified. > > > > When the PTE is fetched, the VMA is checked to see if it has been changed, > > so once the page table is locked, the VMA is valid, so any other changes > > leading to touching this PTE will need to lock the page table, so no > > parallel change is possible at this time. > > > > Compared to the Peter's initial work, this series introduces a spin_trylock > > when dealing with speculative page fault. This is required to avoid dead > > lock when handling a page fault while a TLB invalidate is requested by an > > other CPU holding the PTE. Another change due to a lock dependency issue > > with mapping->i_mmap_rwsem. > > > > In addition some VMA field values which are used once the PTE is unlocked > > at the end the page fault path are saved into the vm_fault structure to > > used the values matching the VMA at the time the PTE was locked. > > > > This series builds on top of v4.13-rc5 and is functional on x86 and > > PowerPC. > > > > Tests have been made using a large commercial in-memory database on a > > PowerPC system with 752 CPU using RFC v5. The results are very encouraging > > since the loading of the 2TB database was faster by 14% with the > > speculative page fault. > > > > You specifically mention loading as most of the page faults will > happen at that time and then the working set will settle down with > very less page faults there after ? That means unless there is > another wave of page faults we wont notice performance improvement > during the runtime. > > > Using ebizzy test [3], which spreads a lot of threads, the result are good > > when running on both a large or a small system. When using kernbench, the > > The performance improvements are greater as there is a lot of creation > and destruction of anon mappings which generates constant flow of page > faults to be handled. > > > result are quite similar which expected as not so much multi threaded > > processes are involved. But there is no performance degradation neither > > which is good. > > If we compile with 'make -j N' there would be a lot of threads but I > guess the problem is SPF does not support handling file mapping IIUC > which limits the performance improvement for some workloads. > > > > > ------------------ > > Benchmarks results > > > > Note these test have been made on top of 4.13-rc3 with the following patch > > from Paul McKenney applied: > > "srcu: Provide ordering for CPU not involved in grace period" [5] > > Is this patch an improvement for SRCU which we are using for walking VMAs. It is a tweak to an earlier patch that parallelizes SRCU callback handling. Thanx, Paul > > Ebizzy: > > ------- > > The test is counting the number of records per second it can manage, the > > higher is the best. I run it like this 'ebizzy -mTRp'. To get consistent > > result I repeated the test 100 times and measure the average result, mean > > deviation, max and min. > > > > - 16 CPUs x86 VM > > Records/s 4.13-rc5 4.13-rc5-spf > > Average 11350.29 21760.36 > > Mean deviation 396.56 881.40 > > Max 13773 26194 > > Min 10567 19223 > > > > - 80 CPUs Power 8 node: > > Records/s 4.13-rc5 4.13-rc5-spf > > Average 33904.67 58847.91 > > Mean deviation 789.40 1753.19 > > Max 36703 68958 > > Min 31759 55125 > > > > Can you also mention % improvement or degradation in a new column. > > > The number of record per second is far better with the speculative page > > fault. > > The mean deviation is higher with the speculative page fault, may be > > because sometime the fault are not handled in a speculative way leading to > > more variation. > > we need to analyze that. Why speculative page faults failed on those > occasions for exact same workload. > > > > > > > Kernbench: > > ---------- > > This test is building a 4.12 kernel using platform default config. The > > build has been run 5 times each time. > > > > - 16 CPUs x86 VM > > Average Half load -j 8 Run (std deviation) > > 4.13.0-rc5 4.13.0-rc5-spf > > Elapsed Time 166.574 (0.340779) 145.754 (0.776325) > > User Time 1080.77 (2.05871) 999.272 (4.12142) > > System Time 204.594 (1.02449) 116.362 (1.22974) > > Percent CPU 771.2 (1.30384) 765 (0.707107) > > Context Switches 46590.6 (935.591) 66316.4 (744.64) > > Sleeps 84421.2 (596.612) 85186 (523.041) > > > > > > Average Optimal load -j 16 Run (std deviation) > > 4.13.0-rc5 4.13.0-rc5-spf > > Elapsed Time 85.422 (0.42293) 74.81 (0.419345) > > User Time 1031.79 (51.6557) 954.912 (46.8439) > > System Time 186.528 (19.0575) 107.514 (9.36902) > > Percent CPU 1059.2 (303.607) 1056.8 (307.624) > > Context Switches 67240.3 (21788.9) 89360.6 (24299.9) > > Sleeps 89607.8 (5511.22) 90372.5 (5490.16) > > > > The elapsed time is a bit shorter in the case of the SPF release, but the > > impact less important since there are less multithreaded processes involved > > here. > > > > - 80 CPUs Power 8 node: > > Average Half load -j 40 Run (std deviation) > > 4.13.0-rc5 4.13.0-rc5-spf > > Elapsed Time 117.176 (0.824093) 116.792 (0.695392) > > User Time 4412.34 (24.29) 4396.02 (24.4819) > > System Time 131.106 (1.28343) 133.452 (0.708851) > > Percent CPU 3876.8 (18.1439) 3877.6 (21.9955) > > Context Switches 72470.2 (466.181) 72971 (673.624) > > Sleeps 161294 (2284.85) 161946 (2217.9) > > > > Average Optimal load -j 80 Run (std deviation) > > 4.13.0-rc5 4.13.0-rc5-spf > > Elapsed Time 111.176 (1.11123) 111.242 (0.801542) > > User Time 5930.03 (1600.07) 5929.89 (1617) > > System Time 166.258 (37.0662) 169.337 (37.8419) > > Percent CPU 5378.5 (1584.16) 5385.6 (1590.24) > > Context Switches 117389 (47350.1) 130132 (60256.3) > > Sleeps 163354 (4153.9) 163219 (2251.27) > > > > Can you also mention % improvement or degradation in a new column. > > > Here the elapsed time is a bit shorter using the spf release, but we > > remain in the error margin. It has to be noted that this system is not > > correctly balanced on the NUMA point of view as all the available memory is > > attached to one core. > > Why different NUMA configuration would have changed the outcome ? > > > > > ------------------------ > > Changes since v1: > > - Remove PERF_COUNT_SW_SPF_FAILED perf event. > > - Add tracing events to details speculative page fault failures. > > - Cache VMA fields values which are used once the PTE is unlocked at the > > end of the page fault events. > > Why is this required ?
[toc] | [prev] | [next] | [standalone]
| From | Laurent Dufour <ldufour@linux.vnet.ibm.com> |
|---|---|
| Date | 2017-08-25 11:50 +0200 |
| Message-ID | <uijpx-2m3-27@gated-at.bofh.it> |
| In reply to | #1716110 |
On 21/08/2017 08:28, Anshuman Khandual wrote:
> On 08/18/2017 03:34 AM, Laurent Dufour wrote:
>> This is a port on kernel 4.13 of the work done by Peter Zijlstra to
>> handle page fault without holding the mm semaphore [1].
>>
>> The idea is to try to handle user space page faults without holding the
>> mmap_sem. This should allow better concurrency for massively threaded
>> process since the page fault handler will not wait for other threads memory
>> layout change to be done, assuming that this change is done in another part
>> of the process's memory space. This type page fault is named speculative
>> page fault. If the speculative page fault fails because of a concurrency is
>> detected or because underlying PMD or PTE tables are not yet allocating, it
>> is failing its processing and a classic page fault is then tried.
>>
>> The speculative page fault (SPF) has to look for the VMA matching the fault
>> address without holding the mmap_sem, so the VMA list is now managed using
>> SRCU allowing lockless walking. The only impact would be the deferred file
>> derefencing in the case of a file mapping, since the file pointer is
>> released once the SRCU cleaning is done. This patch relies on the change
>> done recently by Paul McKenney in SRCU which now runs a callback per CPU
>> instead of per SRCU structure [1].
>>
>> The VMA's attributes checked during the speculative page fault processing
>> have to be protected against parallel changes. This is done by using a per
>> VMA sequence lock. This sequence lock allows the speculative page fault
>> handler to fast check for parallel changes in progress and to abort the
>> speculative page fault in that case.
>>
>> Once the VMA is found, the speculative page fault handler would check for
>> the VMA's attributes to verify that the page fault has to be handled
>> correctly or not. Thus the VMA is protected through a sequence lock which
>> allows fast detection of concurrent VMA changes. If such a change is
>> detected, the speculative page fault is aborted and a *classic* page fault
>> is tried. VMA sequence locks are added when VMA attributes which are
>> checked during the page fault are modified.
>>
>> When the PTE is fetched, the VMA is checked to see if it has been changed,
>> so once the page table is locked, the VMA is valid, so any other changes
>> leading to touching this PTE will need to lock the page table, so no
>> parallel change is possible at this time.
>>
>> Compared to the Peter's initial work, this series introduces a spin_trylock
>> when dealing with speculative page fault. This is required to avoid dead
>> lock when handling a page fault while a TLB invalidate is requested by an
>> other CPU holding the PTE. Another change due to a lock dependency issue
>> with mapping->i_mmap_rwsem.
>>
>> In addition some VMA field values which are used once the PTE is unlocked
>> at the end the page fault path are saved into the vm_fault structure to
>> used the values matching the VMA at the time the PTE was locked.
>>
>> This series builds on top of v4.13-rc5 and is functional on x86 and
>> PowerPC.
>>
>> Tests have been made using a large commercial in-memory database on a
>> PowerPC system with 752 CPU using RFC v5. The results are very encouraging
>> since the loading of the 2TB database was faster by 14% with the
>> speculative page fault.
>>
>
> You specifically mention loading as most of the page faults will
> happen at that time and then the working set will settle down with
> very less page faults there after ? That means unless there is
> another wave of page faults we wont notice performance improvement
> during the runtime.
I just captured performance statistic during the database loading then
since the database was not stimulated, there was no page faults generated.
Further tests will be made while the database is running but I didn't have
the framework to do so right now.
>
>> Using ebizzy test [3], which spreads a lot of threads, the result are good
>> when running on both a large or a small system. When using kernbench, the
>
> The performance improvements are greater as there is a lot of creation
> and destruction of anon mappings which generates constant flow of page
> faults to be handled.
>
>> result are quite similar which expected as not so much multi threaded
>> processes are involved. But there is no performance degradation neither
>> which is good.
>
> If we compile with 'make -j N' there would be a lot of threads but I
> guess the problem is SPF does not support handling file mapping IIUC
> which limits the performance improvement for some workloads.
Yes but that test is showing that there is no performance degradation which
is good.
>>
>> ------------------
>> Benchmarks results
>>
>> Note these test have been made on top of 4.13-rc3 with the following patch
>> from Paul McKenney applied:
>> "srcu: Provide ordering for CPU not involved in grace period" [5]
>
> Is this patch an improvement for SRCU which we are using for walking VMAs.
>
>>
>> Ebizzy:
>> -------
>> The test is counting the number of records per second it can manage, the
>> higher is the best. I run it like this 'ebizzy -mTRp'. To get consistent
>> result I repeated the test 100 times and measure the average result, mean
>> deviation, max and min.
>>
>> - 16 CPUs x86 VM
>> Records/s 4.13-rc5 4.13-rc5-spf
>> Average 11350.29 21760.36
>> Mean deviation 396.56 881.40
>> Max 13773 26194
>> Min 10567 19223
>>
>> - 80 CPUs Power 8 node:
>> Records/s 4.13-rc5 4.13-rc5-spf
>> Average 33904.67 58847.91
>> Mean deviation 789.40 1753.19
>> Max 36703 68958
>> Min 31759 55125
>>
>
> Can you also mention % improvement or degradation in a new column.
Fair enough:
- 16 CPUs x86 VM
Records/s 4.13-rc5 4.13-rc5-spf
Average 11350.29 21760.36 +92%
Mean deviation 396.56 881.40 +122%
Max 13773 26194 +90%
Min 10567 19223 +82%
- 80 CPUs Power 8 node:
Records/s 4.13-rc5 4.13-rc5-spf
Average 33904.67 58847.91 +74%
Mean deviation 789.40 1753.19 +122%
Max 36703 68958 +88%
Min 31759 55125 +74%
>
>> The number of record per second is far better with the speculative page
>> fault.
>> The mean deviation is higher with the speculative page fault, may be
>> because sometime the fault are not handled in a speculative way leading to
>> more variation.
>
> we need to analyze that. Why speculative page faults failed on those
> occasions for exact same workload.
That's not even clear that the mean deviation increasing is due to
speculative page fault failure. This will need to be study, but even if the
mean deviation is more important, the result are far better anyway.
>>
>>
>> Kernbench:
>> ----------
>> This test is building a 4.12 kernel using platform default config. The
>> build has been run 5 times each time.
>>
>> - 16 CPUs x86 VM
>> Average Half load -j 8 Run (std deviation)
>> 4.13.0-rc5 4.13.0-rc5-spf
>> Elapsed Time 166.574 (0.340779) 145.754 (0.776325)
>> User Time 1080.77 (2.05871) 999.272 (4.12142)
>> System Time 204.594 (1.02449) 116.362 (1.22974)
>> Percent CPU 771.2 (1.30384) 765 (0.707107)
>> Context Switches 46590.6 (935.591) 66316.4 (744.64)
>> Sleeps 84421.2 (596.612) 85186 (523.041)
>
>
>>
>> Average Optimal load -j 16 Run (std deviation)
>> 4.13.0-rc5 4.13.0-rc5-spf
>> Elapsed Time 85.422 (0.42293) 74.81 (0.419345)
>> User Time 1031.79 (51.6557) 954.912 (46.8439)
>> System Time 186.528 (19.0575) 107.514 (9.36902)
>> Percent CPU 1059.2 (303.607) 1056.8 (307.624)
>> Context Switches 67240.3 (21788.9) 89360.6 (24299.9)
>> Sleeps 89607.8 (5511.22) 90372.5 (5490.16)
>>
>> The elapsed time is a bit shorter in the case of the SPF release, but the
>> impact less important since there are less multithreaded processes involved
>> here.
>>
>> - 80 CPUs Power 8 node:
>> Average Half load -j 40 Run (std deviation)
>> 4.13.0-rc5 4.13.0-rc5-spf
>> Elapsed Time 117.176 (0.824093) 116.792 (0.695392)
>> User Time 4412.34 (24.29) 4396.02 (24.4819)
>> System Time 131.106 (1.28343) 133.452 (0.708851)
>> Percent CPU 3876.8 (18.1439) 3877.6 (21.9955)
>> Context Switches 72470.2 (466.181) 72971 (673.624)
>> Sleeps 161294 (2284.85) 161946 (2217.9)
>>
>> Average Optimal load -j 80 Run (std deviation)
>> 4.13.0-rc5 4.13.0-rc5-spf
>> Elapsed Time 111.176 (1.11123) 111.242 (0.801542)
>> User Time 5930.03 (1600.07) 5929.89 (1617)
>> System Time 166.258 (37.0662) 169.337 (37.8419)
>> Percent CPU 5378.5 (1584.16) 5385.6 (1590.24)
>> Context Switches 117389 (47350.1) 130132 (60256.3)
>> Sleeps 163354 (4153.9) 163219 (2251.27)
>>
>
> Can you also mention % improvement or degradation in a new column.
Fair enough:
- 16 CPUs x86 VM
Average Half load -j 8 Run (std deviation)
4.13.0-rc5 4.13.0-rc5-spf
Elapsed Time 166.574 (0.340779) 145.754 (0.776325) -12.5%
User Time 1080.77 (2.05871) 999.272 (4.12142) -7.54%
System Time 204.594 (1.02449) 116.362 (1.22974) -43.13%
Percent CPU 771.2 (1.30384) 765 (0.707107) -0.8%
Context Switches 46590.6 (935.591) 66316.4 (744.64) +42.34%
Sleeps 84421.2 (596.612) 85186 (523.041) +0.9%
Average Optimal load -j 16 Run (std deviation)
4.13.0-rc5 4.13.0-rc5-spf
Elapsed Time 85.422 (0.42293) 74.81 (0.419345) -12.42%
User Time 1031.79 (51.6557) 954.912 (46.8439) -7.45%
System Time 186.528 (19.0575) 107.514 (9.36902) -42.36%
Percent CPU 1059.2 (303.607) 1056.8 (307.624) -0.23%
Context Switches 67240.3 (21788.9) 89360.6 (24299.9) +32.9%
Sleeps 89607.8 (5511.22) 90372.5 (5490.16) +0.85%
- 80 CPUs Power 8 node:
Average Half load -j 40 Run (std deviation)
4.13.0-rc5 4.13.0-rc5-spf
Elapsed Time 117.176 (0.824093) 116.792 (0.695392) -0.33%
User Time 4412.34 (24.29) 4396.02 (24.4819) -0.37%
System Time 131.106 (1.28343) 133.452 (0.708851) +1.79%
Percent CPU 3876.8 (18.1439) 3877.6 (21.9955) +0.02%
Context Switches 72470.2 (466.181) 72971 (673.624) +0.69%
Sleeps 161294 (2284.85) 161946 (2217.9) +0.40%
Average Optimal load -j 80 Run (std deviation)
4.13.0-rc5 4.13.0-rc5-spf
Elapsed Time 111.176 (1.11123) 111.242 (0.801542) +0.06%
User Time 5930.03 (1600.07) 5929.89 (1617) +0%
System Time 166.258 (37.0662) 169.337 (37.8419) +1.85%
Percent CPU 5378.5 (1584.16) 5385.6 (1590.24) +0.13%
Context Switches 117389 (47350.1) 130132 (60256.3) +10.86%
Sleeps 163354 (4153.9) 163219 (2251.27) -0.08%
>> Here the elapsed time is a bit shorter using the spf release, but we
>> remain in the error margin. It has to be noted that this system is not
>> correctly balanced on the NUMA point of view as all the available memory is
>> attached to one core.
>
> Why different NUMA configuration would have changed the outcome ?
I guess, process will have been scheduled nearest the memory, or spread in
a different way on the core if memory will be attached to.
>>
>> ------------------------
>> Changes since v1:
>> - Remove PERF_COUNT_SW_SPF_FAILED perf event.
>> - Add tracing events to details speculative page fault failures.
>> - Cache VMA fields values which are used once the PTE is unlocked at the
>> end of the page fault events.
>
> Why is this required ?
Please see patch 07/20 for details.
Cheers,
Laurent.
[toc] | [prev] | [standalone]
Page 2 of 2 — ← Prev page 1 [2]
Back to top | Article view | linux.kernel
csiph-web