| Message ID | 20181018202318.9131-5-josef@toxicpanda.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drop the mmap_sem when doing IO in the fault path | expand |
On Thu, Oct 18, 2018 at 04:23:15PM -0400, Josef Bacik wrote: > If we drop the mmap_sem we have to redo the vma lookup which requires > redoing the fault handler. Chances are we will just come back to the > same page, so save this page in our vmf->cached_page and reuse it in the > next loop through the fault handler. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > --- > mm/filemap.c | 30 ++++++++++++++++++++++++++++-- > 1 file changed, 28 insertions(+), 2 deletions(-) > > diff --git a/mm/filemap.c b/mm/filemap.c > index 65395ee132a0..5212ab637832 100644 > --- a/mm/filemap.c > +++ b/mm/filemap.c > @@ -2530,13 +2530,38 @@ vm_fault_t filemap_fault(struct vm_fault *vmf) > pgoff_t offset = vmf->pgoff; > int flags = vmf->flags; > pgoff_t max_off; > - struct page *page; > + struct page *page = NULL; > + struct page *cached_page = vmf->cached_page; > vm_fault_t ret = 0; > > max_off = DIV_ROUND_UP(i_size_read(inode), PAGE_SIZE); > if (unlikely(offset >= max_off)) > return VM_FAULT_SIGBUS; > > + /* > + * We may have read in the page already and have a page from an earlier > + * loop. If so we need to see if this page is still valid, and if not > + * do the whole dance over again. > + */ > + if (cached_page) { > + if (flags & FAULT_FLAG_KILLABLE) { > + error = lock_page_killable(cached_page); > + if (error) { > + up_read(&mm->mmap_sem); > + goto out_retry; > + } > + } else > + lock_page(cached_page); > + vmf->cached_page = NULL; > + if (cached_page->mapping == mapping && > + cached_page->index == offset) { > + page = cached_page; > + goto have_cached_page; > + } > + unlock_page(cached_page); > + put_page(cached_page); > + } > + Can you factor this out so the main code path doesn't get any more complex than it already is? i.e. something like: error = vmf_has_cached_page(vmf, &page); if (error) goto out_retry; if (page) goto have_cached_page; -dave.
diff --git a/mm/filemap.c b/mm/filemap.c index 65395ee132a0..5212ab637832 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -2530,13 +2530,38 @@ vm_fault_t filemap_fault(struct vm_fault *vmf) pgoff_t offset = vmf->pgoff; int flags = vmf->flags; pgoff_t max_off; - struct page *page; + struct page *page = NULL; + struct page *cached_page = vmf->cached_page; vm_fault_t ret = 0; max_off = DIV_ROUND_UP(i_size_read(inode), PAGE_SIZE); if (unlikely(offset >= max_off)) return VM_FAULT_SIGBUS; + /* + * We may have read in the page already and have a page from an earlier + * loop. If so we need to see if this page is still valid, and if not + * do the whole dance over again. + */ + if (cached_page) { + if (flags & FAULT_FLAG_KILLABLE) { + error = lock_page_killable(cached_page); + if (error) { + up_read(&mm->mmap_sem); + goto out_retry; + } + } else + lock_page(cached_page); + vmf->cached_page = NULL; + if (cached_page->mapping == mapping && + cached_page->index == offset) { + page = cached_page; + goto have_cached_page; + } + unlock_page(cached_page); + put_page(cached_page); + } + /* * Do we have something in the page cache already? */ @@ -2587,6 +2612,7 @@ vm_fault_t filemap_fault(struct vm_fault *vmf) put_page(page); goto retry_find; } +have_cached_page: VM_BUG_ON_PAGE(page->index != offset, page); /* @@ -2677,7 +2703,7 @@ vm_fault_t filemap_fault(struct vm_fault *vmf) if (fpin) fput(fpin); if (page) - put_page(page); + vmf->cached_page = page; return ret | VM_FAULT_RETRY; } EXPORT_SYMBOL(filemap_fault);
If we drop the mmap_sem we have to redo the vma lookup which requires redoing the fault handler. Chances are we will just come back to the same page, so save this page in our vmf->cached_page and reuse it in the next loop through the fault handler. Signed-off-by: Josef Bacik <josef@toxicpanda.com> --- mm/filemap.c | 30 ++++++++++++++++++++++++++++-- 1 file changed, 28 insertions(+), 2 deletions(-)