linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Hugh Dickins <hughd@google.com>,
	Matthew Wilcox <willy@infradead.org>,
	 "Kirill A . Shutemov" <kirill@shutemov.name>
Cc: Linux-MM <linux-mm@kvack.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	 linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	Amir Goldstein <amir73il@gmail.com>
Subject: [PATCH 0/4] Some more lock_page work..
Date: Tue, 13 Oct 2020 12:59:41 -0700	[thread overview]
Message-ID: <CAHk-=wgkD+sVx3cHAAzhVO5orgksY=7i8q6mbzwBjN0+4XTAUw@mail.gmail.com> (raw)

So this is all just preliminary, but I'd really like to have people
think more about the page fault handling of the page lock, and I have
a small experimental series of patches for people to look at and maybe
get the discussion started.

The three first patches are really just fairly trivial cleanups. They
also likely don't really matter, because the *bulk* of all faults -
particularly the ones that really shouldn't need any page locking
games - should be all about just "filemap_map_pages()". Which is that
optimistic "let's insert pages from the page cache as efficiently as
possible" case.

That's how all the normal private pages that don't actually get
modified (so executables, but also any load that uses mmap as a
zero-copy read) should generally get populated.

That code doesn't actually do "lock_page()" itself (because it all
runs under the RCU read lock), but it does to do a trylock, and give
up if the page was locked. Which is fine as long as you don't get any
contention, but any concurrent faults of the same page in different
address spaces will then just mess with other faulters and cause it to
fall out of the fast path.

And looking at that code, I'm pretty sure it doesn't actually *need*
the page lock. It wants the page lock for two reasons:

 - the truncation worries (which may or may not be relevant - xfs
wraps the map_pages with xfs_ilock)

 - compound page worries (sub-page mapcount updates and page splitting issues)

The compound page case I'm not sure about, but it's probably fine to
just lock the page in that case - once we end up actually just mapping
a hugepage, the number of page faults should be small enough that it
probably doesn't matter.

The truncation thing could be handled like xfs does, but honestly, I
think it can equally well be handled by just doing some operations in
the right order, and double-checking that we don't race with truncate.
IOW, first increasing the page mapcount, and then re-checking that the
page still isn't locked and the mapping is still valid, and reachable
in the xarray.

Because we can still just drop out of this loop and not populate the
page table if we see anything odd going on, but if *this* code doesn't
bother getting the page lock (and we make the COW code not do it
either), then in all the normal cases you will never have that "fall
out of the common case".

IOW, I think right now the thing that makes us fall back to the actual
page lock is this code itself: by doing the 'trylock", it will make
other users of the same page not able to do the fast-case. And I think
the trylock is unnecessary.

ANYWAY. The patch I have here isn't actually that "just do the checks
in the right order" patch. No, it's a dirty nasty "a private mapping
doesn't need to be so careful" patch. Ugly, brutish and nasty. Not the
right thing at all. But I'm doing it here simply because I wanted to
test it out and get people to look at this.

This code is "tested" in the sense that it builds for me, and I'm
actually running it right now. But I haven't actually stress-tested it
or tried to see if it gets rid of some page lock heavy cases.

Comments?

                         Linus


             reply	other threads:[~2020-10-13 20:00 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-13 19:59 Linus Torvalds [this message]
2020-10-13 20:03 ` Linus Torvalds
2020-10-14 13:05   ` Kirill A. Shutemov
2020-10-14 16:53     ` Linus Torvalds
2020-10-14 18:15       ` Matthew Wilcox
2020-10-15 10:41         ` Kirill A. Shutemov
2020-10-15  9:43       ` Kirill A. Shutemov
2020-10-15 16:44         ` Linus Torvalds
2020-10-14  5:50 ` Hugh Dickins
     [not found] ` <4794a3fa3742a5e84fb0f934944204b55730829b.camel@lca.pw>
2020-10-15  2:44   ` Linus Torvalds
2020-10-15 15:16     ` Possible deadlock in fuse write path (Was: Re: [PATCH 0/4] Some more lock_page work..) Vivek Goyal
2020-10-15 19:55       ` Vivek Goyal
2020-10-15 21:21         ` Linus Torvalds
2020-10-16 10:02           ` Miklos Szeredi
2020-10-16 12:27             ` Matthew Wilcox
2020-10-20 20:42             ` Vivek Goyal
2020-10-21  7:40               ` Miklos Szeredi
2020-10-21 20:12                 ` Vivek Goyal
2020-10-28 20:29                   ` Miklos Szeredi
2021-02-09 10:01                     ` Miklos Szeredi
2021-02-09 19:09                       ` Vivek Goyal
2020-10-16 18:19           ` Vivek Goyal
2020-10-16 18:24             ` Linus Torvalds
2020-10-16 18:24               ` Linus Torvalds
2020-10-16 23:03             ` Dave Chinner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAHk-=wgkD+sVx3cHAAzhVO5orgksY=7i8q6mbzwBjN0+4XTAUw@mail.gmail.com' \
    --to=torvalds@linux-foundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=amir73il@gmail.com \
    --cc=hughd@google.com \
    --cc=kirill@shutemov.name \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=willy@infradead.org \
    --subject='Re: [PATCH 0/4] Some more lock_page work..' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).