On Mon, Mar 31, 2008 at 10:43 PM, Balbir Singh
<balbir@linux.vnet.ibm.com> wrote:
quoted text
>  -static struct mem_cgroup *mem_cgroup_from_task(struct task_struct *p)
>  +struct mem_cgroup *mem_cgroup_from_task(struct task_struct *p)
>   {
>         return container_of(task_subsys_state(p, mem_cgroup_subsys_id),
>                                 struct mem_cgroup, css);
>   }

This should probably be inlined in the header file if it's needed
outside this file.
quoted text
>  +static inline void mm_fork_init_owner(struct task_struct *p)
>  +{
>  +}

I think this is stale.

quoted text
>  +
>  +void mm_update_next_owner(struct mm_struct *mm)
>  +{
>  +       struct task_struct *c, *g, *p = current;
>  +
>  +       /*
>  +        * This routine should not be called for init_task
>  +        */
>  +       BUG_ON(p == p->parent);

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.

Maybe also BUG_ON(p != mm->owner) ?

quoted text
>  +       list_for_each_entry(c, &p->children, sibling) {
>  +               if (c->mm && (c->mm == mm))

Since mm != NULL, no need to test for c->mm since if it's NULL then c->mm != mm

quoted text
>  +assign_new_owner:
>  +       BUG_ON(c == p);
>  +       task_lock(c);
>  +       if (c->mm != mm) {
>  +               task_unlock(c);
>  +               goto retry;
>  +       }
>  +       mm->owner = c;

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