lkml.org 
[lkml]   [2008]   [Mar]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC][-mm] Memory controller add mm->owner
On Mon, Mar 24, 2008 at 7:01 AM, Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
> --- linux-2.6.25-rc5/include/linux/mm_types.h~memory-controller-add-mm-owner 2008-03-20 13:35:09.000000000 +0530
> +++ linux-2.6.25-rc5-balbir/include/linux/mm_types.h 2008-03-20 15:11:05.000000000 +0530
> @@ -228,7 +228,10 @@ struct mm_struct {
> rwlock_t ioctx_list_lock;
> struct kioctx *ioctx_list;
> #ifdef CONFIG_CGROUP_MEM_RES_CTLR
> - struct mem_cgroup *mem_cgroup;
> + struct task_struct *owner; /* The thread group leader that */
> + /* owns the mm_struct. This */
> + /* might be useful even outside */
> + /* of the config option */
> #endif

This should probably be controlled by something like a CONFIG_MM_OWNER
that's selected by any Kconfig option (mem cgroup, etc) that needs
mm->owner to be maintained.

> @@ -248,12 +248,40 @@ void mm_init_cgroup(struct mm_struct *mm
>
> mem = mem_cgroup_from_task(p);
> css_get(&mem->css);
> - mm->mem_cgroup = mem;
> + mm->owner = p;
> +}
> +
> +void mem_cgroup_fork_init(struct task_struct *p)
> +{
> + struct mm_struct *mm = get_task_mm(p);
> + struct mem_cgroup *mem, *oldmem;
> + if (!mm)
> + return;
> +
> + /*
> + * Initial owner at mm_init_cgroup() time is the task itself.
> + * The thread group leader had not been setup then
> + */
> + oldmem = mem_cgroup_from_task(mm->owner);
> + /*
> + * Override the mm->owner after we know the thread group later
> + */
> + mm->owner = p->group_leader;
> + mem = mem_cgroup_from_task(mm->owner);
> + css_get(&mem->css);
> + css_put(&oldmem->css);
> + mmput(mm);
> }
>
> void mm_free_cgroup(struct mm_struct *mm)
> {
> - css_put(&mm->mem_cgroup->css);
> + struct mem_cgroup *mem;
> +
> + /*
> + * TODO: Should we assign mm->owner to NULL here?
> + */
> + mem = mem_cgroup_from_task(mm->owner);
> + css_put(&mem->css);
> }

It seems to me that the code to setup/maintain mm->owner should be
independent of the control groups, but should be part of the generic
fork/exit code.

Also, if mm->owner exits but mm is still alive (unlikely, but could
happen with weird custom threading libraries?) then we need to
reassign mm->owner to one of the other users of the mm (by looking
first in the thread group, then among the parents/siblings/children,
and then among all processes as a last resort?)

>
> - rcu_read_lock();
> - mem = rcu_dereference(mm->mem_cgroup);
> + mem = mem_cgroup_from_task(mm->owner);

I think we still need the rcu_read_lock(), since mm->owner can move
cgroups any time.

>
> @@ -1069,7 +1096,6 @@ static void mem_cgroup_move_task(struct
> goto out;
>
> css_get(&mem->css);
> - rcu_assign_pointer(mm->mem_cgroup, mem);
> css_put(&old_mem->css);
>

We shouldn't need reference counting on this pointer, since the
cgroups framework won't allow a subsystem to be freed while it has any
tasks in it.

Paul


\
 
 \ /
  Last update: 2008-03-24 16:07    [from the cache]
©2003-2011 Jasper Spaans