Re: [PATCH] memcg: drop type MEM_CGROUP_CHARGE_TYPE_DROP

[KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>]

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.
> >
> >> ---
> >> A mm/memcontrol.c | A  A 5 -----
> >> A 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 {
> >> A  A  A  MEM_CGROUP_CHARGE_TYPE_SHMEM, A  /* used by page migration of shmem */
> >> A  A  A  MEM_CGROUP_CHARGE_TYPE_FORCE, A  /* used by force_empty */
> >> A  A  A  MEM_CGROUP_CHARGE_TYPE_SWAPOUT, /* for accounting swapcache */
> >> - A  A  MEM_CGROUP_CHARGE_TYPE_DROP, A  A /* a page was unused swap cache */
> >> A  A  A  NR_CHARGE_TYPE,
> >> A };
> >>
> >> @@ -3000,7 +2999,6 @@ __mem_cgroup_uncharge_common(struct page *page, enum charge_type ctype)
> >>
> >> A  A  A  switch (ctype) {
> >> A  A  A  case MEM_CGROUP_CHARGE_TYPE_MAPPED:
> >> - A  A  case MEM_CGROUP_CHARGE_TYPE_DROP:
> >> A  A  A  A  A  A  A  /* See mem_cgroup_prepare_migration() */
> >> A  A  A  A  A  A  A  if (page_mapped(page) || PageCgroupMigration(pc))
> >> A  A  A  A  A  A  A  A  A  A  A  goto unlock_out;
> >> @@ -3121,9 +3119,6 @@ mem_cgroup_uncharge_swapcache(struct page *page, swp_entry_t ent, bool swapout)
> >> A  A  A  struct mem_cgroup *memcg;
> >> A  A  A  int ctype = MEM_CGROUP_CHARGE_TYPE_SWAPOUT;
> >>
> >> - A  A  if (!swapout) /* this was a swap cache but the swap is unused ! */
> >> - A  A  A  A  A  A  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.

Thanks,
-Kame

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