lkml.org 
[lkml]   [2010]   [Jun]   [11]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [RFC][PATCH] memcg remove css_get/put per pages v2
> @@ -2432,15 +2463,18 @@ mem_cgroup_uncharge_swapcache(struct pag
> if (!swapout) /* this was a swap cache but the swap is unused ! */
> ctype = MEM_CGROUP_CHARGE_TYPE_DROP;
>
> - memcg = __mem_cgroup_uncharge_common(page, ctype);
> + memcg = try_get_mem_cgroup_from_page(page);
> + if (!memcg)
> + return;
> +
> + __mem_cgroup_uncharge_common(page, ctype);
>
> /* record memcg information */
> - if (do_swap_account && swapout && memcg) {
> + if (do_swap_account && swapout) {
> swap_cgroup_record(ent, css_id(&memcg->css));
> mem_cgroup_get(memcg);
> }
> - if (swapout && memcg)
> - css_put(&memcg->css);
> + css_put(&memcg->css);
> }
> #endif
>
hmm, this change seems to cause a problem.
I can see under flow of mem->memsw and "swap" field in memory.stat.

I think doing swap_cgroup_record() against mem_cgroup which is not returned
by __mem_cgroup_uncharge_common() is a bad behavior.

How about doing like this ? We can safely access mem_cgroup while it has
memory.usage, iow, before we call res_counter_uncharge().
After this change, it seems to work well.

---
mm/memcontrol.c | 22 +++++++++-------------
1 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 6e7c1c9..2fae26f 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -2362,10 +2362,6 @@ __mem_cgroup_uncharge_common(struct page *page, enum charge_type ctype)
break;
}

- if (!mem_cgroup_is_root(mem))
- __do_uncharge(mem, ctype);
- if (ctype == MEM_CGROUP_CHARGE_TYPE_SWAPOUT)
- mem_cgroup_swap_statistics(mem, true);
mem_cgroup_charge_statistics(mem, pc, false);

ClearPageCgroupUsed(pc);
@@ -2379,6 +2375,12 @@ __mem_cgroup_uncharge_common(struct page *page, enum charge_type ctype)
unlock_page_cgroup(pc);

memcg_check_events(mem, page);
+ if (ctype == MEM_CGROUP_CHARGE_TYPE_SWAPOUT) {
+ mem_cgroup_swap_statistics(mem, true);
+ mem_cgroup_get(mem);
+ }
+ if (!mem_cgroup_is_root(mem))
+ __do_uncharge(mem, ctype);

return mem;

@@ -2463,18 +2465,12 @@ mem_cgroup_uncharge_swapcache(struct page *page, swp_entry_t ent, bool swapout)
if (!swapout) /* this was a swap cache but the swap is unused ! */
ctype = MEM_CGROUP_CHARGE_TYPE_DROP;

- memcg = try_get_mem_cgroup_from_page(page);
- if (!memcg)
- return;
-
- __mem_cgroup_uncharge_common(page, ctype);
+ memcg = __mem_cgroup_uncharge_common(page, ctype);

/* record memcg information */
- if (do_swap_account && swapout) {
+ if (do_swap_account && swapout && memcg)
+ /* We've already done mem_cgroup_get() in uncharge_common(). */
swap_cgroup_record(ent, css_id(&memcg->css));
- mem_cgroup_get(memcg);
- }
- css_put(&memcg->css);
}
#endif


\
 
 \ /
  Last update: 2010-06-11 06:53    [W:0.147 / U:0.116 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site