OpenVZ Forum - RDF feed
type: channel
items: [RFC][PATCH] another swap controller for cgroup
items: Re: [RFC][PATCH] another swap controller for cgroup

[RFC][PATCH] another swap controller for cgroup
type: item
description: hi, the following is another swap controller, which was designed and implemented independently from nishimura-san's one. some random differences from nishimura-san's one: - counts and limits the number of ptes with swap entries instead of on-disk swap slots. - no swapon-time memory allocation. - anonymous objects (shmem) are not accounted. - precise wrt moving tasks between cgroups. this patch contains some unrelated small fixes which i've posted separately: - exe_file fput botch fix - cgroup_rmdir EBUSY fix any comments? YAMAMOTO Takashi --- linux-2.6.25-rc3-mm1/init/Kconfig.BACKUP 2008-03-05 15:45:50.000000000 +0900 +++ linux-2.6.25-rc3-mm1/init/Kconfig 2008-03-12 11:52:48.000000000 +0900 @@ -379,6 +379,12 @@ config CGROUP_MEM_RES_CTLR Only enable when you're ok with these trade offs and really sure you need the memory resource controller. +config CGROUP_SWAP_RES_CTLR + bool "Swap Resource Controller for Control Groups" + depends on CGROUPS && RESOURCE_COUNTERS + help + XXX TBD + config SYSFS_DEPRECATED bool "Create deprecated sysfs files" depends on SYSFS --- linux-2.6.25-rc3-mm1/mm/swapfile.c.BACKUP 2008-03-05 15:45:52.000000000 +0900 +++ linux-2.6.25-rc3-mm1/mm/swapfile.c 2008-03-14 17:25:40.000000000 +0900 @@ -28,6 +28,7 @@ #include <linux/capability.h> #include <linux/syscalls.h> #include <linux/memcontrol.h> +#include <linux/swapcontrol.h> #include <asm/pgtable.h> #include <asm/tlbflush.h> @@ -526,6 +527,7 @@ static int unuse_pte(struct vm_area_stru } inc_mm_counter(vma->vm_mm, anon_rss); + swap_cgroup_uncharge(pmd_page(*pmd)); get_page(page); set_pte_at(vma->vm_mm, addr, pte, pte_mkold(mk_pte(page, vma->vm_page_prot))); --- linux-2.6.25-rc3-mm1/mm/Makefile.BACKUP 2008-03-05 15:45:51.000000000 +0900 +++ linux-2.6.25-rc3-mm1/mm/Makefile 2008-03-12 11:53:31.000000000 +0900 @@ -33,4 +33,5 @@ obj-$(CONFIG_MIGRATION) += migrate.o obj-$(CONFIG_SMP) += allocpercpu.o obj-$(CONFIG_QUICKLIST) += quicklist.o obj-$(CONFIG_CGROUP_MEM_RES_CTLR) += memcontrol.o +obj-$(CONF…
creator: yamamoto
date: 2008-03-17T02:04:07-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: YAMAMOTO Takashi wrote: > hi, > > the following is another swap controller, which was designed and > implemented independently from nishimura-san's one. > > some random differences from nishimura-san's one: > - counts and limits the number of ptes with swap entries instead of > on-disk swap slots. > - no swapon-time memory allocation. > - anonymous objects (shmem) are not accounted. > - precise wrt moving tasks between cgroups. > > this patch contains some unrelated small fixes which i've posted separately: > - exe_file fput botch fix > - cgroup_rmdir EBUSY fix > > any comments? > Hi, YAMAMOTO-San, Thanks for the patch. I'll review and test it. I'll get back soon Balbir -- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Balbir Singh
date: 2008-03-17T05:11:51-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: Hi, Yamamoto-san. I'm reviewing and testing your patch now. I think your implementation is better because: - the group to be charged is determined correctly at the point of swapout, without fixing the behavior of move_task of memcg. (I think the behavior of move_task of memcg should be fixed anyway.) - the group to be uncharged is remembered in page struct of pmd, so there is no need to add array of pointers to swap_info_struct. > - anonymous objects (shmem) are not accounted. IMHO, shmem should be accounted. I agree it's difficult in your implementation, but are you going to support it? Thanks, Daisuke Nishimura. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Daisuke Nishimura
date: 2008-03-17T08:15:16-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: > > - anonymous objects (shmem) are not accounted. > IMHO, shmem should be accounted. > I agree it's difficult in your implementation, > but are you going to support it? it should be trivial to track how much swap an anonymous object is using. i'm not sure how it should be associated with cgroups, tho. YAMAMOTO Takashi _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: yamamoto
date: 2008-03-17T08:50:03-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: Daisuke Nishimura wrote: > Hi, Yamamoto-san. > > I'm reviewing and testing your patch now. > In building kernel infinitely(in a cgroup of memory.limit=64M and swap.limit=128M, with swappiness=100), almost all of the swap (1GB) is consumed as swap cache after a day or so. As a result, processes are occasionally OOM-killed even when the swap.usage of the group doesn't exceed the limit. I don't know why the swap cache uses up swap space. I will test whether a similar issue happens without your patch. Do you have any thoughts? BTW, I think that it would be better, in the sence of isolating memory resource, if there is a framework to limit the usage of swap cache. Thanks, Daisuke Nishimura. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Daisuke Nishimura
date: 2008-03-24T12:10:14-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: Daisuke Nishimura wrote: > Daisuke Nishimura wrote: >> Hi, Yamamoto-san. >> >> I'm reviewing and testing your patch now. >> > > In building kernel infinitely(in a cgroup of > memory.limit=64M and swap.limit=128M, with swappiness=100), > almost all of the swap (1GB) is consumed as swap cache > after a day or so. > As a result, processes are occasionally OOM-killed even when > the swap.usage of the group doesn't exceed the limit. > > I don't know why the swap cache uses up swap space. > I will test whether a similar issue happens without your patch. > Do you have any thoughts? > > > BTW, I think that it would be better, in the sence of > isolating memory resource, if there is a framework > to limit the usage of swap cache. We had this earlier, but dropped it later due to issues related to swap readahead and assigning the pages to the correct cgroup. -- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Balbir Singh
date: 2008-03-24T12:22:08-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: hi, > Daisuke Nishimura wrote: > > Hi, Yamamoto-san. > > > > I'm reviewing and testing your patch now. > > > > In building kernel infinitely(in a cgroup of > memory.limit=64M and swap.limit=128M, with swappiness=100), > almost all of the swap (1GB) is consumed as swap cache > after a day or so. > As a result, processes are occasionally OOM-killed even when > the swap.usage of the group doesn't exceed the limit. > > I don't know why the swap cache uses up swap space. > I will test whether a similar issue happens without your patch. > Do you have any thoughts? my patch tends to yield more swap cache because it makes try_to_unmap fail and shrink_page_list leaves swap cache in that case. i'm not sure how it causes 1GB swap cache, tho. YAMAMOTO Takashi > > BTW, I think that it would be better, in the sence of > isolating memory resource, if there is a framework > to limit the usage of swap cache. > > > Thanks, > Daisuke Nishimura. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: yamamoto
date: 2008-03-25T03:10:39-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: YAMAMOTO Takashi wrote: > hi, > >> Daisuke Nishimura wrote: >>> Hi, Yamamoto-san. >>> >>> I'm reviewing and testing your patch now. >>> >> In building kernel infinitely(in a cgroup of >> memory.limit=64M and swap.limit=128M, with swappiness=100), >> almost all of the swap (1GB) is consumed as swap cache >> after a day or so. >> As a result, processes are occasionally OOM-killed even when >> the swap.usage of the group doesn't exceed the limit. >> >> I don't know why the swap cache uses up swap space. >> I will test whether a similar issue happens without your patch. >> Do you have any thoughts? > > my patch tends to yield more swap cache because it makes try_to_unmap > fail and shrink_page_list leaves swap cache in that case. > i'm not sure how it causes 1GB swap cache, tho. > Agree. I suspected that the cause of this problem was the behavior of shrink_page_list as you said, so I thought one of Rik's split-lru patchset: http://lkml.org/lkml/2008/3/4/492 [patch 04/20] free swap space on swap-in/activation would reduce the usage of swap cache to half of the total swap. But it didn't help, so I think there may be some other causes. Thanks, Daisuke Nishimura. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Daisuke Nishimura
date: 2008-03-25T04:35:53-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: Balbir Singh wrote: > Daisuke Nishimura wrote: >> Daisuke Nishimura wrote: >>> Hi, Yamamoto-san. >>> >>> I'm reviewing and testing your patch now. >>> >> In building kernel infinitely(in a cgroup of >> memory.limit=64M and swap.limit=128M, with swappiness=100), >> almost all of the swap (1GB) is consumed as swap cache >> after a day or so. >> As a result, processes are occasionally OOM-killed even when >> the swap.usage of the group doesn't exceed the limit. >> >> I don't know why the swap cache uses up swap space. >> I will test whether a similar issue happens without your patch. >> Do you have any thoughts? >> >> >> BTW, I think that it would be better, in the sence of >> isolating memory resource, if there is a framework >> to limit the usage of swap cache. > > We had this earlier, but dropped it later due to issues related to swap > readahead and assigning the pages to the correct cgroup. > Yes, I know. In my swap subsystem posted before, I charge swap entries and remember to which cgroup each swap entries is charged in an array of pointers. So swap caches is charged as swap not memory, and swap usage including swap cache can be accounted. There may be better solution, and one of the issue of my implementation is swap_cgroup_chage() returns error before reclaiming swap entries which are only used by swap caches. I'm considering this issue now. Thanks, Daisuke Nishimura. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Daisuke Nishimura
date: 2008-03-25T06:46:28-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: > YAMAMOTO Takashi wrote: > > hi, > > > >> Daisuke Nishimura wrote: > >>> Hi, Yamamoto-san. > >>> > >>> I'm reviewing and testing your patch now. > >>> > >> In building kernel infinitely(in a cgroup of > >> memory.limit=64M and swap.limit=128M, with swappiness=100), > >> almost all of the swap (1GB) is consumed as swap cache > >> after a day or so. > >> As a result, processes are occasionally OOM-killed even when > >> the swap.usage of the group doesn't exceed the limit. > >> > >> I don't know why the swap cache uses up swap space. > >> I will test whether a similar issue happens without your patch. > >> Do you have any thoughts? > > > > my patch tends to yield more swap cache because it makes try_to_unmap > > fail and shrink_page_list leaves swap cache in that case. > > i'm not sure how it causes 1GB swap cache, tho. > > > > Agree. > > I suspected that the cause of this problem was the behavior > of shrink_page_list as you said, so I thought one of Rik's > split-lru patchset: > > http://lkml.org/lkml/2008/3/4/492 > [patch 04/20] free swap space on swap-in/activation > > would reduce the usage of swap cache to half of the total swap. > But it didn't help, so I think there may be some other causes. do you mean you tested with the patch in the url? i don't think remove_exclusive_swap_page works for us because our page has more references than it expects. ie. ptes, cache, isolate_page (unless your tree has another change for remove_exclusive_swap_page. i haven't checked other patches in the patchset.) YAMAMOTO Takashi > > > Thanks, > Daisuke Nishimura. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: yamamoto
date: 2008-03-25T08:57:23-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: YAMAMOTO Takashi wrote: >> YAMAMOTO Takashi wrote: >>> hi, >>> >>>> Daisuke Nishimura wrote: >>>>> Hi, Yamamoto-san. >>>>> >>>>> I'm reviewing and testing your patch now. >>>>> >>>> In building kernel infinitely(in a cgroup of >>>> memory.limit=64M and swap.limit=128M, with swappiness=100), >>>> almost all of the swap (1GB) is consumed as swap cache >>>> after a day or so. >>>> As a result, processes are occasionally OOM-killed even when >>>> the swap.usage of the group doesn't exceed the limit. >>>> >>>> I don't know why the swap cache uses up swap space. >>>> I will test whether a similar issue happens without your patch. >>>> Do you have any thoughts? >>> my patch tends to yield more swap cache because it makes try_to_unmap >>> fail and shrink_page_list leaves swap cache in that case. >>> i'm not sure how it causes 1GB swap cache, tho. >>> >> Agree. >> >> I suspected that the cause of this problem was the behavior >> of shrink_page_list as you said, so I thought one of Rik's >> split-lru patchset: >> >> http://lkml.org/lkml/2008/3/4/492 >> [patch 04/20] free swap space on swap-in/activation >> >> would reduce the usage of swap cache to half of the total swap. >> But it didn't help, so I think there may be some other causes. > > do you mean you tested with the patch in the url? > i don't think remove_exclusive_swap_page works for us > because our page has more references than it expects. > ie. ptes, cache, isolate_page > umm... you are right. I missed the fact that isolate_page increases page count. Thanks, Daisuke Nishimura. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Daisuke Nishimura
date: 2008-03-25T12:35:44-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: hi, i tried to reproduce the large swap cache issue, but no luck. can you provide a little more detailed instruction? the following is a new version of the patch. changes from the previous: - fix a bug; update accounting for mremap properly. - some comments and assertions. YAMAMOTO Takashi Signed-off-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp> --- --- linux-2.6.25-rc3-mm1/init/Kconfig.BACKUP 2008-03-05 15:45:50.000000000 +0900 +++ linux-2.6.25-rc3-mm1/init/Kconfig 2008-03-12 11:52:48.000000000 +0900 @@ -379,6 +379,12 @@ config CGROUP_MEM_RES_CTLR Only enable when you're ok with these trade offs and really sure you need the memory resource controller. +config CGROUP_SWAP_RES_CTLR + bool "Swap Resource Controller for Control Groups" + depends on CGROUPS && RESOURCE_COUNTERS + help + XXX TBD + config SYSFS_DEPRECATED bool "Create deprecated sysfs files" depends on SYSFS --- linux-2.6.25-rc3-mm1/mm/swapfile.c.BACKUP 2008-03-05 15:45:52.000000000 +0900 +++ linux-2.6.25-rc3-mm1/mm/swapfile.c 2008-03-14 17:25:40.000000000 +0900 @@ -28,6 +28,7 @@ #include <linux/capability.h> #include <linux/syscalls.h> #include <linux/memcontrol.h> +#include <linux/swapcontrol.h> #include <asm/pgtable.h> #include <asm/tlbflush.h> @@ -526,6 +527,7 @@ static int unuse_pte(struct vm_area_stru } inc_mm_counter(vma->vm_mm, anon_rss); + swap_cgroup_uncharge(pmd_page(*pmd)); get_page(page); set_pte_at(vma->vm_mm, addr, pte, pte_mkold(mk_pte(page, vma->vm_page_prot))); --- linux-2.6.25-rc3-mm1/mm/Makefile.BACKUP 2008-03-05 15:45:51.000000000 +0900 +++ linux-2.6.25-rc3-mm1/mm/Makefile 2008-03-12 11:53:31.000000000 +0900 @@ -33,4 +33,5 @@ obj-$(CONFIG_MIGRATION) += migrate.o obj-$(CONFIG_SMP) += allocpercpu.o obj-$(CONFIG_QUICKLIST) += quicklist.o obj-$(CONFIG_CGROUP_MEM_RES_CTLR) += memcontrol.o +obj-$(CONFIG_CGROUP_SWAP_RES_CTLR) += swapcontrol.o --- linux-2.6.25-rc3-mm1/mm/rmap.c.BACKUP 2008-03-05 15:45:52.000000000 +0900 +++ linux-2.6.25-rc3-mm1/mm/rmap.c 2008-03-17 07:45:16.000000000 +0900 @@…
creator: yamamoto
date: 2008-03-27T06:28:34-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: YAMAMOTO Takashi wrote: > hi, > > i tried to reproduce the large swap cache issue, but no luck. > can you provide a little more detailed instruction? > This issue also happens on generic 2.6.25-rc3-mm1 (with limitting only memory), so I think this issue is not related to your patch. I'm investigating this issue too. Below is my environment and how to reproduce. - System full virtualized xen guest based on RHEL5.1(x86_64). CPU: 2 memory: 2GB swap: 1GB A config of the running kernel(2.6.25-rc3-mm1 with your patch) is attached. - how to reproduce - change swappines to 100 echo 100 >/proc/sys/vm/swappiness - mount cgroup fs # mount -t cgroup -o memory,swap none /cgroup - make cgroup for test # mkdir /cgroup/02 # echo -n 64M >/cgroup/02/memory.limit_in_bytes # echo -n `expr 128 \* 1024 \* 1024` >/cgroup/02/swap.limit_in_bytes - run test # echo $$ >/cgropu/02/tasks # while true; do make clean; make -j2; done In other terminals, I run some monitoring processes, top, "tail -f /var/log/messages", and displaying *.usage_in_bytes every seconds. Thanks, Daisuke Nishimura. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Daisuke Nishimura
date: 2008-03-28T09:00:33-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: > YAMAMOTO Takashi wrote: > > hi, > > > > i tried to reproduce the large swap cache issue, but no luck. > > can you provide a little more detailed instruction? > > > This issue also happens on generic 2.6.25-rc3-mm1 > (with limitting only memory), so I think this issue is not > related to your patch. > I'm investigating this issue too. > > Below is my environment and how to reproduce. > > - System > full virtualized xen guest based on RHEL5.1(x86_64). > CPU: 2 > memory: 2GB > swap: 1GB > A config of the running kernel(2.6.25-rc3-mm1 with your patch) > is attached. > > - how to reproduce > - change swappines to 100 > > echo 100 >/proc/sys/vm/swappiness > > - mount cgroup fs > > # mount -t cgroup -o memory,swap none /cgroup > > - make cgroup for test > > # mkdir /cgroup/02 > # echo -n 64M >/cgroup/02/memory.limit_in_bytes > # echo -n `expr 128 \* 1024 \* 1024` >/cgroup/02/swap.limit_in_bytes > > - run test > > # echo $$ >/cgropu/02/tasks > # while true; do make clean; make -j2; done > > In other terminals, I run some monitoring processes, top, > "tail -f /var/log/messages", and displaying *.usage_in_bytes > every seconds. > > > Thanks, > Daisuke Nishimura. what i tried was essentially same. for me, once vm_swap_full() got true, swap cache stopped growing as expected. http://people.valinux.co.jp/~yamamoto/swap.png it was taken by running while :;do swapon -s|tail -1;sleep 1;done > foo in an unlimited cgroup, and then plotted by gnuplot. plot "foo" u 4 as my system has 1GB swap configured, the vm_swap_full() threshold is around 500MB. YAMAMOTO Takashi _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: yamamoto
date: 2008-04-08T03:29:37-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: the following is a new version of the patch. changes from the previous: - fix a BUG_ON in swap_cgroup_attach and add a comment about it. YAMAMOTO Takashi Signed-off-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp> --- --- linux-2.6.25-rc3-mm1/init/Kconfig.BACKUP 2008-03-05 15:45:50.000000000 +0900 +++ linux-2.6.25-rc3-mm1/init/Kconfig 2008-03-12 11:52:48.000000000 +0900 @@ -379,6 +379,12 @@ config CGROUP_MEM_RES_CTLR Only enable when you're ok with these trade offs and really sure you need the memory resource controller. +config CGROUP_SWAP_RES_CTLR + bool "Swap Resource Controller for Control Groups" + depends on CGROUPS && RESOURCE_COUNTERS + help + XXX TBD + config SYSFS_DEPRECATED bool "Create deprecated sysfs files" depends on SYSFS --- linux-2.6.25-rc3-mm1/mm/swapfile.c.BACKUP 2008-03-05 15:45:52.000000000 +0900 +++ linux-2.6.25-rc3-mm1/mm/swapfile.c 2008-03-14 17:25:40.000000000 +0900 @@ -28,6 +28,7 @@ #include <linux/capability.h> #include <linux/syscalls.h> #include <linux/memcontrol.h> +#include <linux/swapcontrol.h> #include <asm/pgtable.h> #include <asm/tlbflush.h> @@ -526,6 +527,7 @@ static int unuse_pte(struct vm_area_stru } inc_mm_counter(vma->vm_mm, anon_rss); + swap_cgroup_uncharge(pmd_page(*pmd)); get_page(page); set_pte_at(vma->vm_mm, addr, pte, pte_mkold(mk_pte(page, vma->vm_page_prot))); --- linux-2.6.25-rc3-mm1/mm/Makefile.BACKUP 2008-03-05 15:45:51.000000000 +0900 +++ linux-2.6.25-rc3-mm1/mm/Makefile 2008-03-12 11:53:31.000000000 +0900 @@ -33,4 +33,5 @@ obj-$(CONFIG_MIGRATION) += migrate.o obj-$(CONFIG_SMP) += allocpercpu.o obj-$(CONFIG_QUICKLIST) += quicklist.o obj-$(CONFIG_CGROUP_MEM_RES_CTLR) += memcontrol.o +obj-$(CONFIG_CGROUP_SWAP_RES_CTLR) += swapcontrol.o --- linux-2.6.25-rc3-mm1/mm/rmap.c.BACKUP 2008-03-05 15:45:52.000000000 +0900 +++ linux-2.6.25-rc3-mm1/mm/rmap.c 2008-03-17 07:45:16.000000000 +0900 @@ -49,6 +49,7 @@ #include <linux/module.h> #include <linux/kallsyms.h> #include <linux/memcontrol.h> +#include <linux/swapcontrol.h> #incl…
creator: yamamoto
date: 2008-04-10T07:40:04-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: > > > - anonymous objects (shmem) are not accounted. > > IMHO, shmem should be accounted. > > I agree it's difficult in your implementation, > > but are you going to support it? > > it should be trivial to track how much swap an anonymous object is using. > i'm not sure how it should be associated with cgroups, tho. > > YAMAMOTO Takashi i implemented shmem swap accounting. see below. YAMAMOTO Takashi the following is another swap controller, which was designed and implemented independently from nishimura-san's one. some random differences from nishimura-san's one: - counts and limits the number of ptes with swap entries instead of on-disk swap slots. (except swap slots used by anonymous objects, which are counted differently. see below.) - no swapon-time memory allocation. - precise wrt moving tasks between cgroups. - [NEW] anonymous objects (shmem) are associated to a cgroup when they are created and the number of on-disk swap slots used by them are counted for the cgroup. the association is persistent except cgroup removal, in which case, its associated objects are moved to init_mm's cgroup. Signed-off-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp> --- --- linux-2.6.25-rc8-mm2/mm/swapcontrol.c.BACKUP 2008-04-21 12:06:44.000000000 +0900 +++ linux-2.6.25-rc8-mm2/mm/swapcontrol.c 2008-04-28 14:16:03.000000000 +0900 @@ -0,0 +1,447 @@ + +/* + * swapcontrol.c COPYRIGHT FUJITSU LIMITED 2008 + * + * Author: yamamoto@valinux.co.jp + */ + +#include <linux/err.h> +#include <linux/cgroup.h> +#include <linux/hugetlb.h> +#include <linux/res_counter.h> +#include <linux/pagemap.h> +#include <linux/slab.h> +#include <linux/swap.h> +#include <linux/swapcontrol.h> +#include <linux/swapops.h> +#include <linux/shmem_fs.h> + +struct swap_cgroup { + struct cgroup_subsys_state scg_css; + struct res_counter scg_counter; + struct list_head scg_shmem_list; +}; + +#define task_to_css(task) task_subsys_state((task), swap_cgroup_subsys_id) +#define css_to_scg(css) container_of((css), struct swa…
creator: yamamoto
date: 2008-04-29T22:50:47-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: YAMAMOTO Takashi wrote: >>>> - anonymous objects (shmem) are not accounted. >>> IMHO, shmem should be accounted. >>> I agree it's difficult in your implementation, >>> but are you going to support it? >> it should be trivial to track how much swap an anonymous object is using. >> i'm not sure how it should be associated with cgroups, tho. >> >> YAMAMOTO Takashi > > i implemented shmem swap accounting. see below. > > YAMAMOTO Takashi > Hi, YAMAMOTO san. Thank you for implementing this feature. I will review it. BTW, I'm just trying to make my swapcontroller patch that is rebased on recent kernel and implemented as part of memory controller. I'm going to submit it by the middle of May. Thanks, Daisuke Nishimura. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Daisuke Nishimura
date: 2008-04-30T04:09:44-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: * YAMAMOTO Takashi <yamamoto@valinux.co.jp> [2008-04-30 07:50:47]: > > > > - anonymous objects (shmem) are not accounted. > > > IMHO, shmem should be accounted. > > > I agree it's difficult in your implementation, > > > but are you going to support it? > > > > it should be trivial to track how much swap an anonymous object is using. > > i'm not sure how it should be associated with cgroups, tho. > > > > YAMAMOTO Takashi > > i implemented shmem swap accounting. see below. > > YAMAMOTO Takashi > > > the following is another swap controller, which was designed and > implemented independently from nishimura-san's one. > Hi, Thanks for the patches and your patience. I've just applied your patches on top of 2.6.25-mm1 (it had a minor reject, that I've fixed). I am building and testing the patches along with KAMEZAWA-San's low overhead patches. > some random differences from nishimura-san's one: > - counts and limits the number of ptes with swap entries instead of > on-disk swap slots. (except swap slots used by anonymous objects, > which are counted differently. see below.) > - no swapon-time memory allocation. > - precise wrt moving tasks between cgroups. > - [NEW] anonymous objects (shmem) are associated to a cgroup when they are > created and the number of on-disk swap slots used by them are counted for > the cgroup. the association is persistent except cgroup removal, > in which case, its associated objects are moved to init_mm's cgroup. > > > Signed-off-by: YAMAMOTO Takashi <yamamoto@valinux.co.jp> > --- > > --- linux-2.6.25-rc8-mm2/mm/swapcontrol.c.BACKUP 2008-04-21 12:06:44.000000000 +0900 > +++ linux-2.6.25-rc8-mm2/mm/swapcontrol.c 2008-04-28 14:16:03.000000000 +0900 > @@ -0,0 +1,447 @@ > + > +/* > + * swapcontrol.c COPYRIGHT FUJITSU LIMITED 2008 > + * > + * Author: yamamoto@valinux.co.jp > + */ > + > +#include <linux/err.h> > +#include <linux/cgroup.h> > +#include <linux/hugetlb.h> My powerpc build fails, we need to move hugetlb.h down to the bottom > +#include <…
creator: Balbir Singh
date: 2008-05-05T06:11:42-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: hi, > Hi, Thanks for the patches and your patience. I've just applied your > patches on top of 2.6.25-mm1 (it had a minor reject, that I've fixed). > I am building and testing the patches along with KAMEZAWA-San's low > overhead patches. thanks. > > +#include <linux/err.h> > > +#include <linux/cgroup.h> > > +#include <linux/hugetlb.h> > > My powerpc build fails, we need to move hugetlb.h down to the bottom what's the error message? > > +struct swap_cgroup { > > + struct cgroup_subsys_state scg_css; > > Can't we call this just css. Since the structure is swap_cgroup it > already has the required namespace required to distinguish it from > other css's. Please see page 4 of "The practice of programming", be > consistent. The same comment applies to all members below. i don't have the book. i like this kind of prefixes as it's grep-friendly. > > +#define task_to_css(task) task_subsys_state((task), swap_cgroup_subsys_id) > > +#define css_to_scg(css) container_of((css), struct swap_cgroup, scg_css) > > +#define cg_to_css(cg) cgroup_subsys_state((cg), swap_cgroup_subsys_id) > > +#define cg_to_scg(cg) css_to_scg(cg_to_css(cg)) > > Aren't static inline better than macros? I would suggest moving to > them. sounds like a matter of preference. either ok for me. > > +static struct swap_cgroup * > > +swap_cgroup_prepare_ptp(struct page *ptp, struct mm_struct *mm) > > +{ > > + struct swap_cgroup *scg = ptp->ptp_swap_cgroup; > > + > > Is this routine safe w.r.t. concurrent operations, modifications to > ptp_swap_cgroup? it's always accessed with the page table locked. > > + BUG_ON(mm == NULL); > > + BUG_ON(mm->swap_cgroup == NULL); > > + if (scg == NULL) { > > + /* > > + * see swap_cgroup_attach. > > + */ > > + smp_rmb(); > > + scg = mm->swap_cgroup; > > With the mm->owner patches, we need not maintain a separate > mm->swap_cgroup. i don't think the mm->owner patch, at least with the current form, can replace it. > > + /* > > + * swap_cgroup_attach is in progress. > > + */ > > + > …
creator: yamamoto
date: 2008-05-07T05:50:44-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: YAMAMOTO Takashi wrote: > hi, > >> Hi, Thanks for the patches and your patience. I've just applied your >> patches on top of 2.6.25-mm1 (it had a minor reject, that I've fixed). >> I am building and testing the patches along with KAMEZAWA-San's low >> overhead patches. > > thanks. > >>> +#include <linux/err.h> >>> +#include <linux/cgroup.h> >>> +#include <linux/hugetlb.h> >> My powerpc build fails, we need to move hugetlb.h down to the bottom > > what's the error message? > It's unable to find the hugetlb call, I think is_hugetlb_vma() or so. >>> +struct swap_cgroup { >>> + struct cgroup_subsys_state scg_css; >> Can't we call this just css. Since the structure is swap_cgroup it >> already has the required namespace required to distinguish it from >> other css's. Please see page 4 of "The practice of programming", be >> consistent. The same comment applies to all members below. > > i don't have the book. > i like this kind of prefixes as it's grep-friendly. > >>> +#define task_to_css(task) task_subsys_state((task), swap_cgroup_subsys_id) >>> +#define css_to_scg(css) container_of((css), struct swap_cgroup, scg_css) >>> +#define cg_to_css(cg) cgroup_subsys_state((cg), swap_cgroup_subsys_id) >>> +#define cg_to_scg(cg) css_to_scg(cg_to_css(cg)) >> Aren't static inline better than macros? I would suggest moving to >> them. > > sounds like a matter of preference. > either ok for me. > There are other advantages, like better type checking of the arguments. The compiler might even determine that it's better of making a function call instead of inlining it (rare, but possible). >>> +static struct swap_cgroup * >>> +swap_cgroup_prepare_ptp(struct page *ptp, struct mm_struct *mm) >>> +{ >>> + struct swap_cgroup *scg = ptp->ptp_swap_cgroup; >>> + >> Is this routine safe w.r.t. concurrent operations, modifications to >> ptp_swap_cgroup? > > it's always accessed with the page table locked. > >>> + BUG_ON(mm == NULL); >>> + BUG_ON(mm->swap_cgroup == NULL); >>> + if (scg == NULL) { …
creator: Balbir Singh
date: 2008-05-08T15:43:50-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: > >>> + BUG_ON(mm == NULL); > >>> + BUG_ON(mm->swap_cgroup == NULL); > >>> + if (scg == NULL) { > >>> + /* > >>> + * see swap_cgroup_attach. > >>> + */ > >>> + smp_rmb(); > >>> + scg = mm->swap_cgroup; > >> With the mm->owner patches, we need not maintain a separate > >> mm->swap_cgroup. > > > > i don't think the mm->owner patch, at least with the current form, > > can replace it. > > > > Could you please mention what the limitations are? We could get those fixed or > take another serious look at the mm->owner patches. for example, its callback can't sleep. > >>> + /* > >>> + * swap_cgroup_attach is in progress. > >>> + */ > >>> + > >>> + res_counter_charge_force(&newscg->scg_counter, PAGE_CACHE_SIZE); > >> So, we force the counter to go over limit? > > > > yes. > > > > as newscg != oldscg here means the task is being moved between cgroups, > > this instance of res_counter_charge_force should not matter much. > > > > Isn't it bad to force a group to go over it's limit due to migration? we don't have many choices as far as ->attach can't fail. although we can have racy checks in ->can_attach, i'm not happy with it. > >> We need to write actual numbers here? Can't we keep the write > >> interface consistent with the memory controller? > > > > i'm not sure what you mean here. can you explain a bit more? > > do you mean K, M, etc? > > > > Yes, I mean the same format that memparse() uses. i'll take a look. > >> Is moving to init_mm (root > >> cgroup) a good idea? Ideally with support for hierarchies, if we ever > >> do move things, it should be to the parent cgroup. > > > > i chose init_mm because there seemed to be no consensus about > > cgroup hierarchy semantics. > > > > I would suggest that we fail deletion of a group for now. I have a set of > patches for the cgroup hierarchy semantics. I think the parent is the best place > to move it. ok. > >>> + info->swap_cgroup = newscg; > >>> + res_counter_uncharge(&oldscg->scg_counter, bytes); > >>> + res_counter_charge_force…
creator: yamamoto
date: 2008-05-14T03:21:25-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: On Tue, May 13, 2008 at 8:21 PM, YAMAMOTO Takashi <yamamoto@valinux.co.jp> wrote: > > note that a failure can affect other subsystems which belong to > the same hierarchy as well, and, even worse, a back-out attempt can also fail. > i'm afraid that we need to play some kind of transaction-commit game, > which can make subsystems too complex to implement properly. > I was considering something like that - every call to can_attach() would be guaranteed to be followed by either a call to attach() or to a new method called cancel_attach(). Then the subsystem would just need to ensure that nothing could happen which would cause the attach to become invalid between the two calls. Or possibly, since for some subsystems that might involve holding a spinlock, we should extend it to: After a successful call to can_attach(), either abort_attach() or commit_attach() will be called; these calls are not allowed to sleep, and cgroup.c will not sleep between calls.. If commit_attach() is called, it will be followed shortly by attach(), which is allowed to sleep. Paul _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Paul Menage
date: 2008-05-14T03:27:59-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: On Tue, May 13, 2008 at 8:21 PM, YAMAMOTO Takashi <yamamoto@valinux.co.jp> wrote: > > > > Could you please mention what the limitations are? We could get those fixed or > > take another serious look at the mm->owner patches. > > for example, its callback can't sleep. > You need to be able to sleep in order to take mmap_sem, right? Since I think that the other current user of the mm->owner callback probably also needs mmap_sem, it might make sense to take mmap_sem in mm_update_next_owner() prior to locking the old owner, and hold it across the callback, which would presumably solve the problem. > > > > Isn't it bad to force a group to go over it's limit due to migration? > > we don't have many choices as far as ->attach can't fail. > although we can have racy checks in ->can_attach, i'm not happy with it. can_attach() isn't racy iff you ensure that a successful result from can_attach() can't be invalidated by any code not holding cgroup_mutex. The existing user of can_attach() is cpusets, and the only way to make an attachable cpuset non-attachable is to remove its last node or cpu. The only code that can do this (update_nodemask, update_cpumask, and common_cpu_mem_hotplug_unplug) all call cgroup_lock() to ensure that this synchronization occurs. Of course, having lots of datapath operations also take cgroup_mutex would be really painful, so it's not practical to use for things that can become non-attachable due to a process consuming some resources. This is part of the reason that I started working on the lock-mode patches that I sent out yesterday, in order to make finer-grained locking simpler. I'm going to rework those to make the locking more explicit, and I'll bear this use case in mind while I'm doing it. A few comments on the patch: - you're not really limiting swap usage, you're limiting swapped-out address space. So it looks as though if a process has swapped out most of its address space, and forks a child, the total "swap" charge for the cgroup will doubl…
creator: Paul Menage
date: 2008-05-14T08:44:38-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: > On Tue, May 13, 2008 at 8:21 PM, YAMAMOTO Takashi > <yamamoto@valinux.co.jp> wrote: > > > > > > Could you please mention what the limitations are? We could get those fixed or > > > take another serious look at the mm->owner patches. > > > > for example, its callback can't sleep. > > > > You need to be able to sleep in order to take mmap_sem, right? yes. besides that, i prefer not to hold a spinlock when traversing PTEs as it can take somewhat long. > Of course, having lots of datapath operations also take cgroup_mutex > would be really painful, so it's not practical to use for things that > can become non-attachable due to a process consuming some resources. > This is part of the reason that I started working on the lock-mode > patches that I sent out yesterday, in order to make finer-grained > locking simpler. I'm going to rework those to make the locking more > explicit, and I'll bear this use case in mind while I'm doing it. thanks. > A few comments on the patch: > > - you're not really limiting swap usage, you're limiting swapped-out > address space. So it looks as though if a process has swapped out most > of its address space, and forks a child, the total "swap" charge for > the cgroup will double. Is that correct? yes. > If so, why is this better > than charging for actual swap usage? its behaviour is more determinstic and it uses less memory. (than nishimura-san's one, which charges for actual swap usage.) > - what will happen if someone creates non-NPTL threads, which share an > mm but not a thread group (so each of them is a thread group leader)? a thread which is most recently assigned to a cgroup will "win". > - if you were to store a pointer in the page rather than the "a pointer"? a pointer to what? > swap_cgroup pointer, then (in combination with mm->owner) you wouldn't > need to do the rebinding to the new swap_cgroup when a process moves > to a different cgroup - you could instead keep a "swapped pte" count > in the mm, and just charge that to the…
creator: yamamoto
date: 2008-05-15T06:23:18-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: On Wed, May 14, 2008 at 11:23 PM, YAMAMOTO Takashi <yamamoto@valinux.co.jp> wrote: > > > > > > > You need to be able to sleep in order to take mmap_sem, right? > > yes. OK, well in the thread on balbir's vm limit controller, we discussed the idea of having mmap_sem held across calls to mm_update_next_owner(), and hence the cgroup callbacks. Would that help if mmap_sem were already held when you get the mm_owner_changed() callback. > > > A few comments on the patch: > > > > - you're not really limiting swap usage, you're limiting swapped-out > > address space. So it looks as though if a process has swapped out most > > of its address space, and forks a child, the total "swap" charge for > > the cgroup will double. Is that correct? > > yes. > > > > If so, why is this better > > than charging for actual swap usage? > > its behaviour is more determinstic and it uses less memory. > (than nishimura-san's one, which charges for actual swap usage.) > Using less memory is good, but maybe not worth it if the result isn't so useful. I'd say that it's less deterministic than nishimura-san's controller - with his you just need to know how much swap is in use (which you can tell by observing the app on a real system) but with yours you also have to know whether there are any processes sharing anon pages (but not mms). Now it's true that if all the apps you need to run do an execve() after forking, then the number of swap ptes really does track the amount of swap space in use pretty accurately, since there's not going to be any sharing of anon memory between mms. And it might be that people decide that the reduced memory overhead justifies this limitation. But I think it should be made explicit in the patch description and documentation that this controller achieves its reduced overhead at the cost of giving (IMO) bogus results on a rather ancient but still perfectly legitimate class of Unix application. (The apache httpd server used to work this way, for instance. It may still bu…
creator: Paul Menage
date: 2008-05-15T07:19:39-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: > > > If so, why is this better > > > than charging for actual swap usage? > > > > its behaviour is more determinstic and it uses less memory. > > (than nishimura-san's one, which charges for actual swap usage.) > > > > Using less memory is good, but maybe not worth it if the result isn't so useful. > > I'd say that it's less deterministic than nishimura-san's controller - > with his you just need to know how much swap is in use (which you can > tell by observing the app on a real system) but with yours you also > have to know whether there are any processes sharing anon pages (but > not mms). deterministic in the sense that, even when two or more processes from different cgroups are sharing a page, both of them, rather than only unlucky one, are always charged. another related advantage is that it's possible to move charges quite precisely when moving a task among cgroups. > Now it's true that if all the apps you need to run do an execve() > after forking, then the number of swap ptes really does track the > amount of swap space in use pretty accurately, since there's not going > to be any sharing of anon memory between mms. And it might be that > people decide that the reduced memory overhead justifies this > limitation. But I think it should be made explicit in the patch > description and documentation that this controller achieves its > reduced overhead at the cost of giving (IMO) bogus results on a rather > ancient but still perfectly legitimate class of Unix application. (The > apache httpd server used to work this way, for instance. It may still > but I've not looked at it in a while). fair enough. > > > - what will happen if someone creates non-NPTL threads, which share an > > > mm but not a thread group (so each of them is a thread group leader)? > > > > a thread which is most recently assigned to a cgroup will "win". > > > > Doesn't that risk triggering the BUG_ON(mm->swap_cgroup != oldscg) in > swap_cgroup_attach() ? which version of the patch you are loo…
creator: yamamoto
date: 2008-05-15T08:56:06-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: On 2008/05/15 17:56 +0900, YAMAMOTO Takashi wrote: >>> > If so, why is this better >>> > than charging for actual swap usage? >>> >>> its behaviour is more determinstic and it uses less memory. >>> (than nishimura-san's one, which charges for actual swap usage.) >>> Consuming more memory cannot be helped for my controller... >> Using less memory is good, but maybe not worth it if the result isn't so useful. >> >> I'd say that it's less deterministic than nishimura-san's controller - >> with his you just need to know how much swap is in use (which you can >> tell by observing the app on a real system) but with yours you also >> have to know whether there are any processes sharing anon pages (but >> not mms). > > deterministic in the sense that, even when two or more processes > from different cgroups are sharing a page, both of them, rather than > only unlucky one, are always charged. > I'm not sure whether this behavior itself is good or bad, but I think it's not good idea to make memory controller, which charges only one process for a shared page, and swap controller behave differently. I think it will be confusing for users. At least, I would feel it strange. > another related advantage is that it's possible to move charges > quite precisely when moving a task among cgroups. > Moving charges is one of future todo of my controller. But, as you say, it won't be so precise as yours. Thanks, Daisuke Nishimura. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Daisuke Nishimura
date: 2008-05-15T12:01:53-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: > > deterministic in the sense that, even when two or more processes > > from different cgroups are sharing a page, both of them, rather than > > only unlucky one, are always charged. > > > > I'm not sure whether this behavior itself is good or bad, > but I think it's not good idea to make memory controller, > which charges only one process for a shared page, > and swap controller behave differently. > I think it will be confusing for users. At least, > I would feel it strange. i agree that yours can be better integrated with the memory controller. unlike yours, mine was designed to be independent from the memory controller as far as possible. (i don't want to complicate the memory controller.) YAMAMOTO Takashi _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: yamamoto
date: 2008-05-19T04:14:57-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: > BTW, I'm just trying to make my swapcontroller patch > that is rebased on recent kernel and implemented > as part of memory controller. > I'm going to submit it by the middle of May. what's the status of this? YAMAMOTO Takashi _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: yamamoto
date: 2008-05-22T04:46:56-00:00

Re: [RFC][PATCH] another swap controller for cgroup
type: item
description: On 2008/05/22 13:46 +0900, YAMAMOTO Takashi wrote: >> BTW, I'm just trying to make my swapcontroller patch >> that is rebased on recent kernel and implemented >> as part of memory controller. >> I'm going to submit it by the middle of May. > > what's the status of this? > > YAMAMOTO Takashi > Sorry. I will post it very soon. Actually, I've been writing mails :-) Daisuke Nishimura. _______________________________________________ Containers mailing list Containers@lists.linux-foundation.org https://lists.linux-foundation.org/mailman/listinfo/containers
creator: Daisuke Nishimura
date: 2008-05-22T04:54:04-00:00