On Mon, Mar 24, 2008 at 7:01 AM, Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
quoted text
>  --- 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.

quoted text
>  @@ -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?)

quoted text
>
>  -       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.

quoted text
>
>  @@ -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
--
unsubscribe notice
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/