login
Header Space

 
 

Re: [RFC][-mm] Add an owner to the mm_struct (v3)

Previous thread: [PATCH] IA64: use goto to jump out do/while_each_thread by Li Zefan on Tuesday, April 1, 2008 - 12:29 am. (1 message)

Next thread: linux-next: Tree for April 1 by Stephen Rothwell on Tuesday, April 1, 2008 - 2:52 am. (6 messages)
To: Paul Menage <menage@...>, Pavel Emelianov <xemul@...>
Cc: Hugh Dickins <hugh@...>, Sudhir Kumar <skumar@...>, YAMAMOTO Takashi <yamamoto@...>, <lizf@...>, <linux-kernel@...>, <taka@...>, <linux-mm@...>, David Rientjes <rientjes@...>, Balbir Singh <balbir@...>, Andrew Morton <akpm@...>, KAMEZAWA Hiroyuki <kamezawa.hiroyu@...>
Date: Tuesday, April 1, 2008 - 1:43 am

This patch removes the mem_cgroup member from mm_struct and instead adds
an owner. This approach was suggested by Paul Menage. The advantage of
this approach is that, once the mm-&gt;owner is known, using the subsystem
id, the cgroup can be determined. It also allows several control groups
that are virtually grouped by mm_struct, to exist independent of the memory
controller i.e., without adding mem_cgroup's for each controller,
to mm_struct.

A new config option CONFIG_MM_OWNER is added and the memory resource
controller selects this config option.

