Skip to contentSkip to conversation

RSS Container, make page_referenced() container aware

PATCH In review Balbir Singh 2 participants 3 messages Apr 26, 2007 – Apr 27, 2007

Subsystems (4)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 RSS Container, make page_referenced() container aware

Hi, Pavel,

This patch should help with the shared page issue of one container holding shared pages in a another container (the container that brought in the page -- by first touch) hostage.

Signature · 4 lines
	Warm Regards,
	Balbir Singh
	Linux Technology Center
	IBM, ISTL

Make page_referenced() container aware. Without this patch, page_referenced() can cause a page to be skipped while reclaiming pages. This patch ensures that other containers do not hold pages in a particular container hostage. It is required to ensure that shared pages are freed from a container when they are not actively referenced from the container that brought them in

---

 include/linux/rmap.h |    5 +++--
 mm/rmap.c            |   26 ++++++++++++++++++++------
 mm/vmscan.c          |    4 ++--
 3 files changed, 25 insertions(+), 10 deletions(-)
mm/vmscan.c renamed from mm/vmscan.c~rss-implement-per-container-page-referenced +2 -2
diff -puN mm/vmscan.c~rss-implement-per-container-page-referenced mm/vmscan.c
@@ -489,7 +489,7 @@ static unsigned long shrink_page_list(st
 		if (PageWriteback(page))
 			goto keep_locked;
 
-		referenced = page_referenced(page, 1);
+		referenced = page_referenced(page, 1, sc->cnt);
 		/* In active use or really unfreeable?  Activate it. */
 		if (referenced && page_mapping_inuse(page))
 			goto activate_locked;
@@ -852,7 +852,7 @@ force_reclaim_mapped:
 		if (page_mapped(page)) {
 			if (!reclaim_mapped ||
 			    (total_swap_pages == 0 && PageAnon(page)) ||
-			    page_referenced(page, 0)) {
+			    page_referenced(page, 0, sc->cnt)) {
 				list_add(&page->lru, &l_active);
 				continue;
 			}
mm/rmap.c renamed from mm/rmap.c~rss-implement-per-container-page-referenced +20 -6
diff -puN mm/rmap.c~rss-implement-per-container-page-referenced mm/rmap.c
@@ -318,7 +318,7 @@ out:
 	return referenced;
 }
 
-static int page_referenced_anon(struct page *page)
+static int page_referenced_anon(struct page *page, struct rss_container *cnt)
 {
 	unsigned int mapcount;
 	struct anon_vma *anon_vma;
@@ -331,6 +331,13 @@ static int page_referenced_anon(struct p
 
 	mapcount = page_mapcount(page);
 	list_for_each_entry(vma, &anon_vma->head, anon_vma_node) {
+		/*
+		 * If we are reclaiming on behalf of a container, skip
+		 * counting on behalf of references from different
+		 * containers
+		 */
+		if (cnt && (vma->vm_mm->rss_container != cnt))
+			continue;
 		referenced += page_referenced_one(page, vma, &mapcount);
 		if (!mapcount)
 			break;
@@ -350,7 +357,7 @@ static int page_referenced_anon(struct p
  *
  * This function is only called from page_referenced for object-based pages.
  */
-static int page_referenced_file(struct page *page)
+static int page_referenced_file(struct page *page, struct rss_container *cnt)
 {
 	unsigned int mapcount;
 	struct address_space *mapping = page->mapping;
@@ -383,6 +390,13 @@ static int page_referenced_file(struct p
 	mapcount = page_mapcount(page);
 
 	vma_prio_tree_foreach(vma, &iter, &mapping->i_mmap, pgoff, pgoff) {
+		/*
+		 * If we are reclaiming on behalf of a container, skip
+		 * counting on behalf of references from different
+		 * containers
+		 */
+		if (cnt && (vma->vm_mm->rss_container != cnt))
+			continue;
 		if ((vma->vm_flags & (VM_LOCKED|VM_MAYSHARE))
 				  == (VM_LOCKED|VM_MAYSHARE)) {
 			referenced++;
@@ -405,7 +419,7 @@ static int page_referenced_file(struct p
  * Quick test_and_clear_referenced for all mappings to a page,
  * returns the number of ptes which referenced the page.
  */
-int page_referenced(struct page *page, int is_locked)
+int page_referenced(struct page *page, int is_locked, struct rss_container *cnt)
 {
 	int referenced = 0;
 
@@ -417,14 +431,14 @@ int page_referenced(struct page *page, i
 
 	if (page_mapped(page) && page->mapping) {
 		if (PageAnon(page))
-			referenced += page_referenced_anon(page);
+			referenced += page_referenced_anon(page, cnt);
 		else if (is_locked)
-			referenced += page_referenced_file(page);
+			referenced += page_referenced_file(page, cnt);
 		else if (TestSetPageLocked(page))
 			referenced++;
 		else {
 			if (page->mapping)
-				referenced += page_referenced_file(page);
+				referenced += page_referenced_file(page, cnt);
 			unlock_page(page);
 		}
 	}
include/linux/rmap.h renamed from include/linux/rmap.h~rss-implement-per-container-page-referenced +3 -2
diff -puN include/linux/rmap.h~rss-implement-per-container-page-referenced include/linux/rmap.h
@@ -8,6 +8,7 @@
 #include <linux/slab.h>
 #include <linux/mm.h>
 #include <linux/spinlock.h>
+#include <linux/rss_container.h>
 
 /*
  * The anon_vma heads a list of private "related" vmas, to scan if
@@ -93,7 +94,7 @@ static inline void page_dup_rmap(struct 
 /*
  * Called from mm/vmscan.c to handle paging out
  */
-int page_referenced(struct page *, int is_locked);
+int page_referenced(struct page *, int is_locked, struct rss_container *cnt);
 int try_to_unmap(struct page *, int ignore_refs);
 
 /*
@@ -121,7 +122,7 @@ int page_mkclean(struct page *);
 #define anon_vma_prepare(vma)	(0)
 #define anon_vma_link(vma)	do {} while (0)
 
-#define page_referenced(page,l) TestClearPageReferenced(page)
+#define page_referenced(page,l,cnt) TestClearPageReferenced(page)
 #define try_to_unmap(page, refs) SWAP_FAIL
 
 static inline int page_mkclean(struct page *page)

_

#1 lore raw
Quoted 5 lines · Balbir Singh

Hi, Pavel,

This patch should help with the shared page issue of one container holding shared pages in a another container (the container that brought in the page -- by first touch) hostage.

The shared pages accounting is tricky. Actually we planned not to do it right now, but later, when (if) this will be accepted and we'll move forward to the fractions accounting.

Quoted 54 lines

------------------------------------------------------------------------

Make page_referenced() container aware. Without this patch, page_referenced() can cause a page to be skipped while reclaiming pages. This patch ensures that other containers do not hold pages in a particular container hostage. It is required to ensure that shared pages are freed from a container when they are not actively referenced from the container that brought them in

---

 include/linux/rmap.h |    5 +++--
 mm/rmap.c            |   26 ++++++++++++++++++++------
 mm/vmscan.c          |    4 ++--
 3 files changed, 25 insertions(+), 10 deletions(-)

diff -puN mm/vmscan.c~rss-implement-per-container-page-referenced mm/vmscan.c

mm/vmscan.c renamed from mm/vmscan.c~rss-implement-per-container-page-referenced +2 -2
@@ -489,7 +489,7 @@ static unsigned long shrink_page_list(st
 		if (PageWriteback(page))
 			goto keep_locked;
 
-		referenced = page_referenced(page, 1);
+		referenced = page_referenced(page, 1, sc->cnt);
 		/* In active use or really unfreeable?  Activate it. */
 		if (referenced && page_mapping_inuse(page))
 			goto activate_locked;
@@ -852,7 +852,7 @@ force_reclaim_mapped:
 		if (page_mapped(page)) {
 			if (!reclaim_mapped ||
 			    (total_swap_pages == 0 && PageAnon(page)) ||
-			    page_referenced(page, 0)) {
+			    page_referenced(page, 0, sc->cnt)) {
 				list_add(&page->lru, &l_active);
 				continue;
 			}

diff -puN mm/rmap.c~rss-implement-per-container-page-referenced mm/rmap.c

mm/rmap.c renamed from mm/rmap.c~rss-implement-per-container-page-referenced +1 -1
@@ -318,7 +318,7 @@ out:
 	return referenced;
 }
 
-static int page_referenced_anon(struct page *page)
+static int page_referenced_anon(struct page *page, struct rss_container *cnt)
 {
 	unsigned int mapcount;
 	struct anon_vma *anon_vma;

mm/rmap.c static int page_referenced_anon(struct p · line 340

Quoted 10 lines
mm/rmap.c renamed from mm/rmap.c~rss-implement-per-container-page-referenced +7 -0
@@ -331,6 +331,13 @@ static int page_referenced_anon(struct p
 
 	mapcount = page_mapcount(page);
 	list_for_each_entry(vma, &anon_vma->head, anon_vma_node) {
+		/*
+		 * If we are reclaiming on behalf of a container, skip
+		 * counting on behalf of references from different
+		 * containers
+		 */
+		if (cnt && (vma->vm_mm->rss_container != cnt))
+			continue;

An #ifdef missed - mm->rss_container makes sense with the CONFIG_RSS_COUNTAINER set only.

Quoted 83 lines
mm/rmap.c renamed from mm/rmap.c~rss-implement-per-container-page-referenced +12 -5
 		referenced += page_referenced_one(page, vma, &mapcount);
 		if (!mapcount)
 			break;
@@ -350,7 +357,7 @@ static int page_referenced_anon(struct p
  *
  * This function is only called from page_referenced for object-based pages.
  */
-static int page_referenced_file(struct page *page)
+static int page_referenced_file(struct page *page, struct rss_container *cnt)
 {
 	unsigned int mapcount;
 	struct address_space *mapping = page->mapping;
@@ -383,6 +390,13 @@ static int page_referenced_file(struct p
 	mapcount = page_mapcount(page);
 
 	vma_prio_tree_foreach(vma, &iter, &mapping->i_mmap, pgoff, pgoff) {
+		/*
+		 * If we are reclaiming on behalf of a container, skip
+		 * counting on behalf of references from different
+		 * containers
+		 */
+		if (cnt && (vma->vm_mm->rss_container != cnt))
+			continue;
 		if ((vma->vm_flags & (VM_LOCKED|VM_MAYSHARE))
 				  == (VM_LOCKED|VM_MAYSHARE)) {
 			referenced++;
@@ -405,7 +419,7 @@ static int page_referenced_file(struct p
  * Quick test_and_clear_referenced for all mappings to a page,
  * returns the number of ptes which referenced the page.
  */
-int page_referenced(struct page *page, int is_locked)
+int page_referenced(struct page *page, int is_locked, struct rss_container *cnt)
 {
 	int referenced = 0;
 
@@ -417,14 +431,14 @@ int page_referenced(struct page *page, i
 
 	if (page_mapped(page) && page->mapping) {
 		if (PageAnon(page))
-			referenced += page_referenced_anon(page);
+			referenced += page_referenced_anon(page, cnt);
 		else if (is_locked)
-			referenced += page_referenced_file(page);
+			referenced += page_referenced_file(page, cnt);
 		else if (TestSetPageLocked(page))
 			referenced++;
 		else {
 			if (page->mapping)
-				referenced += page_referenced_file(page);
+				referenced += page_referenced_file(page, cnt);
 			unlock_page(page);
 		}
 	}

diff -puN include/linux/rmap.h~rss-implement-per-container-page-referenced include/linux/rmap.h

include/linux/rmap.h renamed from include/linux/rmap.h~rss-implement-per-container-page-referenced +3 -2
@@ -8,6 +8,7 @@
 #include <linux/slab.h>
 #include <linux/mm.h>
 #include <linux/spinlock.h>
+#include <linux/rss_container.h>
 
 /*
  * The anon_vma heads a list of private "related" vmas, to scan if
@@ -93,7 +94,7 @@ static inline void page_dup_rmap(struct 
 /*
  * Called from mm/vmscan.c to handle paging out
  */
-int page_referenced(struct page *, int is_locked);
+int page_referenced(struct page *, int is_locked, struct rss_container *cnt);
 int try_to_unmap(struct page *, int ignore_refs);
 
 /*
@@ -121,7 +122,7 @@ int page_mkclean(struct page *);
 #define anon_vma_prepare(vma)	(0)
 #define anon_vma_link(vma)	do {} while (0)
 
-#define page_referenced(page,l) TestClearPageReferenced(page)
+#define page_referenced(page,l,cnt) TestClearPageReferenced(page)
 #define try_to_unmap(page, refs) SWAP_FAIL
 
 static inline int page_mkclean(struct page *page)

_

#2 lore raw
Quoted 12 lines · Pavel Emelianov
Quoted 5 lines · Balbir Singh

Hi, Pavel,

This patch should help with the shared page issue of one container holding shared pages in a another container (the container that brought in the page -- by first touch) hostage.

The shared pages accounting is tricky. Actually we planned not to do it right now, but later, when (if) this will be accepted and we'll move forward to the fractions accounting.

Yes, but this patch ensures that we do not hit bottle necks when pages are shared

Quoted 54 lines
Quoted 54 lines

------------------------------------------------------------------------

Make page_referenced() container aware. Without this patch, page_referenced() can cause a page to be skipped while reclaiming pages. This patch ensures that other containers do not hold pages in a particular container hostage. It is required to ensure that shared pages are freed from a container when they are not actively referenced from the container that brought them in

---

 include/linux/rmap.h |    5 +++--
 mm/rmap.c            |   26 ++++++++++++++++++++------
 mm/vmscan.c          |    4 ++--
 3 files changed, 25 insertions(+), 10 deletions(-)

diff -puN mm/vmscan.c~rss-implement-per-container-page-referenced mm/vmscan.c

mm/vmscan.c renamed from mm/vmscan.c~rss-implement-per-container-page-referenced +2 -2
@@ -489,7 +489,7 @@ static unsigned long shrink_page_list(st
 		if (PageWriteback(page))
 			goto keep_locked;
 
-		referenced = page_referenced(page, 1);
+		referenced = page_referenced(page, 1, sc->cnt);
 		/* In active use or really unfreeable?  Activate it. */
 		if (referenced && page_mapping_inuse(page))
 			goto activate_locked;
@@ -852,7 +852,7 @@ force_reclaim_mapped:
 		if (page_mapped(page)) {
 			if (!reclaim_mapped ||
 			    (total_swap_pages == 0 && PageAnon(page)) ||
-			    page_referenced(page, 0)) {
+			    page_referenced(page, 0, sc->cnt)) {
 				list_add(&page->lru, &l_active);
 				continue;
 			}

diff -puN mm/rmap.c~rss-implement-per-container-page-referenced mm/rmap.c

mm/rmap.c renamed from mm/rmap.c~rss-implement-per-container-page-referenced +1 -1
@@ -318,7 +318,7 @@ out:
 	return referenced;
 }
 
-static int page_referenced_anon(struct page *page)
+static int page_referenced_anon(struct page *page, struct rss_container *cnt)
 {
 	unsigned int mapcount;
 	struct anon_vma *anon_vma;
@@ -331,6 +331,13 @@ static int page_referenced_anon(struct p
 
Quoted 11 lines

mm/rmap.c static int page_referenced_anon(struct p · line 340

Quoted 9 lines
mm/rmap.c renamed from mm/rmap.c~rss-implement-per-container-page-referenced +7 -0
 	mapcount = page_mapcount(page);
 	list_for_each_entry(vma, &anon_vma->head, anon_vma_node) {
+		/*
+		 * If we are reclaiming on behalf of a container, skip
+		 * counting on behalf of references from different
+		 * containers
+		 */
+		if (cnt && (vma->vm_mm->rss_container != cnt))
+			continue;

An #ifdef missed - mm->rss_container makes sense with the CONFIG_RSS_COUNTAINER set only.

Good catch. I'll fix it!

Quoted 85 lines
Quoted 84 lines
mm/rmap.c renamed from mm/rmap.c~rss-implement-per-container-page-referenced +12 -5
 		referenced += page_referenced_one(page, vma, &mapcount);
 		if (!mapcount)
 			break;
@@ -350,7 +357,7 @@ static int page_referenced_anon(struct p
  *
  * This function is only called from page_referenced for object-based pages.
  */
-static int page_referenced_file(struct page *page)
+static int page_referenced_file(struct page *page, struct rss_container *cnt)
 {
 	unsigned int mapcount;
 	struct address_space *mapping = page->mapping;
@@ -383,6 +390,13 @@ static int page_referenced_file(struct p
 	mapcount = page_mapcount(page);
 
 	vma_prio_tree_foreach(vma, &iter, &mapping->i_mmap, pgoff, pgoff) {
+		/*
+		 * If we are reclaiming on behalf of a container, skip
+		 * counting on behalf of references from different
+		 * containers
+		 */
+		if (cnt && (vma->vm_mm->rss_container != cnt))
+			continue;
 		if ((vma->vm_flags & (VM_LOCKED|VM_MAYSHARE))
 				  == (VM_LOCKED|VM_MAYSHARE)) {
 			referenced++;
@@ -405,7 +419,7 @@ static int page_referenced_file(struct p
  * Quick test_and_clear_referenced for all mappings to a page,
  * returns the number of ptes which referenced the page.
  */
-int page_referenced(struct page *page, int is_locked)
+int page_referenced(struct page *page, int is_locked, struct rss_container *cnt)
 {
 	int referenced = 0;
 
@@ -417,14 +431,14 @@ int page_referenced(struct page *page, i
 
 	if (page_mapped(page) && page->mapping) {
 		if (PageAnon(page))
-			referenced += page_referenced_anon(page);
+			referenced += page_referenced_anon(page, cnt);
 		else if (is_locked)
-			referenced += page_referenced_file(page);
+			referenced += page_referenced_file(page, cnt);
 		else if (TestSetPageLocked(page))
 			referenced++;
 		else {
 			if (page->mapping)
-				referenced += page_referenced_file(page);
+				referenced += page_referenced_file(page, cnt);
 			unlock_page(page);
 		}
 	}

diff -puN include/linux/rmap.h~rss-implement-per-container-page-referenced include/linux/rmap.h

include/linux/rmap.h renamed from include/linux/rmap.h~rss-implement-per-container-page-referenced +3 -2
@@ -8,6 +8,7 @@
 #include <linux/slab.h>
 #include <linux/mm.h>
 #include <linux/spinlock.h>
+#include <linux/rss_container.h>
 
 /*
  * The anon_vma heads a list of private "related" vmas, to scan if
@@ -93,7 +94,7 @@ static inline void page_dup_rmap(struct 
 /*
  * Called from mm/vmscan.c to handle paging out
  */
-int page_referenced(struct page *, int is_locked);
+int page_referenced(struct page *, int is_locked, struct rss_container *cnt);
 int try_to_unmap(struct page *, int ignore_refs);
 
 /*
@@ -121,7 +122,7 @@ int page_mkclean(struct page *);
 #define anon_vma_prepare(vma)	(0)
 #define anon_vma_link(vma)	do {} while (0)
 
-#define page_referenced(page,l) TestClearPageReferenced(page)
+#define page_referenced(page,l,cnt) TestClearPageReferenced(page)
 #define try_to_unmap(page, refs) SWAP_FAIL
 
 static inline int page_mkclean(struct page *page)

_

Signature · 4 lines
	Thanks,
	Balbir Singh
	Linux Technology Center
	IBM, ISTL