All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrea Arcangeli <andrea@qumranet.com>
To: Robin Holt <holt@sgi.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>,
	Christoph Lameter <clameter@sgi.com>,
	akpm@linux-foundation.org, Nick Piggin <npiggin@suse.de>,
	Steve Wise <swise@opengridcomputing.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	linux-mm@kvack.org, Kanoj Sarcar <kanojsarcar@yahoo.com>,
	Roland Dreier <rdreier@cisco.com>, Jack Steiner <steiner@sgi.com>,
	linux-kernel@vger.kernel.org, Avi Kivity <avi@qumranet.com>,
	kvm-devel@lists.sourceforge.net, general@lists.openfabrics.org,
	Hugh Dickins <hugh@veritas.com>
Subject: Re: [PATCH 1 of 9] Lock the entire mm to prevent any mmu related operation to happen
Date: Sat, 26 Apr 2008 02:57:26 +0200	[thread overview]
Message-ID: <20080426005726.GA9514@duo.random> (raw)
In-Reply-To: <20080425192532.GA19717@sgi.com>

On Fri, Apr 25, 2008 at 02:25:32PM -0500, Robin Holt wrote:
> I think you still need mm_lock (unless I miss something).  What happens
> when one callout is scanning mmu_notifier_invalidate_range_start() and
> you unlink.  That list next pointer with LIST_POISON1 which is a really
> bad address for the processor to track.

Ok, _release list_del_init qcan't race with that because it happens in
exit_mmap when no other mmu notifier can trigger anymore.

_unregister can run concurrently but it does list_del_rcu, that only
overwrites the pprev pointer with LIST_POISON2. The
mmu_notifier_invalidate_range_start won't crash on LIST_POISON1 thanks
to srcu.

Actually I did more changes than necessary, for example I noticed the
mmu_notifier_register can return a list_add_head instead of
list_add_head_rcu. _register can't race against _release thanks to the
mm_users temporary or implicit pin. _register can't race against
_unregister thanks to the mmu_notifier_mm->lock. And register can't
race against all other mmu notifiers thanks to the mm_lock.

At this time I've no other pending patches on top of v14-pre3 other
than the below micro-optimizing cleanup. It'd be great to have
confirmation that v14-pre3 passes GRU/XPMEM regressions tests as well
as my KVM testing already passed successfully on it. I'll forward
v14-pre3 mmu-notifier-core plus the below to Andrew tomorrow, I'm
trying to be optimistic here! ;)

diff --git a/mm/mmu_notifier.c b/mm/mmu_notifier.c
--- a/mm/mmu_notifier.c
+++ b/mm/mmu_notifier.c
@@ -187,7 +187,7 @@ int mmu_notifier_register(struct mmu_not
 	 * current->mm or explicitly with get_task_mm() or similar).
 	 */
 	spin_lock(&mm->mmu_notifier_mm->lock);
-	hlist_add_head_rcu(&mn->hlist, &mm->mmu_notifier_mm->list);
+	hlist_add_head(&mn->hlist, &mm->mmu_notifier_mm->list);
 	spin_unlock(&mm->mmu_notifier_mm->lock);
 out_unlock:
 	mm_unlock(mm, &data);

WARNING: multiple messages have this Message-ID (diff)
From: Andrea Arcangeli <andrea@qumranet.com>
To: Robin Holt <holt@sgi.com>
Cc: Nick Piggin <npiggin@suse.de>,
	Steve Wise <swise@opengridcomputing.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	kvm-devel@lists.sourceforge.net,
	Kanoj Sarcar <kanojsarcar@yahoo.com>,
	Roland Dreier <rdreier@cisco.com>,
	linux-kernel@vger.kernel.org, Avi Kivity <avi@qumranet.com>,
	linux-mm@kvack.org, general@lists.openfabrics.org,
	Hugh Dickins <hugh@veritas.com>,
	akpm@linux-foundation.org, Jack Steiner <steiner@sgi.com>,
	Christoph Lameter <clameter@sgi.com>
Subject: Re: [PATCH 1 of 9] Lock the entire mm to prevent any mmu related	operation to happen
Date: Sat, 26 Apr 2008 02:57:26 +0200	[thread overview]
Message-ID: <20080426005726.GA9514@duo.random> (raw)
In-Reply-To: <20080425192532.GA19717@sgi.com>

On Fri, Apr 25, 2008 at 02:25:32PM -0500, Robin Holt wrote:
> I think you still need mm_lock (unless I miss something).  What happens
> when one callout is scanning mmu_notifier_invalidate_range_start() and
> you unlink.  That list next pointer with LIST_POISON1 which is a really
> bad address for the processor to track.

Ok, _release list_del_init qcan't race with that because it happens in
exit_mmap when no other mmu notifier can trigger anymore.

