Memory controller soft limit patches (v2)

mm/memcontrol.c +3 -0

+static DEFINE_SPINLOCK(memcg_soft_limit_tree_lock);
+
+/*
  * The memory controller data structure. The memory controller controls both
  * page cache and RSS per cgroup. We would eventually like to provide
  * statistics based on the statistics developed by Rik Van Riel for clock-pro,

mm/memcontrol.c +16 -0

 enum charge_type {
 	MEM_CGROUP_CHARGE_TYPE_CACHE = 0,
 	MEM_CGROUP_CHARGE_TYPE_MAPPED,

@@ -214,6 +229,41 @@ static void mem_cgroup_get(struct mem_cgroup *mem);
 static void mem_cgroup_put(struct mem_cgroup *mem);
 static struct mem_cgroup *parent_mem_cgroup(struct mem_cgroup *mem);
 
+static void mem_cgroup_insert_exceeded(struct mem_cgroup *mem)
+{
+	struct rb_node **p = &mem_cgroup_soft_limit_exceeded_groups.rb_node;
+	struct rb_node *parent = NULL;
+	struct mem_cgroup *mem_node;
+	unsigned long flags;
+
+	spin_lock_irqsave(&memcg_soft_limit_tree_lock, flags);
+	while (*p) {
+		parent = *p;
+		mem_node = rb_entry(parent, struct mem_cgroup, mem_cgroup_node);
+		if (mem->usage_in_excess < mem_node->usage_in_excess)
+			p = &(*p)->rb_left;
+		/*
+		 * We can't avoid mem cgroups that are over their soft
+		 * limit by the same amount

mm/memcontrol.c +15 -0

+
+static void mem_cgroup_remove_exceeded(struct mem_cgroup *mem)
+{
+	unsigned long flags;
+	spin_lock_irqsave(&memcg_soft_limit_tree_lock, flags);
+	rb_erase(&mem->mem_cgroup_node, &mem_cgroup_soft_limit_exceeded_groups);
+	spin_unlock_irqrestore(&memcg_soft_limit_tree_lock, flags);
+}
+
 static void mem_cgroup_charge_statistics(struct mem_cgroup *mem,
 					 struct page_cgroup *pc,
 					 bool charge)

@@ -897,6 +947,39 @@ static void record_last_oom(struct mem_cgroup *mem)
 	mem_cgroup_walk_tree(mem, NULL, record_last_oom_cb);
 }
 
+static void mem_cgroup_check_and_update_tree(struct mem_cgroup *mem,
+						bool time_check)
+{
+	unsigned long long prev_usage_in_excess, new_usage_in_excess;
+	bool updated_tree = false;
+	unsigned long next_update = 0;

mm/memcontrol.c +34 -4

+		if (prev_usage_in_excess)
+			mem_cgroup_remove_exceeded(mem);
+		mem_cgroup_insert_exceeded(mem);
+		updated_tree = true;
+	} else if (prev_usage_in_excess && !new_usage_in_excess) {
+		mem_cgroup_remove_exceeded(mem);
+		updated_tree = true;
+	}
+
+	if (updated_tree) {
+		spin_lock_irqsave(&memcg_soft_limit_tree_lock, flags);
+		mem->last_tree_update = jiffies;
+		mem->usage_in_excess = new_usage_in_excess;
+		spin_unlock_irqrestore(&memcg_soft_limit_tree_lock, flags);
+	}
+	mem_cgroup_put(mem);
+}
 
 /*
  * Unlike exported interface, "oom" parameter is added. if oom==true,

@@ -906,9 +989,9 @@ static int __mem_cgroup_try_charge(struct mm_struct *mm,
 			gfp_t gfp_mask, struct mem_cgroup **memcg,
 			bool oom)
 {
-	struct mem_cgroup *mem, *mem_over_limit;
+	struct mem_cgroup *mem, *mem_over_limit, *mem_over_soft_limit;
 	int nr_retries = MEM_CGROUP_RECLAIM_RETRIES;
-	struct res_counter *fail_res;
+	struct res_counter *fail_res, *soft_fail_res = NULL;
 
 	if (unlikely(test_thread_flag(TIF_MEMDIE))) {
 		/* Don't account this! */

@@ -938,12 +1021,13 @@ static int __mem_cgroup_try_charge(struct mm_struct *mm,
 		int ret;
 		bool noswap = false;
 
-		ret = res_counter_charge(&mem->res, PAGE_SIZE, &fail_res);
+		ret = res_counter_charge(&mem->res, PAGE_SIZE, &fail_res,
+						&soft_fail_res);
 		if (likely(!ret)) {
 			if (!do_swap_account)
 				break;
 			ret = res_counter_charge(&mem->memsw, PAGE_SIZE,
-							&fail_res);
+							&fail_res, NULL);
 			if (likely(!ret))
 				break;
 			/* mem+swap counter fails */

@@ -985,6 +1069,13 @@ static int __mem_cgroup_try_charge(struct mm_struct *mm,
 			goto nomem;
 		}
 	}
+
+	if (soft_fail_res) {
+		mem_over_soft_limit =
+			mem_cgroup_from_res_counter(soft_fail_res, res);
+		mem_cgroup_check_and_update_tree(mem_over_soft_limit, true);
+	}
+	mem_cgroup_check_and_update_tree(mem, true);
 	return 0;
 nomem:
 	css_put(&mem->css);

@@ -1422,6 +1513,7 @@ __mem_cgroup_uncharge_common(struct page *page, enum charge_type ctype)
 	mz = page_cgroup_zoneinfo(pc);
 	unlock_page_cgroup(pc);
 
+	mem_cgroup_check_and_update_tree(mem, true);
 	/* at swapout, this memcg will be accessed to record to swap */
 	if (ctype != MEM_CGROUP_CHARGE_TYPE_SWAPOUT)
 		css_put(&mem->css);

