Re: [patch 05/11] mm: move memcg hierarchy reclaim to generic reclaim code
From: Michal Hocko <hidden>
Date: 2011-09-20 13:09:19
Also in:
lkml
On Mon 12-09-11 12:57:22, Johannes Weiner wrote:
Memory cgroup limit reclaim and traditional global pressure reclaim will soon share the same code to reclaim from a hierarchical tree of memory cgroups. In preparation of this, move the two right next to each other in shrink_zone().
I like the way how you've split mem_cgroup_hierarchical_reclaim into mem_cgroup_reclaim and mem_cgroup_soft_reclaim and I guess this deserves a note in the patch description. Especially that mem_cgroup_reclaim is hierarchical even though it doesn't use mem_cgroup_iter directly but rather via do_try_to_free_pages and shrink_zone. I am not sure I see how shrink_mem_cgroup_zone works. See comments and questions bellow:
Signed-off-by: Johannes Weiner <redacted> --- include/linux/memcontrol.h | 25 ++++++- mm/memcontrol.c | 167 ++++++++++++++++++++++---------------------- mm/vmscan.c | 43 ++++++++++- 3 files changed, 147 insertions(+), 88 deletions(-)
[...]
quoted hunk ↗ jump to hunk
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index f4b404e..413e1f8 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c
[...]
quoted hunk ↗ jump to hunk
@@ -783,19 +781,33 @@ struct mem_cgroup *try_get_mem_cgroup_from_mm(struct mm_struct *mm) return memcg; } -struct mem_cgroup_iter { - struct zone *zone; - int priority; - unsigned int generation; -}; - -static struct mem_cgroup *mem_cgroup_iter(struct mem_cgroup *root, - struct mem_cgroup *prev, - struct mem_cgroup_iter *iter) +/** + * mem_cgroup_iter - iterate over memory cgroup hierarchy + * @root: hierarchy root + * @prev: previously returned memcg, NULL on first invocation + * @iter: token for partial walks, NULL for full walks + * + * Returns references to children of the hierarchy starting at @root,
I guess you meant "starting at @prev"
+ * or @root itself, or %NULL after a full round-trip.
+ *
+ * Caller must pass the return value in @prev on subsequent
+ * invocations for reference counting, or use mem_cgroup_iter_break()
+ * to cancel a hierarchy walk before the round-trip is complete.
+ *
+ * Reclaimers can specify a zone and a priority level in @iter to
+ * divide up the memcgs in the hierarchy among all concurrent
+ * reclaimers operating on the same zone and priority.
+ */
+struct mem_cgroup *mem_cgroup_iter(struct mem_cgroup *root,
+ struct mem_cgroup *prev,
+ struct mem_cgroup_iter *iter)
{
struct mem_cgroup *mem = NULL;
int id = 0;
+ if (mem_cgroup_disabled())
+ return NULL;
+
if (!root)
root = root_mem_cgroup;
[...]
quoted hunk ↗ jump to hunk
@@ -1479,6 +1496,41 @@ u64 mem_cgroup_get_limit(struct mem_cgroup *memcg) return min(limit, memsw); } +static unsigned long mem_cgroup_reclaim(struct mem_cgroup *mem, + gfp_t gfp_mask, + unsigned long flags) +{ + unsigned long total = 0; + bool noswap = false; + int loop; + + if (flags & MEM_CGROUP_RECLAIM_NOSWAP) + noswap = true; + else if (!(flags & MEM_CGROUP_RECLAIM_SHRINK) && mem->memsw_is_minimum) + noswap = true; + + for (loop = 0; loop < MEM_CGROUP_MAX_RECLAIM_LOOPS; loop++) { + if (loop) + drain_all_stock_async(mem); + total += try_to_free_mem_cgroup_pages(mem, gfp_mask, noswap); + /* + * Avoid freeing too much when shrinking to resize the + * limit. XXX: Shouldn't the margin check be enough?
I guess the MEM_CGROUP_RECLAIM_SHRINK condition should help shrinkers to die more easily on signal even if they make some progress.
+ */ + if (total && (flags & MEM_CGROUP_RECLAIM_SHRINK)) + break; + if (mem_cgroup_margin(mem)) + break; + /* + * If nothing was reclaimed after two attempts, there + * may be no reclaimable pages in this hierarchy. + */ + if (loop && !total) + break; + } + return total; +} + /** * test_mem_cgroup_node_reclaimable * @mem: the target memcg
[...]
quoted hunk ↗ jump to hunk
diff --git a/mm/vmscan.c b/mm/vmscan.c index 92f4e22..8419e8f 100644 --- a/mm/vmscan.c +++ b/mm/vmscan.c@@ -2104,12 +2104,43 @@ restart: static void shrink_zone(int priority, struct zone *zone, struct scan_control *sc) { - struct mem_cgroup_zone mz = { - .mem_cgroup = sc->target_mem_cgroup, + struct mem_cgroup *root = sc->target_mem_cgroup; + struct mem_cgroup_iter iter = { .zone = zone, + .priority = priority, }; + struct mem_cgroup *mem; + + if (global_reclaim(sc)) { + struct mem_cgroup_zone mz = { + .mem_cgroup = NULL, + .zone = zone, + }; + + shrink_mem_cgroup_zone(priority, &mz, sc); + return; + } + + mem = mem_cgroup_iter(root, NULL, &iter); + do { + struct mem_cgroup_zone mz = { + .mem_cgroup = mem, + .zone = zone, + }; - shrink_mem_cgroup_zone(priority, &mz, sc); + shrink_mem_cgroup_zone(priority, &mz, sc); + /* + * Limit reclaim has historically picked one memcg and + * scanned it with decreasing priority levels until + * nr_to_reclaim had been reclaimed. This priority + * cycle is thus over after a single memcg. + */ + if (!global_reclaim(sc)) {
How can we have global_reclaim(sc) == true here? Shouldn't we just check how much have we reclaimed from that group and iterate only if it wasn't sufficient (at least SWAP_CLUSTER_MAX)?
+ mem_cgroup_iter_break(root, mem); + break; + } + mem = mem_cgroup_iter(root, mem, &iter); + } while (mem); } /*
[...] -- Michal Hocko SUSE Labs SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic -- 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>