On 12/04/2012 08:34 PM, Michal Hocko wrote:
> On Tue 04-12-12 12:17:21, Michal Hocko wrote:
>> On Tue 04-12-12 16:36:11, Jeff Liu wrote:
> [...]
>>> + * arrive here multiple times. But we only allocate pages for swap
>>> + * cgroup when the first child memcg was created.
>>> + */
>>> +int swap_cgroup_init(void)
>>> +{
>>> + int type;
>>> +
>>> + if (!do_swap_account)
>>> + return 0;
>>> +
>>> + if (atomic_add_return(1, &swap_cgroup_initialized) != 1)
>>> + return 0;
>>> +
>>> + mutex_lock(&swap_cgroup_mutex);
>>> + for (type = 0; type < MAX_SWAPFILES; type++) {
>>> + if (swap_cgroup_alloc_pages(type) < 0) {
>>
>> Why do you initialize MAX_SWAPFILES rather than nr_swapfiles?
>>
>> Besides that swap_cgroup_alloc_pages is not sufficient because it
>> doesn't allocate ctrl->map but it tries to put pages in it.
>
> Sorry, I have missed that you have kept ctrl->map initialization in
> swap_cgroup_swapon so this is not an issue.
> I think you can do better if swap_cgroup_swapon only initialized
> ctrl->length and deferred all the rest to swap_cgroup_alloc_pages (or
> its original name as it suits better) including the map allocation which
> is currently done in swap_cgroup_swapon.
Definitely, It's better to defer ctrl->map array allocation up to that
phase, thank you for pointing it out. :)
I'll fix those issues according to your comments in another email.
-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>