NOTE: This patch was developed on top of 2.6.25-rc5-mm1 and is applied on top
of the memory-controller-move-to-own-slab patch (which is already present
in the Andrew's patchset).

I am indebted to Paul Menage for the several reviews of this patchset
and helping me make it lighter and simpler.

This patch was tested on a powerpc box, by running a task under the memory
resource controller and moving it across groups at a constant interval.

Signed-off-by: Balbir Singh &lt;balbir@linux.vnet.ibm.com&gt;
---

 fs/exec.c                  |    1 
 include/linux/init_task.h  |    2 -
 include/linux/memcontrol.h |   17 ++---------
 include/linux/mm_types.h   |    5 ++-
 include/linux/sched.h      |   24 ++++++++++++++++
 init/Kconfig               |   15 ++++++++++
 kernel/exit.c              |   65 +++++++++++++++++++++++++++++++++++++++++++++
 kernel/fork.c              |   11 +++++--
 mm/memcontrol.c            |   21 ++------------
 9 files changed, 124 insertions(+), 37 deletions(-)

diff -puN include/linux/mm_types.h~memory-controller-add-mm-owner include/linux/mm_types.h
--- linux-2.6.25-rc5/include/linux/mm_types.h~memory-controller-add-mm-owner	2008-03-28 09:30:47.000000000 +0530
+++ linux-2.6.25-rc5-balbir/include/linux/mm_types.h	2008-03-31 14:53:04.000000000 +0530
@@ -227,8 +227,9 @@ struct mm_struct {
 	/* aio bits */
 	rwlock_t		ioctx_list_lock;
 	struct kioctx		*ioctx_list;
-#ifdef CONFIG_CGROUP_MEM_RES_CTLR
-	stru...
To: Balbir Singh <balbir@...>
Cc: Pavel Emelianov <xemul@...>, Hugh Dickins <hugh@...>, Sudhir Kumar <skumar@...>, YAMAMOTO Takashi <yamamoto@...>, <lizf@...>, <linux-kernel@...>, <taka@...>, <linux-mm@...>, David Rientjes <rientjes@...>, Andrew Morton <akpm@...>, KAMEZAWA Hiroyuki <kamezawa.hiroyu@...>
Date: Tuesday, April 1, 2008 - 2:16 am

On Mon, Mar 31, 2008 at 10:43 PM, Balbir Singh

This should probably be inlined in the header file if it's needed


I think (as you mentioned earlier) that we need an RCU critical
section in this function, in order for the tasklist traversal to be
safe.



Here we'll want to call vm_cgroup_update_mm_owner(), to adjust the
accounting. (Or if in future we end up with more than a couple of
subsystems that want notification at this time, we'll want to call
cgroup_update_mm_owner() and have it call any interested subsystems.

Paul
--
To: Paul Menage <menage@...>
Cc: Pavel Emelianov <xemul@...>, Hugh Dickins <hugh@...>, Sudhir Kumar <skumar@...>, YAMAMOTO Takashi <yamamoto@...>, <lizf@...>, <linux-kernel@...>, <taka@...>, <linux-mm@...>, David Rientjes <rientjes@...>, Andrew Morton <akpm@...>, KAMEZAWA Hiroyuki <kamezawa.hiroyu@...>
Date: Tuesday, April 1, 2008 - 2:23 am

I thought about it, but that also means we need to export struct mem_cgroup into




I don't think we need to adjust accounting, since only mm-&gt;owner is changing and
not the cgroup to which the task/mm belongs. Do we really need to notify? I


-- 
	Warm Regards,
	Balbir Singh
	Linux Technology Center
	IBM, ISTL
--
To: <balbir@...>
Cc: Pavel Emelianov <xemul@...>, Hugh Dickins <hugh@...>, Sudhir Kumar <skumar@...>, YAMAMOTO Takashi <yamamoto@...>, <lizf@...>, <linux-kernel@...>, <taka@...>, <linux-mm@...>, David Rientjes <rientjes@...>, Andrew Morton <akpm@...>, KAMEZAWA Hiroyuki <kamezawa.hiroyu@...>
Date: Tuesday, April 1, 2008 - 2:48 am

On Mon, Mar 31, 2008 at 11:23 PM, Balbir Singh

It's possible but unlikely that the new owner is in a different cgroup.

Paul
--
To: Paul Menage <menage@...>
Cc: Pavel Emelianov <xemul@...>, Hugh Dickins <hugh@...>, Sudhir Kumar <skumar@...>, YAMAMOTO Takashi <yamamoto@...>, <lizf@...>, <linux-kernel@...>, <taka@...>, <linux-mm@...>, David Rientjes <rientjes@...>, Andrew Morton <akpm@...>, KAMEZAWA Hiroyuki <kamezawa.hiroyu@...>
Date: Tuesday, April 1, 2008 - 3:26 am

Hmmm... that can never happen with thread groups, since mm-&gt;owner is
p-&gt;group_leader and that never exits unless all threads are gone (it can
explicitly change groups though). Without thread groups, the new owner can
belong to a different cgroup, so we might need notification.


-- 
	Warm Regards,
	Balbir Singh
	Linux Technology Center
	IBM, ISTL
--
To: Paul Menage <menage@...>, YAMAMOTO Takashi <yamamoto@...>
Cc: <balbir@...>, Pavel Emelianov <xemul@...>, Hugh Dickins <hugh@...>, Sudhir Kumar <skumar@...>, <lizf@...>, <linux-kernel@...>, <taka@...>, <linux-mm@...>, David Rientjes <rientjes@...>, Andrew Morton <akpm@...>, KAMEZAWA Hiroyuki <kamezawa.hiroyu@...>
Date: Tuesday, April 1, 2008 - 4:13 am

Thinking out aloud

If mm-&gt;owner changes and belongs to a different cgroup, we have a whole new
problem. We need to determine all tasks that share the mm and belong to a
particular cgroup, which changed since the new owner belongs to a different
cgroup and then update the charge.


-- 
	Warm Regards,
	Balbir Singh
	Linux Technology Center
	IBM, ISTL
--
To: <balbir@...>
Cc: <menage@...>, <xemul@...>, <hugh@...>, <skumar@...>, <lizf@...>, <linux-kernel@...>, <taka@...>, <linux-mm@...>, <rientjes@...>, <akpm@...>, <kamezawa.hiroyu@...>
Date: Tuesday, April 1, 2008 - 2:03 am

changing mm-&gt;owner without notifying controllers makes it difficult to use.
can you provide a notification mechanism?

YAMAMOTO Takashi
--
To: YAMAMOTO Takashi <yamamoto@...>
Cc: <menage@...>, <xemul@...>, <hugh@...>, <skumar@...>, <lizf@...>, <linux-kernel@...>, <taka@...>, <linux-mm@...>, <rientjes@...>, <akpm@...>, <kamezawa.hiroyu@...>
Date: Tuesday, April 1, 2008 - 2:25 am

But mm-&gt;owner is just a way to get to the correct cgroup and that does not
change when mm-&gt;owner changes. Do we really need this notification? For the
virtual memory controller, move_task() is sufficient, not sure why mm-&gt;owner is
required.

-- 
	Warm Regards,
	Balbir Singh
	Linux Technology Center
	IBM, ISTL
--
To: YAMAMOTO Takashi <yamamoto@...>
Cc: <balbir@...>, <xemul@...>, <hugh@...>, <skumar@...>, <lizf@...>, <linux-kernel@...>, <taka@...>, <linux-mm@...>, <rientjes@...>, <akpm@...>, <kamezawa.hiroyu@...>
Date: Tuesday, April 1, 2008 - 2:06 am

On Mon, Mar 31, 2008 at 11:03 PM, YAMAMOTO Takashi

Yes, I think that call will need to be in the task_lock() critical
section in which we update mm-&gt;owner.

Right now I think the only user that needs to be notified at that
point is Balbir's virtual address limits controller.

Paul
--
To: <menage@...>
Cc: <balbir@...>, <xemul@...>, <hugh@...>, <skumar@...>, <lizf@...>, <linux-kernel@...>, <taka@...>, <linux-mm@...>, <rientjes@...>, <akpm@...>, <kamezawa.hiroyu@...>
Date: Tuesday, April 1, 2008 - 2:24 am

i have some code for which i might want to use mm-&gt;owner.
it does somewhat complicated things like acquiring mm_sem and
traversing ptes in its -&gt;attach hook.  (if you want to read the code, search
"Subject: [RFC][PATCH] another swap controller for cgroup" in ML archive.)

probably i don't need to use mm-&gt;owner, but it's better if mm-&gt;owner can
handle more cases anyway.

YAMAMOTO Takashi
--
Previous thread: [PATCH] IA64: use goto to jump out do/while_each_thread by Li Zefan on Tuesday, April 1, 2008 - 12:29 am. (1 message)

Next thread: linux-next: Tree for April 1 by Stephen Rothwell on Tuesday, April 1, 2008 - 2:52 am. (6 messages)
speck-geostationary