mm/hugetlb_cgroup: Add list_del to remove unused page from hugepage_activelist when hugepage migration

PATCH In review Wanpeng Li 2 participants 5 messages Jul 11, 2012

Subsystems (3)

Memory management cgroup HugeTLB

lore

Share & tools

lore

Wanpeng Li

author

2012-07-11 03:51 UTC #0 lore raw

PATCH 1/1 mm/hugetlb_cgroup: Add list_del to remove unused page from hugepage_activelist when hugepage migration

From: Wanpeng Li <[email protected]>

hugepage_activelist is used to track currently used HugeTLB pages. We can find the in-use HugeTLB pages to support HugeTLB cgroup removal. Don't keep unused page in hugetlb_activelist too long. Otherwise, on cgroup removal we should update the unused page's charge to parent count. To reduce this overhead, remove unused page from hugepage_activelist immediately.

Signed-off-by Wanpeng Li <[email protected]>

---
 mm/hugetlb_cgroup.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

mm/hugetlb_cgroup.c +1 -0

index b834e8d..d819d66 100644
@@ -398,6 +398,7 @@ void hugetlb_cgroup_migrate(struct page *oldhpage, struct page *newhpage)
 	spin_lock(&hugetlb_lock);
 	h_cg = hugetlb_cgroup_from_page(oldhpage);
 	set_hugetlb_cgroup(oldhpage, NULL);
+	list_del(&oldhpage->lru);
 
 	/* move the h_cg details to new cgroup */
 	set_hugetlb_cgroup(newhpage, h_cg);

Signature · 7 lines

1.7.5.4

--
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/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Aneesh Kumar K.V

2012-07-11 08:32 UTC #1 lore raw

Quoted 19 lines · Wanpeng Li

From: Wanpeng Li <[email protected]>
hugepage_activelist is used to track currently used HugeTLB pages. We can find the in-use HugeTLB pages to support HugeTLB cgroup removal. Don't keep unused page in hugetlb_activelist too long. Otherwise, on cgroup removal we should update the unused page's charge to parent count. To reduce this overhead, remove unused page from hugepage_activelist immediately.
Signed-off-by Wanpeng Li <[email protected]>
---
 mm/hugetlb_cgroup.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
mm/hugetlb_cgroup.c +0 -0
index b834e8d..d819d66 100644

mm/hugetlb_cgroup.c void hugetlb_cgroup_migrate(struct page *oldhpage, struct page *newhpage) · line 404

Quoted 7 lines

mm/hugetlb_cgroup.c +1 -0

@@ -398,6 +398,7 @@ void hugetlb_cgroup_migrate(struct page *oldhpage, struct page *newhpage)
 	spin_lock(&hugetlb_lock);
 	h_cg = hugetlb_cgroup_from_page(oldhpage);
 	set_hugetlb_cgroup(oldhpage, NULL);
+	list_del(&oldhpage->lru);
 
 	/* move the h_cg details to new cgroup */
 	set_hugetlb_cgroup(newhpage, h_cg);

put_page on the oldhpage will do that. If we do list_del here free_huge_page() will have error list_move().

-aneesh

--
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/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Wanpeng Li

author

2012-07-11 09:17 UTC #2 lore raw

Quoted 26 lines · Aneesh Kumar K.V

Quoted 24 lines · Wanpeng Li
From: Wanpeng Li <[email protected]>
hugepage_activelist is used to track currently used HugeTLB pages. We can find the in-use HugeTLB pages to support HugeTLB cgroup removal. Don't keep unused page in hugetlb_activelist too long. Otherwise, on cgroup removal we should update the unused page's charge to parent count. To reduce this overhead, remove unused page from hugepage_activelist immediately.
Signed-off-by Wanpeng Li <[email protected]>
---
 mm/hugetlb_cgroup.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
mm/hugetlb_cgroup.c +1 -0
index b834e8d..d819d66 100644
@@ -398,6 +398,7 @@ void hugetlb_cgroup_migrate(struct page *oldhpage, struct page *newhpage)
 	spin_lock(&hugetlb_lock);
 	h_cg = hugetlb_cgroup_from_page(oldhpage);
 	set_hugetlb_cgroup(oldhpage, NULL);
+	list_del(&oldhpage->lru);
 

Quoted 5 lines

mm/hugetlb_cgroup.c void hugetlb_cgroup_migrate(struct page *oldhpage, struct page *newhpage) · line 404
Quoted 2 lines
mm/hugetlb_cgroup.c +0 -0
 	/* move the h_cg details to new cgroup */
 	set_hugetlb_cgroup(newhpage, h_cg);
