(2013/03/06 17:30), Glauber Costa wrote:
> On 03/06/2013 04:27 AM, Kamezawa Hiroyuki wrote:
>> (2013/03/05 22:10), Glauber Costa wrote:
>>> For the root memcg, there is no need to rely on the res_counters if hierarchy
>>> is enabled The sum of all mem cgroups plus the tasks in root itself, is
>>> necessarily the amount of memory used for the whole system. Since those figures
>>> are already kept somewhere anyway, we can just return them here, without too
>>> much hassle.
>>>
>>> Limit and soft limit can't be set for the root cgroup, so they are left at
>>> RESOURCE_MAX. Failcnt is left at 0, because its actual meaning is how many
>>> times we failed allocations due to the limit being hit. We will fail
>>> allocations in the root cgroup, but the limit will never the reason.
>>>
>>> Signed-off-by: Glauber Costa <glommer@xxxxxxxxxxxxx>
>>> CC: Michal Hocko <mhocko@xxxxxxx>
>>> CC: Kamezawa Hiroyuki <kamezawa.hiroyu@xxxxxxxxxxxxxx>
>>> CC: Johannes Weiner <hannes@xxxxxxxxxxx>
>>> CC: Mel Gorman <mgorman@xxxxxxx>
>>> CC: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
>>
>> I think this patch's calculation is wrong.
>>
> where exactly ?
>
>>> ---
>>> mm/memcontrol.c | 64 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>> 1 file changed, 64 insertions(+)
>>>
>>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>>> index b8b363f..bfbf1c2 100644
>>> --- a/mm/memcontrol.c
>>> +++ b/mm/memcontrol.c
>>> @@ -4996,6 +4996,56 @@ static inline u64 mem_cgroup_usage(struct mem_cgroup *memcg, bool swap)
>>> return val << PAGE_SHIFT;
>>> }
>>>
>>> +static u64 memcg_read_root_rss(void)
>>> +{
>>> + struct task_struct *p;
>>> +
>>> + u64 rss = 0;
>>> + read_lock(&tasklist_lock);
>>> + for_each_process(p) {
>>> + if (!p->mm)
>>> + continue;
>>> + task_lock(p);
>>> + rss += get_mm_rss(p->mm);
>>> + task_unlock(p);
>>> + }
>>> + read_unlock(&tasklist_lock);
>>> + return rss;
>>> +}
>>
>> I think you can use rcu_read_lock() instead of tasklist_lock.
>> Isn't it enough to use NR_ANON_LRU rather than this ?
>
> Is it really just ANON_LRU ? get_mm_rss also include filepages, which
> are not in this list.
And mlocked ones counted as Unevictable
>
> Maybe if we sum up *all* LRUs we would get the right result ?
>
_MEM...i.e. ...usage_in_bytes is the sum of all LRUs.
> About the tasklist lock, if I get values from the LRUs, maybe. Otherwise
> it is still necessary, no ?
tasklist is RCU list and we don't need locking at reading values, I think.
Thanks,
-Kame
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@xxxxxxxxx. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>