_unregister can run concurrently but it does list_del_rcu, that only
overwrites the pprev pointer with LIST_POISON2. The
mmu_notifier_invalidate_range_start won't crash on LIST_POISON1 thanks
to srcu.

Actually I did more changes than necessary, for example I noticed the
mmu_notifier_register can return a list_add_head instead of
list_add_head_rcu. _register can't race against _release thanks to the
mm_users temporary or implicit pin. _register can't race against
_unregister thanks to the mmu_notifier_mm->lock. And register can't
race against all other mmu notifiers thanks to the mm_lock.

At this time I've no other pending patches on top of v14-pre3 other
than the below micro-optimizing cleanup. It'd be great to have
confirmation that v14-pre3 passes GRU/XPMEM regressions tests as well
as my KVM testing already passed successfully on it. I'll forward
v14-pre3 mmu-notifier-core plus the below to Andrew tomorrow, I'm
trying to be optimistic here! ;)

diff --git a/mm/mmu_notifier.c b/mm/mmu_notifier.c
--- a/mm/mmu_notifier.c
+++ b/mm/mmu_notifier.c
@@ -187,7 +187,7 @@ int mmu_notifier_register(struct mmu_not
 	 * current->mm or explicitly with get_task_mm() or similar).
 	 */
 	spin_lock(&mm->mmu_notifier_mm->lock);
-	hlist_add_head_rcu(&mn->hlist, &mm->mmu_notifier_mm->list);
+	hlist_add_head(&mn->hlist, &mm->mmu_notifier_mm->list);
 	spin_unlock(&mm->mmu_notifier_mm->lock);
 out_unlock:
 	mm_unlock(mm, &data);

-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference 
Don't miss this year's exciting event. There's still time to save $100. 
Use priority code J8TL2D2. 
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone

WARNING: multiple messages have this Message-ID (diff)
From: Andrea Arcangeli <andrea@qumranet.com>
To: Robin Holt <holt@sgi.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>,
	Christoph Lameter <clameter@sgi.com>,
	akpm@linux-foundation.org, Nick Piggin <npiggin@suse.de>,
	Steve Wise <swise@opengridcomputing.com>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	linux-mm@kvack.org, Kanoj Sarcar <kanojsarcar@yahoo.com>,
	Roland Dreier <rdreier@cisco.com>, Jack Steiner <steiner@sgi.com>,
	linux-kernel@vger.kernel.org, Avi Kivity <avi@qumranet.com>,
	kvm-devel@lists.sourceforge.net, general@lists.openfabrics.org,
	Hugh Dickins <hugh@veritas.com>
Subject: Re: [PATCH 1 of 9] Lock the entire mm to prevent any mmu related operation to happen
Date: Sat, 26 Apr 2008 02:57:26 +0200	[thread overview]
Message-ID: <20080426005726.GA9514@duo.random> (raw)
In-Reply-To: <20080425192532.GA19717@sgi.com>

On Fri, Apr 25, 2008 at 02:25:32PM -0500, Robin Holt wrote:
> I think you still need mm_lock (unless I miss something).  What happens
> when one callout is scanning mmu_notifier_invalidate_range_start() and
> you unlink.  That list next pointer with LIST_POISON1 which is a really
> bad address for the processor to track.

Ok, _release list_del_init qcan't race with that because it happens in
exit_mmap when no other mmu notifier can trigger anymore.

_unregister can run concurrently but it does list_del_rcu, that only
overwrites the pprev pointer with LIST_POISON2. The
mmu_notifier_invalidate_range_start won't crash on LIST_POISON1 thanks
to srcu.

Actually I did more changes than necessary, for example I noticed the
mmu_notifier_register can return a list_add_head instead of
list_add_head_rcu. _register can't race against _release thanks to the
mm_users temporary or implicit pin. _register can't race against
_unregister thanks to the mmu_notifier_mm->lock. And register can't
race against all other mmu notifiers thanks to the mm_lock.

At this time I've no other pending patches on top of v14-pre3 other
than the below micro-optimizing cleanup. It'd be great to have
confirmation that v14-pre3 passes GRU/XPMEM regressions tests as well
as my KVM testing already passed successfully on it. I'll forward
v14-pre3 mmu-notifier-core plus the below to Andrew tomorrow, I'm
trying to be optimistic here! ;)

diff --git a/mm/mmu_notifier.c b/mm/mmu_notifier.c
--- a/mm/mmu_notifier.c
+++ b/mm/mmu_notifier.c
@@ -187,7 +187,7 @@ int mmu_notifier_register(struct mmu_not
 	 * current->mm or explicitly with get_task_mm() or similar).
 	 */
 	spin_lock(&mm->mmu_notifier_mm->lock);
