memcg: prevent endless loop when charging huge pages to near-limit group

If reclaim after a failed charging was unsuccessful, the limits are checked again, just in case they settled by means of other tasks. This is all fine as long as every charge is of size PAGE_SIZE, because in that case, being below the limit means having at least PAGE_SIZE bytes available. But with transparent huge pages, we may end up in an endless loop where charging and reclaim fail, but we keep going because the limits are not yet exceeded, although not allowing for a huge page. Fix this up by explicitely checking for enough room, not just whether we are within limits. Signed-off-by: Johannes Weiner <hannes@cmpxchg.org> Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Reviewed-by: Minchan Kim <minchan.kim@gmail.com> Cc: Balbir Singh <balbir@linux.vnet.ibm.com> Cc: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

memcg: prevent endless loop when charging huge pages to near-limit group
If reclaim after a failed charging was unsuccessful, the limits are checked again, just in case they settled by means of other tasks. This is all fine as long as every charge is of size PAGE_SIZE, because in that case, being below the limit means having at least PAGE_SIZE bytes available. But with transparent huge pages, we may end up in an endless loop where charging and reclaim fail, but we keep going because the limits are not yet exceeded, although not allowing for a huge page. Fix this up by explicitely checking for enough room, not just whether we are within limits. Signed-off-by: Johannes Weiner <hannes@cmpxchg.org> Acked-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> Reviewed-by: Minchan Kim <minchan.kim@gmail.com> Cc: Balbir Singh <balbir@linux.vnet.ibm.com> Cc: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> Signed-off-by: Andrew Morton <akpm@linux-foundation.org> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Johannes Weiner · Linus Torvalds
1 parent 9221edb712
Showing 2 changed files with 48 additions and 7 deletions Side-by-side Diff
include/linux/res_counter.h
mm/memcontrol.c
@@ -182,6 +182,26 @@
 	return ret;
 }
  
+/**
+ * res_counter_check_margin - check if the counter allows charging
+ * @cnt: the resource counter to check
+ * @bytes: the number of bytes to check the remaining space against
+ *
+ * Returns a boolean value on whether the counter can be charged
+ * @bytes or whether this would exceed the limit.
+ */
+static inline bool res_counter_check_margin(struct res_counter *cnt,
+					    unsigned long bytes)
+{
+	bool ret;
+	unsigned long flags;
+
+	spin_lock_irqsave(&cnt->lock, flags);
+	ret = cnt->limit - cnt->usage >= bytes;
+	spin_unlock_irqrestore(&cnt->lock, flags);
+	return ret;
+}
+
 static inline bool res_counter_check_under_soft_limit(struct res_counter *cnt)
 {
 	bool ret;
@@ -1111,6 +1111,23 @@
 	return false;
 }
  
+/**
+ * mem_cgroup_check_margin - check if the memory cgroup allows charging
+ * @mem: memory cgroup to check
+ * @bytes: the number of bytes the caller intends to charge
+ *
+ * Returns a boolean value on whether @mem can be charged @bytes or
+ * whether this would exceed the limit.
+ */
+static bool mem_cgroup_check_margin(struct mem_cgroup *mem, unsigned long bytes)
+{
+	if (!res_counter_check_margin(&mem->res, bytes))
+		return false;
+	if (do_swap_account && !res_counter_check_margin(&mem->memsw, bytes))
+		return false;
+	return true;
+}
+
 static unsigned int get_swappiness(struct mem_cgroup *memcg)
 {
 	struct cgroup *cgrp = memcg->css.cgroup;
  
  
@@ -1852,15 +1869,19 @@
 		return CHARGE_WOULDBLOCK;
  
 	ret = mem_cgroup_hierarchical_reclaim(mem_over_limit, NULL,
-					gfp_mask, flags);
+					      gfp_mask, flags);
+	if (mem_cgroup_check_margin(mem_over_limit, csize))
+		return CHARGE_RETRY;
 	/*
-	 * try_to_free_mem_cgroup_pages() might not give us a full
-	 * picture of reclaim. Some pages are reclaimed and might be
-	 * moved to swap cache or just unmapped from the cgroup.
-	 * Check the limit again to see if the reclaim reduced the
-	 * current usage of the cgroup before giving up
+	 * Even though the limit is exceeded at this point, reclaim
+	 * may have been able to free some pages.  Retry the charge
+	 * before killing the task.
+	 *
+	 * Only for regular pages, though: huge pages are rather
+	 * unlikely to succeed so close to the limit, and we fall back
+	 * to regular pages anyway in case of failure.
 	 */
