On 12/04/2012 08:54 PM, Michal Hocko wrote:
> On Tue 04-12-12 16:36:17, Jeff Liu wrote:
>> - Call swap_cgroup_init() when the first child memcg was created.
>> - Free pages from swap cgroup once the latest child memcg was removed.
>
> This should be a separate patch
>
>> - Teach swap_cgroup_record()/swap_cgroup_cmpxchg()/swap_cgroup_lookup_id()
>> to aware of the new static variable. i.e, swap_cgroup_initialized, so that
>> they would not perform further jobs if swap cgroup is not active.
>
> This should be a preparatory patch.
Ok, these changes will be reflect in next round of post.
>
>>
>> Signed-off-by: Jie Liu <jeff.liu@xxxxxxxxxx>
>> CC: 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>
>> ---
>> mm/memcontrol.c | 3 +++
>> mm/page_cgroup.c | 9 +++++++++
>> 2 files changed, 12 insertions(+)
>>
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index dd39ba0..1f14375 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -4982,6 +4982,8 @@ mem_cgroup_create(struct cgroup *cont)
>> }
>> hotcpu_notifier(memcg_cpu_hotplug_callback, 0);
>> } else {
>> + if (swap_cgroup_init())
>> + goto free_out;
>> parent = mem_cgroup_from_cont(cont->parent);
>> memcg->use_hierarchy = parent->use_hierarchy;
>> memcg->oom_kill_disable = parent->oom_kill_disable;
>> @@ -5046,6 +5048,7 @@ static void mem_cgroup_destroy(struct cgroup *cont)
>> struct mem_cgroup *memcg = mem_cgroup_from_cont(cont);
>>
>> kmem_cgroup_destroy(memcg);
>> + swap_cgroup_destroy();
>>
>> mem_cgroup_put(memcg);
>> }
>> diff --git a/mm/page_cgroup.c b/mm/page_cgroup.c
>> index f1b257b..63c6789 100644
>> --- a/mm/page_cgroup.c
>> +++ b/mm/page_cgroup.c
>> @@ -429,6 +429,9 @@ unsigned short swap_cgroup_cmpxchg(swp_entry_t ent,
>> unsigned long flags;
>> unsigned short retval;
>>
>> + if (!atomic_read(&swap_cgroup_initialized))
>> + return 0;
>> +
>> sc = lookup_swap_cgroup(ent, &ctrl);
>>
>> spin_lock_irqsave(&ctrl->lock, flags);
>> @@ -456,6 +459,9 @@ unsigned short swap_cgroup_record(swp_entry_t ent, unsigned short id)
>> unsigned short old;
>> unsigned long flags;
>>
>> + if (!atomic_read(&swap_cgroup_initialized))
>> + return 0;
>> +
>> sc = lookup_swap_cgroup(ent, &ctrl);
>>
>> spin_lock_irqsave(&ctrl->lock, flags);
>> @@ -474,6 +480,9 @@ unsigned short swap_cgroup_record(swp_entry_t ent, unsigned short id)
>> */
>> unsigned short lookup_swap_cgroup_id(swp_entry_t ent)
>> {
>> + if (!atomic_read(&swap_cgroup_initialized))
>> + return 0;
>> +
>> return lookup_swap_cgroup(ent, NULL)->id;
>
> I think that the swap_cgroup_initialized test should be stuffed into
> lookup_swap_cgroup because it would be less fragile.
Originally, I spreaded this test over those functions in order to avoid adding
additional check against the return value of lookup_swap_cgroup(), but it's
really stupid, will follow up with your suggestions.
Thanks,
-Jeff
--
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>