On Tue, 13 Mar 2018, Laurent Dufour wrote: > diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c > index 866446cf2d9a..104f3cc86b51 100644 > --- a/arch/powerpc/mm/fault.c > +++ b/arch/powerpc/mm/fault.c > @@ -392,6 +392,9 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address, > unsigned long error_code) > { > struct vm_area_struct * vma; > +#ifdef CONFIG_SPECULATIVE_PAGE_FAULT > + struct vm_area_struct *spf_vma = NULL; > +#endif > struct mm_struct *mm = current->mm; > unsigned int flags = FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_KILLABLE; > int is_exec = TRAP(regs) == 0x400; > @@ -459,6 +462,20 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address, > if (is_exec) > flags |= FAULT_FLAG_INSTRUCTION; > > +#ifdef CONFIG_SPECULATIVE_PAGE_FAULT > + if (is_user && (atomic_read(&mm->mm_users) > 1)) { > + /* let's try a speculative page fault without grabbing the > + * mmap_sem. > + */ > + fault = handle_speculative_fault(mm, address, flags, &spf_vma); > + if (!(fault & VM_FAULT_RETRY)) { > + perf_sw_event(PERF_COUNT_SW_SPF, 1, > + regs, address); > + goto done; > + } > + } > +#endif /* CONFIG_SPECULATIVE_PAGE_FAULT */ > + Can't you elimiate all #ifdef's in this patch if handle_speculative_fault() can be passed is_user and return some error code that fallback is needed? Maybe reuse VM_FAULT_FALLBACK? > /* When running in the kernel we expect faults to occur only to > * addresses in user space. All other faults represent errors in the > * kernel and should generate an OOPS. Unfortunately, in the case of an > @@ -489,7 +506,16 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address, > might_sleep(); > } > > - vma = find_vma(mm, address); > +#ifdef CONFIG_SPECULATIVE_PAGE_FAULT > + if (spf_vma) { > + if (can_reuse_spf_vma(spf_vma, address)) > + vma = spf_vma; > + else > + vma = find_vma(mm, address); > + spf_vma = NULL; > + } else > +#endif > + vma = find_vma(mm, address); > if (unlikely(!vma)) > return bad_area(regs, address); > if (likely(vma->vm_start <= address)) I think the code quality here could be improved such that you can pass mm, &spf_vma, and address and some helper function would return spf_vma if can_reuse_spf_vma() is true (and do *spf_vma to NULL) or otherwise return find_vma(mm, address). Also, spf_vma is being set to NULL because of VM_FAULT_RETRY, but does it make sense to retry handle_speculative_fault() in this case since we've dropped mm->mmap_sem and there may have been a writer queued behind it? > @@ -568,6 +594,9 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address, > > up_read(¤t->mm->mmap_sem); > > +#ifdef CONFIG_SPECULATIVE_PAGE_FAULT > +done: > +#endif > if (unlikely(fault & VM_FAULT_ERROR)) > return mm_fault_error(regs, address, fault); > And things like this are trivially handled by doing done: __maybe_unused