Re: [PATCH] memcg: drop type MEM_CGROUP_CHARGE_TYPE_DROP

[Bob Liu <lliubbo@gmail.com>]

On Wed, Dec 7, 2011 at 7:03 PM, KAMEZAWA Hiroyuki
<kamezawa.hiroyu@jp.fujitsu.com> wrote:
> On Wed, 7 Dec 2011 18:30:46 +0800
> Bob Liu <lliubbo@gmail.com> wrote:
>
>> uncharge will happen only when !page_mapped(page) no matter MEM_CGROUP_CHARGE_TYPE_DROP
>> or MEM_CGROUP_CHARGE_TYPE_SWAPOUT when called from mem_cgroup_uncharge_swapcache().
>> i think it's no difference, so drop it.
>>
>> Signed-off-by: Bob Liu <lliubbo@gmail.com>
>
> I think you didn't test at all.
>
>> ---
>>  mm/memcontrol.c |    5 -----
>>  1 files changed, 0 insertions(+), 5 deletions(-)
>>
>> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
>> index 6aff93c..02a2988 100644
>> --- a/mm/memcontrol.c
>> +++ b/mm/memcontrol.c
>> @@ -339,7 +339,6 @@ enum charge_type {
>>       MEM_CGROUP_CHARGE_TYPE_SHMEM,   /* used by page migration of shmem */
>>       MEM_CGROUP_CHARGE_TYPE_FORCE,   /* used by force_empty */
>>       MEM_CGROUP_CHARGE_TYPE_SWAPOUT, /* for accounting swapcache */
>> -     MEM_CGROUP_CHARGE_TYPE_DROP,    /* a page was unused swap cache */
>>       NR_CHARGE_TYPE,
>>  };
>>
>> @@ -3000,7 +2999,6 @@ __mem_cgroup_uncharge_common(struct page *page, enum charge_type ctype)
>>
>>       switch (ctype) {
>>       case MEM_CGROUP_CHARGE_TYPE_MAPPED:
>> -     case MEM_CGROUP_CHARGE_TYPE_DROP:
>>               /* See mem_cgroup_prepare_migration() */
>>               if (page_mapped(page) || PageCgroupMigration(pc))
>>                       goto unlock_out;
>> @@ -3121,9 +3119,6 @@ mem_cgroup_uncharge_swapcache(struct page *page, swp_entry_t ent, bool swapout)
>>       struct mem_cgroup *memcg;
>>       int ctype = MEM_CGROUP_CHARGE_TYPE_SWAPOUT;
>>
>> -     if (!swapout) /* this was a swap cache but the swap is unused ! */
>> -             ctype = MEM_CGROUP_CHARGE_TYPE_DROP;
>> -
>
> Then, here , what ctype must be if !swapout ?
>

I think MEM_CGROUP_CHARGE_TYPE_SWAPOUT is okay, i didn't get the point
that the benefit of using MEM_CGROUP_CHARGE_TYPE_DROP.

If use  MEM_CGROUP_CHARGE_TYPE_SWAPOUT, page_mapped(page) also checked in
__mem_cgroup_uncharge_common().

Maybe i missed something. Thanks.

3003         case MEM_CGROUP_CHARGE_TYPE_DROP:
3004                 /* See mem_cgroup_prepare_migration() */
3005                 if (page_mapped(page) || PageCgroupMigration(pc))
3006                         goto unlock_out;
3007                 break;
3008         case MEM_CGROUP_CHARGE_TYPE_SWAPOUT:
3009                 if (!PageAnon(page)) {  /* Shared memory */
3010                         if (page->mapping && !page_is_file_cache(page))
3011                                 goto unlock_out;
3012                 } else if (page_mapped(page)) /* Anon */
3013                                 goto unlock_out;
3014                 break;

> Nack.
>
> Thanks,
> -Kame
>


-- 
Regards,
--Bob

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>