Skip to contentSkip to conversation

mm: memcontrol: fix cgroup creation failure after many small jobs

PATCH In review Johannes Weiner 4 participants 14 messages Jun 16, 2016 – Jul 14, 2016

Subsystems (4)
lore
Share & tools
lore

Johannes Weiner

author maintainer
#0 lore raw

PATCH 1/1 mm: memcontrol: fix cgroup creation failure after many small jobs

The memory controller has quite a bit of state that usually outlives the cgroup and pins its CSS until said state disappears. At the same time it imposes a 16-bit limit on the CSS ID space to economically store IDs in the wild. Consequently, when we use cgroups to contain frequent but small and short-lived jobs that leave behind some page cache, we quickly run into the 64k limitations of outstanding CSSs. Creating a new cgroup fails with -ENOSPC while there are only a few, or even no user-visible cgroups in existence.

Although pinning CSSs past cgroup removal is common, there are only two instances that actually need a CSS ID after a cgroup is deleted:
cache shadow entries and swapout records.

Cache shadow entries reference the ID weakly and can deal with the CSS having disappeared when it's looked up later. They pose no hurdle.

Swap-out records do need to pin the css to hierarchically attribute swapins after the cgroup has been deleted; though the only pages that remain swapped out after a process exits are tmpfs/shmem pages. Those references are under the user's control and thus manageable.

This patch introduces a private 16bit memcg ID and switches swap and cache shadow entries over to using that. It then decouples the CSS lifetime from the CSS ID lifetime, such that a CSS ID can be recycled when the CSS is only pinned by common objects that don't need an ID.

This script demonstrates the problem by faulting one cache page in a new cgroup and deleting it again:

set -e
mkdir -p pages
for x in seq 128000; do
[ $((x % 1000)) -eq 0 ] && echo $x
mkdir /cgroup/foo
echo $$ >/cgroup/foo/cgroup.procs
echo trex >pages/$x
echo $$ >/cgroup/cgroup.procs
rmdir /cgroup/foo
done

When run on an unpatched kernel, we eventually run out of possible CSS IDs even though there is no visible cgroup existing anymore:

[root@ham ~]# ./cssidstress.sh
[...]
65000
mkdir: cannot create directory '/cgroup/foo': No space left on device

After this patch, the CSS IDs get released upon cgroup destruction and the cache and css objects get released once memory reclaim kicks in.

---
 include/linux/cgroup.h     |  3 ++-
 include/linux/memcontrol.h | 25 +++++++++------------
 kernel/cgroup.c            | 22 ++++++++++++++++--
 mm/memcontrol.c            | 56 ++++++++++++++++++++++++++++++++++++++++------
 mm/slab_common.c           |  4 ++--
 5 files changed, 83 insertions(+), 27 deletions(-)
include/linux/cgroup.h +2 -1
index a20320c666fd..6510bf291d36 100644
@@ -85,9 +85,10 @@ struct cgroup_subsys_state *cgroup_get_e_css(struct cgroup *cgroup,
 					     struct cgroup_subsys *ss);
 struct cgroup_subsys_state *css_tryget_online_from_dir(struct dentry *dentry,
 						       struct cgroup_subsys *ss);
-
 struct cgroup *cgroup_get_from_path(const char *path);
 
+void css_id_free(struct cgroup_subsys_state *css);
+
 int cgroup_attach_task_all(struct task_struct *from, struct task_struct *);
 int cgroup_transfer_tasks(struct cgroup *to, struct cgroup *from);
 
