* [PATCH 0/6] further anon_vma fixes & optimizations
@ 2010-06-21 20:31 Rik van Riel
2010-06-21 20:33 ` [PATCH -mm 1/6] mmap: remove unnecessary lock from __vma_link Rik van Riel
` (6 more replies)
0 siblings, 7 replies; 11+ messages in thread
From: Rik van Riel @ 2010-06-21 20:31 UTC (permalink / raw
To: linux-kernel, akpm, aarcange, linux-mm
A collection of anon_vma fixes and optimizations from Andrea's
transparent hugepage tree.
--
All rights reversed.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH -mm 1/6] mmap: remove unnecessary lock from __vma_link
2010-06-21 20:31 [PATCH 0/6] further anon_vma fixes & optimizations Rik van Riel
@ 2010-06-21 20:33 ` Rik van Riel
2010-06-21 20:33 ` [PATCH -mm 2/6] rmap: always add new vmas at the end Rik van Riel
` (5 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Rik van Riel @ 2010-06-21 20:33 UTC (permalink / raw
To: linux-kernel; +Cc: akpm, aarcange, linux-mm
From: Andrea Arcangeli <aarcange@redhat.com>
Subject: remove unnecessary lock from __vma_link
There's no anon-vma related mangling happening inside __vma_link anymore so no
need of anon_vma locking there.
Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
Signed-off-by: Rik van Riel <riel@redhat.com>
---
diff --git a/mm/mmap.c b/mm/mmap.c
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -469,12 +469,10 @@ static void vma_link(struct mm_struct *m
spin_lock(&mapping->i_mmap_lock);
vma->vm_truncate_count = mapping->truncate_count;
}
- vma_lock_anon_vma(vma);
__vma_link(mm, vma, prev, rb_link, rb_parent);
__vma_link_file(vma);
- vma_unlock_anon_vma(vma);
if (mapping)
spin_unlock(&mapping->i_mmap_lock);
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH -mm 2/6] rmap: always add new vmas at the end
2010-06-21 20:31 [PATCH 0/6] further anon_vma fixes & optimizations Rik van Riel
2010-06-21 20:33 ` [PATCH -mm 1/6] mmap: remove unnecessary lock from __vma_link Rik van Riel
@ 2010-06-21 20:33 ` Rik van Riel
2010-06-22 21:08 ` Andrew Morton
2010-06-21 20:34 ` [PATCH -mm 3/6] ksm: fix ksm swapin time optimization Rik van Riel
` (4 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Rik van Riel @ 2010-06-21 20:33 UTC (permalink / raw
To: linux-kernel; +Cc: akpm, aarcange, linux-mm
From: Andrea Arcangeli <aarcange@redhat.com>
Subject: always add new vmas at the end
Make sure to always add new VMAs at the end of the list. This
is important so rmap_walk does not miss a VMA that was created
during the rmap_walk.
The old code got this right most of the time due to luck, but
was buggy when anon_vma_prepare reused a mergeable anon_vma.
Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
Signed-off-by: Rik van Riel <riel@redhat.com>
---
diff --git a/mm/rmap.c b/mm/rmap.c
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -149,7 +149,7 @@ int anon_vma_prepare(struct vm_area_stru
avc->anon_vma = anon_vma;
avc->vma = vma;
list_add(&avc->same_vma, &vma->anon_vma_chain);
- list_add(&avc->same_anon_vma, &anon_vma->head);
+ list_add_tail(&avc->same_anon_vma, &anon_vma->head);
allocated = NULL;
avc = NULL;
}
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH -mm 3/6] ksm: fix ksm swapin time optimization
2010-06-21 20:31 [PATCH 0/6] further anon_vma fixes & optimizations Rik van Riel
2010-06-21 20:33 ` [PATCH -mm 1/6] mmap: remove unnecessary lock from __vma_link Rik van Riel
2010-06-21 20:33 ` [PATCH -mm 2/6] rmap: always add new vmas at the end Rik van Riel
@ 2010-06-21 20:34 ` Rik van Riel
2010-06-23 17:47 ` Andrea Arcangeli
2010-06-21 20:35 ` [PATCH -mm 4/6] always use anon_vma root pointer Rik van Riel
` (3 subsequent siblings)
6 siblings, 1 reply; 11+ messages in thread
From: Rik van Riel @ 2010-06-21 20:34 UTC (permalink / raw
To: linux-kernel; +Cc: akpm, aarcange, linux-mm, avi
From: Andrea Arcangeli <aarcange@redhat.com>
Subject: fix ksm swapin time optimization
The new anon-vma code, was suboptimal and it lead to erratic invocation of
ksm_does_need_to_copy. That leads to host hangs or guest vnc lockup, or weird
behavior. It's unclear why ksm_does_need_to_copy is unstable but the point is
that when KSM is not in use, ksm_does_need_to_copy must never run or we bounce
pages for no good reason. I suspect the same hangs will happen with KVM swaps.
But this at least fixes the regression in the new-anon-vma code and it only let
KSM bugs triggers when KSM is in use.
The code in do_swap_page likely doesn't cope well with a not-swapcache,
especially the memcg code.
Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
Signed-off-by: Rik van Riel <riel@redhat.com>
---
diff --git a/include/linux/ksm.h b/include/linux/ksm.h
--- a/include/linux/ksm.h
+++ b/include/linux/ksm.h
@@ -78,7 +78,7 @@ static inline struct page *ksm_might_nee
struct anon_vma *anon_vma = page_anon_vma(page);
if (!anon_vma ||
- (anon_vma == vma->anon_vma &&
+ (anon_vma->root == vma->anon_vma->root &&
page->index == linear_page_index(vma, address)))
return page;
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH -mm 4/6] always use anon_vma root pointer
2010-06-21 20:31 [PATCH 0/6] further anon_vma fixes & optimizations Rik van Riel
` (2 preceding siblings ...)
2010-06-21 20:34 ` [PATCH -mm 3/6] ksm: fix ksm swapin time optimization Rik van Riel
@ 2010-06-21 20:35 ` Rik van Riel
2010-06-21 20:38 ` [PATCH -mm 5/6] resurrect page_address_in_vma anon_vma check Rik van Riel
` (2 subsequent siblings)
6 siblings, 0 replies; 11+ messages in thread
From: Rik van Riel @ 2010-06-21 20:35 UTC (permalink / raw
To: linux-kernel; +Cc: akpm, aarcange, linux-mm
From: Andrea Arcangeli <aarcange@redhat.com>
Subject: always use anon_vma root pointer
Always use anon_vma->root pointer instead of anon_vma_chain.prev.
Also optimize the map-paths, if a mapping is already established no need to
overwrite it with root anon-vma list, we can keep the more finegrined anon-vma
and skip the overwrite: see the PageAnon check in !exclusive case. This is also
the optimization that hidden the ksm bug as this tends to make
ksm_might_need_to_copy skip the copy, but only the proper fix to
ksm_might_need_to_copy guarantees not triggering the ksm bug unless ksm is in
use. this is an optimization only...
Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
Signed-off-by: Rik van Riel <riel@redhat.com>
diff --git a/mm/rmap.c b/mm/rmap.c
index 006f223..2d9504d 100644
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -776,15 +776,13 @@ static void __page_set_anon_rmap(struct page *page,
* If the page isn't exclusively mapped into this vma,
* we must use the _oldest_ possible anon_vma for the
* page mapping!
- *
- * So take the last AVC chain entry in the vma, which is
- * the deepest ancestor, and use the anon_vma from that.
*/
if (!exclusive) {
- struct anon_vma_chain *avc;
- avc = list_entry(vma->anon_vma_chain.prev, struct anon_vma_chain, same_vma);
- anon_vma = avc->anon_vma;
- }
+ if (PageAnon(page))
+ return;
+ anon_vma = anon_vma->root;
+ } else
+ BUG_ON(PageAnon(page));
anon_vma = (void *) anon_vma + PAGE_MAPPING_ANON;
page->mapping = (struct address_space *) anon_vma;
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH -mm 5/6] resurrect page_address_in_vma anon_vma check
2010-06-21 20:31 [PATCH 0/6] further anon_vma fixes & optimizations Rik van Riel
` (3 preceding siblings ...)
2010-06-21 20:35 ` [PATCH -mm 4/6] always use anon_vma root pointer Rik van Riel
@ 2010-06-21 20:38 ` Rik van Riel
2010-06-21 20:39 ` [PATCH -mm 6/6] add anon_vma bug checks Rik van Riel
2010-06-21 21:48 ` [PATCH 0/6] further anon_vma fixes & optimizations Andrea Arcangeli
6 siblings, 0 replies; 11+ messages in thread
From: Rik van Riel @ 2010-06-21 20:38 UTC (permalink / raw
To: linux-kernel; +Cc: akpm, aarcange, linux-mm, Naoya Horiguchi
From: Andrea Arcangeli <aarcange@redhat.com>
Subject: resurrect page_address_in_vma anon_vma check
With root anon-vma it's trivial to keep doing the usual check as in
old-anon-vma code.
Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
Signed-off-by: Rik van Riel <riel@redhat.com>
---
diff --git a/mm/rmap.c b/mm/rmap.c
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -362,9 +362,10 @@ vma_address(struct page *page, struct vm
*/
unsigned long page_address_in_vma(struct page *page, struct vm_area_struct *vma)
{
- if (PageAnon(page))
- ;
- else if (page->mapping && !(vma->vm_flags & VM_NONLINEAR)) {
+ if (PageAnon(page)) {
+ if (vma->anon_vma->root != page_anon_vma(page)->root)
+ return -EFAULT;
+ } else if (page->mapping && !(vma->vm_flags & VM_NONLINEAR)) {
if (!vma->vm_file ||
vma->vm_file->f_mapping != page->mapping)
return -EFAULT;
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH -mm 6/6] add anon_vma bug checks
2010-06-21 20:31 [PATCH 0/6] further anon_vma fixes & optimizations Rik van Riel
` (4 preceding siblings ...)
2010-06-21 20:38 ` [PATCH -mm 5/6] resurrect page_address_in_vma anon_vma check Rik van Riel
@ 2010-06-21 20:39 ` Rik van Riel
2010-06-21 21:48 ` [PATCH 0/6] further anon_vma fixes & optimizations Andrea Arcangeli
6 siblings, 0 replies; 11+ messages in thread
From: Rik van Riel @ 2010-06-21 20:39 UTC (permalink / raw
To: linux-kernel; +Cc: akpm, aarcange, linux-mm
From: Andrea Arcangeli <aarcange@redhat.com>
Subject: add anon_vma bug checks
Verify the refcounting doesn't go wrong, and resurrect the check in
__page_check_anon_rmap as in old anon-vma code.
Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
Signed-off-by: Rik van Riel <riel@redhat.com>
---
diff --git a/mm/rmap.c b/mm/rmap.c
--- a/mm/rmap.c
+++ b/mm/rmap.c
@@ -815,6 +815,7 @@ static void __page_check_anon_rmap(struc
* are initially only visible via the pagetables, and the pte is locked
* over the call to page_add_new_anon_rmap.
*/
+ BUG_ON(page_anon_vma(page)->root != vma->anon_vma->root);
BUG_ON(page->index != linear_page_index(vma, address));
#endif
}
@@ -1405,6 +1406,7 @@ int try_to_munlock(struct page *page)
*/
void drop_anon_vma(struct anon_vma *anon_vma)
{
+ BUG_ON(atomic_read(&anon_vma->external_refcount) <= 0);
if (atomic_dec_and_lock(&anon_vma->external_refcount, &anon_vma->root->lock)) {
struct anon_vma *root = anon_vma->root;
int empty = list_empty(&anon_vma->head);
@@ -1416,6 +1418,7 @@ void drop_anon_vma(struct anon_vma *anon
* the refcount on the root and check if we need to free it.
*/
if (empty && anon_vma != root) {
+ BUG_ON(atomic_read(&root->external_refcount) <= 0);
last_root_user = atomic_dec_and_test(&root->external_refcount);
root_empty = list_empty(&root->head);
}
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 0/6] further anon_vma fixes & optimizations
2010-06-21 20:31 [PATCH 0/6] further anon_vma fixes & optimizations Rik van Riel
` (5 preceding siblings ...)
2010-06-21 20:39 ` [PATCH -mm 6/6] add anon_vma bug checks Rik van Riel
@ 2010-06-21 21:48 ` Andrea Arcangeli
6 siblings, 0 replies; 11+ messages in thread
From: Andrea Arcangeli @ 2010-06-21 21:48 UTC (permalink / raw
To: Rik van Riel; +Cc: linux-kernel, akpm, linux-mm
On Mon, Jun 21, 2010 at 04:31:46PM -0400, Rik van Riel wrote:
> A collection of anon_vma fixes and optimizations from Andrea's
> transparent hugepage tree.
Thanks!
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH -mm 2/6] rmap: always add new vmas at the end
2010-06-21 20:33 ` [PATCH -mm 2/6] rmap: always add new vmas at the end Rik van Riel
@ 2010-06-22 21:08 ` Andrew Morton
2010-06-22 21:10 ` Andrea Arcangeli
0 siblings, 1 reply; 11+ messages in thread
From: Andrew Morton @ 2010-06-22 21:08 UTC (permalink / raw
To: Rik van Riel; +Cc: linux-kernel, aarcange, linux-mm
On Mon, 21 Jun 2010 16:33:49 -0400
Rik van Riel <riel@redhat.com> wrote:
> From: Andrea Arcangeli <aarcange@redhat.com>
> Subject: always add new vmas at the end
>
> Make sure to always add new VMAs at the end of the list. This
> is important so rmap_walk does not miss a VMA that was created
> during the rmap_walk.
>
> The old code got this right most of the time due to luck, but
> was buggy when anon_vma_prepare reused a mergeable anon_vma.
>
> Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
> Signed-off-by: Rik van Riel <riel@redhat.com>
> ---
>
> diff --git a/mm/rmap.c b/mm/rmap.c
> --- a/mm/rmap.c
> +++ b/mm/rmap.c
> @@ -149,7 +149,7 @@ int anon_vma_prepare(struct vm_area_stru
> avc->anon_vma = anon_vma;
> avc->vma = vma;
> list_add(&avc->same_vma, &vma->anon_vma_chain);
> - list_add(&avc->same_anon_vma, &anon_vma->head);
> + list_add_tail(&avc->same_anon_vma, &anon_vma->head);
> allocated = NULL;
> avc = NULL;
> }
Should this go into 2.6.35?
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH -mm 2/6] rmap: always add new vmas at the end
2010-06-22 21:08 ` Andrew Morton
@ 2010-06-22 21:10 ` Andrea Arcangeli
0 siblings, 0 replies; 11+ messages in thread
From: Andrea Arcangeli @ 2010-06-22 21:10 UTC (permalink / raw
To: Andrew Morton; +Cc: Rik van Riel, linux-kernel, linux-mm
On Tue, Jun 22, 2010 at 02:08:22PM -0700, Andrew Morton wrote:
> On Mon, 21 Jun 2010 16:33:49 -0400
> Rik van Riel <riel@redhat.com> wrote:
>
> > From: Andrea Arcangeli <aarcange@redhat.com>
> > Subject: always add new vmas at the end
> >
> > Make sure to always add new VMAs at the end of the list. This
> > is important so rmap_walk does not miss a VMA that was created
> > during the rmap_walk.
> >
> > The old code got this right most of the time due to luck, but
> > was buggy when anon_vma_prepare reused a mergeable anon_vma.
> >
> > Signed-off-by: Andrea Arcangeli <aarcange@redhat.com>
> > Signed-off-by: Rik van Riel <riel@redhat.com>
> > ---
> >
> > diff --git a/mm/rmap.c b/mm/rmap.c
> > --- a/mm/rmap.c
> > +++ b/mm/rmap.c
> > @@ -149,7 +149,7 @@ int anon_vma_prepare(struct vm_area_stru
> > avc->anon_vma = anon_vma;
> > avc->vma = vma;
> > list_add(&avc->same_vma, &vma->anon_vma_chain);
> > - list_add(&avc->same_anon_vma, &anon_vma->head);
> > + list_add_tail(&avc->same_anon_vma, &anon_vma->head);
> > allocated = NULL;
> > avc = NULL;
> > }
>
> Should this go into 2.6.35?
Well migrate got broken anyway in 2.6.34, so until the whole
root-anon-vma patchqueue is merged, it's not going to provide a safe
migrate anyway and it can as well wait with the rest.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH -mm 3/6] ksm: fix ksm swapin time optimization
2010-06-21 20:34 ` [PATCH -mm 3/6] ksm: fix ksm swapin time optimization Rik van Riel
@ 2010-06-23 17:47 ` Andrea Arcangeli
0 siblings, 0 replies; 11+ messages in thread
From: Andrea Arcangeli @ 2010-06-23 17:47 UTC (permalink / raw
To: Rik van Riel; +Cc: linux-kernel, akpm, linux-mm, avi
On Mon, Jun 21, 2010 at 04:34:39PM -0400, Rik van Riel wrote:
> From: Andrea Arcangeli <aarcange@redhat.com>
> Subject: fix ksm swapin time optimization
>
> The new anon-vma code, was suboptimal and it lead to erratic invocation of
> ksm_does_need_to_copy. That leads to host hangs or guest vnc lockup, or weird
> behavior. It's unclear why ksm_does_need_to_copy is unstable but the point is
> that when KSM is not in use, ksm_does_need_to_copy must never run or we bounce
BTW, I'm debugging why ksm_does_need_to_copy breaks things... probably
I found something already, maybe not. I'll let you know as soon as I
have a fix. In the meantime the one above is a fix needed to avoid
calling ksm_does_need_to_copy erratically even when KSM is off (which
also avoids the bug to trigger).
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2010-06-23 17:47 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-06-21 20:31 [PATCH 0/6] further anon_vma fixes & optimizations Rik van Riel
2010-06-21 20:33 ` [PATCH -mm 1/6] mmap: remove unnecessary lock from __vma_link Rik van Riel
2010-06-21 20:33 ` [PATCH -mm 2/6] rmap: always add new vmas at the end Rik van Riel
2010-06-22 21:08 ` Andrew Morton
2010-06-22 21:10 ` Andrea Arcangeli
2010-06-21 20:34 ` [PATCH -mm 3/6] ksm: fix ksm swapin time optimization Rik van Riel
2010-06-23 17:47 ` Andrea Arcangeli
2010-06-21 20:35 ` [PATCH -mm 4/6] always use anon_vma root pointer Rik van Riel
2010-06-21 20:38 ` [PATCH -mm 5/6] resurrect page_address_in_vma anon_vma check Rik van Riel
2010-06-21 20:39 ` [PATCH -mm 6/6] add anon_vma bug checks Rik van Riel
2010-06-21 21:48 ` [PATCH 0/6] further anon_vma fixes & optimizations Andrea Arcangeli
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 read-only IMAP folder(s) and NNTP newsgroup(s).