linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Andrea Arcangeli <andrea@qumranet.com>
To: Christoph Lameter <clameter@sgi.com>
Cc: Robin Holt <holt@sgi.com>, Avi Kivity <avi@qumranet.com>,
	Izik Eidus <izike@qumranet.com>,
	kvm-devel@lists.sourceforge.net,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	steiner@sgi.com, linux-kernel@vger.kernel.org,
	linux-mm@kvack.org, daniel.blueman@quadrics.com
Subject: Re: mmu_notifier: close hole in fork
Date: Fri, 1 Feb 2008 01:01:58 +0100	[thread overview]
Message-ID: <20080201000158.GT7185@v2.random> (raw)
In-Reply-To: <Pine.LNX.4.64.0801311355260.27804@schroedinger.engr.sgi.com>

On Thu, Jan 31, 2008 at 02:01:43PM -0800, Christoph Lameter wrote:
> Talking to Robin and Jack we found taht we still have a hole during fork. 
> Fork may set a pte writeprotect. At that point the remote pte are 
> not marked readonly(!). Remote writes may occur to pages that are marked 
> readonly locally without this patch.

Good catch! This was missing also in my #v5 (KVM doesn't need that
because the only possible cows on sptes can be generated by ksm, but
it would have been a problem for GRU). The more I think about it, the
more I think GRU will run so nicely with follow_page taking zero
additional locking and with the nicely scalable PT lock that doesn't
require 1024 threads to trash on the same lock on 1024 different cpus
if they access addresses that aren't in the same naturally aligned 2M
chunk of virtual address space. I understood follow_page was
performance critical for GRU, if yes, then I hope my _dual_ approach
is by far the best for at least GRU (and KVM of course for the very
same reason), and of course it'll fit XPMEM too the moment you add
invalidate_range_start/end too.

Signed-off-by: Andrea Arcangeli <andrea@qumranet.com>

diff --git a/mm/memory.c b/mm/memory.c
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -494,6 +494,7 @@ static int copy_pte_range(struct mm_stru
 	spinlock_t *src_ptl, *dst_ptl;
 	int progress = 0;
 	int rss[2];
+	unsigned long start;
 
 again:
 	rss[1] = rss[0] = 0;
@@ -505,6 +506,7 @@ again:
 	spin_lock_nested(src_ptl, SINGLE_DEPTH_NESTING);
 	arch_enter_lazy_mmu_mode();
 
+	start = addr;
 	do {
 		/*
 		 * We are holding two locks at this point - either of them
@@ -525,6 +527,8 @@ again:
 	} while (dst_pte++, src_pte++, addr += PAGE_SIZE, addr != end);
 
 	arch_leave_lazy_mmu_mode();
+	if (is_cow_mapping(vma->vm_flags))
+		mmu_notifier(invalidate_pages, vma->vm_mm, start, addr);
 	spin_unlock(src_ptl);
 	pte_unmap_nested(src_pte - 1);
 	add_mm_rss(dst_mm, rss[0], rss[1]);

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2008-02-01  0:01 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-01-31  4:57 [patch 0/3] [RFC] MMU Notifiers V4 Christoph Lameter
2008-01-31  4:57 ` [patch 1/3] mmu_notifier: Core code Christoph Lameter
2008-02-01  1:56   ` Jack Steiner
2008-02-01  2:24     ` Robin Holt
2008-02-01  2:37       ` Jack Steiner
2008-02-01  2:39         ` Christoph Lameter
2008-02-01  2:31   ` Robin Holt
2008-02-01  2:39     ` Christoph Lameter
2008-02-01  2:47       ` Robin Holt
2008-02-01  3:01         ` Christoph Lameter
2008-02-01  3:01       ` Jack Steiner
2008-02-01  3:03         ` Christoph Lameter
2008-02-01  3:52   ` Robin Holt
2008-02-01  3:58     ` Christoph Lameter
2008-02-01  4:15       ` Robin Holt
2008-02-03  1:33       ` Andrea Arcangeli
2008-02-04 19:13         ` Christoph Lameter
2008-01-31  4:57 ` [patch 2/3] mmu_notifier: Callbacks to invalidate address ranges Christoph Lameter
2008-01-31 12:31   ` Andrea Arcangeli
2008-01-31 20:07     ` Christoph Lameter
2008-01-31 22:01     ` mmu_notifier: close hole in fork Christoph Lameter
2008-01-31 22:16       ` mmu_notifier: reduce size of mm_struct if !CONFIG_MMU_NOTIFIER Christoph Lameter
2008-01-31 22:21       ` mmu_notifier: Move mmu_notifier_release up to get rid of the invalidat_all() callback Christoph Lameter
2008-02-01  0:13         ` Andrea Arcangeli
2008-02-01  1:52           ` Christoph Lameter
2008-02-01  1:57           ` mmu_notifier: invalidate_range for move_page_tables Christoph Lameter
2008-02-01  2:38             ` Robin Holt
2008-02-01  2:41               ` Christoph Lameter
2008-02-01  0:01       ` Andrea Arcangeli [this message]
2008-02-01  1:48         ` mmu_notifier: close hole in fork Christoph Lameter
2008-02-01  4:24   ` [patch 2/3] mmu_notifier: Callbacks to invalidate address ranges Robin Holt
2008-02-01  4:43     ` Christoph Lameter
2008-02-01 10:32       ` Robin Holt
2008-02-01 10:37         ` Robin Holt
2008-02-01 19:13         ` Christoph Lameter
2008-01-31  4:57 ` [patch 3/3] mmu_notifier: invalidate_page callbacks Christoph Lameter
2008-01-31 17:18 ` [PATCH] mmu notifiers #v5 Andrea Arcangeli
2008-01-31 20:18   ` Christoph Lameter
2008-01-31 23:09     ` Christoph Lameter
2008-01-31 23:41       ` Andrea Arcangeli
2008-02-01  1:44         ` Christoph Lameter
2008-02-01 12:09           ` Andrea Arcangeli
2008-02-01 19:23             ` Christoph Lameter
2008-02-03  2:17               ` Andrea Arcangeli
2008-02-03  3:14                 ` Jack Steiner
2008-02-03  3:33                   ` Andrea Arcangeli
2008-02-04 19:09                 ` Christoph Lameter
2008-02-05  5:25                   ` Andrea Arcangeli
2008-02-05  6:11                     ` Christoph Lameter
2008-02-05 18:08                       ` Andrea Arcangeli
2008-02-05 18:17                         ` Christoph Lameter
2008-02-05 20:55                           ` Andrea Arcangeli
2008-02-05 22:06                             ` Christoph Lameter
2008-02-05 22:12                               ` Robin Holt
2008-02-05 22:26                               ` Andrea Arcangeli
2008-02-05 23:10                                 ` Christoph Lameter
2008-02-05 23:47                                   ` Andrea Arcangeli
2008-02-06  0:04                                     ` Christoph Lameter
2008-01-31 23:28     ` Andrea Arcangeli
2008-02-01  1:37       ` Christoph Lameter
2008-02-01  2:23         ` Robin Holt
2008-02-01  2:26           ` Christoph Lameter
2008-02-01 12:00         ` Andrea Arcangeli

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=20080201000158.GT7185@v2.random \
    --to=andrea@qumranet.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=avi@qumranet.com \
    --cc=clameter@sgi.com \
    --cc=daniel.blueman@quadrics.com \
    --cc=holt@sgi.com \
    --cc=izike@qumranet.com \
    --cc=kvm-devel@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=steiner@sgi.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).