lkml.org 
[lkml]   [2011]   [Sep]   [26]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3 1/7] Basic kernel memory functionality for the Memory Controller
>>   #endif
>>
>> -
>> +#ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM
>> +int do_kmem_account __read_mostly = 1;
>> +#else
>> +#define do_kmem_account 0
>> +#endif
>
>
> Hmm, do we really need this boot option ?
> From my experience to have swap-accounting boot option,
> this scares us ;) I think config is enough.

If no one else wants it, I can remove it. I personally
don't need it, just wanted to follow the convention laid down by swap here.

>
>
>
>> /*
>> * Statistics for memory cgroup.
>> */
>> @@ -270,6 +274,10 @@ struct mem_cgroup {
>> */
>> struct res_counter memsw;
>> /*
>> + * the counter to account for kmem usage.
>> + */
>> + struct res_counter kmem;
>> + /*
>> * Per cgroup active and inactive list, similar to the
>> * per zone LRU lists.
>> */
>> @@ -321,6 +329,11 @@ struct mem_cgroup {
>> */
>> unsigned long move_charge_at_immigrate;
>> /*
>> + * Should kernel memory limits be stabilished independently
>> + * from user memory ?
>> + */
>> + int kmem_independent;
>> + /*
>> * percpu counter.
>> */
>> struct mem_cgroup_stat_cpu *stat;
>> @@ -388,9 +401,14 @@ enum charge_type {
>> };
>>
>> /* for encoding cft->private value on file */
>> -#define _MEM (0)
>> -#define _MEMSWAP (1)
>> -#define _OOM_TYPE (2)
>> +
>> +enum mem_type {
>> + _MEM = 0,
>> + _MEMSWAP,
>> + _OOM_TYPE,
>> + _KMEM,
>> +};
>> +
>
> ok, nice clean up.
>
>
>> #define MEMFILE_PRIVATE(x, val) (((x)<< 16) | (val))
>> #define MEMFILE_TYPE(val) (((val)>> 16)& 0xffff)
>> #define MEMFILE_ATTR(val) ((val)& 0xffff)
>> @@ -3943,10 +3961,15 @@ static inline u64 mem_cgroup_usage(struct mem_cgroup *mem, bool swap)
>> u64 val;
>>
>> if (!mem_cgroup_is_root(mem)) {
>> + val = 0;
>> + if (!mem->kmem_independent)
>> + val = res_counter_read_u64(&mem->kmem, RES_USAGE);
>
>> if (!swap)
>> - return res_counter_read_u64(&mem->res, RES_USAGE);
>> + val += res_counter_read_u64(&mem->res, RES_USAGE);
>> else
>> - return res_counter_read_u64(&mem->memsw, RES_USAGE);
>> + val += res_counter_read_u64(&mem->memsw, RES_USAGE);
>> +
>> + return val;
>> }
>>
>> val = mem_cgroup_recursive_stat(mem, MEM_CGROUP_STAT_CACHE);
>> @@ -3979,6 +4002,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;
>> +
>> default:
>> BUG();
>> break;
>> @@ -4756,6 +4783,21 @@ static int mem_cgroup_reset_vmscan_stat(struct cgroup *cgrp,
>> return 0;
>> }
>>
>> +#ifdef CONFIG_CGROUP_MEM_RES_CTLR_KMEM
>> +static u64 kmem_limit_independent_read(struct cgroup *cont, struct cftype *cft)
>> +{
>> + return mem_cgroup_from_cont(cont)->kmem_independent;
>> +}
>> +
>> +static int kmem_limit_independent_write(struct cgroup *cont, struct cftype *cft,
>> + u64 val)
>> +{
>> + cgroup_lock();
>> + mem_cgroup_from_cont(cont)->kmem_independent = !!val;
>> + cgroup_unlock();
>
> Hm. This code allows that parent/child can have different settings.
> Could you add parent-child check as..
>
> "If parent sets use_hierarchy==1, children must have the same kmem_independent value
> with parant's one."
Agree.
> How do you think ? I think a hierarchy must have the same config.
Yes, I think this is reasonable.

>
> BTW...I don't like naming a little ;)
>
> memory->consolidated/shared/?????_kmem_accounting ?
> Or
> memory->kmem_independent_accounting ?
>
> or some better naming ?

I can go with kmem_independent_accounting if you like, it is fine
by me.




\
 
 \ /
  Last update: 2011-09-27 00:47    [W:0.195 / U:0.212 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site