mm: Simplify anon_vma lifetime rules
Subsystems (4)
cover mm: Simplify anon_vma lifetime rules
As per Linus' request, isolate these three patches.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 1/3 mm: Rename drop_anon_vma to put_anon_vma
The normal code pattern used in the kernel is: get/put.
--- include/linux/rmap.h | 4 ++-- mm/ksm.c | 23 +++++------------------ mm/migrate.c | 4 ++-- mm/rmap.c | 4 ++-- 4 files changed, 11 insertions(+), 24 deletions(-)
include/linux/rmap.h +2 -2
@@ -87,7 +87,7 @@ static inline void get_anon_vma(struct a atomic_inc(&anon_vma->external_refcount); } -void drop_anon_vma (struct anon_vma *); +void put_anon_vma (struct anon_vma *);#else static inline void anonvma_external_refcount_init (struct anon_vma *anon_vma) {
@@ -102,7 +102,7 @@ static inline void get_anon_vma(struct a { } -static inline void drop_anon_vma (struct anon_vma *anon_vma) +static inline void put_anon_vma (struct anon_vma *anon_vma) { }#endif /* CONFIG_KSM */
mm/ksm.c +5 -18
@@ -301,20 +301,6 @@ static inline int in_stable_tree(structreturn rmap_item->address &STABLE_FLAG ; } -static void hold_anon_vma (struct rmap_item *rmap_item, -struct anon_vma *anon_vma) -{ - rmap_item->anon_vma = anon_vma; -get_anon_vma (anon_vma); -} - -static void ksm_drop_anon_vma (struct rmap_item *rmap_item) -{ -struct anon_vma *anon_vma = rmap_item->anon_vma; - -drop_anon_vma (anon_vma); -} -/* * ksmd, and unmerge_and_remove_all_rmap_items(), must not touch an mm's * page tables after it has passed through ksm_exit() - which, if necessary,
@@ -397,7 +383,7 @@ static void break_cow(struct rmap_item ** It is not an accident that whenever we want to break COW * to undo, we also need to drop a reference to the anon_vma. */ -ksm_drop_anon_vma (rmap_item); +put_anon_vma (rmap_item->anon_vma);down_read (&mm->mmap_sem);if (ksm_test_exit (mm))
@@ -466,7 +452,7 @@ static void remove_node_from_stable_tree ksm_pages_sharing--; else ksm_pages_shared--; -ksm_drop_anon_vma (rmap_item); +put_anon_vma (rmap_item->anon_vma); rmap_item->address &=PAGE_MASK ;cond_resched (); }
@@ -554,7 +540,7 @@ static void remove_rmap_item_from_tree(selse ksm_pages_shared--; -ksm_drop_anon_vma (rmap_item); +put_anon_vma (rmap_item->anon_vma); rmap_item->address &=PAGE_MASK ; }else if (rmap_item->address &UNSTABLE_FLAG ) {
@@ -949,7 +935,8 @@ static int try_to_merge_with_ksm_page(st goto out;/* Must get reference to anon_vma while still holding mmap_sem */ -hold_anon_vma (rmap_item, vma->anon_vma); + rmap_item->anon_vma = vma->anon_vma; +get_anon_vma (vma->anon_vma); out:up_read (&mm->mmap_sem);return err;
mm/migrate.c +2 -2
@@ -764,7 +764,7 @@ static int unmap_and_move(new_page_t get/* Drop an anon_vma reference if we took one */ if (anon_vma) -drop_anon_vma (anon_vma); +put_anon_vma (anon_vma); uncharge:if (!charge)
@@ -857,7 +857,7 @@ static int unmap_and_move_huge_page(new_remove_migration_ptes (hpage, hpage);if (anon_vma) -drop_anon_vma (anon_vma); +put_anon_vma (anon_vma); out:unlock_page (hpage);
mm/rmap.c +2 -2
@@ -278,7 +278,7 @@ static void anon_vma_unlink(struct anon_if (empty) {/* We no longer need the root anon_vma */ if (anon_vma->root != anon_vma) -drop_anon_vma (anon_vma->root); +put_anon_vma (anon_vma->root);anon_vma_free (anon_vma); } }
@@ -1489,7 +1489,7 @@ int try_to_munlock(struct page *page)* we know we are the last user, nobody else can get a reference and we * can do the freeing without the lock. */ -void drop_anon_vma (struct anon_vma *anon_vma) +void put_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)) {
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 1/3] mm: Rename drop_anon_vma to put_anon_vma
On 02/17/2011 11:19 AM, Peter Zijlstra wrote:
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 1/3] mm: Rename drop_anon_vma to put_anon_vma
In reply to Peter ZijlstraQuoted 5 lines · Peter Zijlstra
The normal code pattern used in the kernel is: get/put.
Reviewed-by KAMEZAWA Hiroyuki <[email protected]>Acked-by Hugh Dickins <[email protected]>Signed-off-by Peter Zijlstra <[email protected]>
Signature · 2 lines
Mel Gorman SUSE Labs
PATCH 2/3 mm: Move anon_vma ref out from under CONFIG_foo
In reply to Peter ZijlstraWe need the anon_vma refcount unconditionally to simplify the anon_vma lifetime rules.
--- include/linux/rmap.h | 40 ++++------------------------------------ mm/rmap.c | 14 ++++++-------- 2 files changed, 10 insertions(+), 44 deletions(-)
include/linux/rmap.h +4 -36
@@ -27,18 +27,15 @@struct anon_vma {struct anon_vma *root;/* Root of this anon_vma tree */ spinlock_t lock;/* Serialize access to vma list */ -#if defined(CONFIG_KSM ) || defined(CONFIG_MIGRATION ) -/* -* The external_refcount is taken by either KSM or page migration -* to take a reference to an anon_vma when there is no +* The refcount is taken on an anon_vma when there is no * guarantee that the vma of page tables will exist for * the duration of the operation. A caller that takes * the reference is responsible for clearing up the * anon_vma if they are the last user on release */ -atomic_t external_refcount; -#endif +atomic_t refcount; +/* * NOTE: the LSB of the head.next is set by * mm_take_all_locks() _after_ taking the above lock. So the
@@ -71,41 +68,12 @@ struct anon_vma_chain { };#ifdef CONFIG_MMU -#if defined(CONFIG_KSM ) || defined(CONFIG_MIGRATION ) -static inline void anonvma_external_refcount_init (struct anon_vma *anon_vma) -{ -atomic_set (&anon_vma->external_refcount,0 ); -} - -static inline int anonvma_external_refcount (struct anon_vma *anon_vma) -{ -return atomic_read (&anon_vma->external_refcount); -} -static inline void get_anon_vma (struct anon_vma *anon_vma) { -atomic_inc (&anon_vma->external_refcount); +atomic_inc (&anon_vma->refcount); }void put_anon_vma (struct anon_vma *); -#else -static inline void anonvma_external_refcount_init (struct anon_vma *anon_vma) -{ -} - -static inline int anonvma_external_refcount (struct anon_vma *anon_vma) -{ -return 0 ; -} - -static inline void get_anon_vma (struct anon_vma *anon_vma) -{ -} - -static inline void put_anon_vma (struct anon_vma *anon_vma) -{ -} -#endif /* CONFIG_KSM */ static inline struct anon_vma *page_anon_vma (struct page *page) {
mm/rmap.c +6 -8
@@ -272,7 +272,7 @@ static void anon_vma_unlink(struct anon_list_del (&anon_vma_chain->same_anon_vma);/* We must garbage collect the anon_vma if it's empty */ - empty =list_empty (&anon_vma->head) && !anonvma_external_refcount (anon_vma); + empty =list_empty (&anon_vma->head) && !atomic_read (&anon_vma->refcount);anon_vma_unlock (anon_vma);if (empty) {
@@ -303,7 +303,7 @@ static void anon_vma_ctor(void *data)struct anon_vma *anon_vma = data;spin_lock_init (&anon_vma->lock); -anonvma_external_refcount_init (anon_vma); +atomic_set (&anon_vma->refcount,0 );INIT_LIST_HEAD (&anon_vma->head); }
@@ -1482,7 +1482,6 @@ int try_to_munlock(struct page *page)return try_to_unmap_file (page,TTU_MUNLOCK ); } -#if defined(CONFIG_KSM ) || defined(CONFIG_MIGRATION )/* * Drop an anon_vma refcount, freeing the anon_vma and anon_vma->root * if necessary. Be careful to do all the tests under the lock. Once
@@ -1491,8 +1490,8 @@ int try_to_munlock(struct page *page)*/ void put_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)) { +BUG_ON (atomic_read (&anon_vma->refcount) <=0 ); +if (atomic_dec_and_lock (&anon_vma->refcount, &anon_vma->root->lock)) {struct anon_vma *root = anon_vma->root;int empty =list_empty (&anon_vma->head);int last_root_user =0 ;
@@ -1503,8 +1502,8 @@ void put_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); +BUG_ON (atomic_read (&root->refcount) <=0 ); + last_root_user =atomic_dec_and_test (&root->refcount); root_empty =list_empty (&root->head); }anon_vma_unlock (anon_vma);
@@ -1516,7 +1515,6 @@ void put_anon_vma(struct anon_vma *anon_ } } } -#endif #ifdef CONFIG_MIGRATION /*
Re: [PATCH 2/3] mm: Move anon_vma ref out from under CONFIG_foo
On 02/17/2011 11:19 AM, Peter Zijlstra wrote:
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 3/3 mm: Simplify anon_vma refcounts
In reply to Peter ZijlstraThis patch changes the anon_vma refcount to be 0 when the object is free. It does this by adding 1 ref to being in use in the anon_vma structure (iow. the anon_vma->head list is not empty).
This allows a simpler release scheme without having to check both the refcount and the list as well as avoids taking a ref for each entry on the list.
--- include/linux/rmap.h | 11 +++++-- mm/rmap.c | 79 ++++++++++++++++++--------------------------------- 2 files changed, 37 insertions(+), 53 deletions(-)
include/linux/rmap.h +9 -2
@@ -73,7 +73,13 @@ static inline void get_anon_vma(struct aatomic_inc (&anon_vma->refcount); } -void put_anon_vma (struct anon_vma *); +void __put_anon_vma (struct anon_vma *anon_vma); + +static inline void put_anon_vma (struct anon_vma *anon_vma) +{ +if (atomic_dec_and_test (&anon_vma->refcount)) +__put_anon_vma (anon_vma); +}static inline struct anon_vma *page_anon_vma (struct page *page) {
@@ -116,7 +122,6 @@ void unlink_anon_vmas(struct vm_area_strint anon_vma_clone (struct vm_area_struct *,struct vm_area_struct *);int anon_vma_fork (struct vm_area_struct *,struct vm_area_struct *);void __anon_vma_link (struct vm_area_struct *); -void anon_vma_free (struct anon_vma *);static inline void anon_vma_merge (struct vm_area_struct *vma,struct vm_area_struct *next)
@@ -125,6 +130,8 @@ static inline void anon_vma_merge(structunlink_anon_vmas (next); } +struct anon_vma *page_get_anon_vma (struct page *page); +/* * rmap interfaces called when adding or removing pte of page */
mm/rmap.c +28 -51
126 lines not on this page · raw message
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 3/3] mm: Simplify anon_vma refcounts
On 02/17/2011 11:19 AM, Peter Zijlstra wrote:
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 3/3] mm: Simplify anon_vma refcounts
In reply to Peter ZijlstraQuoted 7 lines · Peter Zijlstra
mm/rmap.c +5
-0+void __put_anon_vma (struct anon_vma *anon_vma) +{ + if (anon_vma->root != anon_vma) + put_anon_vma(anon_vma->root); + anon_vma_free(anon_vma);}
So this makes me nervous. It looks like recursion.
Now, I don't think we can ever get a chain of these things (because the root should be the root of everything), but I still preferred the older code that made that "one-level root" case explicit, and didn't have recursion.
IOW, even though it should be entirely equivalent, I think I'd really prefer something like
void __put_anon_vma(struct anon_vma *anon_vma)
{
struct anon_vma *root = anon_vma->root; if (root != anon_vma && atomic_dec_and_test(&root->refcount))
anon_vma_free(root);
anon_vma_free(anon_vma);}
instead. Exactly because it makes it very clear that the "root" is a root, and we're not doing some possibly arbitrarily deep list like the dentry tree (which avoids recursion by open-coding its freeing as a loop).
Hmm? (The above is obviously untested, maybe it has some stupid bug)
Linus
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 3/3] mm: Simplify anon_vma refcounts
Quoted 11 lines · Linus Torvalds
mm/rmap.c int try_to_munlock(struct page *page) · line 1494
Quoted 7 lines · Peter Zijlstra
mm/rmap.c +5
-0+void __put_anon_vma (struct anon_vma *anon_vma) +{ +if (anon_vma->root != anon_vma) +put_anon_vma (anon_vma->root); +anon_vma_free (anon_vma); }So this makes me nervous. It looks like recursion.
Quoted 2 lines
Now, I don't think we can ever get a chain of these things (because the root should be the root of everything),
Exactly.
Quoted 14 lines
but I still preferred the
older code that made that "one-level root" case explicit, and didn't have recursion.IOW, even though it should be entirely equivalent, I think I'd really prefer something like
void __put_anon_vma(struct anon_vma *anon_vma) { struct anon_vma *root = anon_vma->root;if (root != anon_vma && atomic_dec_and_test(&root->refcount)) anon_vma_free(root); anon_vma_free(anon_vma);
Quoted 8 lines
}
instead. Exactly because it makes it very clear that the "root" is a root, and we're not doing some possibly arbitrarily deep list like the dentry tree (which avoids recursion by open-coding its freeing as a loop).
Hmm? (The above is obviously untested, maybe it has some stupid bug)
Looks about right, I'll give it a spin.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCH 3/3] mm: Simplify anon_vma refcounts
In reply to Linus TorvaldsSubject: mm: Simplify anon_vma refcounts
From: Peter Zijlstra <[email protected]>
Date: Fri, 26 Nov 2010 15:38:49 +0100
This patch changes the anon_vma refcount to be 0 when the object is free. It does this by adding 1 ref to being in use in the anon_vma structure (iow. the anon_vma->head list is not empty).
This allows a simpler release scheme without having to check both the refcount and the list as well as avoids taking a ref for each entry on the list.
--- include/linux/rmap.h | 11 +++++-- mm/rmap.c | 78 ++++++++++++++++++--------------------------------- 2 files changed, 38 insertions(+), 51 deletions(-)
include/linux/rmap.h +9 -2
@@ -73,7 +73,13 @@ static inline void get_anon_vma(struct aatomic_inc (&anon_vma->refcount); } -void put_anon_vma (struct anon_vma *); +void __put_anon_vma (struct anon_vma *anon_vma); + +static inline void put_anon_vma (struct anon_vma *anon_vma) +{ +if (atomic_dec_and_test (&anon_vma->refcount)) +__put_anon_vma (anon_vma); +}static inline struct anon_vma *page_anon_vma (struct page *page) {
@@ -116,7 +122,6 @@ void unlink_anon_vmas(struct vm_area_strint anon_vma_clone (struct vm_area_struct *,struct vm_area_struct *);int anon_vma_fork (struct vm_area_struct *,struct vm_area_struct *);void __anon_vma_link (struct vm_area_struct *); -void anon_vma_free (struct anon_vma *);static inline void anon_vma_merge (struct vm_area_struct *vma,struct vm_area_struct *next)
@@ -125,6 +130,8 @@ static inline void anon_vma_merge(structunlink_anon_vmas (next); } +struct anon_vma *page_get_anon_vma (struct page *page); +/* * rmap interfaces called when adding or removing pte of page */
mm/rmap.c +29 -49
127 lines not on this page · raw message
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Rik van Riel
Acked-byRe: [PATCH 3/3] mm: Simplify anon_vma refcounts
Quoted 8 lines · Peter Zijlstra
Subject: mm: Simplify
anon_vmarefcounts
From: Peter Zijlstra<[email protected]>
Date: Fri, 26 Nov 2010 15:38:49 +0100This patch changes the
anon_vmarefcount to be 0 when the object is free. It does this by adding 1 ref to being in use in theanon_vmastructure (iow. theanon_vma->headlist is not empty).
Quoted 7 lines
This allows a simpler release scheme without having to check both the refcount and the list as well as avoids taking a ref for each entry on the list.
Reviewed-by KAMEZAWA Hiroyuki<[email protected]>Acked-by Hugh Dickins<[email protected]>Signed-off-by Peter Zijlstra<[email protected]>
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Mel Gorman
Acked-byRe: [PATCH 3/3] mm: Simplify anon_vma refcounts
In reply to Peter ZijlstraQuoted 8 lines · Peter Zijlstra
Subject: mm: Simplify
anon_vmarefcounts
From: Peter Zijlstra <[email protected]>
Date: Fri, 26 Nov 2010 15:38:49 +0100This patch changes the
anon_vmarefcount to be 0 when the object is free. It does this by adding 1 ref to being in use in theanon_vmastructure (iow. theanon_vma->headlist is not empty).
Quoted 7 lines
This allows a simpler release scheme without having to check both the refcount and the list as well as avoids taking a ref for each entry on the list.
Reviewed-by KAMEZAWA Hiroyuki <[email protected]>Acked-by Hugh Dickins <[email protected]>Signed-off-by Peter Zijlstra <[email protected]>
Signature · 2 lines
Mel Gorman SUSE Labs
Quoted 1 line · Peter Zijlstra
As per Linus' request, isolate these three patches.
--- include/linux/rmap.h | 45 ++++++--------------------- mm/ksm.c | 23 +++----------- mm/migrate.c | 4 +- mm/rmap.c | 83 +++++++++++++++++---------------------------------- 4 files changed, 46 insertions(+), 109 deletions(-)
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>