Messages in this thread |  | | | Date | Mon, 31 Mar 2008 23:16:19 -0700 | | From | "Paul Menage" <> | | Subject | Re: [RFC][-mm] Add an owner to the mm_struct (v3) | |
On Mon, Mar 31, 2008 at 10:43 PM, Balbir Singh
<balbir@linux.vnet.ibm.com> wrote:
> -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.
> +static inline void mm_fork_init_owner(struct task_struct *p)
> +{
> +}
I think this is stale.
> +
> +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) ?
> + 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
> +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
|  |