Re: [-mm] Add an owner to the mm_struct (v2)

Balbir Singh
2008-03-28T12:58:20Z

Balbir Singh wrote:
> Paul Menage wrote:
>> On Fri, Mar 28, 2008 at 1:23 AM, Balbir Singh <balbir@linux.vnet.ibm.com> wrote:
>>> diff -puN include/linux/mm_types.h~memory-controller-add-mm-owner include/linux/mm_types.h
>>> --- linux-2.6.25-rc5/include/linux/mm_types.h~memory-controller-add-mm-owner 2008-03-28 09:30:47.000000000 +0530
>>> +++ linux-2.6.25-rc5-balbir/include/linux/mm_types.h 2008-03-28 12:26:59.000000000 +0530
>>> @@ -227,8 +227,10 @@ struct mm_struct {
>>> /* aio bits */
>>> rwlock_t ioctx_list_lock;
>>> struct kioctx *ioctx_list;
>>> -#ifdef CONFIG_CGROUP_MEM_RES_CTLR
>>> - struct mem_cgroup *mem_cgroup;
>>> +#ifdef CONFIG_MM_OWNER
>>> + spinlock_t owner_lock;
>>> + struct task_struct *owner; /* The thread group leader that */
>>> + /* owns the mm_struct. */
>>> #endif
>> I'm not convinced that we need the spinlock. Just use the simple rule
>> that you can only modify mm->owner if:
>>
>> - mm->owner points to current
>> - the new owner is a user of mm
>
> This will always hold, otherwise it cannot be the new owner :)
>
>> - you hold task_lock() for the new owner (which is necessary anyway to
>> ensure that the new owner's mm doesn't change while you're updating
>> mm->owner)
>>

Thinking more, I don't think it makes sense for us to overload task_lock() to do
the mm->owner handling (we don't want to mix lock domains). task_lock() is used
for several things

1. We don't want to make task_lock() rules more complicated by having it protect
an mm member to save space
2. We don't want more contention on task_lock()

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