Messages in this thread | | | Date | Mon, 24 Mar 2008 08:03:56 -0700 | From | "Paul Menage" <> | Subject | Re: [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
| |