include/linux/memcontrol.h +10 -15
index a805474df4ab..56e6069d2452 100644
@@ -97,6 +97,11 @@ enum mem_cgroup_events_target {
 #define MEM_CGROUP_ID_SHIFT	16
 #define MEM_CGROUP_ID_MAX	USHRT_MAX
 
+struct mem_cgroup_id {
+	int id;
+	atomic_t ref;
+};
+
 struct mem_cgroup_stat_cpu {
 	long count[MEMCG_NR_STAT];
 	unsigned long events[MEMCG_NR_EVENTS];
@@ -172,6 +177,9 @@ enum memcg_kmem_state {
 struct mem_cgroup {
 	struct cgroup_subsys_state css;
 
+	/* Private memcg ID. Used to ID objects that outlive the cgroup */
+	struct mem_cgroup_id id;
+
 	/* Accounted resources */
 	struct page_counter memory;
 	struct page_counter swap;
@@ -330,22 +338,9 @@ static inline unsigned short mem_cgroup_id(struct mem_cgroup *memcg)
 	if (mem_cgroup_disabled())
 		return 0;
 
-	return memcg->css.id;
-}
-
-/**
- * mem_cgroup_from_id - look up a memcg from an id
- * @id: the id to look up
- *
- * Caller must hold rcu_read_lock() and use css_tryget() as necessary.
- */
-static inline struct mem_cgroup *mem_cgroup_from_id(unsigned short id)
-{
-	struct cgroup_subsys_state *css;
-
-	css = css_from_id(id, &memory_cgrp_subsys);
-	return mem_cgroup_from_css(css);
+	return memcg->id.id;
 }
+struct mem_cgroup *mem_cgroup_from_id(unsigned short id);
 
 /**
  * parent_mem_cgroup - find the accounting parent of a memcg
kernel/cgroup.c +20 -2
index 86cb5c6e8932..2e4aff6fd6ec 100644
@@ -4961,10 +4961,10 @@ static void css_free_work_fn(struct work_struct *work)
 	if (ss) {
 		/* css free path */
 		struct cgroup_subsys_state *parent = css->parent;
-		int id = css->id;
 
 		ss->css_free(css);
-		cgroup_idr_remove(&ss->css_idr, id);
+		if (css->id)
+			cgroup_idr_remove(&ss->css_idr, css->id);
 		cgroup_put(cgrp);
 
 		if (parent)
@@ -6205,6 +6205,24 @@ struct cgroup *cgroup_get_from_path(const char *path)
 }
 EXPORT_SYMBOL_GPL(cgroup_get_from_path);
 
+/**
+ * css_id_free - relinquish an existing CSS's ID
+ * @css: the CSS
+ *
+ * This releases the @css's ID and allows it to be recycled while the
+ * CSS continues to exist. This is useful for controllers with state
+ * that extends past a cgroup's lifetime but doesn't need precious ID
+ * address space.
+ *
+ * This invalidates @css->id, and css_from_id() might return NULL or a
+ * new css if the ID has been recycled in the meantime.
+ */
+void css_id_free(struct cgroup_subsys_state *css)
+{
+	cgroup_idr_remove(&css->ss->css_idr, css->id);
+	css->id = 0;
+}
+
 /*
  * sock->sk_cgrp_data handling.  For more info, see sock_cgroup_data
  * definition in cgroup-defs.h.
mm/memcontrol.c +49 -7

117 lines not on this page · raw message

mm/slab_common.c +2 -2
index a65dad7fdcd1..82317abb03ed 100644
@@ -526,8 +526,8 @@ void memcg_create_kmem_cache(struct mem_cgroup *memcg,
 		goto out_unlock;
 
 	cgroup_name(css->cgroup, memcg_name_buf, sizeof(memcg_name_buf));
-	cache_name = kasprintf(GFP_KERNEL, "%s(%d:%s)", root_cache->name,
-			       css->id, memcg_name_buf);
+	cache_name = kasprintf(GFP_KERNEL, "%s(%llu:%s)", root_cache->name,
+			       css->serial_nr, memcg_name_buf);
 	if (!cache_name)
 		goto out_unlock;
 
Signature · 7 lines
2.8.3

--
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>
maintainer
#1 lore raw

Hello,

Looks generally good to me. Some comments below.

Quoted 11 lines · Johannes Weiner
kernel/cgroup.c +7 -0
@@ -6205,6 +6205,24 @@ struct cgroup *cgroup_get_from_path(const char *path)
 }
 EXPORT_SYMBOL_GPL(cgroup_get_from_path);
 
+/**
+ * css_id_free - relinquish an existing CSS's ID
+ * @css: the CSS
+ *
+ * This releases the @css's ID and allows it to be recycled while the
+ * CSS continues to exist. This is useful for controllers with state
+ * that extends past a cgroup's lifetime but doesn't need precious ID

kernel/cgroup.c struct cgroup *cgroup_get_from_path(const char *path) · line 6224

Quoted 10 lines
kernel/cgroup.c +10 -0
+ * address space.
+ *
+ * This invalidates @css->id, and css_from_id() might return NULL or a
+ * new css if the ID has been recycled in the meantime.
+ */
+void css_id_free(struct cgroup_subsys_state *css)
+{
+	cgroup_idr_remove(&css->ss->css_idr, css->id);
+	css->id = 0;
+}

I don't quite get why we're trying to free css->id earlier when memcg is gonna be using its private id anyway. From cgroup core side, the id space isn't restricted.

Quoted 4 lines
mm/memcontrol.c +0 -0
index 75e74408cc8f..1d8a6dffdc25 100644

...

mm/memcontrol.c static struct cftype mem_cgroup_legacy_files[] = { · line 4070

Quoted 4 lines
mm/memcontrol.c +4 -0
+static void mem_cgroup_id_put(struct mem_cgroup *memcg)
+{
+	if (atomic_dec_and_test(&memcg->id.ref)) {
+		idr_remove(&mem_cgroup_idr, memcg->id.id);

Maybe this should do "memcg->id.id = 0"?

Quoted 6 lines
mm/memcontrol.c +6 -0
+		css_id_free(&memcg->css);
+		css_put(&memcg->css);
+	}
+}
+
+/**

mm/memcontrol.c static struct cftype mem_cgroup_legacy_files[] = { · line 4086

Quoted 10 lines
mm/memcontrol.c +10 -0
+ * mem_cgroup_from_id - look up a memcg from a memcg id
+ * @id: the memcg id to look up
+ *
+ * Caller must hold rcu_read_lock().
+ */
+struct mem_cgroup *mem_cgroup_from_id(unsigned short id)
+{
+	WARN_ON_ONCE(!rcu_read_lock_held());
+	return id > 0 ? idr_find(&mem_cgroup_idr, id) : NULL;
+}

css_from_id() has it too but I don't think id > 0 test is necessary. We prolly should take it out of css_from_id() too.

It might be useful to add comment explaining why memcg needs private ids.

Thanks.

Signature · 7 lines
tejun

--
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>
author maintainer
#2 lore raw
Quoted 18 lines · Tejun Heo
Quoted 17 lines · Johannes Weiner
kernel/cgroup.c +13 -0
@@ -6205,6 +6205,24 @@ struct cgroup *cgroup_get_from_path(const char *path)
 }
 EXPORT_SYMBOL_GPL(cgroup_get_from_path);
 
+/**
+ * css_id_free - relinquish an existing CSS's ID
+ * @css: the CSS
+ *
+ * This releases the @css's ID and allows it to be recycled while the
+ * CSS continues to exist. This is useful for controllers with state
+ * that extends past a cgroup's lifetime but doesn't need precious ID
+ * address space.
+ *
+ * This invalidates @css->id, and css_from_id() might return NULL or a
+ * new css if the ID has been recycled in the meantime.
+ */
+void css_id_free(struct cgroup_subsys_state *css)
Quoted 8 lines

kernel/cgroup.c struct cgroup *cgroup_get_from_path(const char *path) · line 6224

Quoted 4 lines
kernel/cgroup.c +4 -0
+{
+	cgroup_idr_remove(&css->ss->css_idr, css->id);
+	css->id = 0;
+}

I don't quite get why we're trying to free css->id earlier when memcg is gonna be using its private id anyway. From cgroup core side, the id space isn't restricted.

For some reason I was thinking of CSS ID being restricted as well, but of course the only restriction is what's enforced in memcg onlining. I deleted it.

Quoted 11 lines
Quoted 4 lines
mm/memcontrol.c +0 -0
index 75e74408cc8f..1d8a6dffdc25 100644

...

mm/memcontrol.c static struct cftype mem_cgroup_legacy_files[] = { · line 4070

Quoted 4 lines
mm/memcontrol.c +4 -0
+static void mem_cgroup_id_put(struct mem_cgroup *memcg)
+{
+	if (atomic_dec_and_test(&memcg->id.ref)) {
+		idr_remove(&mem_cgroup_idr, memcg->id.id);

Maybe this should do "memcg->id.id = 0"?

Added.

Quoted 11 lines
Quoted 11 lines
mm/memcontrol.c +11 -0
+		css_id_free(&memcg->css);
+		css_put(&memcg->css);
+	}
+}
+
+/**
+ * mem_cgroup_from_id - look up a memcg from a memcg id
+ * @id: the memcg id to look up
+ *
+ * Caller must hold rcu_read_lock().
+ */
Quoted 8 lines

mm/memcontrol.c static struct cftype mem_cgroup_legacy_files[] = { · line 4086

Quoted 5 lines
mm/memcontrol.c +5 -0
+struct mem_cgroup *mem_cgroup_from_id(unsigned short id)
+{
+	WARN_ON_ONCE(!rcu_read_lock_held());
+	return id > 0 ? idr_find(&mem_cgroup_idr, id) : NULL;
+}

css_from_id() has it too but I don't think id > 0 test is necessary. We prolly should take it out of css_from_id() too.

Yeah, idr_find() just returns NULL for index 0 - no warning. I removed it from my patch and added a patch to remove it in css_from_id().

Quoted 2 lines

It might be useful to add comment explaining why memcg needs private ids.

Good point. I put an intro comment above the mem_cgroup_idr definition that explains why we need a private space.

Thanks

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

author maintainer
#3 lore raw

PATCH 1/3 cgroup: fix idr leak for the first cgroup root

The valid cgroup hierarchy ID range includes 0, so we can't filter for positive numbers when freeing it, or it'll leak the first ID. No big deal, just disruptive when reading the code.

The ID is freed during error handling and when the reference count hits zero, so the double-free test is not necessary; remove it.

---
 kernel/cgroup.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)
kernel/cgroup.c +1 -7
index 86cb5c6e8932..36fc0ff506c3 100644
@@ -1158,18 +1158,12 @@ static void cgroup_exit_root_id(struct cgroup_root *root)
 {
 	lockdep_assert_held(&cgroup_mutex);
 
-	if (root->hierarchy_id) {
-		idr_remove(&cgroup_hierarchy_idr, root->hierarchy_id);
-		root->hierarchy_id = 0;
-	}
+	idr_remove(&cgroup_hierarchy_idr, root->hierarchy_id);
 }
 
 static void cgroup_free_root(struct cgroup_root *root)
 {
 	if (root) {
-		/* hierarchy ID should already have been released */
-		WARN_ON_ONCE(root->hierarchy_id);
-
 		idr_destroy(&root->cgroup_idr);
 		kfree(root);
 	}
Signature · 1 line
2.8.3
author maintainer
#4 lore raw

PATCH 2/3 cgroup: remove unnecessary 0 check from css_from_id()

In reply to Johannes Weiner

css_idr allocation starts at 1, so index 0 will never point to an item. css_from_id() currently filters that before asking idr_find(), but idr_find() would also just return NULL, so this is not needed.

---
 kernel/cgroup.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/cgroup.c +1 -1
index 36fc0ff506c3..78f6d18ff0af 100644
@@ -6162,7 +6162,7 @@ struct cgroup_subsys_state *css_tryget_online_from_dir(struct dentry *dentry,
 struct cgroup_subsys_state *css_from_id(int id, struct cgroup_subsys *ss)
 {
 	WARN_ON_ONCE(!rcu_read_lock_held());
-	return id > 0 ? idr_find(&ss->css_idr, id) : NULL;
+	return idr_find(&ss->css_idr, id);
 }
 
 /**
Signature · 1 line
2.8.3
maintainer
#5 lore raw

Re: [PATCH 2/3] cgroup: remove unnecessary 0 check from css_from_id()

Quoted 5 lines · Johannes Weiner

css_idr allocation starts at 1, so index 0 will never point to an item. css_from_id() currently filters that before asking idr_find(), but idr_find() would also just return NULL, so this is not needed.

Applied 1-2 to cgroup/for-4.8.

Thanks.

Signature · 1 line
tejun
author maintainer
#6 lore raw

PATCH 3/3 mm: memcontrol: fix cgroup creation failure after many small jobs

In reply to Johannes Weiner

The memory controller has quite a bit of state that usually outlives the cgroup and pins its CSS until said state disappears. At the same time it imposes a 16-bit limit on the CSS ID space to economically store IDs in the wild. Consequently, when we use cgroups to contain frequent but small and short-lived jobs that leave behind some page cache, we quickly run into the 64k limitations of outstanding CSSs. Creating a new cgroup fails with -ENOSPC while there are only a few, or even no user-visible cgroups in existence.

Although pinning CSSs past cgroup removal is common, there are only two instances that actually need an ID after a cgroup is deleted:
cache shadow entries and swapout records.

Cache shadow entries reference the ID weakly and can deal with the CSS having disappeared when it's looked up later. They pose no hurdle.

Swap-out records do need to pin the css to hierarchically attribute swapins after the cgroup has been deleted; though the only pages that remain swapped out after offlining are tmpfs/shmem pages. And those references are under the user's control, so they are manageable.

This patch introduces a private 16-bit memcg ID and switches swap and cache shadow entries over to using that. This ID can then be recycled after offlining when the CSS remains pinned only by objects that don't specifically need it.

This script demonstrates the problem by faulting one cache page in a new cgroup and deleting it again:

set -e
mkdir -p pages
for x in seq 128000; do
[ $((x % 1000)) -eq 0 ] && echo $x
mkdir /cgroup/foo
echo $$ >/cgroup/foo/cgroup.procs
echo trex >pages/$x
echo $$ >/cgroup/cgroup.procs
rmdir /cgroup/foo
done

When run on an unpatched kernel, we eventually run out of possible IDs even though there are no visible cgroups:

[root@ham ~]# ./cssidstress.sh
[...]
65000
mkdir: cannot create directory '/cgroup/foo': No space left on device

After this patch, the IDs get released upon cgroup destruction and the cache and css objects get released once memory reclaim kicks in.

---
 include/linux/memcontrol.h | 25 ++++++--------
 mm/memcontrol.c            | 82 ++++++++++++++++++++++++++++++++++++++++++----
 mm/slab_common.c           |  4 +--
 3 files changed, 87 insertions(+), 24 deletions(-)
include/linux/memcontrol.h +10 -15
index a805474df4ab..56e6069d2452 100644
@@ -97,6 +97,11 @@ enum mem_cgroup_events_target {
 #define MEM_CGROUP_ID_SHIFT	16
 #define MEM_CGROUP_ID_MAX	USHRT_MAX
 
+struct mem_cgroup_id {
+	int id;
+	atomic_t ref;
+};
+
 struct mem_cgroup_stat_cpu {
 	long count[MEMCG_NR_STAT];
 	unsigned long events[MEMCG_NR_EVENTS];
@@ -172,6 +177,9 @@ enum memcg_kmem_state {
 struct mem_cgroup {
 	struct cgroup_subsys_state css;
 
+	/* Private memcg ID. Used to ID objects that outlive the cgroup */
+	struct mem_cgroup_id id;
+
 	/* Accounted resources */
 	struct page_counter memory;
 	struct page_counter swap;
@@ -330,22 +338,9 @@ static inline unsigned short mem_cgroup_id(struct mem_cgroup *memcg)
 	if (mem_cgroup_disabled())
 		return 0;
 
-	return memcg->css.id;
-}
-
-/**
- * mem_cgroup_from_id - look up a memcg from an id
- * @id: the id to look up
- *
- * Caller must hold rcu_read_lock() and use css_tryget() as necessary.
- */
-static inline struct mem_cgroup *mem_cgroup_from_id(unsigned short id)
-{
-	struct cgroup_subsys_state *css;
-
-	css = css_from_id(id, &memory_cgrp_subsys);
-	return mem_cgroup_from_css(css);
+	return memcg->id.id;
 }
+struct mem_cgroup *mem_cgroup_from_id(unsigned short id);
 
 /**
  * parent_mem_cgroup - find the accounting parent of a memcg
mm/memcontrol.c +75 -7

143 lines not on this page · raw message

mm/slab_common.c +2 -2
index a65dad7fdcd1..82317abb03ed 100644
@@ -526,8 +526,8 @@ void memcg_create_kmem_cache(struct mem_cgroup *memcg,
 		goto out_unlock;
 
 	cgroup_name(css->cgroup, memcg_name_buf, sizeof(memcg_name_buf));
-	cache_name = kasprintf(GFP_KERNEL, "%s(%d:%s)", root_cache->name,
-			       css->id, memcg_name_buf);
+	cache_name = kasprintf(GFP_KERNEL, "%s(%llu:%s)", root_cache->name,
+			       css->serial_nr, memcg_name_buf);
 	if (!cache_name)
 		goto out_unlock;
 
Signature · 7 lines
2.8.3

--
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>
maintainer Acked-by on v1 3/3
#7 lore raw
Quoted 8 lines · Johannes Weiner

The memory controller has quite a bit of state that usually outlives the cgroup and pins its CSS until said state disappears. At the same time it imposes a 16-bit limit on the CSS ID space to economically store IDs in the wild. Consequently, when we use cgroups to contain frequent but small and short-lived jobs that leave behind some page cache, we quickly run into the 64k limitations of outstanding CSSs. Creating a new cgroup fails with -ENOSPC while there are only a few, or even no user-visible cgroups in existence.

Thanks.

Signature · 7 lines
tejun

--
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>
#8 lore raw
In reply to Johannes Weiner
Quoted 29 lines · Johannes Weiner

The memory controller has quite a bit of state that usually outlives the cgroup and pins its CSS until said state disappears. At the same time it imposes a 16-bit limit on the CSS ID space to economically store IDs in the wild. Consequently, when we use cgroups to contain frequent but small and short-lived jobs that leave behind some page cache, we quickly run into the 64k limitations of outstanding CSSs. Creating a new cgroup fails with -ENOSPC while there are only a few, or even no user-visible cgroups in existence.

Although pinning CSSs past cgroup removal is common, there are only two instances that actually need an ID after a cgroup is deleted:
cache shadow entries and swapout records.

Cache shadow entries reference the ID weakly and can deal with the CSS having disappeared when it's looked up later. They pose no hurdle.

Swap-out records do need to pin the css to hierarchically attribute swapins after the cgroup has been deleted; though the only pages that remain swapped out after offlining are tmpfs/shmem pages. And those references are under the user's control, so they are manageable.

This patch introduces a private 16-bit memcg ID and switches swap and cache shadow entries over to using that. This ID can then be recycled after offlining when the CSS remains pinned only by objects that don't specifically need it.

This script demonstrates the problem by faulting one cache page in a new cgroup and deleting it again:

Quoted 10 lines

set -e
mkdir -p pages
for x in seq 128000; do
[ $((x % 1000)) -eq 0 ] && echo $x
mkdir /cgroup/foo
echo $$ >/cgroup/foo/cgroup.procs
echo trex >pages/$x
echo $$ >/cgroup/cgroup.procs
rmdir /cgroup/foo
done

Perhaps you could send this script to the LTP project to have this as a regression test?

Quoted 9 lines

When run on an unpatched kernel, we eventually run out of possible IDs even though there are no visible cgroups:

[root@ham ~]# ./cssidstress.sh
[...]
65000
mkdir: cannot create directory '/cgroup/foo': No space left on device

Quoted 4 lines

After this patch, the IDs get released upon cgroup destruction and the cache and css objects get released once memory reclaim kicks in.

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

Reviewed-by on v1 3/3
#9 lore raw
In reply to Johannes Weiner
Quoted 43 lines · Johannes Weiner

The memory controller has quite a bit of state that usually outlives the cgroup and pins its CSS until said state disappears. At the same time it imposes a 16-bit limit on the CSS ID space to economically store IDs in the wild. Consequently, when we use cgroups to contain frequent but small and short-lived jobs that leave behind some page cache, we quickly run into the 64k limitations of outstanding CSSs. Creating a new cgroup fails with -ENOSPC while there are only a few, or even no user-visible cgroups in existence.

Although pinning CSSs past cgroup removal is common, there are only two instances that actually need an ID after a cgroup is deleted:
cache shadow entries and swapout records.

Cache shadow entries reference the ID weakly and can deal with the CSS having disappeared when it's looked up later. They pose no hurdle.

Swap-out records do need to pin the css to hierarchically attribute swapins after the cgroup has been deleted; though the only pages that remain swapped out after offlining are tmpfs/shmem pages. And those references are under the user's control, so they are manageable.

This patch introduces a private 16-bit memcg ID and switches swap and cache shadow entries over to using that. This ID can then be recycled after offlining when the CSS remains pinned only by objects that don't specifically need it.

This script demonstrates the problem by faulting one cache page in a new cgroup and deleting it again:

set -e
mkdir -p pages
for x in seq 128000; do
[ $((x % 1000)) -eq 0 ] && echo $x
mkdir /cgroup/foo
echo $$ >/cgroup/foo/cgroup.procs
echo trex >pages/$x
echo $$ >/cgroup/cgroup.procs
rmdir /cgroup/foo
done

When run on an unpatched kernel, we eventually run out of possible IDs even though there are no visible cgroups:

Quoted 7 lines

[root@ham ~]# ./cssidstress.sh
[...]
65000
mkdir: cannot create directory '/cgroup/foo': No space left on device

After this patch, the IDs get released upon cgroup destruction and the cache and css objects get released once memory reclaim kicks in.

With 65K cgroups it will take the reclaimer a substantial amount of time to iterate over all of them, which might result in latency spikes. Probably, to avoid that, we could move pages from a dead cgroup's lru to its parent's one on offline while still leaving dead cgroups pinned, like we do in case of list_lru entries.

Quoted 2 lines

One nit below.

...

Quoted 23 lines
mm/memcontrol.c +15 -0
index 75e74408cc8f..dc92b2df2585 100644
@@ -4057,6 +4057,60 @@ static struct cftype mem_cgroup_legacy_files[] = {
 	{ },	/* terminate */
 };
 
+/*
+ * Private memory cgroup IDR
+ *
+ * Swap-out records and page cache shadow entries need to store memcg
+ * references in constrained space, so we maintain an ID space that is
+ * limited to 16 bit (MEM_CGROUP_ID_MAX), limiting the total number of
+ * memory-controlled cgroups to 64k.
+ *
+ * However, there usually are many references to the oflline CSS after
+ * the cgroup has been destroyed, such as page cache or reclaimable
+ * slab objects, that don't need to hang on to the ID. We want to keep
+ * those dead CSS from occupying IDs, or we might quickly exhaust the
+ * relatively small ID space and prevent the creation of new cgroups
+ * even when there are much fewer than 64k cgroups - possibly none.
+ *

mm/memcontrol.c static struct cftype mem_cgroup_legacy_files[] = { · line 4084

Quoted 10 lines
mm/memcontrol.c +10 -0
+ * Maintain a private 16-bit ID space for memcg, and allow the ID to
+ * be freed and recycled when it's no longer needed, which is usually
+ * when the CSS is offlined.
+ *
+ * The only exception to that are records of swapped out tmpfs/shmem
+ * pages that need to be attributed to live ancestors on swapin. But
+ * those references are manageable from userspace.
+ */
+
+static struct idr mem_cgroup_idr;

static DEFINE_IDR(mem_cgroup_idr);

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

author maintainer
#10 lore raw
Quoted 9 lines · Vladimir Davydov
Quoted 2 lines · Johannes Weiner

After this patch, the IDs get released upon cgroup destruction and the cache and css objects get released once memory reclaim kicks in.

With 65K cgroups it will take the reclaimer a substantial amount of time to iterate over all of them, which might result in latency spikes. Probably, to avoid that, we could move pages from a dead cgroup's lru to its parent's one on offline while still leaving dead cgroups pinned, like we do in case of list_lru entries.

Yep, that is true. list_lru is a bit easier because the walker stays in the context of the original LRU list, whereas the cache/anon LRUs are not. We'd have to have mem_cgroup_page_lruvec() etc. do a parent walk to find the closest live ancestor. Maybe you have a better idea?

But it's definitely worth considering. I'll think more about it.

Quoted 3 lines
Quoted 1 line

Thanks!

Quoted 3 lines
Quoted 1 line

+static struct idr mem_cgroup_idr;

static DEFINE_IDR(mem_cgroup_idr);

Oops, good catch. Andrew, could you kindly fold this?

From d1261ede8f975a5fccb2e9125562260e4b4b4f3d Mon Sep 17 00:00:00 2001
From: Johannes Weiner <[email protected]>
Date: Tue, 21 Jun 2016 11:36:13 -0400
Subject: [PATCH] mm: memcontrol: fix cgroup creation failure after many small
jobs fix

init the IDR

---
 mm/memcontrol.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
mm/memcontrol.c +1 -1
index dc92b2df2585..b0de1342eab2 100644
@@ -4081,7 +4081,7 @@ static struct cftype mem_cgroup_legacy_files[] = {
  * those references are manageable from userspace.
  */
 
-static struct idr mem_cgroup_idr;
+static DEFINE_IDR(mem_cgroup_idr);
 
 static void mem_cgroup_id_get(struct mem_cgroup *memcg)
 {
Signature · 7 lines
2.8.3

--
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>
#11 lore raw
In reply to Johannes Weiner
Quoted 21 lines · Johannes Weiner

The memory controller has quite a bit of state that usually outlives the cgroup and pins its CSS until said state disappears. At the same time it imposes a 16-bit limit on the CSS ID space to economically store IDs in the wild. Consequently, when we use cgroups to contain frequent but small and short-lived jobs that leave behind some page cache, we quickly run into the 64k limitations of outstanding CSSs. Creating a new cgroup fails with -ENOSPC while there are only a few, or even no user-visible cgroups in existence.

Although pinning CSSs past cgroup removal is common, there are only two instances that actually need a CSS ID after a cgroup is deleted:
cache shadow entries and swapout records.

Cache shadow entries reference the ID weakly and can deal with the CSS having disappeared when it's looked up later. They pose no hurdle.

Swap-out records do need to pin the css to hierarchically attribute swapins after the cgroup has been deleted; though the only pages that remain swapped out after a process exits are tmpfs/shmem pages. Those references are under the user's control and thus manageable.

Quoted 4 lines

This patch introduces a private 16bit memcg ID and switches swap and cache shadow entries over to using that. It then decouples the CSS lifetime from the CSS ID lifetime, such that a CSS ID can be recycled when the CSS is only pinned by common objects that don't need an ID.

There's already id which is only used for online memory cgroups - it's kmemcg_id. May be, instead of introducing one more idr, we could name it generically and reuse it for shadow entries?

Regarding swap entries, would it really make much difference if we used 4 bytes per swap page instead of 2? For a 100 GB swap it'd increase overhead from 50 MB up to 100 MB, which still doesn't seem too much IMO, so may be just use plain unrestricted css->id for swap entries?

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

author maintainer
#12 lore raw
Quoted 22 lines · Vladimir Davydov
Quoted 21 lines · Johannes Weiner

The memory controller has quite a bit of state that usually outlives the cgroup and pins its CSS until said state disappears. At the same time it imposes a 16-bit limit on the CSS ID space to economically store IDs in the wild. Consequently, when we use cgroups to contain frequent but small and short-lived jobs that leave behind some page cache, we quickly run into the 64k limitations of outstanding CSSs. Creating a new cgroup fails with -ENOSPC while there are only a few, or even no user-visible cgroups in existence.

Although pinning CSSs past cgroup removal is common, there are only two instances that actually need a CSS ID after a cgroup is deleted:
cache shadow entries and swapout records.

Cache shadow entries reference the ID weakly and can deal with the CSS having disappeared when it's looked up later. They pose no hurdle.

Swap-out records do need to pin the css to hierarchically attribute swapins after the cgroup has been deleted; though the only pages that remain swapped out after a process exits are tmpfs/shmem pages. Those references are under the user's control and thus manageable.

Quoted 8 lines
Quoted 4 lines

This patch introduces a private 16bit memcg ID and switches swap and cache shadow entries over to using that. It then decouples the CSS lifetime from the CSS ID lifetime, such that a CSS ID can be recycled when the CSS is only pinned by common objects that don't need an ID.

There's already id which is only used for online memory cgroups - it's kmemcg_id. May be, instead of introducing one more idr, we could name it generically and reuse it for shadow entries?

Good point. But it seems mem_cgroup_idr is more generic, it makes sense to switch slab accounting over to that. I'll look into that, but as a refactoring patch on top of this fix.

Quoted 4 lines

Regarding swap entries, would it really make much difference if we used 4 bytes per swap page instead of 2? For a 100 GB swap it'd increase overhead from 50 MB up to 100 MB, which still doesn't seem too much IMO, so may be just use plain unrestricted css->id for swap entries?

Yes and no. I agree that the increased consumption wouldn't be too crazy, but if we have to maintain a 16-bit ID anyway, we might as well use it for swap too to save that space. I don't think tmpfs and shmem pins past offlining will be common enough to significantly eat into the ID space of online cgroups.

author maintainer
#13 lore raw
In reply to Johannes Weiner

Hi Andrew,

this issue dates back quite a bit and wasn't reported until now, so I didn't tag it for stable. However, it seems that larger scale setups are now running into this as they upgrade their kernels, and several people have run into this independently now. Could you please add:

Fixes b2052564e66d ("mm: memcontrol: continue cache reclaim from offlined groups")

and send it linusward?

Thank you

Quoted 43 lines · Johannes Weiner

The memory controller has quite a bit of state that usually outlives the cgroup and pins its CSS until said state disappears. At the same time it imposes a 16-bit limit on the CSS ID space to economically store IDs in the wild. Consequently, when we use cgroups to contain frequent but small and short-lived jobs that leave behind some page cache, we quickly run into the 64k limitations of outstanding CSSs. Creating a new cgroup fails with -ENOSPC while there are only a few, or even no user-visible cgroups in existence.

Although pinning CSSs past cgroup removal is common, there are only two instances that actually need a CSS ID after a cgroup is deleted:
cache shadow entries and swapout records.

Cache shadow entries reference the ID weakly and can deal with the CSS having disappeared when it's looked up later. They pose no hurdle.

Swap-out records do need to pin the css to hierarchically attribute swapins after the cgroup has been deleted; though the only pages that remain swapped out after a process exits are tmpfs/shmem pages. Those references are under the user's control and thus manageable.

This patch introduces a private 16bit memcg ID and switches swap and cache shadow entries over to using that. It then decouples the CSS lifetime from the CSS ID lifetime, such that a CSS ID can be recycled when the CSS is only pinned by common objects that don't need an ID.

This script demonstrates the problem by faulting one cache page in a new cgroup and deleting it again:

set -e
mkdir -p pages
for x in seq 128000; do
[ $((x % 1000)) -eq 0 ] && echo $x
mkdir /cgroup/foo
echo $$ >/cgroup/foo/cgroup.procs
echo trex >pages/$x
echo $$ >/cgroup/cgroup.procs
rmdir /cgroup/foo
done

When run on an unpatched kernel, we eventually run out of possible CSS IDs even though there is no visible cgroup existing anymore:

Quoted 7 lines

[root@ham ~]# ./cssidstress.sh
[...]
65000
mkdir: cannot create directory '/cgroup/foo': No space left on device

After this patch, the CSS IDs get released upon cgroup destruction and the cache and css objects get released once memory reclaim kicks in.

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