| Message ID | 20230711202047.3818697-5-willy@infradead.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Avoid the mmap lock for fault-around | expand |
On Tue, Jul 11, 2023 at 1:20 PM Matthew Wilcox (Oracle) <willy@infradead.org> wrote: > > Push the check down from __handle_mm_fault(). There's a mild upside to > this patch in that we'll allocate the page tables while under the VMA > lock rather than the mmap lock, reducing the hold time on the mmap lock, > since the retry will find the page tables already populated. > > Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> Took some time but seems safe to me... Reviewed-by: Suren Baghdasaryan <surenb@google.com> > --- > mm/memory.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/mm/memory.c b/mm/memory.c > index 6eda5c5f2069..52f7fdd78380 100644 > --- a/mm/memory.c > +++ b/mm/memory.c > @@ -4924,6 +4924,11 @@ static vm_fault_t handle_pte_fault(struct vm_fault *vmf) > { > pte_t entry; > > + if ((vmf->flags & FAULT_FLAG_VMA_LOCK) && !vma_is_anonymous(vmf->vma)) { > + vma_end_read(vmf->vma); > + return VM_FAULT_RETRY; > + } > + > if (unlikely(pmd_none(*vmf->pmd))) { > /* > * Leave __pte_alloc() until later: because vm_ops->fault may > @@ -5020,11 +5025,6 @@ static vm_fault_t __handle_mm_fault(struct vm_area_struct *vma, > p4d_t *p4d; > vm_fault_t ret; > > - if ((flags & FAULT_FLAG_VMA_LOCK) && !vma_is_anonymous(vma)) { > - vma_end_read(vma); > - return VM_FAULT_RETRY; > - } > - > pgd = pgd_offset(mm, address); > p4d = p4d_alloc(mm, pgd, address); > if (!p4d) > -- > 2.39.2 >
On Tue, Jul 11, 2023 at 10:20 PM Matthew Wilcox (Oracle) <willy@infradead.org> wrote: > Push the check down from __handle_mm_fault(). There's a mild upside to > this patch in that we'll allocate the page tables while under the VMA > lock rather than the mmap lock, reducing the hold time on the mmap lock, > since the retry will find the page tables already populated. This commit, by moving the check from __handle_mm_fault() to handle_pte_fault(), also makes the non-anonymous THP paths (including the DAX huge fault handling) reachable for VMA-locked faults, right? Is that intentional?
On Mon, Jul 24, 2023 at 05:46:57PM +0200, Jann Horn wrote: > On Tue, Jul 11, 2023 at 10:20 PM Matthew Wilcox (Oracle) > <willy@infradead.org> wrote: > > Push the check down from __handle_mm_fault(). There's a mild upside to > > this patch in that we'll allocate the page tables while under the VMA > > lock rather than the mmap lock, reducing the hold time on the mmap lock, > > since the retry will find the page tables already populated. > > This commit, by moving the check from __handle_mm_fault() to > handle_pte_fault(), also makes the non-anonymous THP paths (including > the DAX huge fault handling) reachable for VMA-locked faults, right? > Is that intentional? Oof, this patch is all kinds of buggy. Will split this into several pieces. Thanks!
diff --git a/mm/memory.c b/mm/memory.c index 6eda5c5f2069..52f7fdd78380 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -4924,6 +4924,11 @@ static vm_fault_t handle_pte_fault(struct vm_fault *vmf) { pte_t entry; + if ((vmf->flags & FAULT_FLAG_VMA_LOCK) && !vma_is_anonymous(vmf->vma)) { + vma_end_read(vmf->vma); + return VM_FAULT_RETRY; + } + if (unlikely(pmd_none(*vmf->pmd))) { /* * Leave __pte_alloc() until later: because vm_ops->fault may @@ -5020,11 +5025,6 @@ static vm_fault_t __handle_mm_fault(struct vm_area_struct *vma, p4d_t *p4d; vm_fault_t ret; - if ((flags & FAULT_FLAG_VMA_LOCK) && !vma_is_anonymous(vma)) { - vma_end_read(vma); - return VM_FAULT_RETRY; - } - pgd = pgd_offset(mm, address); p4d = p4d_alloc(mm, pgd, address); if (!p4d)
Push the check down from __handle_mm_fault(). There's a mild upside to this patch in that we'll allocate the page tables while under the VMA lock rather than the mmap lock, reducing the hold time on the mmap lock, since the retry will find the page tables already populated. Signed-off-by: Matthew Wilcox (Oracle) <willy@infradead.org> --- mm/memory.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)