linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hugh Dickins <hughd@google.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Yang Shi <yang.shi@linux.alibaba.com>,
	ktkhai@virtuozzo.com, jhubbard@nvidia.com, hughd@google.com,
	aarcange@redhat.com, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: [PATCH mmotm] mm: ksm: do not block on page lock when searching stable tree fix
Date: Mon, 18 Feb 2019 21:26:46 -0800 (PST)	[thread overview]
Message-ID: <alpine.LSU.2.11.1902182122280.6914@eggly.anvils> (raw)

I hit the kernel BUG at mm/ksm.c:809! quite easily under KSM swapping
load.  That's the BUG_ON(age > 1) in remove_rmap_item_from_tree().

There is a comment above it, but explaining in more detail: KSM saves
effort by not fully maintaining the unstable tree like a proper RB
tree throughout, but at the start of each pass forgetting the old tree
and rebuilding anew from scratch. But that means that whenever it looks
like we need to remove an item from the unstable tree, we have to check
whether it has already been linked into the new tree this time around
(hence rb_erase needed), or it's just a free-floating leftover from the
previous tree.

"age" 0 or 1 says which: but if it's more than 1, then something has
gone wrong: cmp_and_merge_page() was forgetting to remove the item
in the new EBUSY case.

Signed-off-by: Hugh Dickins <hughd@google.com>
---
Fix to fold into
mm-ksm-do-not-block-on-page-lock-when-searching-stable-tree.patch

I like that patch better now it has the mods suggested by John Hubbard;
but what I'd still really prefer to do is to make the patch unnecessary,
by reworking that window of KSM page migration so that there's just no
need for stable_tree_search() to take page lock.  We would all prefer
that.  However, each time I've gone to do so, it's turned out to need
more care than I expected, and I run out of time.  So, let's go with
what we have, and one day I might perhaps get back to it.

 mm/ksm.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

--- mmotm/mm/ksm.c	2019-02-14 15:16:13.000000000 -0800
+++ linux/mm/ksm.c	2019-02-18 20:36:44.707310427 -0800
@@ -2082,10 +2082,6 @@ static void cmp_and_merge_page(struct pa
 
 	/* We first start with searching the page inside the stable tree */
 	kpage = stable_tree_search(page);
-
-	if (PTR_ERR(kpage) == -EBUSY)
-		return;
-
 	if (kpage == page && rmap_item->head == stable_node) {
 		put_page(kpage);
 		return;
@@ -2094,6 +2090,9 @@ static void cmp_and_merge_page(struct pa
 	remove_rmap_item_from_tree(rmap_item);
 
 	if (kpage) {
+		if (PTR_ERR(kpage) == -EBUSY)
+			return;
+
 		err = try_to_merge_with_ksm_page(rmap_item, page, kpage);
 		if (!err) {
 			/*

             reply	other threads:[~2019-02-19  5:26 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-19  5:26 Hugh Dickins [this message]
2019-02-19 18:11 ` [PATCH mmotm] mm: ksm: do not block on page lock when searching stable tree fix Yang Shi

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=alpine.LSU.2.11.1902182122280.6914@eggly.anvils \
    --to=hughd@google.com \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=jhubbard@nvidia.com \
    --cc=ktkhai@virtuozzo.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=yang.shi@linux.alibaba.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).