put_page on the oldhpage will do that. If we do list_del here free_huge_page() will have error list_move().

I don't know if move the page to other lists when migration in order to reduce overhead I mentioned make sense. :-)

Regards,
Wanpeng Li

Quoted 2 lines

-aneesh

Aneesh Kumar K.V

2012-07-11 09:45 UTC #3 lore raw

Quoted 27 lines · Wanpeng Li

Quoted 26 lines · Aneesh Kumar K.V
Quoted 24 lines · Wanpeng Li
From: Wanpeng Li <[email protected]>
hugepage_activelist is used to track currently used HugeTLB pages. We can find the in-use HugeTLB pages to support HugeTLB cgroup removal. Don't keep unused page in hugetlb_activelist too long. Otherwise, on cgroup removal we should update the unused page's charge to parent count. To reduce this overhead, remove unused page from hugepage_activelist immediately.
Signed-off-by Wanpeng Li <[email protected]>
---
 mm/hugetlb_cgroup.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
mm/hugetlb_cgroup.c +1 -0
index b834e8d..d819d66 100644
@@ -398,6 +398,7 @@ void hugetlb_cgroup_migrate(struct page *oldhpage, struct page *newhpage)
 	spin_lock(&hugetlb_lock);
 	h_cg = hugetlb_cgroup_from_page(oldhpage);
 	set_hugetlb_cgroup(oldhpage, NULL);
+	list_del(&oldhpage->lru);
 

Quoted 9 lines

Quoted 5 lines
mm/hugetlb_cgroup.c void hugetlb_cgroup_migrate(struct page *oldhpage, struct page *newhpage) · line 404
Quoted 2 lines
mm/hugetlb_cgroup.c +0 -0
 	/* move the h_cg details to new cgroup */
 	set_hugetlb_cgroup(newhpage, h_cg);
put_page on the oldhpage will do that. If we do list_del here free_huge_page() will have error list_move().
I don't know if move the page to other lists when migration in order to reduce overhead I mentioned make sense. :-)

Even if we are in the active_list, we don't get anything wrong because hugetlb cgroup is set to NULL for the old hugetlb page. The above patch is wrong and will result in crash when we try to put_page the oldhpage.

-aneesh

Wanpeng Li

author

2012-07-11 10:16 UTC #4 lore raw

Quoted 35 lines · Aneesh Kumar K.V

Quoted 33 lines · Wanpeng Li
Quoted 31 lines · Aneesh Kumar K.V
Quoted 19 lines · Wanpeng Li
From: Wanpeng Li <[email protected]>
hugepage_activelist is used to track currently used HugeTLB pages. We can find the in-use HugeTLB pages to support HugeTLB cgroup removal. Don't keep unused page in hugetlb_activelist too long. Otherwise, on cgroup removal we should update the unused page's charge to parent count. To reduce this overhead, remove unused page from hugepage_activelist immediately.
Signed-off-by Wanpeng Li <[email protected]>
---
 mm/hugetlb_cgroup.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
mm/hugetlb_cgroup.c +0 -0
index b834e8d..d819d66 100644
mm/hugetlb_cgroup.c void hugetlb_cgroup_migrate(struct page *oldhpage, struct page *newhpage) · line 404
Quoted 7 lines
mm/hugetlb_cgroup.c +1 -0
@@ -398,6 +398,7 @@ void hugetlb_cgroup_migrate(struct page *oldhpage, struct page *newhpage)
 	spin_lock(&hugetlb_lock);
 	h_cg = hugetlb_cgroup_from_page(oldhpage);
 	set_hugetlb_cgroup(oldhpage, NULL);
+	list_del(&oldhpage->lru);
 
 	/* move the h_cg details to new cgroup */
 	set_hugetlb_cgroup(newhpage, h_cg);
put_page on the oldhpage will do that. If we do list_del here free_huge_page() will have error list_move().

Quoted 7 lines

Quoted 3 lines
I don't know if move the page to other lists when migration in order to reduce overhead I mentioned make sense. :-)
Even if we are in the active_list, we don't get anything wrong because hugetlb cgroup is set to NULL for the old hugetlb page. The above patch is wrong and will result in crash when we try to put_page the oldhpage.

Hi aneesh, I am happy we are in the same company. I am a guy in LTC China. I want to know if there any features or works about hugetlb cgroup I can do with you!

Regards,
Wanpeng Li

Quoted 8 lines

-aneesh
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/