On Tue, Sep 15, 2020 at 12:39:38PM +0200, Jan Kara wrote:
> Hi Matthew!
>
> On Tue 15-09-20 04:32:10, Matthew Wilcox wrote:
> > On Sat, Sep 12, 2020 at 09:44:15AM -0500, Michael Larabel wrote:
> > > Interesting, I'll fire up some cross-filesystem benchmarks with those tests
> > > today and report back shortly with the difference.
> >
> > If you have time, perhaps you'd like to try this patch. It tries to
> > handle page faults locklessly when possible, which should be the case
> > where you're seeing page lock contention. I've tried to be fairly
> > conservative in this patch; reducing page lock acquisition should be
> > possible in more cases.
>
> So I'd be somewhat uneasy with this optimization. The thing is that e.g.
> page migration relies on page lock protecting page from being mapped? How
> does your patch handle that? I'm also not sure if the rmap code is really
> ready for new page reverse mapping being added without holding page lock...
I admit to not even having looked at the page migration code. This
patch was really to demonstrate that it's _possible_ to do page faults
without taking the page lock.
It's possible to expand the ClearPageUptodate page validity protocol
beyond mm/truncate.c, of course. We can find all necessary places to
change by grepping for 'page_mapped'. Some places (eg the invalidate2
path) can't safely ClearPageUptodate before their existing call to
unmap_mapping_pages(), and those places will have to add a second
test-and-call.
It seems to me the page_add_file_rmap() is fine with being called
without the page lock, unless the page is compound. So we could
make sure not to use this new protocol for THPs ...
+++ b/mm/filemap.c
@@ -2604,7 +2604,7 @@ vm_fault_t filemap_fault(struct vm_fault *vmf)
if (fpin)
goto out_retry;
- if (likely(PageUptodate(page))) {
+ if (likely(PageUptodate(page) && !PageTransHuge(page))) {
ret |= VM_FAULT_UPTODATE;
goto uptodate;
}
diff --git a/mm/memory.c b/mm/memory.c
index 53c8ef2bb38b..6981e8738df4 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -3460,6 +3460,9 @@ static vm_fault_t __do_fault(struct vm_fault *vmf)
return VM_FAULT_HWPOISON;
}
+ /* rmap needs THP pages to be locked in case it's mlocked */
+ VM_BUG_ON((ret & VM_FAULT_UPTODATE) && PageTransHuge(page));
+
return ret;
}
