Balbir Singh wrote:
quoted text
> 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
--
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/