[PATCH -mmotm 0/2] memcg: move charge of file cache/shmem

[PATCH -mmotm 0/2] memcg: move charge of file cache/shmem

[Daisuke Nishimura]

These are patches based on mmotm-2010-03-24-14-48 to support moving charges of
file cache/shmem at task migration.

[1/2] memcg move charge of file cache at task migration
[2/2] memcg move charge of shmem at task migration

I think they are rather small and simple than expected.
Any comments are welcome.

Thanks,
Daisuke Nishimura.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH -mmotm 1/2] memcg move charge of file cache at task migration

[Daisuke Nishimura]

This patch adds support for moving charge of file cache. It's enabled by setting
bit 1 of <target cgroup>/memory.move_charge_at_immigrate.

Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
---
 Documentation/cgroups/memory.txt |    6 ++++--
 mm/memcontrol.c                  |   14 +++++++++++---
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt
index 1b5bd04..f53d220 100644
--- a/Documentation/cgroups/memory.txt
+++ b/Documentation/cgroups/memory.txt
@@ -461,10 +461,12 @@ charges should be moved.
    0  | A charge of an anonymous page(or swap of it) used by the target task.
       | Those pages and swaps must be used only by the target task. You must
       | enable Swap Extension(see 2.4) to enable move of swap charges.
+ -----+------------------------------------------------------------------------
+   1  | A charge of file cache mmap'ed by the target task. Those pages must be
+      | mmap'ed only by the target task.
 
 Note: Those pages and swaps must be charged to the old cgroup.
