Re: Kernel Benchmarking

[Matthew Wilcox <willy@infradead.org>]

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;
 }