On 02/15/2013 05:32 AM, Greg Thelen wrote:
> On Fri, Feb 08 2013, Glauber Costa wrote:
>
>> With the infrastructure we now have, we can add an element to a memcg
>> LRU list instead of the global list. The memcg lists are still
>> per-node.
>
> [...]
>
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index b9e1941..bfb4b5b 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -3319,6 +3319,36 @@ static inline void memcg_resume_kmem_account(void)
>> current->memcg_kmem_skip_account--;
>> }
>>
>> +static struct mem_cgroup *mem_cgroup_from_kmem_page(struct page *page)
>> +{
>> + struct page_cgroup *pc;
>> + struct mem_cgroup *memcg = NULL;
>> +
>> + pc = lookup_page_cgroup(page);
>> + if (!PageCgroupUsed(pc))
>> + return NULL;
>> +
>> + lock_page_cgroup(pc);
>> + if (PageCgroupUsed(pc))
>> + memcg = pc->mem_cgroup;
>> + unlock_page_cgroup(pc);
>
> Once we drop the lock, is there anything that needs protection
> (e.g. PageCgroupUsed)? If there's no problem, then what's the point of
> taking the lock?
>
This is the same pattern already used in the rest of memcg, and I just
transposing it here. From my understanding, we need to make sure that if
the Used bit is not set, we don't rely on the memcg information. So we
take the lock to guarantee that the big is not cleared in the meantime.
But after that, we should be fine.
Kame, you have any input?
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
