LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] RSS Container, make page_referenced() container aware
@ 2007-04-26 19:02 Balbir Singh
  2007-04-27  9:51 ` Pavel Emelianov
  0 siblings, 1 reply; 3+ messages in thread
From: Balbir Singh @ 2007-04-26 19:02 UTC (permalink / raw)
  To: Pavel Emelianov
  Cc: Andrew Morton, Kirill Korotaev, Vaidyanathan Srinivasan,
	linux kernel mailing list

[-- Attachment #1: Type: text/plain, Size: 261 bytes --]

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.

-- 
	Warm Regards,
	Balbir Singh
	Linux Technology Center
	IBM, ISTL

[-- Attachment #2: rss-implement-per-container-page-referenced.patch --]
[-- Type: text/x-patch, Size: 5366 bytes --]



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

Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com>
---

 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
--- linux-2.6.20/mm/vmscan.c~rss-implement-per-container-page-referenced	2007-04-26 23:28:44.000000000 +0530
+++ linux-2.6.20-balbir/mm/vmscan.c	2007-04-27 00:04:38.000000000 +0530
@@ -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
--- linux-2.6.20/mm/rmap.c~rss-implement-per-container-page-referenced	2007-04-26 23:28:44.000000000 +0530
+++ linux-2.6.20-balbir/mm/rmap.c	2007-04-26 23:33:41.000000000 +0530
@@ -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);
 		}
 	}
diff -puN include/linux/rmap.h~rss-implement-per-container-page-referenced include/linux/rmap.h
--- linux-2.6.20/include/linux/rmap.h~rss-implement-per-container-page-referenced	2007-04-26 23:28:44.000000000 +0530
+++ linux-2.6.20-balbir/include/linux/rmap.h	2007-04-26 23:29:31.000000000 +0530
@@ -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)
_

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] RSS Container, make page_referenced() container aware
  2007-04-26 19:02 [PATCH] RSS Container, make page_referenced() container aware Balbir Singh
@ 2007-04-27  9:51 ` Pavel Emelianov
  2007-04-27 11:11   ` Balbir Singh
  0 siblings, 1 reply; 3+ messages in thread
From: Pavel Emelianov @ 2007-04-27  9:51 UTC (permalink / raw)
  To: balbir
  Cc: Andrew Morton, Kirill Korotaev, Vaidyanathan Srinivasan,
	linux kernel mailing list

Balbir Singh wrote:
> 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.

> 
> ------------------------------------------------------------------------
> 
> 
> 
> 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
> 
> Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com>
> ---
> 
>  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
> --- linux-2.6.20/mm/vmscan.c~rss-implement-per-container-page-referenced	2007-04-26 23:28:44.000000000 +0530
> +++ linux-2.6.20-balbir/mm/vmscan.c	2007-04-27 00:04:38.000000000 +0530
> @@ -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
> --- linux-2.6.20/mm/rmap.c~rss-implement-per-container-page-referenced	2007-04-26 23:28:44.000000000 +0530
> +++ linux-2.6.20-balbir/mm/rmap.c	2007-04-26 23:33:41.000000000 +0530
> @@ -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;

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

>  		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
> --- linux-2.6.20/include/linux/rmap.h~rss-implement-per-container-page-referenced	2007-04-26 23:28:44.000000000 +0530
> +++ linux-2.6.20-balbir/include/linux/rmap.h	2007-04-26 23:29:31.000000000 +0530
> @@ -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)
> _


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] RSS Container, make page_referenced() container aware
  2007-04-27  9:51 ` Pavel Emelianov
@ 2007-04-27 11:11   ` Balbir Singh
  0 siblings, 0 replies; 3+ messages in thread
From: Balbir Singh @ 2007-04-27 11:11 UTC (permalink / raw)
  To: Pavel Emelianov
  Cc: Andrew Morton, Kirill Korotaev, Vaidyanathan Srinivasan,
	linux kernel mailing list

Pavel Emelianov wrote:
> Balbir Singh wrote:
>> 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

>> ------------------------------------------------------------------------
>>
>>
>>
>> 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
>>
>> Signed-off-by: Balbir Singh <balbir@linux.vnet.ibm.com>
>> ---
>>
>>  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
>> --- linux-2.6.20/mm/vmscan.c~rss-implement-per-container-page-referenced	2007-04-26 23:28:44.000000000 +0530
>> +++ linux-2.6.20-balbir/mm/vmscan.c	2007-04-27 00:04:38.000000000 +0530
>> @@ -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
>> --- linux-2.6.20/mm/rmap.c~rss-implement-per-container-page-referenced	2007-04-26 23:28:44.000000000 +0530
>> +++ linux-2.6.20-balbir/mm/rmap.c	2007-04-26 23:33:41.000000000 +0530
>> @@ -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;
> 
> An #ifdef missed - mm->rss_container makes sense with 
> the CONFIG_RSS_COUNTAINER set only.

Good catch. I'll fix it!

> 
>>  		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
>> --- linux-2.6.20/include/linux/rmap.h~rss-implement-per-container-page-referenced	2007-04-26 23:28:44.000000000 +0530
>> +++ linux-2.6.20-balbir/include/linux/rmap.h	2007-04-26 23:29:31.000000000 +0530
>> @@ -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)
>> _
> 


-- 
	Thanks,
	Balbir Singh
	Linux Technology Center
	IBM, ISTL

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2007-04-27 11:12 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-04-26 19:02 [PATCH] RSS Container, make page_referenced() container aware Balbir Singh
2007-04-27  9:51 ` Pavel Emelianov
2007-04-27 11:11   ` Balbir Singh

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 NNTP newsgroup(s).