linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Nicholas Piggin <npiggin@gmail.com>
To: linux-mm@kvack.org
Cc: Nicholas Piggin <npiggin@gmail.com>,
	Suresh Siddha <suresh.b.siddha@intel.com>,
	"David S. Miller" <davem@davemloft.net>,
	Hugh Dickins <hughd@google.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Suresh Siddha <sbsiddha@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>
Subject: [RFC PATCH] mm: generalise COW SMC TLB flushing race comment
Date: Tue, 15 Dec 2020 22:11:19 +1000	[thread overview]
Message-ID: <20201215121119.351650-1-npiggin@gmail.com> (raw)

I'm not sure if I'm completely missing something here, but AFAIKS the
reference to the mysterious "COW SMC race" confuses the issue. The original
changelog and mailing list thread didn't help me either.

This SMC race is where the problem was detected, but isn't the general
problem bigger and more obvious: that the new PTE could be picked
up at any time by any TLB while entries for the old PTE exist in other
TLBs before the TLB flush takes effect?

The case where the iTLB and dTLB of a CPU are pointing at different
pages is an interesting one but follows from the general problem.

The other (minor) thing with the comment I think it makes it a bit
clearer to say what the old code was doing (i.e., it avoids the race
as opposed to what?).

References: 4ce072f1faf29 ("mm: fix a race condition under SMC + COW")
---
 mm/memory.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/mm/memory.c b/mm/memory.c
index ecda25d855ea..fd034b908070 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2880,11 +2880,13 @@ static vm_fault_t wp_page_copy(struct vm_fault *vmf)
 		entry = mk_pte(new_page, vma->vm_page_prot);
 		entry = pte_mkyoung(entry);
 		entry = maybe_mkwrite(pte_mkdirty(entry), vma);
+
 		/*
 		 * Clear the pte entry and flush it first, before updating the
-		 * pte with the new entry. This will avoid a race condition
-		 * seen in the presence of one thread doing SMC and another
-		 * thread doing COW.
+		 * pte with the new entry, to keep TLBs on different CPUs in
+		 * sync. This code used to set the new PTE then flush TLBs, but
+		 * that left a window where the new PTE could be loaded into
+		 * some TLBs while the old PTE remains in others.
 		 */
 		ptep_clear_flush_notify(vma, vmf->address, vmf->pte);
 		page_add_new_anon_rmap(new_page, vma, vmf->address, false);
-- 
2.23.0



             reply	other threads:[~2020-12-15 12:11 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-15 12:11 Nicholas Piggin [this message]
2020-12-15 13:36 ` Matthew Wilcox
2020-12-18 18:58 ` Andrew Morton

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=20201215121119.351650-1-npiggin@gmail.com \
    --to=npiggin@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=davem@davemloft.net \
    --cc=hughd@google.com \
    --cc=linux-mm@kvack.org \
    --cc=peterz@infradead.org \
    --cc=sbsiddha@gmail.com \
    --cc=suresh.b.siddha@intel.com \
    --cc=torvalds@linux-foundation.org \
    /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