Messages in this thread | | | Date | Tue, 4 Jun 2019 15:56:12 +0100 | From | Catalin Marinas <> | Subject | Re: [PATCH V2 4/4] arm64/mm: Drop local variable vm_fault_t from __do_page_fault() |
| |
On Mon, Jun 03, 2019 at 12:11:25PM +0530, Anshuman Khandual wrote: > __do_page_fault() is over complicated with multiple goto statements. This > cleans up the code flow and while there drops local variable vm_fault_t.
I'd change the subject as well here to something like refactor or simplify __do_page_fault().
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c > index 4bb65f3..41fa905 100644 > --- a/arch/arm64/mm/fault.c > +++ b/arch/arm64/mm/fault.c > @@ -397,37 +397,29 @@ static void do_bad_area(unsigned long addr, unsigned int esr, struct pt_regs *re > static vm_fault_t __do_page_fault(struct mm_struct *mm, unsigned long addr, > unsigned int mm_flags, unsigned long vm_flags) > { > - struct vm_area_struct *vma; > - vm_fault_t fault; > + struct vm_area_struct *vma = find_vma(mm, addr); > > - vma = find_vma(mm, addr); > - fault = VM_FAULT_BADMAP; > if (unlikely(!vma)) > - goto out; > - if (unlikely(vma->vm_start > addr)) > - goto check_stack; > + return VM_FAULT_BADMAP; > > /* > * Ok, we have a good vm_area for this memory access, so we can handle > * it. > */ > -good_area: > + if (unlikely(vma->vm_start > addr)) { > + if (!(vma->vm_flags & VM_GROWSDOWN)) > + return VM_FAULT_BADMAP; > + if (expand_stack(vma, addr)) > + return VM_FAULT_BADMAP; > + }
You could have a single return here:
if (unlikely(vma->vm_start > addr) && (!(vma->vm_flags & VM_GROWSDOWN) || expand_stack(vma, addr))) return VM_FAULT_BADMAP;
Not sure it's any clearer though.
-- Catalin
| |