-Note: More type of pages(e.g. file cache, shmem,) will be supported by other
-      bits in future.
+Note: More type of pages(e.g. shmem) will be supported by other bits in future.
 
 8.3 TODO
 
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index f6c9d42..66d2704 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -250,6 +250,7 @@ struct mem_cgroup {
  */
 enum move_type {
 	MOVE_CHARGE_TYPE_ANON,	/* private anonymous page and swap of it */
+	MOVE_CHARGE_TYPE_FILE,	/* private file caches */
 	NR_MOVE_TYPE,
 };
 
@@ -4192,6 +4193,8 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
 	int usage_count = 0;
 	bool move_anon = test_bit(MOVE_CHARGE_TYPE_ANON,
 					&mc.to->move_charge_at_immigrate);
+	bool move_file = test_bit(MOVE_CHARGE_TYPE_FILE,
+					&mc.to->move_charge_at_immigrate);
 
 	if (!pte_present(ptent)) {
 		/* TODO: handle swap of shmes/tmpfs */
@@ -4208,10 +4211,15 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
 		if (!page || !page_mapped(page))
 			return 0;
 		/*
-		 * TODO: We don't move charges of file(including shmem/tmpfs)
-		 * pages for now.
+		 * TODO: We don't move charges of shmem/tmpfs pages for now.
 		 */
-		if (!move_anon || !PageAnon(page))
+		if (PageAnon(page)) {
+			if (!move_anon)
+				return 0;
+		} else if (page_is_file_cache(page)) {
+			if (!move_file)
+				return 0;
+		} else
 			return 0;
 		if (!get_page_unless_zero(page))
 			return 0;
-- 
1.6.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH -mmotm 2/2] memcg move charge of shmem at task migration

[Daisuke Nishimura]

This patch adds support for moving charge of shmem and swap of it. It's enabled
by setting bit 2 of <target cgroup>/memory.move_charge_at_immigrate.

Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
---
 Documentation/cgroups/memory.txt |    7 +++-
 include/linux/swap.h             |    5 +++
 mm/memcontrol.c                  |   52 +++++++++++++++++++++++++------------
 mm/shmem.c                       |   37 +++++++++++++++++++++++++++
 4 files changed, 82 insertions(+), 19 deletions(-)

diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt
index f53d220..b197d60 100644
--- a/Documentation/cgroups/memory.txt
+++ b/Documentation/cgroups/memory.txt
@@ -464,13 +464,16 @@ charges should be moved.
  -----+------------------------------------------------------------------------
    1  | A charge of file cache mmap'ed by the target task. Those pages must be
       | mmap'ed only by the target task.
+ -----+------------------------------------------------------------------------
+   2  | A charge of a tmpfs page(or swap of it) mmap'ed by the target task. A
+      | typical use case of it is ipc shared memory. It must be mmap'ed by the
+      | target task, but unlike above 2 cases, the task may not be the only one.
+      | You must enable Swap Extension(see 2.4) to enable move of swap charges.
 
 Note: Those pages and swaps must be charged to the old cgroup.
-Note: More type of pages(e.g. shmem) will be supported by other bits in future.
 
 8.3 TODO
 
-- Add support for other types of pages(e.g. file cache, shmem, etc.).
 - Implement madvise(2) to let users decide the vma to be moved or not to be
   moved.
 - All of moving charge operations are done under cgroup_mutex. It's not good
diff --git a/include/linux/swap.h b/include/linux/swap.h
index 1f59d93..94ec325 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -285,6 +285,11 @@ extern void kswapd_stop(int nid);
 extern int shmem_unuse(swp_entry_t entry, struct page *page);
 #endif /* CONFIG_MMU */
 
+#ifdef CONFIG_CGROUP_MEM_RES_CTLR
+extern void mem_cgroup_get_shmem_target(struct inode *inode, pgoff_t pgoff,
+					struct page **pagep, swp_entry_t *ent);
+#endif
+
 extern void swap_unplug_io_fn(struct backing_dev_info *, struct page *);
 
 #ifdef CONFIG_SWAP
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 66d2704..99a496c 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -251,6 +251,7 @@ struct mem_cgroup {
 enum move_type {
 	MOVE_CHARGE_TYPE_ANON,	/* private anonymous page and swap of it */
 	MOVE_CHARGE_TYPE_FILE,	/* private file caches */
+	MOVE_CHARGE_TYPE_SHMEM,	/* shared memory and swap of it */
 	NR_MOVE_TYPE,
 };
 
@@ -4195,12 +4196,30 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
 					&mc.to->move_charge_at_immigrate);
 	bool move_file = test_bit(MOVE_CHARGE_TYPE_FILE,
 					&mc.to->move_charge_at_immigrate);
+	bool move_shmem = test_bit(MOVE_CHARGE_TYPE_SHMEM,
+					&mc.to->move_charge_at_immigrate);
+	bool is_shmem = false;
 
 	if (!pte_present(ptent)) {
-		/* TODO: handle swap of shmes/tmpfs */
-		if (pte_none(ptent) || pte_file(ptent))
-			return 0;
-		else if (is_swap_pte(ptent)) {
+		if (pte_none(ptent) || pte_file(ptent)) {
+			struct inode *inode;
+			struct address_space *mapping;
+			pgoff_t pgoff = 0;
+
+			if (!vma->vm_file)
+				return 0;
+			mapping = vma->vm_file->f_mapping;
+			if (!move_shmem || !mapping_cap_swap_backed(mapping))
+				return 0;
+
+			if (pte_none(ptent))
+				pgoff = linear_page_index(vma, addr);
+			if (pte_file(ptent))
+				pgoff = pte_to_pgoff(ptent);
+			inode = vma->vm_file->f_path.dentry->d_inode;
+			mem_cgroup_get_shmem_target(inode, pgoff, &page, &ent);
+			is_shmem = true;
+		} else if (is_swap_pte(ptent)) {
 			ent = pte_to_swp_entry(ptent);
 			if (!move_anon || non_swap_entry(ent))
 				return 0;
@@ -4210,26 +4229,22 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
 		page = vm_normal_page(vma, addr, ptent);
 		if (!page || !page_mapped(page))
 			return 0;
-		/*
-		 * TODO: We don't move charges of shmem/tmpfs pages for now.
-		 */
 		if (PageAnon(page)) {
 			if (!move_anon)
 				return 0;
 		} else if (page_is_file_cache(page)) {
 			if (!move_file)
 				return 0;
-		} else
-			return 0;
+		} else {
+			if (!move_shmem)
+				return 0;
+			is_shmem = true;
+		}
 		if (!get_page_unless_zero(page))
 			return 0;
 		usage_count = page_mapcount(page);
 	}
-	if (usage_count > 1) {
-		/*
-		 * TODO: We don't move charges of shared(used by multiple
-		 * processes) pages for now.
-		 */
+	if (usage_count > 1 && !is_shmem) {
 		if (page)
 			put_page(page);
 		return 0;
@@ -4284,6 +4299,8 @@ static unsigned long mem_cgroup_count_precharge(struct mm_struct *mm)
 
 	down_read(&mm->mmap_sem);
 	for (vma = mm->mmap; vma; vma = vma->vm_next) {
+		bool move_shmem = test_bit(MOVE_CHARGE_TYPE_SHMEM,
+					&mc.to->move_charge_at_immigrate);
 		struct mm_walk mem_cgroup_count_precharge_walk = {
 			.pmd_entry = mem_cgroup_count_precharge_pte_range,
 			.mm = mm,
@@ -4292,7 +4309,7 @@ static unsigned long mem_cgroup_count_precharge(struct mm_struct *mm)
 		if (is_vm_hugetlb_page(vma))
 			continue;
 		/* TODO: We don't move charges of shmem/tmpfs pages for now. */
-		if (vma->vm_flags & VM_SHARED)
+		if ((vma->vm_flags & VM_SHARED) && !move_shmem)
 			continue;
 		walk_page_range(vma->vm_start, vma->vm_end,
 					&mem_cgroup_count_precharge_walk);
@@ -4483,6 +4500,8 @@ static void mem_cgroup_move_charge(struct mm_struct *mm)
 	down_read(&mm->mmap_sem);
 	for (vma = mm->mmap; vma; vma = vma->vm_next) {
 		int ret;
+		bool move_shmem = test_bit(MOVE_CHARGE_TYPE_SHMEM,
+					&mc.to->move_charge_at_immigrate);
 		struct mm_walk mem_cgroup_move_charge_walk = {
 			.pmd_entry = mem_cgroup_move_charge_pte_range,
 			.mm = mm,
@@ -4490,8 +4509,7 @@ static void mem_cgroup_move_charge(struct mm_struct *mm)
 		};
 		if (is_vm_hugetlb_page(vma))
 			continue;
-		/* TODO: We don't move charges of shmem/tmpfs pages for now. */
-		if (vma->vm_flags & VM_SHARED)
+		if ((vma->vm_flags & VM_SHARED) && !move_shmem)
 			continue;
 		ret = walk_page_range(vma->vm_start, vma->vm_end,
 						&mem_cgroup_move_charge_walk);
diff --git a/mm/shmem.c b/mm/shmem.c
index dde4363..cb87365 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2701,3 +2701,40 @@ int shmem_zero_setup(struct vm_area_struct *vma)
 	vma->vm_ops = &shmem_vm_ops;
 	return 0;
 }
+
+#ifdef CONFIG_CGROUP_MEM_RES_CTLR
+/**
+ * mem_cgroup_get_shmem_target - find a page or entry assigned to the shmem file
+ * @inode: the inode to be searched
+ * @pgoff: the offset to be searched
+ * @pagep: the pointer for the found page to be stored
+ * @ent: the pointer for the found swap entry to be stored
+ *
+ * If a page is found, refcount of it is incremented. Callers should handle
+ * these refcount.
+ */
+void mem_cgroup_get_shmem_target(struct inode *inode, pgoff_t pgoff,
+					struct page **pagep, swp_entry_t *ent)
+{
+	swp_entry_t entry = { .val = 0 }, *ptr;
+	struct page *page = NULL;
+	struct shmem_inode_info *info = SHMEM_I(inode);
+
+	if ((pgoff << PAGE_CACHE_SHIFT) >= i_size_read(inode))
+		goto out;
+
+	spin_lock(&info->lock);
+	ptr = shmem_swp_entry(info, pgoff, NULL);
+	if (ptr && ptr->val) {
+		entry.val = ptr->val;
+		page = find_get_page(&swapper_space, entry.val);
+	} else
+		page = find_get_page(inode->i_mapping, pgoff);
+	if (ptr)
+		shmem_swp_unmap(ptr);
+	spin_unlock(&info->lock);
+out:
+	*pagep = page;
+	*ent = entry;
+}
+#endif
-- 
1.6.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH -mmotm 1/2] memcg move charge of file cache at task migration

[KAMEZAWA Hiroyuki]

On Mon, 29 Mar 2010 12:03:21 +0900
Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:

> This patch adds support for moving charge of file cache. It's enabled by setting
> bit 1 of <target cgroup>/memory.move_charge_at_immigrate.
> 
> Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> ---
>  Documentation/cgroups/memory.txt |    6 ++++--
>  mm/memcontrol.c                  |   14 +++++++++++---
>  2 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt
> index 1b5bd04..f53d220 100644
> --- a/Documentation/cgroups/memory.txt
> +++ b/Documentation/cgroups/memory.txt
> @@ -461,10 +461,12 @@ charges should be moved.
>     0  | A charge of an anonymous page(or swap of it) used by the target task.
>        | Those pages and swaps must be used only by the target task. You must
>        | enable Swap Extension(see 2.4) to enable move of swap charges.
> + -----+------------------------------------------------------------------------
> +   1  | A charge of file cache mmap'ed by the target task. Those pages must be
> +      | mmap'ed only by the target task.

Hmm..my English is not good but..
==
A charge of a page cache mapped by the target task. Pages mapped by multiple processes
will not be moved. This "page cache" doesn't include tmpfs.
==

Hmm, "a page mapped only by target task but belongs to other cgroup" will be moved ?
The answer is "NO.".

The code itself seems to work well. So, could you update Documentation ?

Thanks,
-Kame

>  
>  Note: Those pages and swaps must be charged to the old cgroup.
> -Note: More type of pages(e.g. file cache, shmem,) will be supported by other
> -      bits in future.
> +Note: More type of pages(e.g. shmem) will be supported by other bits in future.
>  
>  8.3 TODO
>  
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index f6c9d42..66d2704 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -250,6 +250,7 @@ struct mem_cgroup {
>   */
>  enum move_type {
>  	MOVE_CHARGE_TYPE_ANON,	/* private anonymous page and swap of it */
> +	MOVE_CHARGE_TYPE_FILE,	/* private file caches */
>  	NR_MOVE_TYPE,
>  };
>  
> @@ -4192,6 +4193,8 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
>  	int usage_count = 0;
>  	bool move_anon = test_bit(MOVE_CHARGE_TYPE_ANON,
>  					&mc.to->move_charge_at_immigrate);
> +	bool move_file = test_bit(MOVE_CHARGE_TYPE_FILE,
> +					&mc.to->move_charge_at_immigrate);
>  
>  	if (!pte_present(ptent)) {
>  		/* TODO: handle swap of shmes/tmpfs */
> @@ -4208,10 +4211,15 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
>  		if (!page || !page_mapped(page))
>  			return 0;
>  		/*
> -		 * TODO: We don't move charges of file(including shmem/tmpfs)
> -		 * pages for now.
> +		 * TODO: We don't move charges of shmem/tmpfs pages for now.
>  		 */
> -		if (!move_anon || !PageAnon(page))
> +		if (PageAnon(page)) {
> +			if (!move_anon)
> +				return 0;
> +		} else if (page_is_file_cache(page)) {
> +			if (!move_file)
> +				return 0;
> +		} else
>  			return 0;
>  		if (!get_page_unless_zero(page))
>  			return 0;
> -- 
> 1.6.4
> 
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH -mmotm 2/2] memcg move charge of shmem at task migration

[KAMEZAWA Hiroyuki]

On Mon, 29 Mar 2010 12:03:59 +0900
Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:

> This patch adds support for moving charge of shmem and swap of it. It's enabled
> by setting bit 2 of <target cgroup>/memory.move_charge_at_immigrate.
> 
> Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> ---
>  Documentation/cgroups/memory.txt |    7 +++-
>  include/linux/swap.h             |    5 +++
>  mm/memcontrol.c                  |   52 +++++++++++++++++++++++++------------
>  mm/shmem.c                       |   37 +++++++++++++++++++++++++++
>  4 files changed, 82 insertions(+), 19 deletions(-)
> 
> diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt
> index f53d220..b197d60 100644
> --- a/Documentation/cgroups/memory.txt
> +++ b/Documentation/cgroups/memory.txt
> @@ -464,13 +464,16 @@ charges should be moved.
>   -----+------------------------------------------------------------------------
>     1  | A charge of file cache mmap'ed by the target task. Those pages must be
>        | mmap'ed only by the target task.
> + -----+------------------------------------------------------------------------
> +   2  | A charge of a tmpfs page(or swap of it) mmap'ed by the target task. A
> +      | typical use case of it is ipc shared memory. It must be mmap'ed by the
> +      | target task, but unlike above 2 cases, the task may not be the only one.
> +      | You must enable Swap Extension(see 2.4) to enable move of swap charges.
>  
>  Note: Those pages and swaps must be charged to the old cgroup.
> -Note: More type of pages(e.g. shmem) will be supported by other bits in future.
>  
>  8.3 TODO
>  
> -- Add support for other types of pages(e.g. file cache, shmem, etc.).
>  - Implement madvise(2) to let users decide the vma to be moved or not to be
>    moved.
>  - All of moving charge operations are done under cgroup_mutex. It's not good
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index 1f59d93..94ec325 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -285,6 +285,11 @@ extern void kswapd_stop(int nid);
>  extern int shmem_unuse(swp_entry_t entry, struct page *page);
>  #endif /* CONFIG_MMU */
>  
> +#ifdef CONFIG_CGROUP_MEM_RES_CTLR
> +extern void mem_cgroup_get_shmem_target(struct inode *inode, pgoff_t pgoff,
> +					struct page **pagep, swp_entry_t *ent);
> +#endif
> +
>  extern void swap_unplug_io_fn(struct backing_dev_info *, struct page *);
>  
>  #ifdef CONFIG_SWAP
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 66d2704..99a496c 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -251,6 +251,7 @@ struct mem_cgroup {
>  enum move_type {
>  	MOVE_CHARGE_TYPE_ANON,	/* private anonymous page and swap of it */
>  	MOVE_CHARGE_TYPE_FILE,	/* private file caches */
> +	MOVE_CHARGE_TYPE_SHMEM,	/* shared memory and swap of it */
>  	NR_MOVE_TYPE,
>  };
>  
> @@ -4195,12 +4196,30 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
>  					&mc.to->move_charge_at_immigrate);
>  	bool move_file = test_bit(MOVE_CHARGE_TYPE_FILE,
>  					&mc.to->move_charge_at_immigrate);
> +	bool move_shmem = test_bit(MOVE_CHARGE_TYPE_SHMEM,
> +					&mc.to->move_charge_at_immigrate);
> +	bool is_shmem = false;
>  
>  	if (!pte_present(ptent)) {
> -		/* TODO: handle swap of shmes/tmpfs */
> -		if (pte_none(ptent) || pte_file(ptent))
> -			return 0;
> -		else if (is_swap_pte(ptent)) {
> +		if (pte_none(ptent) || pte_file(ptent)) {
> +			struct inode *inode;
> +			struct address_space *mapping;
> +			pgoff_t pgoff = 0;
> +
> +			if (!vma->vm_file)
> +				return 0;
> +			mapping = vma->vm_file->f_mapping;
> +			if (!move_shmem || !mapping_cap_swap_backed(mapping))
> +				return 0;
> +
> +			if (pte_none(ptent))
> +				pgoff = linear_page_index(vma, addr);
> +			if (pte_file(ptent))
> +				pgoff = pte_to_pgoff(ptent);

Hmm...then, a shmem page is moved even if the task doesn't do page-fault.
Could you clarify
	"All pages in the range mapped by a task will be moved to the new group
	 even if the task doesn't do page fault, i.e. not tasks' RSS."
?
Thanks,
-Kame

> +			inode = vma->vm_file->f_path.dentry->d_inode;
> +			mem_cgroup_get_shmem_target(inode, pgoff, &page, &ent);




> +			is_shmem = true;
> +		} else if (is_swap_pte(ptent)) {
>  			ent = pte_to_swp_entry(ptent);
>  			if (!move_anon || non_swap_entry(ent))
>  				return 0;
> @@ -4210,26 +4229,22 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
>  		page = vm_normal_page(vma, addr, ptent);
>  		if (!page || !page_mapped(page))
>  			return 0;
> -		/*
> -		 * TODO: We don't move charges of shmem/tmpfs pages for now.
> -		 */
>  		if (PageAnon(page)) {
>  			if (!move_anon)
>  				return 0;
>  		} else if (page_is_file_cache(page)) {
>  			if (!move_file)
>  				return 0;
> -		} else
> -			return 0;
> +		} else {
> +			if (!move_shmem)
> +				return 0;
> +			is_shmem = true;
> +		}
>  		if (!get_page_unless_zero(page))
>  			return 0;
>  		usage_count = page_mapcount(page);
>  	}
> -	if (usage_count > 1) {
> -		/*
> -		 * TODO: We don't move charges of shared(used by multiple
> -		 * processes) pages for now.
> -		 */
> +	if (usage_count > 1 && !is_shmem) {
>  		if (page)
>  			put_page(page);
>  		return 0;
> @@ -4284,6 +4299,8 @@ static unsigned long mem_cgroup_count_precharge(struct mm_struct *mm)
>  
>  	down_read(&mm->mmap_sem);
>  	for (vma = mm->mmap; vma; vma = vma->vm_next) {
> +		bool move_shmem = test_bit(MOVE_CHARGE_TYPE_SHMEM,
> +					&mc.to->move_charge_at_immigrate);
>  		struct mm_walk mem_cgroup_count_precharge_walk = {
>  			.pmd_entry = mem_cgroup_count_precharge_pte_range,
>  			.mm = mm,
> @@ -4292,7 +4309,7 @@ static unsigned long mem_cgroup_count_precharge(struct mm_struct *mm)
>  		if (is_vm_hugetlb_page(vma))
>  			continue;
>  		/* TODO: We don't move charges of shmem/tmpfs pages for now. */
> -		if (vma->vm_flags & VM_SHARED)
> +		if ((vma->vm_flags & VM_SHARED) && !move_shmem)
>  			continue;
>  		walk_page_range(vma->vm_start, vma->vm_end,
>  					&mem_cgroup_count_precharge_walk);
> @@ -4483,6 +4500,8 @@ static void mem_cgroup_move_charge(struct mm_struct *mm)
>  	down_read(&mm->mmap_sem);
>  	for (vma = mm->mmap; vma; vma = vma->vm_next) {
>  		int ret;
> +		bool move_shmem = test_bit(MOVE_CHARGE_TYPE_SHMEM,
> +					&mc.to->move_charge_at_immigrate);
>  		struct mm_walk mem_cgroup_move_charge_walk = {
>  			.pmd_entry = mem_cgroup_move_charge_pte_range,
>  			.mm = mm,
> @@ -4490,8 +4509,7 @@ static void mem_cgroup_move_charge(struct mm_struct *mm)
>  		};
>  		if (is_vm_hugetlb_page(vma))
>  			continue;
> -		/* TODO: We don't move charges of shmem/tmpfs pages for now. */
> -		if (vma->vm_flags & VM_SHARED)
> +		if ((vma->vm_flags & VM_SHARED) && !move_shmem)
>  			continue;
>  		ret = walk_page_range(vma->vm_start, vma->vm_end,
>  						&mem_cgroup_move_charge_walk);
> diff --git a/mm/shmem.c b/mm/shmem.c
> index dde4363..cb87365 100644
> --- a/mm/shmem.c
> +++ b/mm/shmem.c
> @@ -2701,3 +2701,40 @@ int shmem_zero_setup(struct vm_area_struct *vma)
>  	vma->vm_ops = &shmem_vm_ops;
>  	return 0;
>  }
> +
> +#ifdef CONFIG_CGROUP_MEM_RES_CTLR
> +/**
> + * mem_cgroup_get_shmem_target - find a page or entry assigned to the shmem file
> + * @inode: the inode to be searched
> + * @pgoff: the offset to be searched
> + * @pagep: the pointer for the found page to be stored
> + * @ent: the pointer for the found swap entry to be stored
> + *
> + * If a page is found, refcount of it is incremented. Callers should handle
> + * these refcount.
> + */
> +void mem_cgroup_get_shmem_target(struct inode *inode, pgoff_t pgoff,
> +					struct page **pagep, swp_entry_t *ent)
> +{
> +	swp_entry_t entry = { .val = 0 }, *ptr;
> +	struct page *page = NULL;
> +	struct shmem_inode_info *info = SHMEM_I(inode);
> +
> +	if ((pgoff << PAGE_CACHE_SHIFT) >= i_size_read(inode))
> +		goto out;
> +
> +	spin_lock(&info->lock);
> +	ptr = shmem_swp_entry(info, pgoff, NULL);
> +	if (ptr && ptr->val) {
> +		entry.val = ptr->val;
> +		page = find_get_page(&swapper_space, entry.val);
> +	} else
> +		page = find_get_page(inode->i_mapping, pgoff);
> +	if (ptr)
> +		shmem_swp_unmap(ptr);
> +	spin_unlock(&info->lock);
> +out:
> +	*pagep = page;
> +	*ent = entry;
> +}
> +#endif
> -- 
> 1.6.4
> 
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH(v2) -mmotm 1/2] memcg move charge of file cache at task migration

[Daisuke Nishimura]

On Mon, 29 Mar 2010 13:15:41 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> On Mon, 29 Mar 2010 12:03:21 +0900
> Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:
> 
> > This patch adds support for moving charge of file cache. It's enabled by setting
> > bit 1 of <target cgroup>/memory.move_charge_at_immigrate.
> > 
> > Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> > ---
> >  Documentation/cgroups/memory.txt |    6 ++++--
> >  mm/memcontrol.c                  |   14 +++++++++++---
> >  2 files changed, 15 insertions(+), 5 deletions(-)
> > 
> > diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt
> > index 1b5bd04..f53d220 100644
> > --- a/Documentation/cgroups/memory.txt
> > +++ b/Documentation/cgroups/memory.txt
> > @@ -461,10 +461,12 @@ charges should be moved.
> >     0  | A charge of an anonymous page(or swap of it) used by the target task.
> >        | Those pages and swaps must be used only by the target task. You must
> >        | enable Swap Extension(see 2.4) to enable move of swap charges.
> > + -----+------------------------------------------------------------------------
> > +   1  | A charge of file cache mmap'ed by the target task. Those pages must be
> > +      | mmap'ed only by the target task.
> 
> Hmm..my English is not good but..
> ==
> A charge of a page cache mapped by the target task. Pages mapped by multiple processes
> will not be moved. This "page cache" doesn't include tmpfs.
> ==
> 
This is more accurate than mine.

> Hmm, "a page mapped only by target task but belongs to other cgroup" will be moved ?
> The answer is "NO.".
> 
> The code itself seems to work well. So, could you update Documentation ?
> 
Thank you for your advice.

This is the updated version.

===
From: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>

This patch adds support for moving charge of file cache. It's enabled by setting
bit 1 of <target cgroup>/memory.move_charge_at_immigrate.

Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
---
v1->v2
  - update a documentation.

 Documentation/cgroups/memory.txt |    7 +++++--
 mm/memcontrol.c                  |   14 +++++++++++---
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt
index 1b5bd04..c624cd2 100644
--- a/Documentation/cgroups/memory.txt
+++ b/Documentation/cgroups/memory.txt
@@ -461,10 +461,13 @@ charges should be moved.
    0  | A charge of an anonymous page(or swap of it) used by the target task.
       | Those pages and swaps must be used only by the target task. You must
       | enable Swap Extension(see 2.4) to enable move of swap charges.
+ -----+------------------------------------------------------------------------
+   1  | A charge of page cache mapped by the target task. Pages mapped by
+      | multiple processes will not be moved. This "page cache" doesn't include
+      | tmpfs.
 
 Note: Those pages and swaps must be charged to the old cgroup.
-Note: More type of pages(e.g. file cache, shmem,) will be supported by other
-      bits in future.
+Note: More type of pages(e.g. shmem) will be supported by other bits in future.
 
 8.3 TODO
 
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index f6c9d42..66d2704 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -250,6 +250,7 @@ struct mem_cgroup {
  */
 enum move_type {
 	MOVE_CHARGE_TYPE_ANON,	/* private anonymous page and swap of it */
+	MOVE_CHARGE_TYPE_FILE,	/* private file caches */
 	NR_MOVE_TYPE,
 };
 
@@ -4192,6 +4193,8 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
 	int usage_count = 0;
 	bool move_anon = test_bit(MOVE_CHARGE_TYPE_ANON,
 					&mc.to->move_charge_at_immigrate);
+	bool move_file = test_bit(MOVE_CHARGE_TYPE_FILE,
+					&mc.to->move_charge_at_immigrate);
 
 	if (!pte_present(ptent)) {
 		/* TODO: handle swap of shmes/tmpfs */
@@ -4208,10 +4211,15 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
 		if (!page || !page_mapped(page))
 			return 0;
 		/*
-		 * TODO: We don't move charges of file(including shmem/tmpfs)
-		 * pages for now.
+		 * TODO: We don't move charges of shmem/tmpfs pages for now.
 		 */
-		if (!move_anon || !PageAnon(page))
+		if (PageAnon(page)) {
+			if (!move_anon)
+				return 0;
+		} else if (page_is_file_cache(page)) {
+			if (!move_file)
+				return 0;
+		} else
 			return 0;
 		if (!get_page_unless_zero(page))
 			return 0;
-- 
1.6.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

[PATCH(v2) -mmotm 2/2] memcg move charge of shmem at task migration

[Daisuke Nishimura]

On Mon, 29 Mar 2010 13:36:45 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> Hmm...then, a shmem page is moved even if the task doesn't do page-fault.
> Could you clarify
> 	"All pages in the range mapped by a task will be moved to the new group
> 	 even if the task doesn't do page fault, i.e. not tasks' RSS."
> ?
I see.

This is the updated version.

===
From: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>

This patch adds support for moving charge of shmem and swap of it. It's enabled
by setting bit 2 of <target cgroup>/memory.move_charge_at_immigrate.

Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
---
v1->v2
  - update a documentation.

 Documentation/cgroups/memory.txt |    9 +++++-
 include/linux/swap.h             |    5 +++
 mm/memcontrol.c                  |   52 +++++++++++++++++++++++++------------
 mm/shmem.c                       |   37 +++++++++++++++++++++++++++
 4 files changed, 84 insertions(+), 19 deletions(-)

diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt
index c624cd2..4755d5e 100644
--- a/Documentation/cgroups/memory.txt
+++ b/Documentation/cgroups/memory.txt
@@ -465,13 +465,18 @@ charges should be moved.
    1  | A charge of page cache mapped by the target task. Pages mapped by
       | multiple processes will not be moved. This "page cache" doesn't include
       | tmpfs.
+ -----+------------------------------------------------------------------------
+   2  | A charge of a tmpfs page(or swap of it) mapped by the target task. A
+      | typical use case of it is ipc shared memory. Unlike above 2 cases, all
+      | pages(and swaps) in the range mapped by the task will be moved even if
+      | the task hasn't done page fault, i.e. they might not be the task's
+      | "RSS", but other task's "RSS" that maps the shared memory. You must
+      | enable Swap Extension(see 2.4) to enable move of swap charges.
 
 Note: Those pages and swaps must be charged to the old cgroup.
-Note: More type of pages(e.g. shmem) will be supported by other bits in future.
 
 8.3 TODO
 
-- Add support for other types of pages(e.g. file cache, shmem, etc.).
 - Implement madvise(2) to let users decide the vma to be moved or not to be
   moved.
 - All of moving charge operations are done under cgroup_mutex. It's not good
diff --git a/include/linux/swap.h b/include/linux/swap.h
index 1f59d93..94ec325 100644
--- a/include/linux/swap.h
+++ b/include/linux/swap.h
@@ -285,6 +285,11 @@ extern void kswapd_stop(int nid);
 extern int shmem_unuse(swp_entry_t entry, struct page *page);
 #endif /* CONFIG_MMU */
 
+#ifdef CONFIG_CGROUP_MEM_RES_CTLR
+extern void mem_cgroup_get_shmem_target(struct inode *inode, pgoff_t pgoff,
+					struct page **pagep, swp_entry_t *ent);
+#endif
+
 extern void swap_unplug_io_fn(struct backing_dev_info *, struct page *);
 
 #ifdef CONFIG_SWAP
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 66d2704..99a496c 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -251,6 +251,7 @@ struct mem_cgroup {
 enum move_type {
 	MOVE_CHARGE_TYPE_ANON,	/* private anonymous page and swap of it */
 	MOVE_CHARGE_TYPE_FILE,	/* private file caches */
+	MOVE_CHARGE_TYPE_SHMEM,	/* shared memory and swap of it */
 	NR_MOVE_TYPE,
 };
 
@@ -4195,12 +4196,30 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
 					&mc.to->move_charge_at_immigrate);
 	bool move_file = test_bit(MOVE_CHARGE_TYPE_FILE,
 					&mc.to->move_charge_at_immigrate);
+	bool move_shmem = test_bit(MOVE_CHARGE_TYPE_SHMEM,
+					&mc.to->move_charge_at_immigrate);
+	bool is_shmem = false;
 
 	if (!pte_present(ptent)) {
-		/* TODO: handle swap of shmes/tmpfs */
-		if (pte_none(ptent) || pte_file(ptent))
-			return 0;
-		else if (is_swap_pte(ptent)) {
+		if (pte_none(ptent) || pte_file(ptent)) {
+			struct inode *inode;
+			struct address_space *mapping;
+			pgoff_t pgoff = 0;
+
+			if (!vma->vm_file)
+				return 0;
+			mapping = vma->vm_file->f_mapping;
+			if (!move_shmem || !mapping_cap_swap_backed(mapping))
+				return 0;
+
+			if (pte_none(ptent))
+				pgoff = linear_page_index(vma, addr);
+			if (pte_file(ptent))
+				pgoff = pte_to_pgoff(ptent);
+			inode = vma->vm_file->f_path.dentry->d_inode;
+			mem_cgroup_get_shmem_target(inode, pgoff, &page, &ent);
+			is_shmem = true;
+		} else if (is_swap_pte(ptent)) {
 			ent = pte_to_swp_entry(ptent);
 			if (!move_anon || non_swap_entry(ent))
 				return 0;
@@ -4210,26 +4229,22 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
 		page = vm_normal_page(vma, addr, ptent);
 		if (!page || !page_mapped(page))
 			return 0;
-		/*
-		 * TODO: We don't move charges of shmem/tmpfs pages for now.
-		 */
 		if (PageAnon(page)) {
 			if (!move_anon)
 				return 0;
 		} else if (page_is_file_cache(page)) {
 			if (!move_file)
 				return 0;
-		} else
-			return 0;
+		} else {
+			if (!move_shmem)
+				return 0;
+			is_shmem = true;
+		}
 		if (!get_page_unless_zero(page))
 			return 0;
 		usage_count = page_mapcount(page);
 	}
-	if (usage_count > 1) {
-		/*
-		 * TODO: We don't move charges of shared(used by multiple
-		 * processes) pages for now.
-		 */
+	if (usage_count > 1 && !is_shmem) {
 		if (page)
 			put_page(page);
 		return 0;
@@ -4284,6 +4299,8 @@ static unsigned long mem_cgroup_count_precharge(struct mm_struct *mm)
 
 	down_read(&mm->mmap_sem);
 	for (vma = mm->mmap; vma; vma = vma->vm_next) {
+		bool move_shmem = test_bit(MOVE_CHARGE_TYPE_SHMEM,
+					&mc.to->move_charge_at_immigrate);
 		struct mm_walk mem_cgroup_count_precharge_walk = {
 			.pmd_entry = mem_cgroup_count_precharge_pte_range,
 			.mm = mm,
@@ -4292,7 +4309,7 @@ static unsigned long mem_cgroup_count_precharge(struct mm_struct *mm)
 		if (is_vm_hugetlb_page(vma))
 			continue;
 		/* TODO: We don't move charges of shmem/tmpfs pages for now. */
-		if (vma->vm_flags & VM_SHARED)
+		if ((vma->vm_flags & VM_SHARED) && !move_shmem)
 			continue;
 		walk_page_range(vma->vm_start, vma->vm_end,
 					&mem_cgroup_count_precharge_walk);
@@ -4483,6 +4500,8 @@ static void mem_cgroup_move_charge(struct mm_struct *mm)
 	down_read(&mm->mmap_sem);
 	for (vma = mm->mmap; vma; vma = vma->vm_next) {
 		int ret;
+		bool move_shmem = test_bit(MOVE_CHARGE_TYPE_SHMEM,
+					&mc.to->move_charge_at_immigrate);
 		struct mm_walk mem_cgroup_move_charge_walk = {
 			.pmd_entry = mem_cgroup_move_charge_pte_range,
 			.mm = mm,
@@ -4490,8 +4509,7 @@ static void mem_cgroup_move_charge(struct mm_struct *mm)
 		};
 		if (is_vm_hugetlb_page(vma))
 			continue;
-		/* TODO: We don't move charges of shmem/tmpfs pages for now. */
-		if (vma->vm_flags & VM_SHARED)
+		if ((vma->vm_flags & VM_SHARED) && !move_shmem)
 			continue;
 		ret = walk_page_range(vma->vm_start, vma->vm_end,
 						&mem_cgroup_move_charge_walk);
diff --git a/mm/shmem.c b/mm/shmem.c
index dde4363..cb87365 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -2701,3 +2701,40 @@ int shmem_zero_setup(struct vm_area_struct *vma)
 	vma->vm_ops = &shmem_vm_ops;
 	return 0;
 }
+
+#ifdef CONFIG_CGROUP_MEM_RES_CTLR
+/**
+ * mem_cgroup_get_shmem_target - find a page or entry assigned to the shmem file
+ * @inode: the inode to be searched
+ * @pgoff: the offset to be searched
+ * @pagep: the pointer for the found page to be stored
+ * @ent: the pointer for the found swap entry to be stored
+ *
+ * If a page is found, refcount of it is incremented. Callers should handle
+ * these refcount.
+ */
+void mem_cgroup_get_shmem_target(struct inode *inode, pgoff_t pgoff,
+					struct page **pagep, swp_entry_t *ent)
+{
+	swp_entry_t entry = { .val = 0 }, *ptr;
+	struct page *page = NULL;
+	struct shmem_inode_info *info = SHMEM_I(inode);
+
+	if ((pgoff << PAGE_CACHE_SHIFT) >= i_size_read(inode))
+		goto out;
+
+	spin_lock(&info->lock);
+	ptr = shmem_swp_entry(info, pgoff, NULL);
+	if (ptr && ptr->val) {
+		entry.val = ptr->val;
+		page = find_get_page(&swapper_space, entry.val);
+	} else
+		page = find_get_page(inode->i_mapping, pgoff);
+	if (ptr)
+		shmem_swp_unmap(ptr);
+	spin_unlock(&info->lock);
+out:
+	*pagep = page;
+	*ent = entry;
+}
+#endif
-- 
1.6.4

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH(v2) -mmotm 1/2] memcg move charge of file cache at task migration

[KAMEZAWA Hiroyuki]

On Tue, 30 Mar 2010 10:32:36 +0900
Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:

> On Mon, 29 Mar 2010 13:15:41 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > On Mon, 29 Mar 2010 12:03:21 +0900
> > Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:
> > 
> > > This patch adds support for moving charge of file cache. It's enabled by setting
> > > bit 1 of <target cgroup>/memory.move_charge_at_immigrate.
> > > 
> > > Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> > > ---
> > >  Documentation/cgroups/memory.txt |    6 ++++--
> > >  mm/memcontrol.c                  |   14 +++++++++++---
> > >  2 files changed, 15 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt
> > > index 1b5bd04..f53d220 100644
> > > --- a/Documentation/cgroups/memory.txt
> > > +++ b/Documentation/cgroups/memory.txt
> > > @@ -461,10 +461,12 @@ charges should be moved.
> > >     0  | A charge of an anonymous page(or swap of it) used by the target task.
> > >        | Those pages and swaps must be used only by the target task. You must
> > >        | enable Swap Extension(see 2.4) to enable move of swap charges.
> > > + -----+------------------------------------------------------------------------
> > > +   1  | A charge of file cache mmap'ed by the target task. Those pages must be
> > > +      | mmap'ed only by the target task.
> > 
> > Hmm..my English is not good but..
> > ==
> > A charge of a page cache mapped by the target task. Pages mapped by multiple processes
> > will not be moved. This "page cache" doesn't include tmpfs.
> > ==
> > 
> This is more accurate than mine.
> 
> > Hmm, "a page mapped only by target task but belongs to other cgroup" will be moved ?
> > The answer is "NO.".
> > 
> > The code itself seems to work well. So, could you update Documentation ?
> > 
> Thank you for your advice.
> 
> This is the updated version.
> 
> ===
> From: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> 
> This patch adds support for moving charge of file cache. It's enabled by setting
> bit 1 of <target cgroup>/memory.move_charge_at_immigrate.
> 
> Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>

Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH(v2) -mmotm 2/2] memcg move charge of shmem at task migration

[KAMEZAWA Hiroyuki]

On Tue, 30 Mar 2010 10:33:01 +0900
Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:

> On Mon, 29 Mar 2010 13:36:45 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > Hmm...then, a shmem page is moved even if the task doesn't do page-fault.
> > Could you clarify
> > 	"All pages in the range mapped by a task will be moved to the new group
> > 	 even if the task doesn't do page fault, i.e. not tasks' RSS."
> > ?
> I see.
> 
> This is the updated version.
> 
> ===
> From: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> 
> This patch adds support for moving charge of shmem and swap of it. It's enabled
> by setting bit 2 of <target cgroup>/memory.move_charge_at_immigrate.
> 
> Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>

Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>

BTW, I'm glad if I can see some clean up around migrations. 
Total clean up after all necessary functions will be good.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH(v2) -mmotm 2/2] memcg move charge of shmem at task migration

[KAMEZAWA Hiroyuki]

On Tue, 30 Mar 2010 10:33:01 +0900
Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:

> On Mon, 29 Mar 2010 13:36:45 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > Hmm...then, a shmem page is moved even if the task doesn't do page-fault.
> > Could you clarify
> > 	"All pages in the range mapped by a task will be moved to the new group
> > 	 even if the task doesn't do page fault, i.e. not tasks' RSS."
> > ?
> I see.
> 
> This is the updated version.
> 

Ah, sorry. one more quesiton.

> ===
> From: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> 
> This patch adds support for moving charge of shmem and swap of it. It's enabled
> by setting bit 2 of <target cgroup>/memory.move_charge_at_immigrate.
> 
> Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> ---
> v1->v2
>   - update a documentation.
> 
>  Documentation/cgroups/memory.txt |    9 +++++-
>  include/linux/swap.h             |    5 +++
>  mm/memcontrol.c                  |   52 +++++++++++++++++++++++++------------
>  mm/shmem.c                       |   37 +++++++++++++++++++++++++++
>  4 files changed, 84 insertions(+), 19 deletions(-)
> 
> diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt
> index c624cd2..4755d5e 100644
> --- a/Documentation/cgroups/memory.txt
> +++ b/Documentation/cgroups/memory.txt
> @@ -465,13 +465,18 @@ charges should be moved.
>     1  | A charge of page cache mapped by the target task. Pages mapped by
>        | multiple processes will not be moved. This "page cache" doesn't include
>        | tmpfs.
> + -----+------------------------------------------------------------------------
> +   2  | A charge of a tmpfs page(or swap of it) mapped by the target task. A
> +      | typical use case of it is ipc shared memory. Unlike above 2 cases, all
> +      | pages(and swaps) in the range mapped by the task will be moved even if
> +      | the task hasn't done page fault, i.e. they might not be the task's
> +      | "RSS", but other task's "RSS" that maps the shared memory. You must
> +      | enable Swap Extension(see 2.4) to enable move of swap charges.
>  
>  Note: Those pages and swaps must be charged to the old cgroup.
> -Note: More type of pages(e.g. shmem) will be supported by other bits in future.
>  
>  8.3 TODO
>  
> -- Add support for other types of pages(e.g. file cache, shmem, etc.).
>  - Implement madvise(2) to let users decide the vma to be moved or not to be
>    moved.
>  - All of moving charge operations are done under cgroup_mutex. It's not good
> diff --git a/include/linux/swap.h b/include/linux/swap.h
> index 1f59d93..94ec325 100644
> --- a/include/linux/swap.h
> +++ b/include/linux/swap.h
> @@ -285,6 +285,11 @@ extern void kswapd_stop(int nid);
>  extern int shmem_unuse(swp_entry_t entry, struct page *page);
>  #endif /* CONFIG_MMU */
>  
> +#ifdef CONFIG_CGROUP_MEM_RES_CTLR
> +extern void mem_cgroup_get_shmem_target(struct inode *inode, pgoff_t pgoff,
> +					struct page **pagep, swp_entry_t *ent);
> +#endif
> +
>  extern void swap_unplug_io_fn(struct backing_dev_info *, struct page *);
>  
>  #ifdef CONFIG_SWAP
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 66d2704..99a496c 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -251,6 +251,7 @@ struct mem_cgroup {
>  enum move_type {
>  	MOVE_CHARGE_TYPE_ANON,	/* private anonymous page and swap of it */
>  	MOVE_CHARGE_TYPE_FILE,	/* private file caches */
> +	MOVE_CHARGE_TYPE_SHMEM,	/* shared memory and swap of it */
>  	NR_MOVE_TYPE,
>  };
>  
> @@ -4195,12 +4196,30 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
>  					&mc.to->move_charge_at_immigrate);
>  	bool move_file = test_bit(MOVE_CHARGE_TYPE_FILE,
>  					&mc.to->move_charge_at_immigrate);
> +	bool move_shmem = test_bit(MOVE_CHARGE_TYPE_SHMEM,
> +					&mc.to->move_charge_at_immigrate);
> +	bool is_shmem = false;
>  
>  	if (!pte_present(ptent)) {
> -		/* TODO: handle swap of shmes/tmpfs */
> -		if (pte_none(ptent) || pte_file(ptent))
> -			return 0;
> -		else if (is_swap_pte(ptent)) {
> +		if (pte_none(ptent) || pte_file(ptent)) {
> +			struct inode *inode;
> +			struct address_space *mapping;
> +			pgoff_t pgoff = 0;
> +
> +			if (!vma->vm_file)
> +				return 0;
> +			mapping = vma->vm_file->f_mapping;
> +			if (!move_shmem || !mapping_cap_swap_backed(mapping))
> +				return 0;
> +
> +			if (pte_none(ptent))
> +				pgoff = linear_page_index(vma, addr);
> +			if (pte_file(ptent))
> +				pgoff = pte_to_pgoff(ptent);
> +			inode = vma->vm_file->f_path.dentry->d_inode;
> +			mem_cgroup_get_shmem_target(inode, pgoff, &page, &ent);
> +			is_shmem = true;
> +		} else if (is_swap_pte(ptent)) {
>  			ent = pte_to_swp_entry(ptent);
>  			if (!move_anon || non_swap_entry(ent))
>  				return 0;
> @@ -4210,26 +4229,22 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
>  		page = vm_normal_page(vma, addr, ptent);
>  		if (!page || !page_mapped(page))
>  			return 0;
> -		/*
> -		 * TODO: We don't move charges of shmem/tmpfs pages for now.
> -		 */
>  		if (PageAnon(page)) {
>  			if (!move_anon)
>  				return 0;
>  		} else if (page_is_file_cache(page)) {
>  			if (!move_file)
>  				return 0;
> -		} else
> -			return 0;
> +		} else {
> +			if (!move_shmem)
> +				return 0;
> +			is_shmem = true;
> +		}
>  		if (!get_page_unless_zero(page))
>  			return 0;
>  		usage_count = page_mapcount(page);
>  	}
> -	if (usage_count > 1) {
> -		/*
> -		 * TODO: We don't move charges of shared(used by multiple
> -		 * processes) pages for now.
> -		 */
> +	if (usage_count > 1 && !is_shmem) {
>  		if (page)
>  			put_page(page);
>  		return 0;
> @@ -4284,6 +4299,8 @@ static unsigned long mem_cgroup_count_precharge(struct mm_struct *mm)
>  
>  	down_read(&mm->mmap_sem);
>  	for (vma = mm->mmap; vma; vma = vma->vm_next) {
> +		bool move_shmem = test_bit(MOVE_CHARGE_TYPE_SHMEM,
> +					&mc.to->move_charge_at_immigrate);
>  		struct mm_walk mem_cgroup_count_precharge_walk = {
>  			.pmd_entry = mem_cgroup_count_precharge_pte_range,
>  			.mm = mm,
> @@ -4292,7 +4309,7 @@ static unsigned long mem_cgroup_count_precharge(struct mm_struct *mm)
>  		if (is_vm_hugetlb_page(vma))
>  			continue;
>  		/* TODO: We don't move charges of shmem/tmpfs pages for now. */
> -		if (vma->vm_flags & VM_SHARED)
> +		if ((vma->vm_flags & VM_SHARED) && !move_shmem)
>  			continue;

SHARED mapped file cache is not moved by patch [1/2] ???
It sounds strange.

Thanks,
-Kame

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH(v2) -mmotm 2/2] memcg move charge of shmem at task migration

[Daisuke Nishimura]

On Tue, 30 Mar 2010 11:23:01 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> SHARED mapped file cache is not moved by patch [1/2] ???
> It sounds strange.
> 
hmm, I'm sorry I'm not so good at user applications, but is it usual to use
VM_SHARED file caches(!tmpfs) ?
And is it better for us to move them only when page_mapcount() == 1 ?


Thanks,
Daisuke Nishimura.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH(v2) -mmotm 2/2] memcg move charge of shmem at task migration

[KAMEZAWA Hiroyuki]

On Tue, 30 Mar 2010 11:49:03 +0900
Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:

> On Tue, 30 Mar 2010 11:23:01 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > SHARED mapped file cache is not moved by patch [1/2] ???
> > It sounds strange.
> > 
> hmm, I'm sorry I'm not so good at user applications, but is it usual to use
> VM_SHARED file caches(!tmpfs) ?
> And is it better for us to move them only when page_mapcount() == 1 ?
> 

Considering shared library which has only one user, moving MAP_SHARED makes sense.
Unfortunately, there are people who creates their own shared library just for
their private dlopen() etc. (shared library for private use...)

So, I think moving MAP_SHARED files makes sense.

Thanks,
-Kame

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH(v2) -mmotm 2/2] memcg move charge of shmem at task migration

[Daisuke Nishimura]

On Tue, 30 Mar 2010 12:11:19 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> On Tue, 30 Mar 2010 11:49:03 +0900
> Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:
> 
> > On Tue, 30 Mar 2010 11:23:01 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > > SHARED mapped file cache is not moved by patch [1/2] ???
> > > It sounds strange.
> > > 
> > hmm, I'm sorry I'm not so good at user applications, but is it usual to use
> > VM_SHARED file caches(!tmpfs) ?
> > And is it better for us to move them only when page_mapcount() == 1 ?
> > 
> 
> Considering shared library which has only one user, moving MAP_SHARED makes sense.
> Unfortunately, there are people who creates their own shared library just for
> their private dlopen() etc. (shared library for private use...)
> 
> So, I think moving MAP_SHARED files makes sense.
> 
Thank you for your explanations.
I'll update my patches to allow to move MAP_SHARED(but page_mapcount() == 1)
file caches, and resend.

Thanks,
Daisuke Nishimura.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH(v2) -mmotm 2/2] memcg move charge of shmem at task migration

[KAMEZAWA Hiroyuki]

On Tue, 30 Mar 2010 13:06:48 +0900
Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:

> On Tue, 30 Mar 2010 12:11:19 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > On Tue, 30 Mar 2010 11:49:03 +0900
> > Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:
> > 
> > > On Tue, 30 Mar 2010 11:23:01 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > > > SHARED mapped file cache is not moved by patch [1/2] ???
> > > > It sounds strange.
> > > > 
> > > hmm, I'm sorry I'm not so good at user applications, but is it usual to use
> > > VM_SHARED file caches(!tmpfs) ?
> > > And is it better for us to move them only when page_mapcount() == 1 ?
> > > 
> > 
> > Considering shared library which has only one user, moving MAP_SHARED makes sense.
> > Unfortunately, there are people who creates their own shared library just for
> > their private dlopen() etc. (shared library for private use...)
> > 
> > So, I think moving MAP_SHARED files makes sense.
> > 
> Thank you for your explanations.
> I'll update my patches to allow to move MAP_SHARED(but page_mapcount() == 1)
> file caches, and resend.
> 

Hmm, considering again...current summary is following...right ?

 - If page is an anon, it's not moved if page_mapcount() > 2.
 - If page is a page cache, it's not moved if page_mapcount() > 2.
 - If page is a shmem, it's not moved regardless of mapcount.
 - If pte is swap, it's not moved refcnt > 2.

I think following is straightforward and simple.

 - If page is an anon or swap of anon, it's not moved if referer > 2. 
   (i.e. inherited from it's parent)
 - If page is file,shmem or swap of shmem, it's moved regardless of referer.
   But pages only under "from" memcg can be moved.

I doubt adding too much speciality to shmem is not good.


How do you think ?

Thanks,
-Kame




--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH(v2) -mmotm 2/2] memcg move charge of shmem at task migration

[Balbir Singh]

* KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2010-03-30 13:51:59]:

> On Tue, 30 Mar 2010 13:06:48 +0900
> Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:
> 
> > On Tue, 30 Mar 2010 12:11:19 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > > On Tue, 30 Mar 2010 11:49:03 +0900
> > > Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:
> > > 
> > > > On Tue, 30 Mar 2010 11:23:01 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > > > > SHARED mapped file cache is not moved by patch [1/2] ???
> > > > > It sounds strange.
> > > > > 
> > > > hmm, I'm sorry I'm not so good at user applications, but is it usual to use
> > > > VM_SHARED file caches(!tmpfs) ?
> > > > And is it better for us to move them only when page_mapcount() == 1 ?
> > > > 
> > > 
> > > Considering shared library which has only one user, moving MAP_SHARED makes sense.
> > > Unfortunately, there are people who creates their own shared library just for
> > > their private dlopen() etc. (shared library for private use...)
> > > 
> > > So, I think moving MAP_SHARED files makes sense.
> > > 

IIRC, the libraries are loaded with MAP_PRIVATE and MAP_SHARED is not
set.

> > Thank you for your explanations.
> > I'll update my patches to allow to move MAP_SHARED(but page_mapcount() == 1)
> > file caches, and resend.
> > 
> 
> Hmm, considering again...current summary is following...right ?
> 
>  - If page is an anon, it's not moved if page_mapcount() > 2.
>  - If page is a page cache, it's not moved if page_mapcount() > 2.
>  - If page is a shmem, it's not moved regardless of mapcount.
>  - If pte is swap, it's not moved refcnt > 2.
> 
> I think following is straightforward and simple.
> 
>  - If page is an anon or swap of anon, it's not moved if referer > 2. 

What is referer in this context? The cgroup refering to the page?

>    (i.e. inherited from it's parent)
>  - If page is file,shmem or swap of shmem, it's moved regardless of referer.
>    But pages only under "from" memcg can be moved.
> 
> I doubt adding too much speciality to shmem is not good.
>

Yep, I tend to agree, but I need to take a closer look again at the
patches. 

-- 
	Three Cheers,
	Balbir

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH(v2) -mmotm 2/2] memcg move charge of shmem at task migration

[KAMEZAWA Hiroyuki]

On Tue, 30 Mar 2010 10:30:50 +0530
Balbir Singh <balbir@linux.vnet.ibm.com> wrote:

> * KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2010-03-30 13:51:59]:
> > > > Considering shared library which has only one user, moving MAP_SHARED makes sense.
> > > > Unfortunately, there are people who creates their own shared library just for
> > > > their private dlopen() etc. (shared library for private use...)
> > > > 
> > > > So, I think moving MAP_SHARED files makes sense.
> > > > 
> 
> IIRC, the libraries are loaded with MAP_PRIVATE and MAP_SHARED is not
> set.
> 
Ah, yes. I was wrong.
But changing handling of MAP_SHARED/MAP_PRIVATE is not good.
It will give much confusion to users.


> > > Thank you for your explanations.
> > > I'll update my patches to allow to move MAP_SHARED(but page_mapcount() == 1)
> > > file caches, and resend.
> > > 
> > 
> > Hmm, considering again...current summary is following...right ?
> > 
> >  - If page is an anon, it's not moved if page_mapcount() > 2.
> >  - If page is a page cache, it's not moved if page_mapcount() > 2.
> >  - If page is a shmem, it's not moved regardless of mapcount.
> >  - If pte is swap, it's not moved refcnt > 2.
> > 
> > I think following is straightforward and simple.
> > 
> >  - If page is an anon or swap of anon, it's not moved if referer > 2. 
> 
> What is referer in this context? The cgroup refering to the page?
> 
page_mapcount(page) + refcnt_to_swap_entry(ent.val)

Bye.
-Kame
> >    (i.e. inherited from it's parent)
> >  - If page is file,shmem or swap of shmem, it's moved regardless of referer.
> >    But pages only under "from" memcg can be moved.
> > 
> > I doubt adding too much speciality to shmem is not good.
> >
> 
> Yep, I tend to agree, but I need to take a closer look again at the
> patches. 
> 
> -- 
> 	Three Cheers,
> 	Balbir
> 

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH(v2) -mmotm 2/2] memcg move charge of shmem at task migration

[Daisuke Nishimura]

On Tue, 30 Mar 2010 10:30:50 +0530, Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
> * KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2010-03-30 13:51:59]:
> 
> > On Tue, 30 Mar 2010 13:06:48 +0900
> > Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:
> > 
> > > On Tue, 30 Mar 2010 12:11:19 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > > > On Tue, 30 Mar 2010 11:49:03 +0900
> > > > Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:
> > > > 
> > > > > On Tue, 30 Mar 2010 11:23:01 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > > > > > SHARED mapped file cache is not moved by patch [1/2] ???
> > > > > > It sounds strange.
> > > > > > 
> > > > > hmm, I'm sorry I'm not so good at user applications, but is it usual to use
> > > > > VM_SHARED file caches(!tmpfs) ?
> > > > > And is it better for us to move them only when page_mapcount() == 1 ?
> > > > > 
> > > > 
> > > > Considering shared library which has only one user, moving MAP_SHARED makes sense.
> > > > Unfortunately, there are people who creates their own shared library just for
> > > > their private dlopen() etc. (shared library for private use...)
> > > > 
> > > > So, I think moving MAP_SHARED files makes sense.
> > > > 
> 
> IIRC, the libraries are loaded with MAP_PRIVATE and MAP_SHARED is not
> set.
> 
Thank you for your information.

> > > Thank you for your explanations.
> > > I'll update my patches to allow to move MAP_SHARED(but page_mapcount() == 1)
> > > file caches, and resend.
> > > 
> > 
> > Hmm, considering again...current summary is following...right ?
> > 
> >  - If page is an anon, it's not moved if page_mapcount() > 2.
> >  - If page is a page cache, it's not moved if page_mapcount() > 2.
> >  - If page is a shmem, it's not moved regardless of mapcount.
> >  - If pte is swap, it's not moved refcnt > 2.
> > 
Right.

> > I think following is straightforward and simple.
> > 
> >  - If page is an anon or swap of anon, it's not moved if referer > 2. 
> 
> What is referer in this context? The cgroup refering to the page?
> 
> >    (i.e. inherited from it's parent)
> >  - If page is file,shmem or swap of shmem, it's moved regardless of referer.
> >    But pages only under "from" memcg can be moved.
> > 
> > I doubt adding too much speciality to shmem is not good.
> >
> 
> Yep, I tend to agree, but I need to take a closer look again at the
> patches. 
> 
I agree it would be more simple. I selected the current policy because
I was not sure whether we should move file caches(!tmpfs) with mapcount > 1,
and, IMHO, shared memory and file caches are different for users.
But it's O.K. for me to change current policy.


Thanks,
Daisuke Nishimura.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH(v2) -mmotm 2/2] memcg move charge of shmem at task migration

[KAMEZAWA Hiroyuki]

On Tue, 30 Mar 2010 14:30:38 +0900
Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:

> On Tue, 30 Mar 2010 10:30:50 +0530, Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
> > * KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2010-03-30 13:51:59]:
> > Yep, I tend to agree, but I need to take a closer look again at the
> > patches. 
> > 
> I agree it would be more simple. I selected the current policy because
> I was not sure whether we should move file caches(!tmpfs) with mapcount > 1,
> and, IMHO, shared memory and file caches are different for users.
> But it's O.K. for me to change current policy.
> 

To explain what I think of, I wrote a patch onto yours. (Maybe overkill for explaination ;)

Summary.

 + adding move_anon, move_file, move_shmem information to move_charge_struct.
 + adding hanlders for each pte types.
 + checking # of referer should be divided to each type.
   It's complicated to catch all cases in one "if" sentense.
 + FILE pages will be moved if it's charged against "from". no mapcount check.
   i.e. FILE pages should be moved even if it's not page-faulted.
 + ANON pages will be moved if it's really private.

For widely shared FILE, "if it's charged against "from"" is enough good limitation.



---
 mm/memcontrol.c |  265 +++++++++++++++++++++++++++++++++++---------------------
 1 file changed, 170 insertions(+), 95 deletions(-)

Index: mmotm-2.6.34-Mar24/mm/memcontrol.c
===================================================================
--- mmotm-2.6.34-Mar24.orig/mm/memcontrol.c
+++ mmotm-2.6.34-Mar24/mm/memcontrol.c
@@ -263,6 +263,10 @@ static struct move_charge_struct {
 	unsigned long moved_charge;
 	unsigned long moved_swap;
 	struct task_struct *moving_task;	/* a task moving charges */
+	/* move type attributes */
+	unsigned move_anon:1;
+	unsigned move_file:1;
+	unsigned move_shmem:1;
 	wait_queue_head_t waitq;		/* a waitq for other context */
 } mc = {
 	.waitq = __WAIT_QUEUE_HEAD_INITIALIZER(mc.waitq),
@@ -4184,6 +4188,112 @@ enum mc_target_type {
 	MC_TARGET_SWAP,
 };
 
+/*
+ * Hanlder for 4 pte types (present, nont, pte_file, swap_entry).
+ */
+static struct page *__mc_handle_present_pte(struct vm_area_struct *vma,
+					unsigned long addr, pte_t ptent)
+{
+	struct page *page = vm_normal_page(vma, addr, ptent);
+
+	if (PageAnon(page)) {
+		/* we don't move shared anon */
+		if (!mc.move_anon || page_mapcount(page) > 2)
+			return NULL;
+	} else if (page_is_file_cache(page)) {
+		if (!mc.move_file)
+			return NULL;
+	} else if (!mc.move_shmem)
+		return NULL;
+
+	if (!get_page_unless_zero(page))
+		return NULL;
+
+	return page;
+}
+
+static struct page *__mc_handle_pte_none(struct vm_area_struct *vma,
+				unsigned long addr, swp_entry_t *ent)
+{
+	struct page *page;
+	pgoff_t pgoff;
+	struct inode *inode;
+	struct address_space *mapping;
+
+	if (!vma->vm_file) /* Fully anonymous vma. */
+		return NULL;
+	inode = vma->vm_file->f_path.dentry->d_inode;
+	mapping = inode->i_mapping;
+
+	pgoff = linear_page_index(vma, addr);
+
+	if (!mapping_cap_swap_backed(mapping)) { /* usual file */
+		if (!mc.move_file)
+			return NULL;
+		page = find_get_page(mapping, pgoff);
+		/* page is moved even if it's not mapped (page-faulted) */
+	} else {
+		/* For shmem and tmpfs. We do swap accounting then... */
+		if (!mc.move_shmem)
+			return NULL;
+		mem_cgroup_get_shmem_target(inode, pgoff, &page, ent);
+	}
+	return page;
+}
+
+static struct page *__mc_handle_pte_file(struct vm_area_struct *vma,
+			unsigned long addr, pte_t ptent, swp_entry_t *ent)
+{
+	struct page *page;
+	pgoff_t pgoff;
+	struct inode *inode;
+	struct address_space *mapping;
+
+	if (!vma->vm_file) /* Fully anonymous vma. */
+		return NULL;
+	inode = vma->vm_file->f_path.dentry->d_inode;
+	mapping = inode->i_mapping;
+
+	pgoff = pte_to_pgoff(ptent);
+
+	if (!mapping_cap_swap_backed(mapping)) { /* usual file */
+		if (!mc.move_file)
+			return NULL;
+		page = find_get_page(mapping, pgoff);
+		/* page is moved even if it's not mapped (page-faulted) */
+	} else {
+		/* shmem, tmpfs file. We do swap accounting then... */
+		if (!mc.move_shmem)
+			return NULL;
+		mem_cgroup_get_shmem_target(inode, pgoff, &page, ent);
+		if (!page && !do_swap_account) {
+			ent->val = 0;
+			return NULL;
+		}
+	}
+	return page;
+}
+
+static struct page *__mc_handle_pte_swap(struct vm_area_struct *vma,
+			unsigned long addr, pte_t ptent, swp_entry_t *ent)
+{
+	int count;
+	struct page *page;
+
+	*ent = pte_to_swp_entry(ptent);
+	if (!do_swap_account || non_swap_entry(*ent))
+		return NULL;
+	count = mem_cgroup_count_swap_user(*ent, &page);
+	if (count > 1) { /* We don't move shared anon */
+		if (page)
+			put_page(page);
+		ent->val = 0;
+		return NULL;
+	}
+	return page;
+}
+
+
 static int is_target_pte_for_mc(struct vm_area_struct *vma,
 		unsigned long addr, pte_t ptent, union mc_target *target)
 {
@@ -4191,70 +4301,27 @@ static int is_target_pte_for_mc(struct v
 	struct page_cgroup *pc;
 	int ret = 0;
 	swp_entry_t ent = { .val = 0 };
-	int usage_count = 0;
-	bool move_anon = test_bit(MOVE_CHARGE_TYPE_ANON,
-					&mc.to->move_charge_at_immigrate);
-	bool move_file = test_bit(MOVE_CHARGE_TYPE_FILE,
-					&mc.to->move_charge_at_immigrate);
-	bool move_shmem = test_bit(MOVE_CHARGE_TYPE_SHMEM,
-					&mc.to->move_charge_at_immigrate);
-	bool is_shmem = false;
 
-	if (!pte_present(ptent)) {
-		if (pte_none(ptent) || pte_file(ptent)) {
-			struct inode *inode;
-			struct address_space *mapping;
-			pgoff_t pgoff = 0;
-
-			if (!vma->vm_file)
-				return 0;
-			mapping = vma->vm_file->f_mapping;
-			if (!move_shmem || !mapping_cap_swap_backed(mapping))
-				return 0;
-
-			if (pte_none(ptent))
-				pgoff = linear_page_index(vma, addr);
-			if (pte_file(ptent))
-				pgoff = pte_to_pgoff(ptent);
-			inode = vma->vm_file->f_path.dentry->d_inode;
-			mem_cgroup_get_shmem_target(inode, pgoff, &page, &ent);
-			is_shmem = true;
-		} else if (is_swap_pte(ptent)) {
-			ent = pte_to_swp_entry(ptent);
-			if (!move_anon || non_swap_entry(ent))
-				return 0;
-			usage_count = mem_cgroup_count_swap_user(ent, &page);
-		}
-	} else {
-		page = vm_normal_page(vma, addr, ptent);
-		if (!page || !page_mapped(page))
-			return 0;
-		if (PageAnon(page)) {
-			if (!move_anon)
-				return 0;
-		} else if (page_is_file_cache(page)) {
-			if (!move_file)
-				return 0;
-		} else {
-			if (!move_shmem)
-				return 0;
-			is_shmem = true;
-		}
-		if (!get_page_unless_zero(page))
-			return 0;
-		usage_count = page_mapcount(page);
-	}
-	if (usage_count > 1 && !is_shmem) {
-		if (page)
-			put_page(page);
+	if (pte_present(ptent))
+		page = __mc_handle_present_pte(vma, addr, ptent);
+	else if (pte_none(ptent) && (mc.move_file || mc.move_shmem))
+		page = __mc_handle_pte_none(vma, addr, &ent);
+	else if (pte_file(ptent) && (mc.move_file || mc.move_shmem))
+		page = __mc_handle_pte_file(vma, addr, ptent, &ent);
+	else if (is_swap_pte(ptent) && mc.move_anon)
+		page = __mc_handle_pte_swap(vma, addr, ptent, &ent);
+	else
 		return 0;
-	}
+
+	if (!page && !ent.val)
+		return 0;
+
 	if (page) {
 		pc = lookup_page_cgroup(page);
 		/*
 		 * Do only loose check w/o page_cgroup lock.
-		 * mem_cgroup_move_account() checks the pc is valid or not under
-		 * the lock.
+		 * mem_cgroup_move_account() checks the pc is valid or
+		 * not under the lock.
 		 */
 		if (PageCgroupUsed(pc) && pc->mem_cgroup == mc.from) {
 			ret = MC_TARGET_PAGE;
@@ -4264,12 +4331,13 @@ static int is_target_pte_for_mc(struct v
 		if (!ret || !target)
 			put_page(page);
 	}
-	/* throught */
-	if (ent.val && do_swap_account && !ret &&
-			css_id(&mc.from->css) == lookup_swap_cgroup(ent)) {
-		ret = MC_TARGET_SWAP;
-		if (target)
-			target->ent = ent;
+	/* Threre is a swap entry and a page doesn't exist or isn't charged */
+	if (!ret && ent.val) {
+		if (css_id(&mc.from->css) == lookup_swap_cgroup(ent)) {
+			ret = MC_TARGET_SWAP;
+			if (target)
+				target->ent = ent;
+		}
 	}
 	return ret;
 }
@@ -4370,6 +4438,9 @@ static void mem_cgroup_clear_mc(void)
 	mc.from = NULL;
 	mc.to = NULL;
 	mc.moving_task = NULL;
+	mc.move_anon = 0;
+	mc.move_file = 0;
+	mc.move_shmem = 0;
 	wake_up_all(&mc.waitq);
 }
 
@@ -4380,37 +4451,44 @@ static int mem_cgroup_can_attach(struct 
 {
 	int ret = 0;
 	struct mem_cgroup *mem = mem_cgroup_from_cont(cgroup);
+	struct mm_struct *mm;
+	struct mem_cgroup *from = mem_cgroup_from_task(p);
 
-	if (mem->move_charge_at_immigrate) {
-		struct mm_struct *mm;
-		struct mem_cgroup *from = mem_cgroup_from_task(p);
+	if (!mem->move_charge_at_immigrate)
+		return 0;
 
-		VM_BUG_ON(from == mem);
+	VM_BUG_ON(from == mem);
 
-		mm = get_task_mm(p);
-		if (!mm)
-			return 0;
-		/* We move charges only when we move a owner of the mm */
-		if (mm->owner == p) {
-			VM_BUG_ON(mc.from);
-			VM_BUG_ON(mc.to);
-			VM_BUG_ON(mc.precharge);
-			VM_BUG_ON(mc.moved_charge);
-			VM_BUG_ON(mc.moved_swap);
-			VM_BUG_ON(mc.moving_task);
-			mc.from = from;
-			mc.to = mem;
-			mc.precharge = 0;
-			mc.moved_charge = 0;
-			mc.moved_swap = 0;
-			mc.moving_task = current;
+	mm = get_task_mm(p);
+	if (!mm)
+		return 0;
+	if (mm->owner != p)
+		goto out;
+	/* We move charges only when we move a owner of the mm */
+	VM_BUG_ON(mc.from);
+	VM_BUG_ON(mc.to);
+	VM_BUG_ON(mc.precharge);
+	VM_BUG_ON(mc.moved_charge);
+	VM_BUG_ON(mc.moved_swap);
+	VM_BUG_ON(mc.moving_task);
+	mc.from = from;
+	mc.to = mem;
+	mc.precharge = 0;
+	mc.moved_charge = 0;
+	mc.moved_swap = 0;
+	mc.moving_task = current;
+	if (test_bit(MOVE_CHARGE_TYPE_ANON, &mem->move_charge_at_immigrate))
+		mc.move_anon = 1;
+	if (test_bit(MOVE_CHARGE_TYPE_FILE, &mem->move_charge_at_immigrate))
+		mc.move_file = 1;
+	if (test_bit(MOVE_CHARGE_TYPE_SHMEM, &mem->move_charge_at_immigrate))
+		mc.move_shmem = 1;
 
-			ret = mem_cgroup_precharge_mc(mm);
-			if (ret)
-				mem_cgroup_clear_mc();
-		}
-		mmput(mm);
-	}
+	ret = mem_cgroup_precharge_mc(mm);
+	if (ret)
+		mem_cgroup_clear_mc();
+out:
+	mmput(mm);
 	return ret;
 }
 
@@ -4500,8 +4578,6 @@ static void mem_cgroup_move_charge(struc
 	down_read(&mm->mmap_sem);
 	for (vma = mm->mmap; vma; vma = vma->vm_next) {
 		int ret;
-		bool move_shmem = test_bit(MOVE_CHARGE_TYPE_SHMEM,
-					&mc.to->move_charge_at_immigrate);
 		struct mm_walk mem_cgroup_move_charge_walk = {
 			.pmd_entry = mem_cgroup_move_charge_pte_range,
 			.mm = mm,
@@ -4509,8 +4585,7 @@ static void mem_cgroup_move_charge(struc
 		};
 		if (is_vm_hugetlb_page(vma))
 			continue;
-		if ((vma->vm_flags & VM_SHARED) && !move_shmem)
-			continue;
+
 		ret = walk_page_range(vma->vm_start, vma->vm_end,
 						&mem_cgroup_move_charge_walk);
 		if (ret)



--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH(v2) -mmotm 1/2] memcg move charge of file cache at task migration

[Balbir Singh]

* nishimura@mxp.nes.nec.co.jp <nishimura@mxp.nes.nec.co.jp> [2010-03-30 10:32:36]:

> On Mon, 29 Mar 2010 13:15:41 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > On Mon, 29 Mar 2010 12:03:21 +0900
> > Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:
> > 
> > > This patch adds support for moving charge of file cache. It's enabled by setting
> > > bit 1 of <target cgroup>/memory.move_charge_at_immigrate.
> > > 
> > > Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> > > ---
> > >  Documentation/cgroups/memory.txt |    6 ++++--
> > >  mm/memcontrol.c                  |   14 +++++++++++---
> > >  2 files changed, 15 insertions(+), 5 deletions(-)
> > > 
> > > diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt
> > > index 1b5bd04..f53d220 100644
> > > --- a/Documentation/cgroups/memory.txt
> > > +++ b/Documentation/cgroups/memory.txt
> > > @@ -461,10 +461,12 @@ charges should be moved.
> > >     0  | A charge of an anonymous page(or swap of it) used by the target task.
> > >        | Those pages and swaps must be used only by the target task. You must
> > >        | enable Swap Extension(see 2.4) to enable move of swap charges.
> > > + -----+------------------------------------------------------------------------
> > > +   1  | A charge of file cache mmap'ed by the target task. Those pages must be
> > > +      | mmap'ed only by the target task.
> > 
> > Hmm..my English is not good but..
> > ==
> > A charge of a page cache mapped by the target task. Pages mapped by multiple processes
> > will not be moved. This "page cache" doesn't include tmpfs.
> > ==
> > 
> This is more accurate than mine.
> 
> > Hmm, "a page mapped only by target task but belongs to other cgroup" will be moved ?
> > The answer is "NO.".
> > 
> > The code itself seems to work well. So, could you update Documentation ?
> > 
> Thank you for your advice.
> 
> This is the updated version.
> 
> ===
> From: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> 
> This patch adds support for moving charge of file cache. It's enabled by setting
> bit 1 of <target cgroup>/memory.move_charge_at_immigrate.
> 
> Signed-off-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> ---
> v1->v2
>   - update a documentation.
> 
>  Documentation/cgroups/memory.txt |    7 +++++--
>  mm/memcontrol.c                  |   14 +++++++++++---
>  2 files changed, 16 insertions(+), 5 deletions(-)
> 
> diff --git a/Documentation/cgroups/memory.txt b/Documentation/cgroups/memory.txt
> index 1b5bd04..c624cd2 100644
> --- a/Documentation/cgroups/memory.txt
> +++ b/Documentation/cgroups/memory.txt
> @@ -461,10 +461,13 @@ charges should be moved.
>     0  | A charge of an anonymous page(or swap of it) used by the target task.
>        | Those pages and swaps must be used only by the target task. You must
>        | enable Swap Extension(see 2.4) to enable move of swap charges.
> + -----+------------------------------------------------------------------------
> +   1  | A charge of page cache mapped by the target task. Pages mapped by
> +      | multiple processes will not be moved. This "page cache" doesn't include
> +      | tmpfs.
> 
>  Note: Those pages and swaps must be charged to the old cgroup.
> -Note: More type of pages(e.g. file cache, shmem,) will be supported by other
> -      bits in future.
> +Note: More type of pages(e.g. shmem) will be supported by other bits in future.
> 
>  8.3 TODO
> 
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index f6c9d42..66d2704 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -250,6 +250,7 @@ struct mem_cgroup {
>   */
>  enum move_type {
>  	MOVE_CHARGE_TYPE_ANON,	/* private anonymous page and swap of it */
> +	MOVE_CHARGE_TYPE_FILE,	/* private file caches */
>  	NR_MOVE_TYPE,
>  };
> 
> @@ -4192,6 +4193,8 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
>  	int usage_count = 0;
>  	bool move_anon = test_bit(MOVE_CHARGE_TYPE_ANON,
>  					&mc.to->move_charge_at_immigrate);
> +	bool move_file = test_bit(MOVE_CHARGE_TYPE_FILE,
> +					&mc.to->move_charge_at_immigrate);
> 
>  	if (!pte_present(ptent)) {
>  		/* TODO: handle swap of shmes/tmpfs */
> @@ -4208,10 +4211,15 @@ static int is_target_pte_for_mc(struct vm_area_struct *vma,
>  		if (!page || !page_mapped(page))
>  			return 0;
>  		/*
> -		 * TODO: We don't move charges of file(including shmem/tmpfs)
> -		 * pages for now.
> +		 * TODO: We don't move charges of shmem/tmpfs pages for now.
>  		 */
> -		if (!move_anon || !PageAnon(page))
> +		if (PageAnon(page)) {
> +			if (!move_anon)
> +				return 0;

if (PageAnon(page) && !move_anon)
        return 0
is easier to read


> +		} else if (page_is_file_cache(page)) {
> +			if (!move_file)
> +				return 0;

if (page_is_file_cache(page) && !move_file)
        return 0

> +		} else
>  			return 0;
>  		if (!get_page_unless_zero(page))
>  			return 0;
> -- 
> 1.6.4
> 

-- 
	Three Cheers,
	Balbir

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH(v2) -mmotm 2/2] memcg move charge of shmem at task migration

[KAMEZAWA Hiroyuki]

On Tue, 30 Mar 2010 14:44:58 +0900
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:

> On Tue, 30 Mar 2010 14:30:38 +0900
> Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:
> 
> > On Tue, 30 Mar 2010 10:30:50 +0530, Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
> > > * KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2010-03-30 13:51:59]:
> > > Yep, I tend to agree, but I need to take a closer look again at the
> > > patches. 
> > > 
> > I agree it would be more simple. I selected the current policy because
> > I was not sure whether we should move file caches(!tmpfs) with mapcount > 1,
> > and, IMHO, shared memory and file caches are different for users.
> > But it's O.K. for me to change current policy.
> > 
> 
> To explain what I think of, I wrote a patch onto yours. (Maybe overkill for explaination ;)
> 
> Summary.
> 
>  + adding move_anon, move_file, move_shmem information to move_charge_struct.
>  + adding hanlders for each pte types.
>  + checking # of referer should be divided to each type.
>    It's complicated to catch all cases in one "if" sentense.
>  + FILE pages will be moved if it's charged against "from". no mapcount check.
>    i.e. FILE pages should be moved even if it's not page-faulted.
>  + ANON pages will be moved if it's really private.
> 
> For widely shared FILE, "if it's charged against "from"" is enough good limitation.
> 
> 

Hmm....how about changing meanings of new flags ?

1 : a charge of page caches are moved. Page cache means cache of regular files
    and shared memory. But only privately mapped pages (mapcount==1) are moved.

2 : a charge of page caches are moved. Page cache means cache of regular files
    and shared memory. They are moved even if it's shared among processes.

When both of 1 and 2 are specified, "2" is used. Anonymous pages will not be
moved if it's shared.

Then, total view of user interface will be simple and I think this will allow
what you want.

Thanks,
-Kame







--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH(v2) -mmotm 2/2] memcg move charge of shmem at task migration

[Daisuke Nishimura]

On Tue, 30 Mar 2010 15:29:58 +0900, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> On Tue, 30 Mar 2010 14:44:58 +0900
> KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> 
> > On Tue, 30 Mar 2010 14:30:38 +0900
> > Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:
> > 
> > > On Tue, 30 Mar 2010 10:30:50 +0530, Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
> > > > * KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> [2010-03-30 13:51:59]:
> > > > Yep, I tend to agree, but I need to take a closer look again at the
> > > > patches. 
> > > > 
> > > I agree it would be more simple. I selected the current policy because
> > > I was not sure whether we should move file caches(!tmpfs) with mapcount > 1,
> > > and, IMHO, shared memory and file caches are different for users.
> > > But it's O.K. for me to change current policy.
> > > 
> > 
> > To explain what I think of, I wrote a patch onto yours. (Maybe overkill for explaination ;)
> > 
> > Summary.
> > 
> >  + adding move_anon, move_file, move_shmem information to move_charge_struct.
> >  + adding hanlders for each pte types.
> >  + checking # of referer should be divided to each type.
> >    It's complicated to catch all cases in one "if" sentense.
> >  + FILE pages will be moved if it's charged against "from". no mapcount check.
> >    i.e. FILE pages should be moved even if it's not page-faulted.
> >  + ANON pages will be moved if it's really private.
> > 
> > For widely shared FILE, "if it's charged against "from"" is enough good limitation.
> > 
> > 
> 
> Hmm....how about changing meanings of new flags ?
> 
> 1 : a charge of page caches are moved. Page cache means cache of regular files
>     and shared memory. But only privately mapped pages (mapcount==1) are moved.
> 
> 2 : a charge of page caches are moved. Page cache means cache of regular files
>     and shared memory. They are moved even if it's shared among processes.
> 
> When both of 1 and 2 are specified, "2" is used. Anonymous pages will not be
> moved if it's shared.
> 
> Then, total view of user interface will be simple and I think this will allow
> what you want.
> 
Thank you for your suggestion. It would be simple.
I'll try in that way.

Thanks,
Daisuke Nishimura.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>