Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1723177
| From | Laurent Dufour <ldufour@linux.vnet.ibm.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PATCH v2 14/20] mm: Provide speculative fault infrastructure |
| Date | 2017-08-30 12:00 +0200 |
| Message-ID | <uk7WW-6Hb-19@gated-at.bofh.it> (permalink) |
| References | (3 earlier) <ujJBg-8hK-9@gated-at.bofh.it> <ujNvc-2sE-23@gated-at.bofh.it> <ujOAW-37c-7@gated-at.bofh.it> <ujP3X-3gG-3@gated-at.bofh.it> <uk3qh-44H-3@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
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;
> }
>
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
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
csiph-web