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 Monday, March 31, 2008 - 9:29 pm. (1 message)

Next thread: linux-next: Tree for April 1 by Stephen Rothwell on Monday, March 31, 2008 - 11:52 pm. (6 messages)
From: Balbir Singh
Date: Monday, March 31, 2008 - 10:43 pm

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->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 <balbir@linux.vnet.ibm.com>
---

 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
-	struct ...
From: YAMAMOTO Takashi
Date: Monday, March 31, 2008 - 11:03 pm

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

YAMAMOTO Takashi
--

From: Paul Menage
Date: Monday, March 31, 2008 - 11:06 pm

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->owner.

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

Paul
--

From: YAMAMOTO Takashi
Date: Monday, March 31, 2008 - 11:24 pm

i have some code for which i might want to use mm->owner.
it does somewhat complicated things like acquiring mm_sem and
traversing ptes in its ->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->owner, but it's better if mm->owner can
handle more cases anyway.

YAMAMOTO Takashi
--

From: Balbir Singh
Date: Monday, March 31, 2008 - 11:25 pm

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

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

From: Paul Menage
Date: Monday, March 31, 2008 - 11:16 pm

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
--

From: Balbir Singh
Date: Monday, March 31, 2008 - 11:23 pm

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->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
--

From: Paul Menage
Date: Monday, March 31, 2008 - 11:48 pm

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
--

From: Balbir Singh
Date: Tuesday, April 1, 2008 - 12:26 am

Hmmm... that can never happen with thread groups, since mm->owner is
p->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
--

From: Balbir Singh
Date: Tuesday, April 1, 2008 - 1:13 am

Thinking out aloud

If mm->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
--

Previous thread: [PATCH] IA64: use goto to jump out do/while_each_thread by Li Zefan on Monday, March 31, 2008 - 9:29 pm. (1 message)

Next thread: linux-next: Tree for April 1 by Stephen Rothwell on Monday, March 31, 2008 - 11:52 pm. (6 messages)