@@ -2346,6 +2438,7 @@ static void __mem_cgroup_free(struct mem_cgroup *mem)
 {
 	int node;
 
+	mem_cgroup_check_and_update_tree(mem, false);
 	free_css_id(&mem_cgroup_subsys, &mem->css);
 
 	for_each_node_state(node, N_POSSIBLE)

@@ -2412,6 +2505,7 @@ mem_cgroup_create(struct cgroup_subsys *ss, struct cgroup *cont)
 	if (cont->parent == NULL) {
 		enable_swap_cgroup();
 		parent = NULL;
+		mem_cgroup_soft_limit_exceeded_groups = RB_ROOT;
 	} else {
 		parent = mem_cgroup_from_cont(cont->parent);
 		mem->use_hierarchy = parent->use_hierarchy;

@@ -2432,6 +2526,8 @@ mem_cgroup_create(struct cgroup_subsys *ss, struct cgroup *cont)
 		res_counter_init(&mem->memsw, NULL);
 	}
 	mem->last_scanned_child = 0;
+	mem->usage_in_excess = 0;
+	mem->last_tree_update = 0;	/* Yes, time begins at 0 here */
 	spin_lock_init(&mem->reclaim_param_lock);
 
 	if (parent)

Signature · 7 lines

-- 
	Balbir
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

mm/memcontrol.c struct mem_cgroup_lru_info { · line 137

Quoted 6 lines

/*
+ * Cgroups above their limits are maintained in a RB-Tree, independent of
+ * their hierarchy representation
+ */

mm/memcontrol.c +2 -0

+
+static struct rb_root mem_cgroup_soft_limit_exceeded_groups;

37 length variable name seems too long.

Quoted 9 lines

mm/memcontrol.c +3 -0

+static DEFINE_SPINLOCK(memcg_soft_limit_tree_lock);
+
+/*
  * The memory controller data structure. The memory controller controls both
  * page cache and RSS per cgroup. We would eventually like to provide
  * statistics based on the statistics developed by Rik Van Riel for clock-pro,

@@ -176,12 +185,18 @@ struct mem_cgroup {
 

mm/memcontrol.c struct mem_cgroup { · line 191

Quoted 6 lines

mm/memcontrol.c +4 -0

 	unsigned int	swappiness;
 
+	struct rb_node mem_cgroup_node;
+	unsigned long long usage_in_excess;
+	unsigned long last_tree_update;
+

no comment fields.

Do usage_in_excess and last_tree_update have what unit? "unsigned long" don't tell me anything.

Quoted 6 lines

mm/memcontrol.c +0 -0

 	/*
 	 * statistics. This must be placed at the end of memcg.
 	 */
 	struct mem_cgroup_stat stat;
 };
 

mm/memcontrol.c struct mem_cgroup { · line 199

Quoted 2 lines

mm/memcontrol.c +2 -0

+#define	MEM_CGROUP_TREE_UPDATE_INTERVAL		(HZ)
+

In general, memory subsystem be considered to shouldn't have timer thing. it's because we expect we get 100x times faster machine after 10 year, at that time, we expect proper timeout value is changed.

Can we make proper stastics, instead?

Quoted 29 lines

mm/memcontrol.c +21 -0

 enum charge_type {
 	MEM_CGROUP_CHARGE_TYPE_CACHE = 0,
 	MEM_CGROUP_CHARGE_TYPE_MAPPED,

@@ -214,6 +229,41 @@ static void mem_cgroup_get(struct mem_cgroup *mem);
 static void mem_cgroup_put(struct mem_cgroup *mem);
 static struct mem_cgroup *parent_mem_cgroup(struct mem_cgroup *mem);
 
+static void mem_cgroup_insert_exceeded(struct mem_cgroup *mem)
+{
+	struct rb_node **p = &mem_cgroup_soft_limit_exceeded_groups.rb_node;
+	struct rb_node *parent = NULL;
+	struct mem_cgroup *mem_node;
+	unsigned long flags;
+
+	spin_lock_irqsave(&memcg_soft_limit_tree_lock, flags);
+	while (*p) {
+		parent = *p;
+		mem_node = rb_entry(parent, struct mem_cgroup, mem_cgroup_node);
+		if (mem->usage_in_excess < mem_node->usage_in_excess)
+			p = &(*p)->rb_left;
+		/*
+		 * We can't avoid mem cgroups that are over their soft
+		 * limit by the same amount
+		 */
+		else if (mem->usage_in_excess >= mem_node->usage_in_excess)
+			p = &(*p)->rb_right;
+	}
+	rb_link_node(&mem->mem_cgroup_node, parent, p);

mm/memcontrol.c static void mem_cgroup_get(struct mem_cgroup *mem); · line 257

Quoted 5 lines

mm/memcontrol.c +5 -0

+	rb_insert_color(&mem->mem_cgroup_node,
+			&mem_cgroup_soft_limit_exceeded_groups);
+	mem->last_tree_update = jiffies;
+	spin_unlock_irqrestore(&memcg_soft_limit_tree_lock, flags);
+}

I think this function is called from page fault hotpath, right? if so, you insert global lock into hotpath!

Quoted 28 lines

mm/memcontrol.c +20 -0

+
+static void mem_cgroup_remove_exceeded(struct mem_cgroup *mem)
+{
+	unsigned long flags;
+	spin_lock_irqsave(&memcg_soft_limit_tree_lock, flags);
+	rb_erase(&mem->mem_cgroup_node, &mem_cgroup_soft_limit_exceeded_groups);
+	spin_unlock_irqrestore(&memcg_soft_limit_tree_lock, flags);
+}
+
 static void mem_cgroup_charge_statistics(struct mem_cgroup *mem,
 					 struct page_cgroup *pc,
 					 bool charge)

@@ -897,6 +947,39 @@ static void record_last_oom(struct mem_cgroup *mem)
 	mem_cgroup_walk_tree(mem, NULL, record_last_oom_cb);
 }
 
+static void mem_cgroup_check_and_update_tree(struct mem_cgroup *mem,
+						bool time_check)
+{
+	unsigned long long prev_usage_in_excess, new_usage_in_excess;
+	bool updated_tree = false;
+	unsigned long next_update = 0;
+	unsigned long flags;
+
+	mem_cgroup_get(mem);
+	prev_usage_in_excess = mem->usage_in_excess;
+	new_usage_in_excess = res_counter_soft_limit_excess(&mem->res);

mm/memcontrol.c static void record_last_oom(struct mem_cgroup *mem) · line 965

Quoted 5 lines

mm/memcontrol.c +5 -0

+
+	if (time_check)
+		next_update = mem->last_tree_update +
+				MEM_CGROUP_TREE_UPDATE_INTERVAL;
+	if (new_usage_in_excess && time_after(jiffies, next_update)) {

incorrect time_after() usage. jiffies can round-tripping. then time_after(jiffies, 0) don't gurantee to return true.

Quoted 95 lines

mm/memcontrol.c +34 -4

+		if (prev_usage_in_excess)
+			mem_cgroup_remove_exceeded(mem);
+		mem_cgroup_insert_exceeded(mem);
+		updated_tree = true;
+	} else if (prev_usage_in_excess && !new_usage_in_excess) {
+		mem_cgroup_remove_exceeded(mem);
+		updated_tree = true;
+	}
+
+	if (updated_tree) {
+		spin_lock_irqsave(&memcg_soft_limit_tree_lock, flags);
+		mem->last_tree_update = jiffies;
+		mem->usage_in_excess = new_usage_in_excess;
+		spin_unlock_irqrestore(&memcg_soft_limit_tree_lock, flags);
+	}
+	mem_cgroup_put(mem);
+}
 
 /*
  * Unlike exported interface, "oom" parameter is added. if oom==true,

@@ -906,9 +989,9 @@ static int __mem_cgroup_try_charge(struct mm_struct *mm,
 			gfp_t gfp_mask, struct mem_cgroup **memcg,
 			bool oom)
 {
-	struct mem_cgroup *mem, *mem_over_limit;
+	struct mem_cgroup *mem, *mem_over_limit, *mem_over_soft_limit;
 	int nr_retries = MEM_CGROUP_RECLAIM_RETRIES;
-	struct res_counter *fail_res;
+	struct res_counter *fail_res, *soft_fail_res = NULL;
 
 	if (unlikely(test_thread_flag(TIF_MEMDIE))) {
 		/* Don't account this! */

@@ -938,12 +1021,13 @@ static int __mem_cgroup_try_charge(struct mm_struct *mm,
 		int ret;
 		bool noswap = false;
 
-		ret = res_counter_charge(&mem->res, PAGE_SIZE, &fail_res);
+		ret = res_counter_charge(&mem->res, PAGE_SIZE, &fail_res,
+						&soft_fail_res);
 		if (likely(!ret)) {
 			if (!do_swap_account)
 				break;
 			ret = res_counter_charge(&mem->memsw, PAGE_SIZE,
-							&fail_res);
+							&fail_res, NULL);
 			if (likely(!ret))
 				break;
 			/* mem+swap counter fails */

@@ -985,6 +1069,13 @@ static int __mem_cgroup_try_charge(struct mm_struct *mm,
 			goto nomem;
 		}
 	}
+
+	if (soft_fail_res) {
+		mem_over_soft_limit =
+			mem_cgroup_from_res_counter(soft_fail_res, res);
+		mem_cgroup_check_and_update_tree(mem_over_soft_limit, true);
+	}
+	mem_cgroup_check_and_update_tree(mem, true);
 	return 0;
 nomem:
 	css_put(&mem->css);

@@ -1422,6 +1513,7 @@ __mem_cgroup_uncharge_common(struct page *page, enum charge_type ctype)
 	mz = page_cgroup_zoneinfo(pc);
 	unlock_page_cgroup(pc);
 
+	mem_cgroup_check_and_update_tree(mem, true);
 	/* at swapout, this memcg will be accessed to record to swap */
 	if (ctype != MEM_CGROUP_CHARGE_TYPE_SWAPOUT)
 		css_put(&mem->css);

@@ -2346,6 +2438,7 @@ static void __mem_cgroup_free(struct mem_cgroup *mem)
 {
 	int node;
 
+	mem_cgroup_check_and_update_tree(mem, false);
 	free_css_id(&mem_cgroup_subsys, &mem->css);
 
 	for_each_node_state(node, N_POSSIBLE)

@@ -2412,6 +2505,7 @@ mem_cgroup_create(struct cgroup_subsys *ss, struct cgroup *cont)
 	if (cont->parent == NULL) {
 		enable_swap_cgroup();
 		parent = NULL;
+		mem_cgroup_soft_limit_exceeded_groups = RB_ROOT;
 	} else {
 		parent = mem_cgroup_from_cont(cont->parent);
 		mem->use_hierarchy = parent->use_hierarchy;

@@ -2432,6 +2526,8 @@ mem_cgroup_create(struct cgroup_subsys *ss, struct cgroup *cont)
 		res_counter_init(&mem->memsw, NULL);
 	}
 	mem->last_scanned_child = 0;
+	mem->usage_in_excess = 0;
+	mem->last_tree_update = 0;	/* Yes, time begins at 0 here */
 	spin_lock_init(&mem->reclaim_param_lock);
 

mm/memcontrol.c mem_cgroup_create(struct cgroup_subsys *ss, struct cgroup *cont) · line 2533

Quoted 2 lines

mm/memcontrol.c +0 -0

 	if (parent)

From: Balbir Singh <[email protected]>

Changelog v2...v1
1. Added support for hierarchical soft limits

This patch allows reclaim from memory cgroups on contention (via the __alloc_pages_internal() path). If a order greater than 0 is specified, we anyway fall back on try_to_free_pages().

memory cgroup soft limit reclaim finds the group that exceeds its soft limit by the largest amount and reclaims pages from it and then reinserts the cgroup into its correct place in the rbtree.

Signed-off-by Balbir Singh <[email protected]>

---

 include/linux/memcontrol.h |    1 
 mm/memcontrol.c            |  105 +++++++++++++++++++++++++++++++++++++++-----
 mm/page_alloc.c            |   10 ++++
 3 files changed, 104 insertions(+), 12 deletions(-)

include/linux/memcontrol.h +1 -0

index 18146c9..a50f73e 100644
@@ -116,6 +116,7 @@ static inline bool mem_cgroup_disabled(void)
 }
 
 extern bool mem_cgroup_oom_called(struct task_struct *task);
+extern unsigned long mem_cgroup_soft_limit_reclaim(gfp_t gfp_mask);
 
 #else /* CONFIG_CGROUP_MEM_RES_CTLR */
 struct mem_cgroup;

mm/memcontrol.c +1 -0

index a2617ac..dd835d3 100644
@@ -188,6 +188,7 @@ struct mem_cgroup {
 	struct rb_node mem_cgroup_node;
 	unsigned long long usage_in_excess;
 	unsigned long last_tree_update;
+	bool on_tree;
 
 	/*
 	 * statistics. This must be placed at the end of memcg.

mm/memcontrol.c +36 -4

 enum charge_type {
 	MEM_CGROUP_CHARGE_TYPE_CACHE = 0,

@@ -229,14 +230,15 @@ static void mem_cgroup_get(struct mem_cgroup *mem);
 static void mem_cgroup_put(struct mem_cgroup *mem);
 static struct mem_cgroup *parent_mem_cgroup(struct mem_cgroup *mem);
 
-static void mem_cgroup_insert_exceeded(struct mem_cgroup *mem)
+static void __mem_cgroup_insert_exceeded(struct mem_cgroup *mem)
 {
 	struct rb_node **p = &mem_cgroup_soft_limit_exceeded_groups.rb_node;
 	struct rb_node *parent = NULL;
 	struct mem_cgroup *mem_node;
-	unsigned long flags;
 
-	spin_lock_irqsave(&memcg_soft_limit_tree_lock, flags);
+	if (mem->on_tree)
+		return;
+
 	while (*p) {
 		parent = *p;
 		mem_node = rb_entry(parent, struct mem_cgroup, mem_cgroup_node);

@@ -253,6 +255,23 @@ static void mem_cgroup_insert_exceeded(struct mem_cgroup *mem)
 	rb_insert_color(&mem->mem_cgroup_node,
 			&mem_cgroup_soft_limit_exceeded_groups);
 	mem->last_tree_update = jiffies;
+	mem->on_tree = true;
+}
+
+static void __mem_cgroup_remove_exceeded(struct mem_cgroup *mem)
+{
+	if (!mem->on_tree)
+		return;
+	rb_erase(&mem->mem_cgroup_node, &mem_cgroup_soft_limit_exceeded_groups);
+	mem->on_tree = false;
+}
+
+static void mem_cgroup_insert_exceeded(struct mem_cgroup *mem)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&memcg_soft_limit_tree_lock, flags);
+	__mem_cgroup_insert_exceeded(mem);
 	spin_unlock_irqrestore(&memcg_soft_limit_tree_lock, flags);
 }
 

@@ -260,10 +279,34 @@ static void mem_cgroup_remove_exceeded(struct mem_cgroup *mem)
 {
 	unsigned long flags;
 	spin_lock_irqsave(&memcg_soft_limit_tree_lock, flags);
-	rb_erase(&mem->mem_cgroup_node, &mem_cgroup_soft_limit_exceeded_groups);
+	__mem_cgroup_remove_exceeded(mem);
 	spin_unlock_irqrestore(&memcg_soft_limit_tree_lock, flags);
 }
 
+static struct mem_cgroup *mem_cgroup_get_largest_soft_limit_exceeding_node(void)
+{
+	struct rb_node *rightmost = NULL;
+	struct mem_cgroup *mem = NULL;
+	unsigned long flags;
+
+	spin_lock_irqsave(&memcg_soft_limit_tree_lock, flags);
+	rightmost = rb_last(&mem_cgroup_soft_limit_exceeded_groups);
+	if (!rightmost)
+		goto done;		/* Nothing to reclaim from */
+
+	mem = rb_entry(rightmost, struct mem_cgroup, mem_cgroup_node);
+	mem_cgroup_get(mem);
+	/*

mm/memcontrol.c +0 -0

 {
 	struct mem_cgroup *victim;
 	int ret, total = 0;

mm/memcontrol.c +26 -4

 	}
 	return total;

@@ -1044,7 +1092,7 @@ static int __mem_cgroup_try_charge(struct mm_struct *mm,
 			goto nomem;
 
 		ret = mem_cgroup_hierarchical_reclaim(mem_over_limit, gfp_mask,
-							noswap, false);
+							noswap, false, false);
 		if (ret)
 			continue;
 

@@ -1686,7 +1734,7 @@ int mem_cgroup_shrink_usage(struct page *page,
 
 	do {
 		progress = mem_cgroup_hierarchical_reclaim(mem,
-					gfp_mask, true, false);
+					gfp_mask, true, false, false);
 		progress += mem_cgroup_check_under_limit(mem);
 	} while (!progress && --retry);
 

@@ -1741,7 +1789,7 @@ static int mem_cgroup_resize_limit(struct mem_cgroup *memcg,
 			break;
 
 		progress = mem_cgroup_hierarchical_reclaim(memcg, GFP_KERNEL,
-						   false, true);
+						   false, true, false);
 		curusage = res_counter_read_u64(&memcg->res, RES_USAGE);
 		/* Usage is reduced ? */
   		if (curusage >= oldusage)

@@ -1789,7 +1837,8 @@ int mem_cgroup_resize_memsw_limit(struct mem_cgroup *memcg,
 		if (!ret)
 			break;
 
-		mem_cgroup_hierarchical_reclaim(memcg, GFP_KERNEL, true, true);
+		mem_cgroup_hierarchical_reclaim(memcg, GFP_KERNEL, true, true,
+						false);
 		curusage = res_counter_read_u64(&memcg->memsw, RES_USAGE);
 		/* Usage is reduced ? */
 		if (curusage >= oldusage)

@@ -1940,6 +1989,38 @@ try_to_free:
 	goto out;
 }
 
+unsigned long mem_cgroup_soft_limit_reclaim(gfp_t gfp_mask)
+{
+	unsigned long nr_reclaimed = 0;
+	struct mem_cgroup *mem;
+	unsigned long flags;
+
+	do {
+		mem = mem_cgroup_get_largest_soft_limit_exceeding_node();
+		if (!mem)
+			break;
+		if (mem_cgroup_is_obsolete(mem)) {
+			mem_cgroup_put(mem);
+			continue;
+		}
+		nr_reclaimed +=
+			mem_cgroup_hierarchical_reclaim(mem, gfp_mask, false,
+							false, true);
+		spin_lock_irqsave(&memcg_soft_limit_tree_lock, flags);
+		mem->usage_in_excess = res_counter_soft_limit_excess(&mem->res);
+		/*
+		 * We need to remove and reinsert the node in its correct

mm/memcontrol.c +3 -0

+
 int mem_cgroup_force_empty_write(struct cgroup *cont, unsigned int event)
 {
 	return mem_cgroup_force_empty(mem_cgroup_from_cont(cont), true);

@@ -2528,6 +2609,8 @@ mem_cgroup_create(struct cgroup_subsys *ss, struct cgroup *cont)
 	mem->last_scanned_child = 0;
 	mem->usage_in_excess = 0;
 	mem->last_tree_update = 0;	/* Yes, time begins at 0 here */
+	mem->on_tree = false;
+
 	spin_lock_init(&mem->reclaim_param_lock);
 
 	if (parent)

mm/page_alloc.c +2 -1

index 7be9386..c50e29b 100644
@@ -1579,7 +1579,15 @@ nofail_alloc:
 	reclaim_state.reclaimed_slab = 0;
 	p->reclaim_state = &reclaim_state;
 
-	did_some_progress = try_to_free_pages(zonelist, order, gfp_mask);
+	did_some_progress = mem_cgroup_soft_limit_reclaim(gfp_mask);
+	/*

Quoted 51 lines

From: Balbir Singh <[email protected]>

Changelog v2...v1
1. Added support for hierarchical soft limits

This patch allows reclaim from memory cgroups on contention (via the __alloc_pages_internal() path). If a order greater than 0 is specified, we anyway fall back on try_to_free_pages().

memory cgroup soft limit reclaim finds the group that exceeds its soft limit by the largest amount and reclaims pages from it and then reinserts the cgroup into its correct place in the rbtree.

Signed-off-by Balbir Singh <[email protected]>

---

 include/linux/memcontrol.h |    1 
 mm/memcontrol.c            |  105 +++++++++++++++++++++++++++++++++++++++-----
 mm/page_alloc.c            |   10 ++++
 3 files changed, 104 insertions(+), 12 deletions(-)

include/linux/memcontrol.h +1 -0

index 18146c9..a50f73e 100644
@@ -116,6 +116,7 @@ static inline bool mem_cgroup_disabled(void)
 }
 
 extern bool mem_cgroup_oom_called(struct task_struct *task);
+extern unsigned long mem_cgroup_soft_limit_reclaim(gfp_t gfp_mask);
 
 #else /* CONFIG_CGROUP_MEM_RES_CTLR */
 struct mem_cgroup;

mm/memcontrol.c +1 -0

index a2617ac..dd835d3 100644
@@ -188,6 +188,7 @@ struct mem_cgroup {
 	struct rb_node mem_cgroup_node;
 	unsigned long long usage_in_excess;
 	unsigned long last_tree_update;
+	bool on_tree;
 
 	/*
 	 * statistics. This must be placed at the end of memcg.

@@ -195,7 +196,7 @@ struct mem_cgroup {
 	struct mem_cgroup_stat stat;
 };
 

mm/memcontrol.c struct mem_cgroup { · line 199

Quoted 2 lines

mm/memcontrol.c +1 -1

-#define	MEM_CGROUP_TREE_UPDATE_INTERVAL		(HZ)
+#define	MEM_CGROUP_TREE_UPDATE_INTERVAL		(HZ/4)

??
moving [3/4] is proper more?

Quoted 70 lines

mm/memcontrol.c +36 -4

 enum charge_type {
 	MEM_CGROUP_CHARGE_TYPE_CACHE = 0,

@@ -229,14 +230,15 @@ static void mem_cgroup_get(struct mem_cgroup *mem);
 static void mem_cgroup_put(struct mem_cgroup *mem);
 static struct mem_cgroup *parent_mem_cgroup(struct mem_cgroup *mem);
 
-static void mem_cgroup_insert_exceeded(struct mem_cgroup *mem)
+static void __mem_cgroup_insert_exceeded(struct mem_cgroup *mem)
 {
 	struct rb_node **p = &mem_cgroup_soft_limit_exceeded_groups.rb_node;
 	struct rb_node *parent = NULL;
 	struct mem_cgroup *mem_node;
-	unsigned long flags;
 
-	spin_lock_irqsave(&memcg_soft_limit_tree_lock, flags);
+	if (mem->on_tree)
+		return;
+
 	while (*p) {
 		parent = *p;
 		mem_node = rb_entry(parent, struct mem_cgroup, mem_cgroup_node);

@@ -253,6 +255,23 @@ static void mem_cgroup_insert_exceeded(struct mem_cgroup *mem)
 	rb_insert_color(&mem->mem_cgroup_node,
 			&mem_cgroup_soft_limit_exceeded_groups);
 	mem->last_tree_update = jiffies;
+	mem->on_tree = true;
+}
+
+static void __mem_cgroup_remove_exceeded(struct mem_cgroup *mem)
+{
+	if (!mem->on_tree)
+		return;
+	rb_erase(&mem->mem_cgroup_node, &mem_cgroup_soft_limit_exceeded_groups);
+	mem->on_tree = false;
+}
+
+static void mem_cgroup_insert_exceeded(struct mem_cgroup *mem)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&memcg_soft_limit_tree_lock, flags);
+	__mem_cgroup_insert_exceeded(mem);
 	spin_unlock_irqrestore(&memcg_soft_limit_tree_lock, flags);
 }
 

@@ -260,10 +279,34 @@ static void mem_cgroup_remove_exceeded(struct mem_cgroup *mem)
 {
 	unsigned long flags;
 	spin_lock_irqsave(&memcg_soft_limit_tree_lock, flags);
-	rb_erase(&mem->mem_cgroup_node, &mem_cgroup_soft_limit_exceeded_groups);
+	__mem_cgroup_remove_exceeded(mem);
 	spin_unlock_irqrestore(&memcg_soft_limit_tree_lock, flags);
 }
 
+static struct mem_cgroup *mem_cgroup_get_largest_soft_limit_exceeding_node(void)
+{
+	struct rb_node *rightmost = NULL;
+	struct mem_cgroup *mem = NULL;
+	unsigned long flags;
+
+	spin_lock_irqsave(&memcg_soft_limit_tree_lock, flags);
+	rightmost = rb_last(&mem_cgroup_soft_limit_exceeded_groups);
+	if (!rightmost)
+		goto done;		/* Nothing to reclaim from */
+
+	mem = rb_entry(rightmost, struct mem_cgroup, mem_cgroup_node);
+	mem_cgroup_get(mem);
+	/*

mm/memcontrol.c static void mem_cgroup_remove_exceeded(struct mem_cgroup *mem) · line 309

Quoted 10 lines

mm/memcontrol.c +10 -0

+	 * Remove the node now but someone else can add it back,
+	 * we will to add it back at the end of reclaim to its correct
+	 * position in the tree.
+	 */
+	__mem_cgroup_remove_exceeded(mem);
+done:
+	spin_unlock_irqrestore(&memcg_soft_limit_tree_lock, flags);
+	return mem;
+}
+

Do you remember we discuss about zone reclaim balancing thing in "reclaim bail out"thread at about 2 month ago?

The "largest exceeding" policy seems to have similar problems. if largest exceeding group is most active,

1. do the largest group activity and charge memory over softlimit.
2. reclaim memory from the largest group.
3. goto 1.

then, system can become livelock.

I think per-group priority is not good idea.

mm/memcontrol.c mem_cgroup_select_victim(struct mem_cgroup *root_mem) · line 933

Quoted 12 lines

mm/memcontrol.c +2 -1

 static void mem_cgroup_charge_statistics(struct mem_cgroup *mem,
 					 struct page_cgroup *pc,
 					 bool charge)

@@ -886,7 +929,8 @@ mem_cgroup_select_victim(struct mem_cgroup *root_mem)
  * If shrink==true, for avoiding to free too much, this returns immedieately.
  */
 static int mem_cgroup_hierarchical_reclaim(struct mem_cgroup *root_mem,
-				   gfp_t gfp_mask, bool noswap, bool shrink)
+				   gfp_t gfp_mask, bool noswap, bool shrink,
+				   bool check_soft)

Now, we get three boolean argument.
So, can we convert "int flags" argument?

I don't think mem_cgroup_hierarchical_reclaim(memcg, GFP_KERNEL, false, true, false) is self-described good look.

Quoted 4 lines

mm/memcontrol.c +0 -0

 {
 	struct mem_cgroup *victim;
 	int ret, total = 0;

mm/memcontrol.c static int mem_cgroup_hierarchical_reclaim(struct mem_cgroup *root_mem, · line 965

Quoted 10 lines

mm/memcontrol.c +5 -1

@@ -913,7 +957,11 @@ static int mem_cgroup_hierarchical_reclaim(struct mem_cgroup *root_mem,
 		if (shrink)
 			return ret;
 		total += ret;
-		if (mem_cgroup_check_under_limit(root_mem))
+
+		if (check_soft) {
+			if (res_counter_soft_limit_excess(&root_mem->res))
+				return 1 + total;
+		} else if (mem_cgroup_check_under_limit(root_mem))
 			return 1 + total;

I don't understand what's mean "1 +".

Quoted 64 lines

mm/memcontrol.c +26 -4

 	}
 	return total;

@@ -1044,7 +1092,7 @@ static int __mem_cgroup_try_charge(struct mm_struct *mm,
 			goto nomem;
 
 		ret = mem_cgroup_hierarchical_reclaim(mem_over_limit, gfp_mask,
-							noswap, false);
+							noswap, false, false);
 		if (ret)
 			continue;
 

@@ -1686,7 +1734,7 @@ int mem_cgroup_shrink_usage(struct page *page,
 
 	do {
 		progress = mem_cgroup_hierarchical_reclaim(mem,
-					gfp_mask, true, false);
+					gfp_mask, true, false, false);
 		progress += mem_cgroup_check_under_limit(mem);
 	} while (!progress && --retry);
 

@@ -1741,7 +1789,7 @@ static int mem_cgroup_resize_limit(struct mem_cgroup *memcg,
 			break;
 
 		progress = mem_cgroup_hierarchical_reclaim(memcg, GFP_KERNEL,
-						   false, true);
+						   false, true, false);
 		curusage = res_counter_read_u64(&memcg->res, RES_USAGE);
 		/* Usage is reduced ? */
   		if (curusage >= oldusage)

@@ -1789,7 +1837,8 @@ int mem_cgroup_resize_memsw_limit(struct mem_cgroup *memcg,
 		if (!ret)
 			break;
 
-		mem_cgroup_hierarchical_reclaim(memcg, GFP_KERNEL, true, true);
+		mem_cgroup_hierarchical_reclaim(memcg, GFP_KERNEL, true, true,
+						false);
 		curusage = res_counter_read_u64(&memcg->memsw, RES_USAGE);
 		/* Usage is reduced ? */
 		if (curusage >= oldusage)

@@ -1940,6 +1989,38 @@ try_to_free:
 	goto out;
 }
 
+unsigned long mem_cgroup_soft_limit_reclaim(gfp_t gfp_mask)
+{
+	unsigned long nr_reclaimed = 0;
+	struct mem_cgroup *mem;
+	unsigned long flags;
+
+	do {
+		mem = mem_cgroup_get_largest_soft_limit_exceeding_node();
+		if (!mem)
+			break;
+		if (mem_cgroup_is_obsolete(mem)) {
+			mem_cgroup_put(mem);
+			continue;
+		}
+		nr_reclaimed +=
+			mem_cgroup_hierarchical_reclaim(mem, gfp_mask, false,
+							false, true);
+		spin_lock_irqsave(&memcg_soft_limit_tree_lock, flags);
+		mem->usage_in_excess = res_counter_soft_limit_excess(&mem->res);
+		/*
+		 * We need to remove and reinsert the node in its correct

mm/memcontrol.c try_to_free: · line 2022

Quoted 10 lines

mm/memcontrol.c +10 -0

+		 * position
+		 */
+		__mem_cgroup_remove_exceeded(mem);
+		if (mem->usage_in_excess)
+			__mem_cgroup_insert_exceeded(mem);
+		spin_unlock_irqrestore(&memcg_soft_limit_tree_lock, flags);
+		mem_cgroup_put(mem);
+	} while (!nr_reclaimed);
+	return nr_reclaimed;
+}

this function is called from reclaim hotpath. but it grab glocal spin lock.. I don't like this.

Quoted 24 lines

mm/memcontrol.c +3 -0

+
 int mem_cgroup_force_empty_write(struct cgroup *cont, unsigned int event)
 {
 	return mem_cgroup_force_empty(mem_cgroup_from_cont(cont), true);

@@ -2528,6 +2609,8 @@ mem_cgroup_create(struct cgroup_subsys *ss, struct cgroup *cont)
 	mem->last_scanned_child = 0;
 	mem->usage_in_excess = 0;
 	mem->last_tree_update = 0;	/* Yes, time begins at 0 here */
+	mem->on_tree = false;
+
 	spin_lock_init(&mem->reclaim_param_lock);
 
 	if (parent)

mm/page_alloc.c +2 -1

index 7be9386..c50e29b 100644
@@ -1579,7 +1579,15 @@ nofail_alloc:
 	reclaim_state.reclaimed_slab = 0;
 	p->reclaim_state = &reclaim_state;
 
-	did_some_progress = try_to_free_pages(zonelist, order, gfp_mask);
+	did_some_progress = mem_cgroup_soft_limit_reclaim(gfp_mask);
+	/*

mm/page_alloc.c nofail_alloc: · line 1593

Quoted 10 lines

mm/page_alloc.c +7 -0

+	 * If we made no progress or need higher order allocations
+	 * try_to_free_pages() is still our best bet, since mem_cgroup
+	 * reclaim does not handle freeing pages greater than order 0
+	 */
+	if (!did_some_progress || order)
+		did_some_progress = try_to_free_pages(zonelist, order,
+							gfp_mask);
 
 	p->reclaim_state = NULL;
 	p->flags &= ~PF_MEMALLOC;

this is very freqently called place. then we want to
- if no memcgroup using, no performance regression.
- if no softlimit but using memcg, performance degression is smaller than 1%.

Do you have any performance number?

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

From: Balbir Singh <[email protected]>

Changelog v2...v1
1. Soft limits now support hierarchies
2. Use spinlocks instead of mutexes for synchronization of the RB tree

Here is v2 of the new soft limit implementation. Soft limits is a new feature for the memory resource controller, something similar has existed in the group scheduler in the form of shares. The CPU controllers interpretation of shares is very different though. We'll compare shares and soft limits below.

Soft limits are the most useful feature to have for environments where the administrator wants to overcommit the system, such that only on memory contention do the limits become active. The current soft limits implementation provides a soft_limit_in_bytes interface for the memory controller and not for memory+swap controller. The implementation maintains an RB-Tree of groups that exceed their soft limit and starts reclaiming from the group that exceeds this limit by the maximum amount.

This is an RFC implementation and is not meant for inclusion

Quoted 21 lines · On Mon, 16 Feb 2009 16:38:44 +0530 Balbir Singh <[email protected]>…

From: Balbir Singh <[email protected]>

Changelog v2...v1
1. Soft limits now support hierarchies
2. Use spinlocks instead of mutexes for synchronization of the RB tree

Here is v2 of the new soft limit implementation. Soft limits is a new feature for the memory resource controller, something similar has existed in the group scheduler in the form of shares. The CPU controllers interpretation of shares is very different though. We'll compare shares and soft limits below.

Soft limits are the most useful feature to have for environments where the administrator wants to overcommit the system, such that only on memory contention do the limits become active. The current soft limits implementation provides a soft_limit_in_bytes interface for the memory controller and not for memory+swap controller. The implementation maintains an RB-Tree of groups that exceed their soft limit and starts reclaiming from the group that exceeds this limit by the maximum amount.

Quoted 2 lines

This is an RFC implementation and is not meant for inclusion

some thoughts after reading patch.

1. As I pointed out, cpuset/mempolicy case is not handled yet.

2. I don't like to change usual direct-memory-reclaim path. It will be obstacles for VM-maintaners to improve memory reclaim. memcg's LRU is designed for shrinking memory usage and not for avoiding memory shortage. IOW, it's slow routine for reclaiming memory for memory shortage.

3. After this patch, res_counter is no longer for general purpose res_counter... It seems to have too many unnecessary accessories for general purpose.

4. please use css_tryget() rather than mem_cgroup_get().

5. please remove mem_cgroup from tree at force_empty or rmdir. Just making memcg->on_tree=false is enough ? I'm in doubt.

6. What happens when the-largest-soft-limit-memcg has tons on Anon on swapless system and memory reclaim cannot make enough progress ?

Quoted 24 lines · KAMEZAWA Hiroyuki

Quoted 21 lines · On Mon, 16 Feb 2009 16:38:44 +0530 Balbir Singh <[email protected]>…

From: Balbir Singh <[email protected]>

Changelog v2...v1
1. Soft limits now support hierarchies
2. Use spinlocks instead of mutexes for synchronization of the RB tree

Here is v2 of the new soft limit implementation. Soft limits is a new feature for the memory resource controller, something similar has existed in the group scheduler in the form of shares. The CPU controllers interpretation of shares is very different though. We'll compare shares and soft limits below.

Soft limits are the most useful feature to have for environments where the administrator wants to overcommit the system, such that only on memory contention do the limits become active. The current soft limits implementation provides a soft_limit_in_bytes interface for the memory controller and not for memory+swap controller. The implementation maintains an RB-Tree of groups that exceed their soft limit and starts reclaiming from the group that exceeds this limit by the maximum amount.

Quoted 6 lines

Quoted 2 lines

This is an RFC implementation and is not meant for inclusion

some thoughts after reading patch.

1. As I pointed out, cpuset/mempolicy case is not handled yet.

That should be esy to do with zonelists passed from reclaim path

Quoted 5 lines

2. I don't like to change usual direct-memory-reclaim path. It will be obstacles for VM-maintaners to improve memory reclaim. memcg's LRU is designed for shrinking memory usage and not for avoiding memory shortage. IOW, it's slow routine for reclaiming memory for memory shortage.

I don't think I agree here. Direct reclaim is the first indication of shortage and if order 0 pages are short, memcg's above their soft limit can be targetted first.

Quoted 2 lines

3. After this patch, res_counter is no longer for general purpose res_counter... It seems to have too many unnecessary accessories for general purpose.

Why not? Soft limits are a feature of any controller. The return of highest ancestor might be the only policy we impose right now. But as new controllers start using res_counter, we can clearly add a policy callback.

Quoted 9 lines

Quoted 4 lines

2. I don't like to change usual direct-memory-reclaim path. It will be obstacles for VM-maintaners to improve memory reclaim. memcg's LRU is designed for shrinking memory usage and not for avoiding memory shortage. IOW, it's slow routine for reclaiming memory for memory shortage.

I don't think I agree here. Direct reclaim is the first indication of shortage and if order 0 pages are short, memcg's above their soft limit can be targetted first.

My "slow" means "the overhead seems to be big". The latency will increase.

About 0-order
In patch 4/4

(quoted diff) +2 -0

+	did_some_progress = mem_cgroup_soft_limit_reclaim(gfp_mask);
+	/*

should be

        if (!order)
            did_some_progress = mem....

Quoted 16 lines · On Tue, 17 Feb 2009 13:03:52 +0900 KAMEZAWA Hiroyuki…

Quoted 9 lines

Quoted 4 lines

2. I don't like to change usual direct-memory-reclaim path. It will be obstacles for VM-maintaners to improve memory reclaim. memcg's LRU is designed for shrinking memory usage and not for avoiding memory shortage. IOW, it's slow routine for reclaiming memory for memory shortage.

I don't think I agree here. Direct reclaim is the first indication of shortage and if order 0 pages are short, memcg's above their soft limit can be targetted first.

My "slow" means "the overhead seems to be big". The latency will increase.

About 0-order
In patch 4/4

(quoted diff) +2 -0

+	did_some_progress = mem_cgroup_soft_limit_reclaim(gfp_mask);
+	/*

should be

Quoted 2 lines

        if (!order)
            did_some_progress = mem....

above is wrong.

if (!order && (gfp_mask & GFP_MOVABLE)) ....Hmm, but this is not correct. I have no good idea to avoid unnecessary works.

BTW, why don't you call soft_limit_reclaim from kswapd's path ?

Quoted 26 lines · On Tue, 17 Feb 2009 08:35:26 +0530 Balbir Singh <[email protected]>…

Quoted 24 lines · KAMEZAWA Hiroyuki

Quoted 21 lines · On Mon, 16 Feb 2009 16:38:44 +0530 Balbir Singh <[email protected]>…

From: Balbir Singh <[email protected]>

Changelog v2...v1
1. Soft limits now support hierarchies
2. Use spinlocks instead of mutexes for synchronization of the RB tree

Here is v2 of the new soft limit implementation. Soft limits is a new feature for the memory resource controller, something similar has existed in the group scheduler in the form of shares. The CPU controllers interpretation of shares is very different though. We'll compare shares and soft limits below.

Soft limits are the most useful feature to have for environments where the administrator wants to overcommit the system, such that only on memory contention do the limits become active. The current soft limits implementation provides a soft_limit_in_bytes interface for the memory controller and not for memory+swap controller. The implementation maintains an RB-Tree of groups that exceed their soft limit and starts reclaiming from the group that exceeds this limit by the maximum amount.

Quoted 9 lines

Quoted 6 lines

Quoted 2 lines

This is an RFC implementation and is not meant for inclusion

some thoughts after reading patch.

1. As I pointed out, cpuset/mempolicy case is not handled yet.

That should be esy to do with zonelists passed from reclaim path

plz do.

Quoted 10 lines

Quoted 5 lines

2. I don't like to change usual direct-memory-reclaim path. It will be obstacles for VM-maintaners to improve memory reclaim. memcg's LRU is designed for shrinking memory usage and not for avoiding memory shortage. IOW, it's slow routine for reclaiming memory for memory shortage.

I don't think I agree here. Direct reclaim is the first indication of shortage and if order 0 pages are short, memcg's above their soft limit can be targetted first.

My "slow" means "the overhead seems to be big". The latency will increase.

About 0-order
In patch 4/4

(quoted diff) +2 -0

+	did_some_progress = mem_cgroup_soft_limit_reclaim(gfp_mask);
+	/*

should be

        if (!order)
            did_some_progress = mem....

I don't want to add any new big burden to kernel hackers of memory management, they work hard to improve memory reclaim. This patch will change the behavior.

BTW, in typical bad case, several threads on cpus goes into memory recalim at once and all thread will visit this memcg's soft-limit tree at once and soft-limit will not work as desired anyway.
You can't avoid this problem at alloc_page() hot-path.

Quoted 4 lines

Quoted 3 lines

3. After this patch, res_counter is no longer for general purpose res_counter... It seems to have too many unnecessary accessories for general purpose.

Quoted 5 lines

Why not? Soft limits are a feature of any controller. The return of highest ancestor might be the only policy we impose right now. But as new controllers start using res_counter, we can clearly add a policy callback.

I think you forget that memroy cgroups is an only controller in which the kernel can reduce the usage of resource without any harmful to users. soft-limit is nonsense for general resources, I think.

Quoted 5 lines · KAMEZAWA Hiroyuki

On Tue, 17 Feb 2009 08:35:26 +0530
Balbir Singh <[email protected]> wrote:
I don't want to add any new big burden to kernel hackers of memory management, they work hard to improve memory reclaim. This patch will change the behavior.

I don't think I agree, this approach suggests that before doing global reclaim, there are several groups that are using more than their share of memory, so it makes sense to reclaim from them first.

Quoted 4 lines

BTW, in typical bad case, several threads on cpus goes into memory recalim at once and all thread will visit this memcg's soft-limit tree at once and soft-limit will not work as desired anyway.
You can't avoid this problem at alloc_page() hot-path.

Even if all threads go into soft-reclaim at once, the tree will become empty after a point and we will just return saying there are no more memcg's to reclaim from (we remove the memcg from the tree when reclaiming), then those threads will go into regular reclaim if there is still memory pressure.

Quoted 9 lines

Quoted 9 lines

Quoted 3 lines

3. After this patch, res_counter is no longer for general purpose res_counter... It seems to have too many unnecessary accessories for general purpose.

Why not? Soft limits are a feature of any controller. The return of highest ancestor might be the only policy we impose right now. But as new controllers start using res_counter, we can clearly add a policy callback.

Quoted 4 lines

I think you forget that memroy cgroups is an only controller in which the kernel can reduce the usage of resource without any harmful to users. soft-limit is nonsense for general resources, I think.

Really? Even for CPUs? soft-limit is a form of shares (please don't confuse with cpu.shares). Soft limits is used as a way of implementing work conserving controllers.

Quoted 19 lines · On Tue, 17 Feb 2009 10:11:10 +0530 Balbir Singh <[email protected]>…

Quoted 5 lines · KAMEZAWA Hiroyuki

On Tue, 17 Feb 2009 08:35:26 +0530
Balbir Singh <[email protected]> wrote:
I don't want to add any new big burden to kernel hackers of memory management, they work hard to improve memory reclaim. This patch will change the behavior.

I don't think I agree, this approach suggests that before doing global reclaim, there are several groups that are using more than their share of memory, so it makes sense to reclaim from them first.

Quoted 4 lines

BTW, in typical bad case, several threads on cpus goes into memory recalim at once and all thread will visit this memcg's soft-limit tree at once and soft-limit will not work as desired anyway.
You can't avoid this problem at alloc_page() hot-path.

Quoted 5 lines

Even if all threads go into soft-reclaim at once, the tree will become empty after a point and we will just return saying there are no more memcg's to reclaim from (we remove the memcg from the tree when reclaiming), then those threads will go into regular reclaim if there is still memory pressure.

Yes. the largest-excess group will be removed. So, it seems that it doesn't work as designed. rbtree is considered as just a hint ? If so, rbtree seems to be overkill.

just a question:
Assume memcg under hierarchy.

   ../group_A/                 usage=1G, soft_limit=900M  hierarchy=1
              01/              usage=200M, soft_limit=100M
              02/              usage=300M, soft_limit=200M
              03/              usage=500M, soft_limit=300M  <==== 200M over.
                 004/          usage=200M, soft_limit=100M
                 005/          usage=300M, soft_limit=200M

At memory shortage, group 03's memory will be reclaimed
- reclaim memory from 03, 03/004, 03/005

When 100M of group 03' memory is reclaimed, group_A 's memory is reclaimd at the same time, implicitly. Doesn't this break your rb-tree ?

I recommend you that soft-limit can be only applied to the node which is top of hierarchy.

Quoted 15 lines

Quoted 14 lines

Quoted 10 lines

Quoted 4 lines

3. After this patch, res_counter is no longer for general purpose res_counter... It seems to have too many unnecessary accessories for general purpose.

Why not? Soft limits are a feature of any controller. The return of highest ancestor might be the only policy we impose right now. But as new controllers start using res_counter, we can clearly add a policy callback.

I think you forget that memroy cgroups is an only controller in which the kernel can reduce the usage of resource without any harmful to users. soft-limit is nonsense for general resources, I think.

Quoted 4 lines

Really? Even for CPUs? soft-limit is a form of shares (please don't confuse with cpu.shares). Soft limits is used as a way of implementing work conserving controllers.

I don't think cpu needs this. It works under share and no hardlimit.

Quoted 44 lines · KAMEZAWA Hiroyuki

Quoted 19 lines · On Tue, 17 Feb 2009 10:11:10 +0530 Balbir Singh <[email protected]>…

Quoted 5 lines · KAMEZAWA Hiroyuki

On Tue, 17 Feb 2009 08:35:26 +0530
Balbir Singh <[email protected]> wrote:
I don't want to add any new big burden to kernel hackers of memory management, they work hard to improve memory reclaim. This patch will change the behavior.

I don't think I agree, this approach suggests that before doing global reclaim, there are several groups that are using more than their share of memory, so it makes sense to reclaim from them first.

Quoted 4 lines

BTW, in typical bad case, several threads on cpus goes into memory recalim at once and all thread will visit this memcg's soft-limit tree at once and soft-limit will not work as desired anyway.
You can't avoid this problem at alloc_page() hot-path.

Quoted 5 lines

Even if all threads go into soft-reclaim at once, the tree will become empty after a point and we will just return saying there are no more memcg's to reclaim from (we remove the memcg from the tree when reclaiming), then those threads will go into regular reclaim if there is still memory pressure.

Yes. the largest-excess group will be removed. So, it seems that it doesn't work as designed. rbtree is considered as just a hint ? If so, rbtree seems to be overkill.

just a question:
Assume memcg under hierarchy.

   ../group_A/                 usage=1G, soft_limit=900M  hierarchy=1
              01/              usage=200M, soft_limit=100M
              02/              usage=300M, soft_limit=200M
              03/              usage=500M, soft_limit=300M  <==== 200M over.
                 004/          usage=200M, soft_limit=100M
                 005/          usage=300M, soft_limit=200M

At memory shortage, group 03's memory will be reclaimed
- reclaim memory from 03, 03/004, 03/005

Quoted 5 lines

When 100M of group 03' memory is reclaimed, group_A 's memory is reclaimd at the same time, implicitly. Doesn't this break your rb-tree ?

I recommend you that soft-limit can be only applied to the node which is top of hierarchy.

Yes, that can be done, but the reason for putting both was to target the right memcg early.

Quoted 49 lines · On Tue, 17 Feb 2009 11:09:03 +0530 Balbir Singh <[email protected]>…

Quoted 47 lines · KAMEZAWA Hiroyuki

Quoted 19 lines · On Tue, 17 Feb 2009 10:11:10 +0530 Balbir Singh <[email protected]>…

Quoted 5 lines · KAMEZAWA Hiroyuki

On Tue, 17 Feb 2009 08:35:26 +0530
Balbir Singh <[email protected]> wrote:
I don't want to add any new big burden to kernel hackers of memory management, they work hard to improve memory reclaim. This patch will change the behavior.

I don't think I agree, this approach suggests that before doing global reclaim, there are several groups that are using more than their share of memory, so it makes sense to reclaim from them first.

Quoted 4 lines

BTW, in typical bad case, several threads on cpus goes into memory recalim at once and all thread will visit this memcg's soft-limit tree at once and soft-limit will not work as desired anyway.
You can't avoid this problem at alloc_page() hot-path.

Quoted 5 lines

Even if all threads go into soft-reclaim at once, the tree will become empty after a point and we will just return saying there are no more memcg's to reclaim from (we remove the memcg from the tree when reclaiming), then those threads will go into regular reclaim if there is still memory pressure.

Yes. the largest-excess group will be removed. So, it seems that it doesn't work as designed. rbtree is considered as just a hint ? If so, rbtree seems to be overkill.

just a question:
Assume memcg under hierarchy.

   ../group_A/                 usage=1G, soft_limit=900M  hierarchy=1
              01/              usage=200M, soft_limit=100M
              02/              usage=300M, soft_limit=200M
              03/              usage=500M, soft_limit=300M  <==== 200M over.
                 004/          usage=200M, soft_limit=100M
                 005/          usage=300M, soft_limit=200M

At memory shortage, group 03's memory will be reclaimed
- reclaim memory from 03, 03/004, 03/005

When 100M of group 03' memory is reclaimed, group_A 's memory is reclaimd at the same time, implicitly. Doesn't this break your rb-tree ?

Quoted 6 lines

Quoted 2 lines

I recommend you that soft-limit can be only applied to the node which is top of hierarchy.

Yes, that can be done, but the reason for putting both was to target the right memcg early.

My point is that sort by rb-tree is broken in above case.