Messages in this thread | | | Date | Thu, 03 Apr 2008 12:13:00 +0530 | From | Balbir Singh <> | Subject | Re: [-mm] Add an owner to the mm_struct (v5) |
| |
KAMEZAWA Hiroyuki wrote: > just nitpicks ;) > > On Thu, 03 Apr 2008 11:29:01 +0530 > Balbir Singh <balbir@linux.vnet.ibm.com> wrote: > >> #define mm_match_cgroup(mm, cgroup) \ >> - ((cgroup) == rcu_dereference((mm)->mem_cgroup)) >> + ((cgroup) == mem_cgroup_from_task((mm)->owner)) >> > > After this patch, above should be > == > #define mm_match_cgroup_css(mm, css, subsys_id) > ((css) == task_subsys_state((mm)->owner, subsys_id) > == > This generic macro may be good for your purpose. >
When we call mm_match_cgroup_*, we don't want to dereference mem_cont->css to get the css, hence we abstract it away. This is called from mm/rmap.c
> >> #endif >> diff -puN init/Kconfig~memory-controller-add-mm-owner init/Kconfig >> --- linux-2.6.25-rc8/init/Kconfig~memory-controller-add-mm-owner 2008-04-03 10:08:23.000000000 +0530 >> +++ linux-2.6.25-rc8-balbir/init/Kconfig 2008-04-03 10:08:23.000000000 +0530 >> @@ -371,9 +371,21 @@ config RESOURCE_COUNTERS >> infrastructure that works with cgroups >> depends on CGROUPS >> >> +config MM_OWNER >> + bool "Enable ownership of mm structure" >> + help >> + This option enables mm_struct's to have an owner. The advantage >> + of this approach is that it allows for several independent memory >> + based cgorup controllers to co-exist independently without too >> + much space overhead > Above is an explanation for this patch. > More simple text is better... How about > == > This is necessary for some cgroup subsystem related to memory management. > ==
Yes, but several other developers have also asked for it. revoke*, swap namespaces, etc will use it. I wanted to have a common definition.
>> + >> + This feature adds fork/exit overhead. So enable this only if >> + you need resource controllers >> + > > >> config CGROUP_MEM_RES_CTLR >> bool "Memory Resource Controller for Control Groups" >> depends on CGROUPS && RESOURCE_COUNTERS >> + select MM_OWNER > > I don't like "select"....this should be > depends on CGROUPS && RESOURCE_COUNTERS && MM_OWNER >
I discussed this will Paul and I think select is better. The user might ignore to enable MM_OWNER and wonder why memory controller or other features are not getting enabled.
> Thanks, > -Kame
Thanks for the review
-- Warm Regards, Balbir Singh Linux Technology Center IBM, ISTL
| |