-	if (ret || mem_cgroup_check_under_limit(mem_over_limit))
+	if (csize == PAGE_SIZE && ret)
 		return CHARGE_RETRY;
  
 	/*
...	...	@@ -182,6 +182,26 @@
182	182	return ret;
183	183	}
184	184
	185	+/**
	186	+ * res_counter_check_margin - check if the counter allows charging
	187	+ * @cnt: the resource counter to check
	188	+ * @bytes: the number of bytes to check the remaining space against
	189	+ *
	190	+ * Returns a boolean value on whether the counter can be charged
	191	+ * @bytes or whether this would exceed the limit.
	192	+ */
	193	+static inline bool res_counter_check_margin(struct res_counter *cnt,
	194	+ unsigned long bytes)
	195	+{
	196	+ bool ret;
	197	+ unsigned long flags;
	198	+
	199	+ spin_lock_irqsave(&cnt->lock, flags);
	200	+ ret = cnt->limit - cnt->usage >= bytes;
	201	+ spin_unlock_irqrestore(&cnt->lock, flags);
	202	+ return ret;
	203	+}
	204	+
185	205	static inline bool res_counter_check_under_soft_limit(struct res_counter *cnt)
186	206	{
187	207	bool ret;
...	...	@@ -1111,6 +1111,23 @@
1111	1111	return false;
1112	1112	}
1113	1113
	1114	+/**
	1115	+ * mem_cgroup_check_margin - check if the memory cgroup allows charging
	1116	+ * @mem: memory cgroup to check
	1117	+ * @bytes: the number of bytes the caller intends to charge
	1118	+ *
	1119	+ * Returns a boolean value on whether @mem can be charged @bytes or
	1120	+ * whether this would exceed the limit.
	1121	+ */
	1122	+static bool mem_cgroup_check_margin(struct mem_cgroup *mem, unsigned long bytes)
	1123	+{
	1124	+ if (!res_counter_check_margin(&mem->res, bytes))
	1125	+ return false;
	1126	+ if (do_swap_account && !res_counter_check_margin(&mem->memsw, bytes))
	1127	+ return false;
	1128	+ return true;
	1129	+}
	1130	+
1114	1131	static unsigned int get_swappiness(struct mem_cgroup *memcg)
1115	1132	{
1116	1133	struct cgroup *cgrp = memcg->css.cgroup;
1117	1134
1118	1135
...	...	@@ -1852,15 +1869,19 @@
1852	1869	return CHARGE_WOULDBLOCK;
1853	1870
1854	1871	ret = mem_cgroup_hierarchical_reclaim(mem_over_limit, NULL,
1855		- gfp_mask, flags);
	1872	+ gfp_mask, flags);
	1873	+ if (mem_cgroup_check_margin(mem_over_limit, csize))
	1874	+ return CHARGE_RETRY;
1856	1875	/*
1857		- * try_to_free_mem_cgroup_pages() might not give us a full
1858		- * picture of reclaim. Some pages are reclaimed and might be
1859		- * moved to swap cache or just unmapped from the cgroup.
1860		- * Check the limit again to see if the reclaim reduced the
1861		- * current usage of the cgroup before giving up
	1876	+ * Even though the limit is exceeded at this point, reclaim
	1877	+ * may have been able to free some pages. Retry the charge
	1878	+ * before killing the task.
	1879	+ *
	1880	+ * Only for regular pages, though: huge pages are rather
	1881	+ * unlikely to succeed so close to the limit, and we fall back
	1882	+ * to regular pages anyway in case of failure.
1862	1883	*/
1863		- if (ret \|\| mem_cgroup_check_under_limit(mem_over_limit))
	1884	+ if (csize == PAGE_SIZE && ret)
1864	1885	return CHARGE_RETRY;
1865	1886
1866	1887	/*