linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Peter Xu <peterx@redhat.com>
To: linux-kernel@vger.kernel.org, linux-mm@kvack.org
Cc: David Hildenbrand <david@redhat.com>,
	peterx@redhat.com, Andrea Arcangeli <aarcange@redhat.com>,
	Yang Shi <shy828301@gmail.com>, Vlastimil Babka <vbabka@suse.cz>,
	Hugh Dickins <hughd@google.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Alistair Popple <apopple@nvidia.com>,
	"Kirill A . Shutemov" <kirill@shutemov.name>
Subject: [PATCH RFC v2 1/2] mm: Don't skip swap entry even if zap_details specified
Date: Mon, 15 Nov 2021 21:49:50 +0800	[thread overview]
Message-ID: <20211115134951.85286-2-peterx@redhat.com> (raw)
In-Reply-To: <20211115134951.85286-1-peterx@redhat.com>

This check existed since the 1st git commit of Linux repository, but at that
time there's no page migration yet so I think it's okay.

With page migration enabled, it should logically be possible that we zap some
shmem pages during migration.  When that happens, IIUC the old code could have
the RSS counter accounted wrong on MM_SHMEMPAGES because we will zap the ptes
without decreasing the counters for the migrating entries.  I have no unit test
to prove it as I don't know an easy way to trigger this condition, though.

Besides, the optimization itself is already confusing IMHO to me in a few points:

  - The wording "skip swap entries" is confusing, because we're not skipping all
    swap entries - we handle device private/exclusive pages before that.

  - The skip behavior is enabled as long as zap_details pointer passed over.
    It's very hard to figure that out for a new zap caller because it's unclear
    why we should skip swap entries when we have zap_details specified.

  - With modern systems, especially performance critical use cases, swap
    entries should be rare, so I doubt the usefulness of this optimization
    since it should be on a slow path anyway.

  - It is not aligned with what we do with huge pmd swap entries, where in
    zap_huge_pmd() we'll do the accounting unconditionally.

This patch drops that trick, so we handle swap ptes coherently.  Meanwhile we
should do the same mapping check upon migration entries too.

Signed-off-by: Peter Xu <peterx@redhat.com>
---
 mm/memory.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index 8f1de811a1dc..e454f3c6aeb9 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1382,16 +1382,14 @@ static unsigned long zap_pte_range(struct mmu_gather *tlb,
 			continue;
 		}
 
-		/* If details->check_mapping, we leave swap entries. */
-		if (unlikely(details))
-			continue;
-
 		if (!non_swap_entry(entry))
 			rss[MM_SWAPENTS]--;
 		else if (is_migration_entry(entry)) {
 			struct page *page;
 
 			page = pfn_swap_entry_to_page(entry);
+			if (unlikely(zap_skip_check_mapping(details, page)))
+				continue;
 			rss[mm_counter(page)]--;
 		}
 		if (unlikely(!free_swap_and_cache(entry)))
-- 
2.32.0


  reply	other threads:[~2021-11-15 13:51 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-15 13:49 [PATCH RFC v2 0/2] mm: Rework zap ptes on swap entries Peter Xu
2021-11-15 13:49 ` Peter Xu [this message]
2021-12-02 11:06   ` [PATCH RFC v2 1/2] mm: Don't skip swap entry even if zap_details specified Alistair Popple
2021-12-03  3:21     ` Peter Xu
2021-12-03  5:33       ` Alistair Popple
2021-12-03  6:59         ` Peter Xu
2022-01-09  1:19   ` Hugh Dickins
2022-01-12 13:18     ` Peter Xu
2022-01-12 13:26       ` Peter Xu
2022-01-13  3:47         ` Hugh Dickins
2022-01-20 10:32           ` Peter Xu
2022-01-21  3:11             ` Peter Xu
2022-01-21  5:11               ` Peter Xu
2022-01-24  6:51                 ` Hugh Dickins
2022-01-24  9:13                   ` Peter Xu
2022-01-24  6:29             ` Hugh Dickins
2022-01-24  8:54               ` Peter Xu
2022-01-24 11:01                 ` Peter Xu
2022-01-10  8:37   ` David Hildenbrand
2022-01-11  7:40     ` Alistair Popple
2022-01-11  9:00       ` David Hildenbrand
2021-11-15 13:49 ` [PATCH RFC v2 2/2] mm: Rework swap handling of zap_pte_range Peter Xu
2021-11-15 13:57   ` Matthew Wilcox
2021-11-16  5:06     ` Peter Xu
2021-11-16  8:51     ` John Hubbard
2021-11-16 13:11       ` Matthew Wilcox
2021-11-16 19:06         ` John Hubbard

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=20211115134951.85286-2-peterx@redhat.com \
    --to=peterx@redhat.com \
    --cc=aarcange@redhat.com \
    --cc=akpm@linux-foundation.org \
    --cc=apopple@nvidia.com \
    --cc=david@redhat.com \
    --cc=hughd@google.com \
    --cc=kirill@shutemov.name \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=shy828301@gmail.com \
    --cc=vbabka@suse.cz \
    /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).