Re: [PATCH] memcg: drop type MEM_CGROUP_CHARGE_TYPE_DROP

[Bob Liu <lliubbo@gmail.com>]

On Thu, Dec 8, 2011 at 9:42 AM, KAMEZAWA Hiroyuki
<kamezawa.hiroyu@jp.fujitsu.com> wrote:
> On Wed, 7 Dec 2011 20:29:24 +0800
> Bob Liu <lliubbo@gmail.com> wrote:
>
>> 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.
>>
> why you don't see 10 more lines.
>
> If SWAPOUT,
>  - record swap out information to swap_cgroup
>  - don't decrease memcg->memsw counter
> If DROP
>  - same as usual MAPPED anon pages.
>
> DROP may be equal to MAPPED. But this swap realted codes are most fragile
> part of memcg and we used another name than MAPPED for taking care.
>

Get it.
Sorry for my noise, i didn't deep into following function.

-- 
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>