lkml.org 
[lkml]   [2008]   [Apr]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC][-mm] Add an owner to the mm_struct (v3)
Paul Menage wrote:
> 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.

I thought about it, but that also means we need to export struct mem_cgroup into
the header file

>> +static inline void mm_fork_init_owner(struct task_struct *p)
>> +{
>> +}
>
> I think this is stale.
>

Yes, it is stale now :)

>> +
>> +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) ?
>

Yes

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

OK

>> +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.
>

I don't think we need to adjust accounting, since only mm->owner is changing and
not the cgroup to which the task/mm belongs. Do we really need to notify? I
don't want to do any notifications under task_lock().

> Paul


--
Warm Regards,
Balbir Singh
Linux Technology Center
IBM, ISTL


\
 
 \ /
  Last update: 2008-04-01 08:39    [W:0.077 / U:0.232 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site