-	hlist_add_head_rcu(&mn->hlist, &mm->mmu_notifier_mm->list);
+	hlist_add_head(&mn->hlist, &mm->mmu_notifier_mm->list);
 	spin_unlock(&mm->mmu_notifier_mm->lock);
 out_unlock:
 	mm_unlock(mm, &data);

--
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>

  reply	other threads:[~2008-04-26  0:57 UTC|newest]

Thread overview: 125+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-04-08 15:44 [PATCH 0 of 9] mmu notifier #v12 Andrea Arcangeli
2008-04-08 15:44 ` Andrea Arcangeli
2008-04-08 15:44 ` [ofa-general] " Andrea Arcangeli
2008-04-08 15:44 ` [PATCH 1 of 9] Lock the entire mm to prevent any mmu related operation to happen Andrea Arcangeli
2008-04-08 15:44   ` Andrea Arcangeli
2008-04-08 15:44   ` [ofa-general] " Andrea Arcangeli
2008-04-16 16:33   ` Robin Holt
2008-04-16 16:33     ` Robin Holt
2008-04-16 16:33     ` [ofa-general] " Robin Holt
2008-04-16 18:35     ` Christoph Lameter
2008-04-16 18:35       ` Christoph Lameter
2008-04-16 18:35       ` [ofa-general] " Christoph Lameter
2008-04-16 19:02       ` Robin Holt
2008-04-16 19:02         ` Robin Holt
2008-04-16 19:02         ` Robin Holt
2008-04-16 19:15         ` Christoph Lameter
2008-04-16 19:15           ` Christoph Lameter
2008-04-16 19:15           ` [ofa-general] " Christoph Lameter
2008-04-17 11:14           ` Robin Holt
2008-04-17 11:14             ` Robin Holt
2008-04-17 11:14             ` [ofa-general] " Robin Holt
2008-04-17 15:51       ` Andrea Arcangeli
2008-04-17 15:51         ` Andrea Arcangeli
2008-04-17 16:36         ` Robin Holt
2008-04-17 16:36           ` Robin Holt
2008-04-17 17:14           ` Andrea Arcangeli
2008-04-17 17:14             ` Andrea Arcangeli
2008-04-17 17:14             ` Andrea Arcangeli
2008-04-17 17:25             ` Robin Holt
2008-04-17 17:25               ` Robin Holt
2008-04-17 17:25               ` [ofa-general] " Robin Holt
2008-04-17 19:10             ` Christoph Lameter
2008-04-17 19:10               ` Christoph Lameter
2008-04-17 22:16               ` Andrea Arcangeli
2008-04-17 22:16                 ` Andrea Arcangeli
2008-04-17 22:16                 ` [ofa-general] " Andrea Arcangeli
2008-04-22  5:06   ` Rusty Russell
2008-04-22  5:06     ` Rusty Russell
2008-04-22  5:06     ` Rusty Russell
2008-04-25 16:56     ` Andrea Arcangeli
2008-04-25 16:56       ` Andrea Arcangeli
2008-04-25 17:04       ` Andrea Arcangeli
2008-04-25 17:04         ` Andrea Arcangeli
2008-04-25 19:25       ` Robin Holt
2008-04-25 19:25         ` Robin Holt
2008-04-25 19:25         ` [ofa-general] " Robin Holt
2008-04-26  0:57         ` Andrea Arcangeli [this message]
2008-04-26  0:57           ` Andrea Arcangeli
2008-04-26  0:57           ` Andrea Arcangeli
2008-04-08 15:44 ` [PATCH 2 of 9] Core of mmu notifiers Andrea Arcangeli
2008-04-08 15:44   ` Andrea Arcangeli
2008-04-08 15:44   ` [ofa-general] " Andrea Arcangeli
2008-04-08 16:26   ` Robin Holt
2008-04-08 16:26     ` Robin Holt
2008-04-08 16:26     ` [ofa-general] " Robin Holt
2008-04-08 17:05     ` Andrea Arcangeli
2008-04-08 17:05       ` Andrea Arcangeli
2008-04-14 19:57   ` Christoph Lameter
2008-04-14 19:57     ` Christoph Lameter
2008-04-14 19:57     ` [ofa-general] " Christoph Lameter
2008-04-14 19:59   ` Christoph Lameter
2008-04-14 19:59     ` Christoph Lameter
2008-04-14 19:59     ` Christoph Lameter
2008-04-08 15:44 ` [PATCH 3 of 9] Moves all mmu notifier methods outside the PT lock (first and not last Andrea Arcangeli
2008-04-08 15:44   ` Andrea Arcangeli
2008-04-08 15:44   ` [ofa-general] " Andrea Arcangeli
2008-04-14 19:57   ` Christoph Lameter
2008-04-14 19:57     ` Christoph Lameter
2008-04-14 19:57     ` Christoph Lameter
2008-04-08 15:44 ` [PATCH 4 of 9] Move the tlb flushing into free_pgtables. The conversion of the locks Andrea Arcangeli
2008-04-08 15:44   ` Andrea Arcangeli
2008-04-08 15:44   ` [ofa-general] " Andrea Arcangeli
2008-04-08 15:44 ` [PATCH 5 of 9] The conversion to a rwsem allows callbacks during rmap traversal Andrea Arcangeli
2008-04-08 15:44   ` Andrea Arcangeli
2008-04-08 15:44   ` [ofa-general] " Andrea Arcangeli
2008-04-08 15:44 ` [PATCH 6 of 9] We no longer abort unmapping in unmap vmas because we can reschedule while Andrea Arcangeli
2008-04-08 15:44   ` Andrea Arcangeli
2008-04-08 15:44   ` [ofa-general] " Andrea Arcangeli
2008-04-08 15:44 ` [PATCH 7 of 9] Convert the anon_vma spinlock to a rw semaphore. This allows concurrent Andrea Arcangeli
2008-04-08 15:44   ` Andrea Arcangeli
2008-04-08 15:44   ` [ofa-general] " Andrea Arcangeli
2008-04-08 15:44 ` [PATCH 8 of 9] XPMEM would have used sys_madvise() except that madvise_dontneed() Andrea Arcangeli
2008-04-08 15:44   ` Andrea Arcangeli
2008-04-08 15:44   ` [ofa-general] " Andrea Arcangeli
2008-04-08 15:44 ` [PATCH 9 of 9] This patch adds a lock ordering rule to avoid a potential deadlock when Andrea Arcangeli
2008-04-08 15:44   ` Andrea Arcangeli
2008-04-08 15:44   ` [ofa-general] " Andrea Arcangeli
2008-04-08 21:46 ` [PATCH 0 of 9] mmu notifier #v12 Avi Kivity
2008-04-08 21:46   ` Avi Kivity
2008-04-08 21:46   ` [ofa-general] " Avi Kivity
2008-04-08 22:06   ` Andrea Arcangeli
2008-04-08 22:06     ` Andrea Arcangeli
2008-04-08 22:06     ` [ofa-general] " Andrea Arcangeli
2008-04-09 13:17 ` Robin Holt
2008-04-09 13:17   ` Robin Holt
2008-04-09 13:17   ` [ofa-general] " Robin Holt
2008-04-09 14:44   ` Andrea Arcangeli
2008-04-09 14:44     ` Andrea Arcangeli
2008-04-09 14:44     ` [ofa-general] " Andrea Arcangeli
2008-04-09 18:55     ` Robin Holt
2008-04-09 18:55       ` Robin Holt
2008-04-09 18:55       ` [ofa-general] " Robin Holt
2008-04-22  7:20       ` Andrea Arcangeli
2008-04-22  7:20         ` Andrea Arcangeli
2008-04-22  7:20         ` [ofa-general] " Andrea Arcangeli
2008-04-22 12:00         ` Andrea Arcangeli
2008-04-22 12:00           ` Andrea Arcangeli
2008-04-22 12:00           ` [ofa-general] " Andrea Arcangeli
2008-04-22 13:01           ` Robin Holt
2008-04-22 13:01             ` Robin Holt
2008-04-22 13:01             ` [ofa-general] " Robin Holt
2008-04-22 13:21             ` Andrea Arcangeli
2008-04-22 13:21               ` Andrea Arcangeli
2008-04-22 13:21               ` [ofa-general] " Andrea Arcangeli
2008-04-22 13:36               ` Robin Holt
2008-04-22 13:36                 ` Robin Holt
2008-04-22 13:36                 ` [ofa-general] " Robin Holt
2008-04-22 13:48                 ` Andrea Arcangeli
2008-04-22 13:48                   ` Andrea Arcangeli
2008-04-22 13:48                   ` Andrea Arcangeli
2008-04-22 15:26                   ` Robin Holt
2008-04-22 15:26                     ` Robin Holt
2008-04-14 23:09 ` Christoph Lameter
2008-04-14 23:09   ` Christoph Lameter
2008-04-14 23:09   ` [ofa-general] " Christoph Lameter

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=20080426005726.GA9514@duo.random \
    --to=andrea@qumranet.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=akpm@linux-foundation.org \
    --cc=avi@qumranet.com \
    --cc=clameter@sgi.com \
    --cc=general@lists.openfabrics.org \
    --cc=holt@sgi.com \
    --cc=hugh@veritas.com \
    --cc=kanojsarcar@yahoo.com \
    --cc=kvm-devel@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=npiggin@suse.de \
    --cc=rdreier@cisco.com \
    --cc=rusty@rustcorp.com.au \
    --cc=steiner@sgi.com \
    --cc=swise@opengridcomputing.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.