On Sat, Sep 12, 2020 at 10:28:29AM +0300, Amir Goldstein wrote:

On Sat, Sep 12, 2020 at 1:40 AM Michael Larabel
[off-list ref] wrote:

quoted

On 9/11/20 5:07 PM, Linus Torvalds wrote:

quoted

On Fri, Sep 11, 2020 at 9:19 AM Linus Torvalds
[off-list ref] wrote:

quoted

Ok, it's probably simply that fairness is really bad for performance
here in general, and that special case is just that - a special case,
not the main issue.

Ahh. It turns out that I should have looked more at the fault path
after all. It was higher up in the profile, but I ignored it because I
found that lock-unlock-lock pattern lower down.

The main contention point is actually filemap_fault(). Your apache
test accesses the 'test.html' file that is mmap'ed into memory, and
all the threads hammer on that one single file concurrently and that
seems to be the main page lock contention.

Which is really sad - the page lock there isn't really all that
interesting, and the normal "read()" path doesn't even take it. But
faulting the page in does so because the page will have a long-term
existence in the page tables, and so there's a worry about racing with
truncate.

Here's an idea (sorry, no patch, about to go out for the day)

What if we cleared PageUptodate in the truncate path?  And then
filemap_fault() looks a lot more like generic_file_buffered_read() where
we check PageUptodate, and only if it's clear do we take the page lock
and call ->readpage.

We'd need to recheck PageUptodate after installing the PTE and zap
it ourselves, and we wouldn't be able to check page_mapped() in the
truncate path any more, which would make me sad.  But there's something
to be said for making faults cheaper.

Even the XFS model where we take the MMAPLOCK_SHARED isn't free -- it's
just write-vs-write on a cacheline instead of a visible contention on
the page lock.