Re: [PATCH v2 1/7] Basic kernel memory functionality for the Memory Controller

Glauber Costa
2011-09-18T03:39:57Z

>> struct mem_cgroup_stat_cpu *stat;
>> @@ -391,6 +404,7 @@ enum charge_type {
>> #define _MEM (0)
>> #define _MEMSWAP (1)
>> #define _OOM_TYPE (2)
>> +#define _KMEM (3)
>
> Ditto. Can we use enum instead?
Yes we can (tm)

>> if (!mem_cgroup_is_root(mem)) {
>> if (!swap)
>> - return res_counter_read_u64(&mem->res, RES_USAGE);
>> + kmem += res_counter_read_u64(&mem->res, RES_USAGE);
>> else
>> - return res_counter_read_u64(&mem->memsw, RES_USAGE);
>> + kmem += res_counter_read_u64(&mem->memsw, RES_USAGE);
>> +
>> + return kmem;
>> }
>>
>> val = mem_cgroup_recursive_stat(mem, MEM_CGROUP_STAT_CACHE);
>
> No kernel memory accounting for root cgroup, right?
Not sure. Maybe kernel memory accounting is useful even for root cgroup.
Same as normal memory accounting... what we want to avoid is kernel
memory limits. OTOH, if we are not limiting it anyway, accounting it is
just useless overhead... Even the statistics can then be gathered
through all
the proc files that show slab usage, I guess?

>
>> @@ -3979,6 +3999,10 @@ static u64 mem_cgroup_read(struct cgroup *cont, struct cftype *cft)
>> else
>> val = res_counter_read_u64(&mem->memsw, name);
>> break;
>> + case _KMEM:
>> + val = res_counter_read_u64(&mem->kmem, name);
>> + break;
>> +
>
> Always zero in root cgroup?

Yes, if we're not accounting, it should be zero. WARN_ON, maybe?