Resource controllers based on process containers

2. Extended containers may register themselves too late. Kernel threads/helpers start forking, opening files and touching pages much earlier. This patchset workarounds this in not-so-cute manner and I'm waiting for Paul's comments on this issue.

Introduce generic structures and routines for
resource accounting.

Each resource accounting container is supposed to
aggregate it, container_subsystem_state and its
resource-specific members within.

------------------------------------------------------------------------

diff -upr linux-2.6.20.orig/include/linux/res_counter.h linux-2.6.20-0/include/linux/res_counter.h

include/linux/res_counter.h +27 -0

@@ -0,0 +1,83 @@
+#ifndef __RES_COUNTER_H__
+#define __RES_COUNTER_H__
+/*
+ * resource counters
+ *
+ * Copyright 2007 OpenVZ SWsoft Inc
+ *
+ * Author: Pavel Emelianov <[email protected]>
+ *
+ */
+
+#include <linux/container.h>
+
+struct res_counter {
+	unsigned long usage;
+	unsigned long limit;
+	unsigned long failcnt;
+	spinlock_t lock;
+};
+
+enum {
+	RES_USAGE,
+	RES_LIMIT,
+	RES_FAILCNT,
+};
+
+ssize_t res_counter_read(struct res_counter *cnt, int member,

include/linux/res_counter.h +14 -0

+static inline int res_counter_charge_locked(struct res_counter *cnt,
+		unsigned long val)
+{
+	if (cnt->usage <= cnt->limit - val) {
+		cnt->usage += val;
+		return 0;
+	}
+
+	cnt->failcnt++;
+	return -ENOMEM;
+}
+
+static inline int res_counter_charge(struct res_counter *cnt,
+		unsigned long val)

include/linux/res_counter.h +22 -0

+static inline void res_counter_uncharge_locked(struct res_counter *cnt,
+		unsigned long val)
+{
+	if (unlikely(cnt->usage < val)) {
+		WARN_ON(1);
+		val = cnt->usage;
+	}
+
+	cnt->usage -= val;
+}
+
+static inline void res_counter_uncharge(struct res_counter *cnt,
+		unsigned long val)
+{
+	unsigned long flags;
+
+	spin_lock_irqsave(&cnt->lock, flags);
+	res_counter_uncharge_locked(cnt, val);
+	spin_unlock_irqrestore(&cnt->lock, flags);
+}
+
+#endif

diff -upr linux-2.6.20.orig/init/Kconfig linux-2.6.20-0/init/Kconfig

init/Kconfig +4 -0

@@ -265,6 +265,10 @@ config CPUSETS
 
 	  Say N if unsure.
 
+config RESOURCE_COUNTERS
+	bool
+	select CONTAINERS
+
 config SYSFS_DEPRECATED
 	bool "Create deprecated sysfs files"
 	default y

diff -upr linux-2.6.20.orig/kernel/Makefile linux-2.6.20-0/kernel/Makefile

kernel/Makefile +1 -0

@@ -51,6 +51,7 @@ obj-$(CONFIG_RELAY) += relay.o
 obj-$(CONFIG_UTS_NS) += utsname.o
 obj-$(CONFIG_TASK_DELAY_ACCT) += delayacct.o
 obj-$(CONFIG_TASKSTATS) += taskstats.o tsacct.o
+obj-$(CONFIG_RESOURCE_COUNTERS) += res_counter.o
 
 ifneq ($(CONFIG_SCHED_NO_NO_OMIT_FRAME_POINTER),y)
 # According to Alan Modra <[email protected]>, the -fno-omit-frame-pointer is

diff -upr linux-2.6.20.orig/kernel/res_counter.c linux-2.6.20-0/kernel/res_counter.c

kernel/res_counter.c +62 -0

@@ -0,0 +1,72 @@
+/*
+ * resource containers
+ *
+ * Copyright 2007 OpenVZ SWsoft Inc
+ *
+ * Author: Pavel Emelianov <[email protected]>
+ *
+ */
+
+#include <linux/parser.h>
+#include <linux/fs.h>
+#include <linux/res_counter.h>
+#include <asm/uaccess.h>
+
+static inline unsigned long *res_counter_member(struct res_counter *cnt, int member)
+{
+	switch (member) {
+	case RES_USAGE:
+		return &cnt->usage;
+	case RES_LIMIT:
+		return &cnt->limit;
+	case RES_FAILCNT:
+		return &cnt->failcnt;
+	};
+
+	BUG();
+	return NULL;
+}
+
+ssize_t res_counter_read(struct res_counter *cnt, int member, 
+		const char __user *userbuf, size_t nbytes, loff_t *pos)
+{
+	unsigned long *val;
+	char buf[64], *s;
+
+	s = buf;
+	val = res_counter_member(cnt, member);
+	s += sprintf(s, "%lu\n", *val);
+	return simple_read_from_buffer((void __user *)userbuf, nbytes,
+			pos, buf, s - buf);
+}
+
+ssize_t res_counter_write(struct res_counter *cnt, int member,
+		const char __user *userbuf, size_t nbytes, loff_t *pos)
+{
+	int ret;
+	char *buf, *end;
+	unsigned long tmp, *val;
+
+	buf = kmalloc(nbytes + 1, GFP_KERNEL);
+	ret = -ENOMEM;
+	if (buf == NULL)
+		goto out;
+
+	buf[nbytes] = 0;
+	ret = -EFAULT;
+	if (copy_from_user(buf, userbuf, nbytes))
+		goto out_free;
+
+	ret = -EINVAL;
+	tmp = simple_strtoul(buf, &end, 10);
+	if (*end != '\0')

This includes setup of RSS container within generic process containers, all the declarations used in RSS accounting, and core code responsible for accounting.

------------------------------------------------------------------------

diff -upr linux-2.6.20.orig/include/linux/rss_container.h linux-2.6.20-0/include/linux/rss_container.h

include/linux/rss_container.h +68 -0

@@ -0,0 +1,68 @@
+#ifndef __RSS_CONTAINER_H__
+#define __RSS_CONTAINER_H__
+/*
+ * RSS container
+ *
+ * Copyright 2007 OpenVZ SWsoft Inc
+ *
+ * Author: Pavel Emelianov <[email protected]>
+ *
+ */
+
+struct page_container;
+struct rss_container;
+
+#ifdef CONFIG_RSS_CONTAINER
+int container_rss_prepare(struct page *, struct vm_area_struct *vma,
+		struct page_container **);
+
+void container_rss_add(struct page_container *);
+void container_rss_del(struct page_container *);
+void container_rss_release(struct page_container *);
+
+int mm_init_container(struct mm_struct *mm, struct task_struct *tsk);
+void mm_free_container(struct mm_struct *mm);
+
+unsigned long container_isolate_pages(unsigned long nr_to_scan,
+		struct rss_container *rss, struct list_head *dst,
+		int active, unsigned long *scanned);
+unsigned long container_nr_physpages(struct rss_container *rss);
+
+unsigned long container_try_to_free_pages(struct rss_container *);
+void container_out_of_memory(struct rss_container *);
+
+void container_rss_init_early(void);
+#else
+static inline int container_rss_prepare(struct page *pg,
+		struct vm_area_struct *vma, struct page_container **pc)
+{
+	*pc = NULL; /* to make gcc happy */
+	return 0;
+}
+
+static inline void container_rss_add(struct page_container *pc)
+{
+}
+
+static inline void container_rss_del(struct page_container *pc)
+{
+}
+
+static inline void container_rss_release(struct page_container *pc)
+{
+}
+
+static inline int mm_init_container(struct mm_struct *mm, struct task_struct *t)
+{
+	return 0;
+}
+
+static inline void mm_free_container(struct mm_struct *mm)
+{
+}
+
+static inline void container_rss_init_early(void)
+{
+}
+#endif
+#endif

diff -upr linux-2.6.20.orig/init/Kconfig linux-2.6.20-0/init/Kconfig

init/Kconfig +0 -0

 config SYSFS_DEPRECATED
 	bool "Create deprecated sysfs files"
 	default y

diff -upr linux-2.6.20.orig/mm/Makefile linux-2.6.20-0/mm/Makefile

mm/Makefile +2 -0

@@ -29,3 +29,5 @@ obj-$(CONFIG_MEMORY_HOTPLUG) += memory_h
 obj-$(CONFIG_FS_XIP) += filemap_xip.o
 obj-$(CONFIG_MIGRATION) += migrate.o
 obj-$(CONFIG_SMP) += allocpercpu.o
+
+obj-$(CONFIG_RSS_CONTAINER) += rss_container.o

diff -upr linux-2.6.20.orig/mm/rss_container.c linux-2.6.20-0/mm/rss_container.c

mm/rss_container.c +19 -0

@@ -0,0 +1,307 @@
+/*
+ * RSS accounting container
+ *
+ * Copyright 2007 OpenVZ SWsoft Inc
+ *
+ * Author: Pavel Emelianov <[email protected]>
+ *
+ */
+
+#include <linux/list.h>
+#include <linux/sched.h>
+#include <linux/mm.h>
+#include <linux/res_counter.h>
+#include <linux/rss_container.h>
+
+static struct container_subsys rss_subsys;
+
+struct rss_container {
+	struct res_counter res;

mm/rss_container.c +32 -0

+static inline struct rss_container *rss_from_cont(struct container *cnt)
+{
+	return container_of(container_subsys_state(cnt, &rss_subsys),
+			struct rss_container, css);
+}
+
+int mm_init_container(struct mm_struct *mm, struct task_struct *tsk)
+{
+	struct rss_container *cnt;
+
+	cnt = rss_from_cont(task_container(tsk, &rss_subsys));
+	if (css_get(&cnt->css))
+		return -EBUSY;
+
+	mm->rss_container = cnt;
+	return 0;
+}
+
+void mm_free_container(struct mm_struct *mm)
+{
+	css_put(&mm->rss_container->css);
+}
+
+int container_rss_prepare(struct page *page, struct vm_area_struct *vma,
+		struct page_container **ppc)
+{
+	struct rss_container *rss;
+	struct page_container *pc;
+
+	rcu_read_lock();
+	rss = rcu_dereference(vma->vm_mm->rss_container);
+	css_get_current(&rss->css);

mm/rss_container.c +27 -0

+		container_out_of_memory(rss);
+		if (test_thread_flag(TIF_MEMDIE))
+			goto out_charge;
+	}
+
+	pc->page = page;
+	pc->cnt = rss;
+	*ppc = pc;
+	return 0;
+
+out_charge:
+	kfree(pc);
+out_nomem:
+	css_put(&rss->css);
+	return -ENOMEM;
+}
+
+void container_rss_release(struct page_container *pc)
+{
+	struct rss_container *rss;
+
+	rss = pc->cnt;
+	res_counter_uncharge(&rss->res, 1);
+	css_put(&rss->css);
+	kfree(pc);
+}
+

mm/rss_container.c +27 -0

+	spin_unlock(&rss->res.lock);
+
+	page_container(pg) = pc;
+}
+
+void container_rss_del(struct page_container *pc)
+{
+	struct page *page;
+	struct rss_container *rss;
+
+	page = pc->page;
+	rss = pc->cnt;
+
+	spin_lock(&rss->res.lock);
+	list_del(&pc->list);
+	res_counter_uncharge_locked(&rss->res, 1);
+	spin_unlock(&rss->res.lock);
+
+	css_put(&rss->css);
+	kfree(pc);
+}
+
+unsigned long container_isolate_pages(unsigned long nr_to_scan,
+		struct rss_container *rss, struct list_head *dst,
+		int active, unsigned long *scanned)
+{
+	unsigned long nr_taken = 0;

mm/rss_container.c +12 -0

+	for (scan = 0; scan < nr_to_scan && !list_empty(src); scan++) {
+		pc = list_entry(src->prev, struct page_container, list);
+		page = pc->page;
+		z = page_zone(page);
+
+		list_move(&pc->list, &pc_list);
+
+		spin_lock(&z->lru_lock);
+		if (PageLRU(page)) {
+			if ((active && PageActive(page)) ||
+					(!active && !PageActive(page))) {
+				if (likely(get_page_unless_zero(page))) {

mm/rss_container.c +22 -0

+	spin_unlock_irq(&rss->res.lock);
+
+	*scanned = scan;
+	return nr_taken;
+}
+
+unsigned long container_nr_physpages(struct rss_container *rss)
+{
+	return rss->res.usage;
+}
+
+static void rss_move_task(struct container_subsys *ss,
+		struct container *cont,
+		struct container *old_cont,
+		struct task_struct *p)
+{
+	struct mm_struct *mm;
+	struct rss_container *rss, *old_rss;
+
+	mm = get_task_mm(p);
+	if (mm == NULL)
+		goto out;

mm/rss_container.c +108 -0

+out_put:
+	mmput(mm);
+out:
+	return;
+}
+
+static int rss_create(struct container_subsys *ss, struct container *cont)
+{
+	struct rss_container *rss;
+
+	rss = kzalloc(sizeof(struct rss_container), GFP_KERNEL);
+	if (rss == NULL)
+		return -ENOMEM;
+
+	res_counter_init(&rss->res);
+	INIT_LIST_HEAD(&rss->page_list);
+	cont->subsys[rss_subsys.subsys_id] = &rss->css;
+	return 0;
+}
+
+static void rss_destroy(struct container_subsys *ss,
+		struct container *cont)
+{
+	kfree(rss_from_cont(cont));
+}
+
+
+static ssize_t rss_read(struct container *cont, struct cftype *cft,
+		struct file *file, char __user *userbuf,
+		size_t nbytes, loff_t *ppos)
+{
+	return res_counter_read(&rss_from_cont(cont)->res, cft->private,
+			userbuf, nbytes, ppos);
+}
+
+static ssize_t rss_write(struct container *cont, struct cftype *cft,
+		struct file *file, const char __user *userbuf,
+		size_t nbytes, loff_t *ppos)
+{
+	return res_counter_write(&rss_from_cont(cont)->res, cft->private,
+			userbuf, nbytes, ppos);
+}
+
+
+static struct cftype rss_usage = {
+	.name = "rss_usage",
+	.private = RES_USAGE,
+	.read = rss_read,
+};
+
+static struct cftype rss_limit = {
+	.name = "rss_limit",
+	.private = RES_LIMIT,
+	.read = rss_read,
+	.write = rss_write,
+};
+
+static struct cftype rss_failcnt = {
+	.name = "rss_failcnt",
+	.private = RES_FAILCNT,
+	.read = rss_read,
+};
+
+static int rss_populate(struct container_subsys *ss,
+		struct container *cont)
+{
+	int rc;
+
+	if ((rc = container_add_file(cont, &rss_usage)) < 0)
+		return rc;
+	if ((rc = container_add_file(cont, &rss_failcnt)) < 0)
+		return rc;
+	if ((rc = container_add_file(cont, &rss_limit)) < 0)
+		return rc;
+
+	return 0;
+}
+
+static struct rss_container init_rss_container;
+
+static __init int rss_create_early(struct container_subsys *ss,
+		struct container *cont)
+{
+	struct rss_container *rss;
+
+	rss = &init_rss_container;
+	res_counter_init(&rss->res);
+	INIT_LIST_HEAD(&rss->page_list);
+	cont->subsys[rss_subsys.subsys_id] = &rss->css;
+	ss->create = rss_create;
+	return 0;
+}
+
+static struct container_subsys rss_subsys = {
+	.name = "rss",
+	.create = rss_create_early,
+	.destroy = rss_destroy,
+	.populate = rss_populate,
+	.attach = rss_move_task,
+};
+
+void __init container_rss_init_early(void)
+{
+	container_register_subsys(&rss_subsys);
+	init_mm.rss_container = rss_from_cont(
+			task_container(&init_task, &rss_subsys));
+	css_get_current(&init_mm.rss_container->css);
+}

This patchset adds RSS, accounting and control and limiting the number of tasks and files within container.

Based on top of Paul Menage's container subsystem v7

RSS controller includes per-container RSS accounter, reclamation and OOM killer. It behaves like standalone machine - when container runs out of resources it tries to reclaim some pages and if it doesn't succeed in it kills some task which mm_struct belongs to container in question.

Num tasks and files containers are very simple and self-descriptive from code.

As discussed before when a task moves from one container to another no resources follow it - they keep holding the container they were allocated in.

Hi Pavel,

Quoted 7 lines · Pavel Emelianov

diff -upr linux-2.6.20.orig/include/linux/sched.h
linux-2.6.20-0/include/linux/sched.h
--- linux-2.6.20.orig/include/linux/sched.h 2007-03-06
13:33:28.000000000 +0300
+++ linux-2.6.20-0/include/linux/sched.h 2007-03-06
13:33:28.000000000 +0300

include/linux/sched.h struct task_struct { · line 1061

Quoted 7 lines

include/linux/sched.h +3 -0

@@ -1052,6 +1055,9 @@ struct task_struct {
 #ifdef CONFIG_FAULT_INJECTION
        int make_it_fail;
 #endif
+#ifdef CONFIG_PROCESS_CONTAINER
+       struct numproc_container *numproc_cnt;
+#endif
 };

Why do you need a pointer added to task_struct? One of the main points of the generic containers is to avoid every different subsystem and resource controller having to add new pointers there.

kernel/numproc_container.c line 35

Quoted 4 lines

kernel/numproc_container.c +4 -0

+
+       rcu_read_lock();
+       np = numproc_from_cont(task_container(current, &numproc_subsys));
+       css_get_current(&np->css);

There's no need to hold a reference here - by definition, the task's container can't go away while the task is in it.

Also, shouldn't you have an attach() method to move the count from one container to another when a task moves?

Paul

Quoted 48 lines · Pavel Emelianov

Introduce generic structures and routines for
resource accounting.

Each resource accounting container is supposed to
aggregate it, container_subsystem_state and its
resource-specific members within.

------------------------------------------------------------------------

diff -upr linux-2.6.20.orig/include/linux/res_counter.h linux-2.6.20-0/include/linux/res_counter.h
--- linux-2.6.20.orig/include/linux/res_counter.h 2007-03-06
13:39:17.000000000 +0300
+++ linux-2.6.20-0/include/linux/res_counter.h 2007-03-06
13:33:28.000000000 +0300

include/linux/res_counter.h +31 -0

@@ -0,0 +1,83 @@
+#ifndef __RES_COUNTER_H__
+#define __RES_COUNTER_H__
+/*
+ * resource counters
+ *
+ * Copyright 2007 OpenVZ SWsoft Inc
+ *
+ * Author: Pavel Emelianov <[email protected]>
+ *
+ */
+
+#include <linux/container.h>
+
+struct res_counter {
+    unsigned long usage;
+    unsigned long limit;
+    unsigned long failcnt;
+    spinlock_t lock;
+};
+
+enum {
+    RES_USAGE,
+    RES_LIMIT,
+    RES_FAILCNT,
+};
+
+ssize_t res_counter_read(struct res_counter *cnt, int member,
+        const char __user *buf, size_t nbytes, loff_t *pos);
+ssize_t res_counter_write(struct res_counter *cnt, int member,
+        const char __user *buf, size_t nbytes, loff_t *pos);
+

include/linux/res_counter.h line 37

Quoted 6 lines

include/linux/res_counter.h +6 -0

+static inline void res_counter_init(struct res_counter *cnt)
+{
+    spin_lock_init(&cnt->lock);
+    cnt->limit = (unsigned long)LONG_MAX;
+}
+

Is there any way to indicate that there are no limits on this container.

LONG_MAX is quite huge, but still when the administrator wants to configure a container to *un-limited usage*, it becomes hard for the administrator.

Quoted 18 lines

include/linux/res_counter.h +18 -0

+static inline int res_counter_charge_locked(struct res_counter *cnt,
+        unsigned long val)
+{
+    if (cnt->usage <= cnt->limit - val) {
+        cnt->usage += val;
+        return 0;
+    }
+
+    cnt->failcnt++;
+    return -ENOMEM;
+}
+
+static inline int res_counter_charge(struct res_counter *cnt,
+        unsigned long val)
+{
+    int ret;
+    unsigned long flags;
+

include/linux/res_counter.h line 61

Quoted 6 lines

include/linux/res_counter.h +6 -0

+    spin_lock_irqsave(&cnt->lock, flags);
+    ret = res_counter_charge_locked(cnt, val);
+    spin_unlock_irqrestore(&cnt->lock, flags);
+    return ret;
+}
+

Will atomic counters help here.

Quoted 127 lines

include/linux/res_counter.h +22 -0

+static inline void res_counter_uncharge_locked(struct res_counter *cnt,
+        unsigned long val)
+{
+    if (unlikely(cnt->usage < val)) {
+        WARN_ON(1);
+        val = cnt->usage;
+    }
+
+    cnt->usage -= val;
+}
+
+static inline void res_counter_uncharge(struct res_counter *cnt,
+        unsigned long val)
+{
+    unsigned long flags;
+
+    spin_lock_irqsave(&cnt->lock, flags);
+    res_counter_uncharge_locked(cnt, val);
+    spin_unlock_irqrestore(&cnt->lock, flags);
+}
+
+#endif

diff -upr linux-2.6.20.orig/init/Kconfig linux-2.6.20-0/init/Kconfig

init/Kconfig 2007-03-06 13:33:28.000000000 +0300 +4 -0

@@ -265,6 +265,10 @@ config CPUSETS
 
       Say N if unsure.
 
+config RESOURCE_COUNTERS
+    bool
+    select CONTAINERS
+
 config SYSFS_DEPRECATED
     bool "Create deprecated sysfs files"
     default y

diff -upr linux-2.6.20.orig/kernel/Makefile
linux-2.6.20-0/kernel/Makefile
--- linux-2.6.20.orig/kernel/Makefile 2007-03-06 13:33:28.000000000
+0300
+++ linux-2.6.20-0/kernel/Makefile 2007-03-06 13:33:28.000000000 +0300

init/Kconfig 2007-03-06 13:33:28.000000000 +0300 +1 -1

@@ -51,6 +51,7 @@ obj-$(CONFIG_RELAY) += relay.o
 obj-$(CONFIG_UTS_NS) += utsname.o
 obj-$(CONFIG_TASK_DELAY_ACCT) += delayacct.o
 obj-$(CONFIG_TASKSTATS) += taskstats.o tsacct.o
+obj-$(CONFIG_RESOURCE_COUNTERS) += res_counter.o
 
 ifneq ($(CONFIG_SCHED_NO_NO_OMIT_FRAME_POINTER),y)
 # According to Alan Modra <[email protected]>, the
-fno-omit-frame-pointer is

diff -upr linux-2.6.20.orig/kernel/res_counter.c
linux-2.6.20-0/kernel/res_counter.c
--- linux-2.6.20.orig/kernel/res_counter.c 2007-03-06
13:39:17.000000000 +0300
+++ linux-2.6.20-0/kernel/res_counter.c 2007-03-06
13:33:28.000000000 +0300

init/Kconfig 2007-03-06 13:33:28.000000000 +0300 +15 -0

@@ -0,0 +1,72 @@
+/*
+ * resource containers
+ *
+ * Copyright 2007 OpenVZ SWsoft Inc
+ *
+ * Author: Pavel Emelianov <[email protected]>
+ *
+ */
+
+#include <linux/parser.h>
+#include <linux/fs.h>
+#include <linux/res_counter.h>
+#include <asm/uaccess.h>
+
+static inline unsigned long *res_counter_member(struct res_counter

*cnt, int member)

init/Kconfig 2007-03-06 13:33:28.000000000 +0300 +54 -0

+{
+    switch (member) {
+    case RES_USAGE:
+        return &cnt->usage;
+    case RES_LIMIT:
+        return &cnt->limit;
+    case RES_FAILCNT:
+        return &cnt->failcnt;
+    };
+
+    BUG();
+    return NULL;
+}
+
+ssize_t res_counter_read(struct res_counter *cnt, int member,
+        const char __user *userbuf, size_t nbytes, loff_t *pos)
+{
+    unsigned long *val;
+    char buf[64], *s;
+
+    s = buf;
+    val = res_counter_member(cnt, member);
+    s += sprintf(s, "%lu\n", *val);
+    return simple_read_from_buffer((void __user *)userbuf, nbytes,
+            pos, buf, s - buf);
+}
+
+ssize_t res_counter_write(struct res_counter *cnt, int member,
+        const char __user *userbuf, size_t nbytes, loff_t *pos)
+{
+    int ret;
+    char *buf, *end;
+    unsigned long tmp, *val;
+
+    buf = kmalloc(nbytes + 1, GFP_KERNEL);
+    ret = -ENOMEM;
+    if (buf == NULL)
+        goto out;
+
+    buf[nbytes] = 0;
+    ret = -EFAULT;
+    if (copy_from_user(buf, userbuf, nbytes))
+        goto out_free;
+
+    ret = -EINVAL;
+    tmp = simple_strtoul(buf, &end, 10);
+    if (*end != '\0')
+        goto out_free;
+
+    val = res_counter_member(cnt, member);
+    *val = tmp;
+    ret = nbytes;
+out_free:
+    kfree(buf);

init/Kconfig 2007-03-06 13:33:28.000000000 +0300

Quoted 4 lines

init/Kconfig 2007-03-06 13:33:28.000000000 +0300 +3 -0

+out:
+    return ret;
+}

These bits look a little out of sync, with no users for these routines in this patch. Won't you get a compiler warning, compiling this bit alone?

Quoted 90 lines · Pavel Emelianov

This includes setup of RSS container within generic process containers, all the declarations used in RSS accounting, and core code responsible for accounting.

------------------------------------------------------------------------

diff -upr linux-2.6.20.orig/include/linux/rss_container.h linux-2.6.20-0/include/linux/rss_container.h
--- linux-2.6.20.orig/include/linux/rss_container.h 2007-03-06
13:39:17.000000000 +0300
+++ linux-2.6.20-0/include/linux/rss_container.h 2007-03-06
13:33:28.000000000 +0300

include/linux/rss_container.h +55 -0

@@ -0,0 +1,68 @@
+#ifndef __RSS_CONTAINER_H__
+#define __RSS_CONTAINER_H__
+/*
+ * RSS container
+ *
+ * Copyright 2007 OpenVZ SWsoft Inc
+ *
+ * Author: Pavel Emelianov <[email protected]>
+ *
+ */
+
+struct page_container;
+struct rss_container;
+
+#ifdef CONFIG_RSS_CONTAINER
+int container_rss_prepare(struct page *, struct vm_area_struct *vma,
+        struct page_container **);
+
+void container_rss_add(struct page_container *);
+void container_rss_del(struct page_container *);
+void container_rss_release(struct page_container *);
+
+int mm_init_container(struct mm_struct *mm, struct task_struct *tsk);
+void mm_free_container(struct mm_struct *mm);
+
+unsigned long container_isolate_pages(unsigned long nr_to_scan,
+        struct rss_container *rss, struct list_head *dst,
+        int active, unsigned long *scanned);
+unsigned long container_nr_physpages(struct rss_container *rss);
+
+unsigned long container_try_to_free_pages(struct rss_container *);
+void container_out_of_memory(struct rss_container *);
+
+void container_rss_init_early(void);
+#else
+static inline int container_rss_prepare(struct page *pg,
+        struct vm_area_struct *vma, struct page_container **pc)
+{
+    *pc = NULL; /* to make gcc happy */
+    return 0;
+}
+
+static inline void container_rss_add(struct page_container *pc)
+{
+}
+
+static inline void container_rss_del(struct page_container *pc)
+{
+}
+
+static inline void container_rss_release(struct page_container *pc)
+{
+}
+
+static inline int mm_init_container(struct mm_struct *mm, struct

task_struct *t)

include/linux/rss_container.h +13 -0

+{
+    return 0;
+}
+
+static inline void mm_free_container(struct mm_struct *mm)
+{
+}
+
+static inline void container_rss_init_early(void)
+{
+}
+#endif
+#endif

diff -upr linux-2.6.20.orig/init/Kconfig linux-2.6.20-0/init/Kconfig

init/Kconfig 2007-03-06 13:33:28.000000000 +0300 +0 -0

@@ -265,6 +265,13 @@ config CPUSETS
     bool
     select CONTAINERS
 

Quoted 8 lines

init/Kconfig 2007-03-06 13:33:28.000000000 +0300 +6 -0

+config RSS_CONTAINER
+    bool "RSS accounting container"
+    select RESOURCE_COUNTERS
+    help
+      Provides a simple Resource Controller for monitoring and
+      controlling the total Resident Set Size of the tasks in a

container
+

The wording looks very familiar :-). It would be useful to add "The reclaim logic is now container aware, when the container goes overlimit
the page reclaimer reclaims pages belonging to this container. If we are unable to reclaim enough pages to satisfy the request, the process is killed with an out of memory warning"

Quoted 39 lines

config SYSFS_DEPRECATED
bool "Create deprecated sysfs files"
default y
diff -upr linux-2.6.20.orig/mm/Makefile linux-2.6.20-0/mm/Makefile

mm/Makefile 2007-03-06 13:33:28.000000000 +0300 renamed from mm/Makefile 2007-02-04 21:44:54.000000000 +0300 +2 -0

@@ -29,3 +29,5 @@ obj-$(CONFIG_MEMORY_HOTPLUG) += memory_h
 obj-$(CONFIG_FS_XIP) += filemap_xip.o
 obj-$(CONFIG_MIGRATION) += migrate.o
 obj-$(CONFIG_SMP) += allocpercpu.o
+
+obj-$(CONFIG_RSS_CONTAINER) += rss_container.o

diff -upr linux-2.6.20.orig/mm/rss_container.c
linux-2.6.20-0/mm/rss_container.c
--- linux-2.6.20.orig/mm/rss_container.c 2007-03-06
13:39:17.000000000 +0300
+++ linux-2.6.20-0/mm/rss_container.c 2007-03-06 13:33:28.000000000
+0300

mm/Makefile 2007-03-06 13:33:28.000000000 +0300 renamed from mm/Makefile 2007-02-04 21:44:54.000000000 +0300 +19 -0

@@ -0,0 +1,307 @@
+/*
+ * RSS accounting container
+ *
+ * Copyright 2007 OpenVZ SWsoft Inc
+ *
+ * Author: Pavel Emelianov <[email protected]>
+ *
+ */
+
+#include <linux/list.h>
+#include <linux/sched.h>
+#include <linux/mm.h>
+#include <linux/res_counter.h>
+#include <linux/rss_container.h>
+
+static struct container_subsys rss_subsys;
+
+struct rss_container {
+    struct res_counter res;

mm/Makefile 2007-03-06 13:33:28.000000000 +0300 line 29

Quoted 10 lines

mm/Makefile 2007-03-06 13:33:28.000000000 +0300 renamed from mm/Makefile 2007-02-04 21:44:54.000000000 +0300 +10 -0

+    struct list_head page_list;
+    struct container_subsys_state css;
+};
+
+struct page_container {
+    struct page *page;
+    struct rss_container *cnt;
+    struct list_head list;
+};
+

Yes, this is what I was planning to get to -- a per container LRU list. But you have just one list, don't you need active and inactive lists? When the global LRU is manipulated, shouldn't this list be updated as well, so that reclaim will pick the best pages.

Quoted 37 lines

mm/Makefile 2007-03-06 13:33:28.000000000 +0300 renamed from mm/Makefile 2007-02-04 21:44:54.000000000 +0300 +37 -0

+static inline struct rss_container *rss_from_cont(struct container *cnt)
+{
+    return container_of(container_subsys_state(cnt, &rss_subsys),
+            struct rss_container, css);
+}
+
+int mm_init_container(struct mm_struct *mm, struct task_struct *tsk)
+{
+    struct rss_container *cnt;
+
+    cnt = rss_from_cont(task_container(tsk, &rss_subsys));
+    if (css_get(&cnt->css))
+        return -EBUSY;
+
+    mm->rss_container = cnt;
+    return 0;
+}
+
+void mm_free_container(struct mm_struct *mm)
+{
+    css_put(&mm->rss_container->css);
+}
+
+int container_rss_prepare(struct page *page, struct vm_area_struct *vma,
+        struct page_container **ppc)
+{
+    struct rss_container *rss;
+    struct page_container *pc;
+
+    rcu_read_lock();
+    rss = rcu_dereference(vma->vm_mm->rss_container);
+    css_get_current(&rss->css);
+    rcu_read_unlock();
+
+    pc = kmalloc(sizeof(struct page_container), GFP_KERNEL);
+    if (pc == NULL)
+        goto out_nomem;

mm/Makefile 2007-03-06 13:33:28.000000000 +0300 line 71

Quoted 5 lines

mm/Makefile 2007-03-06 13:33:28.000000000 +0300 renamed from mm/Makefile 2007-02-04 21:44:54.000000000 +0300 +5 -0

+
+    while (res_counter_charge(&rss->res, 1)) {
+        if (container_try_to_free_pages(rss))
+            continue;
+

The return codes of the functions is a bit confusing, ideally container_try_to_free_pages() should return 0 on success. Also

res_counter_charge() has a WARN_ON(1) if the limit is exceeded.

The system administrator can figure out the details from failcnt, I suspect when the container is running close to it's limit, dmesg will have too many WARNING messages.

How much memory do you try to reclaim in container_try_to_free_pages()?

With my patches, I was planning to export this knob to userspace with a default value. This will help the administrator decide how much of the working set/container LRU should be freed on reaching the limit. I cannot find the definition of container_try_to_free_pages() in this patch.

Quoted 34 lines

mm/Makefile 2007-03-06 13:33:28.000000000 +0300 renamed from mm/Makefile 2007-02-04 21:44:54.000000000 +0300 +32 -0

+        container_out_of_memory(rss);
+        if (test_thread_flag(TIF_MEMDIE))
+            goto out_charge;
+    }
+
+    pc->page = page;
+    pc->cnt = rss;
+    *ppc = pc;
+    return 0;
+
+out_charge:
+    kfree(pc);
+out_nomem:
+    css_put(&rss->css);
+    return -ENOMEM;
+}
+
+void container_rss_release(struct page_container *pc)
+{
+    struct rss_container *rss;
+
+    rss = pc->cnt;
+    res_counter_uncharge(&rss->res, 1);
+    css_put(&rss->css);
+    kfree(pc);
+}
+
+void container_rss_add(struct page_container *pc)
+{
+    struct page *pg;
+    struct rss_container *rss;
+

mm/Makefile 2007-03-06 13:33:28.000000000 +0300 line 108

Quoted 5 lines

mm/Makefile 2007-03-06 13:33:28.000000000 +0300 renamed from mm/Makefile 2007-02-04 21:44:54.000000000 +0300 +5 -0

+    pg = pc->page;
+    rss = pc->cnt;
+
+    spin_lock(&rss->res.lock);
+    list_add(&pc->list, &rss->page_list);

This is not good, it won't give us LRU behaviour which is useful for determining which pages to free.

Quoted 33 lines

mm/Makefile 2007-03-06 13:33:28.000000000 +0300 renamed from mm/Makefile 2007-02-04 21:44:54.000000000 +0300 +33 -0

+    spin_unlock(&rss->res.lock);
+
+    page_container(pg) = pc;
+}
+
+void container_rss_del(struct page_container *pc)
+{
+    struct page *page;
+    struct rss_container *rss;
+
+    page = pc->page;
+    rss = pc->cnt;
+
+    spin_lock(&rss->res.lock);
+    list_del(&pc->list);
+    res_counter_uncharge_locked(&rss->res, 1);
+    spin_unlock(&rss->res.lock);
+
+    css_put(&rss->css);
+    kfree(pc);
+}
+
+unsigned long container_isolate_pages(unsigned long nr_to_scan,
+        struct rss_container *rss, struct list_head *dst,
+        int active, unsigned long *scanned)
+{
+    unsigned long nr_taken = 0;
+    struct page *page;
+    struct page_container *pc;
+    unsigned long scan;
+    struct list_head *src;
+    LIST_HEAD(pc_list);
+    struct zone *z;

mm/Makefile 2007-03-06 13:33:28.000000000 +0300 line 145

Quoted 4 lines

mm/Makefile 2007-03-06 13:33:28.000000000 +0300 renamed from mm/Makefile 2007-02-04 21:44:54.000000000 +0300 +4 -0

+
+    spin_lock_irq(&rss->res.lock);
+    src = &rss->page_list;
+

Which part of the working set are we pushing out, this looks like we are using FIFO to determine which pages to reclaim. This needs to be FIXED.

Quoted 18 lines

mm/Makefile 2007-03-06 13:33:28.000000000 +0300 renamed from mm/Makefile 2007-02-04 21:44:54.000000000 +0300 +18 -0

+    for (scan = 0; scan < nr_to_scan && !list_empty(src); scan++) {
+        pc = list_entry(src->prev, struct page_container, list);
+        page = pc->page;
+        z = page_zone(page);
+
+        list_move(&pc->list, &pc_list);
+
+        spin_lock(&z->lru_lock);
+        if (PageLRU(page)) {
+            if ((active && PageActive(page)) ||
+                    (!active && !PageActive(page))) {
+                if (likely(get_page_unless_zero(page))) {
+                    ClearPageLRU(page);
+                    nr_taken++;
+                    list_move(&page->lru, dst);
+                }
+            }
+        }

mm/Makefile 2007-03-06 13:33:28.000000000 +0300 line 167

Quoted 4 lines

mm/Makefile 2007-03-06 13:33:28.000000000 +0300 renamed from mm/Makefile 2007-02-04 21:44:54.000000000 +0300 +4 -0

+        spin_unlock(&z->lru_lock);
+    }
+
+    list_splice(&pc_list, src);

This would lead to LRU churning, I would recommend using list_splice_tail() instead. Since this code has a lot in common with isolate_lru_pages, it would be nice to reuse the code in vmscan.c

NOTE: Code duplication is a back door for subtle bugs and solving the same issue twice :-)

Quoted 26 lines

mm/Makefile 2007-03-06 13:33:28.000000000 +0300 renamed from mm/Makefile 2007-02-04 21:44:54.000000000 +0300 +26 -0

+    spin_unlock_irq(&rss->res.lock);
+
+    *scanned = scan;
+    return nr_taken;
+}
+
+unsigned long container_nr_physpages(struct rss_container *rss)
+{
+    return rss->res.usage;
+}
+
+static void rss_move_task(struct container_subsys *ss,
+        struct container *cont,
+        struct container *old_cont,
+        struct task_struct *p)
+{
+    struct mm_struct *mm;
+    struct rss_container *rss, *old_rss;
+
+    mm = get_task_mm(p);
+    if (mm == NULL)
+        goto out;
+
+    rss = rss_from_cont(cont);
+    old_rss = rss_from_cont(old_cont);
+    if (old_rss != mm->rss_container)

mm/Makefile 2007-03-06 13:33:28.000000000 +0300 line 199

Quoted 6 lines

mm/Makefile 2007-03-06 13:33:28.000000000 +0300 renamed from mm/Makefile 2007-02-04 21:44:54.000000000 +0300 +6 -0

+        goto out_put;
+
+    css_get_current(&rss->css);
+    rcu_assign_pointer(mm->rss_container, rss);
+    css_put(&old_rss->css);
+

I see that the charges are not migrated. Is that good?

If a user could find a way of migrating his/her task from one container to another, it could create an issue with the user's task taking up a big chunk of the RSS limit.

Can we migrate any task or just the thread group leader. In my patches, I allowed migration of just the thread group leader. Imagine if you have several threads, no matter which container they belong to, their mm gets charged (usage will not show up in the container's usage). This could confuse the system administrator.

Quoted 110 lines

mm/Makefile 2007-03-06 13:33:28.000000000 +0300 renamed from mm/Makefile 2007-02-04 21:44:54.000000000 +0300 +7 -0

+out_put:
+    mmput(mm);
+out:
+    return;
+}
+
+static int rss_create(struct container_subsys *ss, struct container

*cont)

mm/Makefile 2007-03-06 13:33:28.000000000 +0300 renamed from mm/Makefile 2007-02-04 21:44:54.000000000 +0300 +101 -0

+{
+    struct rss_container *rss;
+
+    rss = kzalloc(sizeof(struct rss_container), GFP_KERNEL);
+    if (rss == NULL)
+        return -ENOMEM;
+
+    res_counter_init(&rss->res);
+    INIT_LIST_HEAD(&rss->page_list);
+    cont->subsys[rss_subsys.subsys_id] = &rss->css;
+    return 0;
+}
+
+static void rss_destroy(struct container_subsys *ss,
+        struct container *cont)
+{
+    kfree(rss_from_cont(cont));
+}
+
+
+static ssize_t rss_read(struct container *cont, struct cftype *cft,
+        struct file *file, char __user *userbuf,
+        size_t nbytes, loff_t *ppos)
+{
+    return res_counter_read(&rss_from_cont(cont)->res, cft->private,
+            userbuf, nbytes, ppos);
+}
+
+static ssize_t rss_write(struct container *cont, struct cftype *cft,
+        struct file *file, const char __user *userbuf,
+        size_t nbytes, loff_t *ppos)
+{
+    return res_counter_write(&rss_from_cont(cont)->res, cft->private,
+            userbuf, nbytes, ppos);
+}
+
+
+static struct cftype rss_usage = {
+    .name = "rss_usage",
+    .private = RES_USAGE,
+    .read = rss_read,
+};
+
+static struct cftype rss_limit = {
+    .name = "rss_limit",
+    .private = RES_LIMIT,
+    .read = rss_read,
+    .write = rss_write,
+};
+
+static struct cftype rss_failcnt = {
+    .name = "rss_failcnt",
+    .private = RES_FAILCNT,
+    .read = rss_read,
+};
+
+static int rss_populate(struct container_subsys *ss,
+        struct container *cont)
+{
+    int rc;
+
+    if ((rc = container_add_file(cont, &rss_usage)) < 0)
+        return rc;
+    if ((rc = container_add_file(cont, &rss_failcnt)) < 0)
+        return rc;
+    if ((rc = container_add_file(cont, &rss_limit)) < 0)
+        return rc;
+
+    return 0;
+}
+
+static struct rss_container init_rss_container;
+
+static __init int rss_create_early(struct container_subsys *ss,
+        struct container *cont)
+{
+    struct rss_container *rss;
+
+    rss = &init_rss_container;
+    res_counter_init(&rss->res);
+    INIT_LIST_HEAD(&rss->page_list);
+    cont->subsys[rss_subsys.subsys_id] = &rss->css;
+    ss->create = rss_create;
+    return 0;
+}
+
+static struct container_subsys rss_subsys = {
+    .name = "rss",
+    .create = rss_create_early,
+    .destroy = rss_destroy,
+    .populate = rss_populate,
+    .attach = rss_move_task,
+};
+
+void __init container_rss_init_early(void)
+{
+    container_register_subsys(&rss_subsys);
+    init_mm.rss_container = rss_from_cont(
+            task_container(&init_task, &rss_subsys));
+    css_get_current(&init_mm.rss_container->css);
+}

Quoted 6 lines · Pavel Emelianov

2. Extended containers may register themselves too late. Kernel threads/helpers start forking, opening files and touching pages much earlier. This patchset workarounds this in not-so-cute manner and I'm waiting for Paul's comments on this issue.

Can we not make sure that each subsystem registers itself before any of its resources become usable? So the file counting subsystem should

register at some point before filp_open() becomes usable, and the process counting subsystem should register before it's possible to fork, etc.

Paul

Quoted 12 lines · Pavel Emelianov

This patchset adds RSS, accounting and control and limiting the number of tasks and files within container.

Based on top of Paul Menage's container subsystem v7

RSS controller includes per-container RSS accounter, reclamation and OOM killer. It behaves like standalone machine - when container runs out of resources it tries to reclaim some pages and if it doesn't succeed in it kills some task which mm_struct belongs to container in question.

Quoted 7 lines

Num tasks and files containers are very simple and self-descriptive from code.

As discussed before when a task moves from one container to another no resources follow it - they keep holding the container they were allocated in.

I have one problem with the patchset, I cannot compile the patches individually and some of the code is hard to read as it depends on functions from future patches. Patch 2, 3 and 4 fail to compile without patch 5 applied.

Patch 1 failed to apply with a reject in kernel/Makefile I applied it on top of 2.6.20 with all of Paul Menage's patches (all 7).

Quoted 22 lines · Balbir Singh

Quoted 19 lines · Pavel Emelianov

This patchset adds RSS, accounting and control and limiting the number of tasks and files within container.

Based on top of Paul Menage's container subsystem v7

RSS controller includes per-container RSS accounter, reclamation and OOM killer. It behaves like standalone machine - when container runs out of resources it tries to reclaim some pages and if it doesn't succeed in it kills some task which mm_struct belongs to container in question.

Num tasks and files containers are very simple and self-descriptive from code.

As discussed before when a task moves from one container to another no resources follow it - they keep holding the container they were allocated in.

Quoted 8 lines

I have one problem with the patchset, I cannot compile the patches individually and some of the code is hard to read as it depends on functions from future patches. Patch 2, 3 and 4 fail to compile without patch 5 applied.

Patch 1 failed to apply with a reject in kernel/Makefile I applied it on top of 2.6.20 with all of Paul Menage's patches (all 7).

The idea is:

Task may be "the entity that allocates the resources" and "the entity that is a resource allocated".

When task is the first entity it may move across containers (that is implemented in your patches). When task is a resource it shouldn't move across containers like files or pages do.

More generally - allocated resources hold reference to original container till they die. No resource migration is performed.

Did I express my idea cleanly?

Quoted 49 lines · Balbir Singh

Quoted 48 lines · Pavel Emelianov

Introduce generic structures and routines for
resource accounting.

Each resource accounting container is supposed to
aggregate it, container_subsystem_state and its
resource-specific members within.

------------------------------------------------------------------------

diff -upr linux-2.6.20.orig/include/linux/res_counter.h linux-2.6.20-0/include/linux/res_counter.h
--- linux-2.6.20.orig/include/linux/res_counter.h 2007-03-06
13:39:17.000000000 +0300
+++ linux-2.6.20-0/include/linux/res_counter.h 2007-03-06
13:33:28.000000000 +0300

include/linux/res_counter.h +31 -0

@@ -0,0 +1,83 @@
+#ifndef __RES_COUNTER_H__
+#define __RES_COUNTER_H__
+/*
+ * resource counters
+ *
+ * Copyright 2007 OpenVZ SWsoft Inc
+ *
+ * Author: Pavel Emelianov <[email protected]>
+ *
+ */
+
+#include <linux/container.h>
+
+struct res_counter {
+    unsigned long usage;
+    unsigned long limit;
+    unsigned long failcnt;
+    spinlock_t lock;
+};
+
+enum {
+    RES_USAGE,
+    RES_LIMIT,
+    RES_FAILCNT,
+};
+
+ssize_t res_counter_read(struct res_counter *cnt, int member,
+        const char __user *buf, size_t nbytes, loff_t *pos);
+ssize_t res_counter_write(struct res_counter *cnt, int member,
+        const char __user *buf, size_t nbytes, loff_t *pos);
+

Quoted 8 lines

include/linux/res_counter.h line 37

Quoted 6 lines

include/linux/res_counter.h +6 -0

+static inline void res_counter_init(struct res_counter *cnt)
+{
+    spin_lock_init(&cnt->lock);
+    cnt->limit = (unsigned long)LONG_MAX;
+}
+

Is there any way to indicate that there are no limits on this container.

Yes - LONG_MAX is essentially a "no limit" value as no container will ever have such many files :)

Quoted 22 lines

LONG_MAX is quite huge, but still when the administrator wants to configure a container to *un-limited usage*, it becomes hard for the administrator.

Quoted 18 lines

include/linux/res_counter.h +18 -0

+static inline int res_counter_charge_locked(struct res_counter *cnt,
+        unsigned long val)
+{
+    if (cnt->usage <= cnt->limit - val) {
+        cnt->usage += val;
+        return 0;
+    }
+
+    cnt->failcnt++;
+    return -ENOMEM;
+}
+
+static inline int res_counter_charge(struct res_counter *cnt,
+        unsigned long val)
+{
+    int ret;
+    unsigned long flags;
+

Quoted 8 lines

include/linux/res_counter.h line 61

Quoted 6 lines

include/linux/res_counter.h +6 -0

+    spin_lock_irqsave(&cnt->lock, flags);
+    ret = res_counter_charge_locked(cnt, val);
+    spin_unlock_irqrestore(&cnt->lock, flags);
+    return ret;
+}
+

Will atomic counters help here.

I'm afraid no. We have to atomically check for limit and alter one of usage or failcnt depending on the checking result. Making this with atomic_xxx ops will require at least two ops.

If we'll remove failcnt this would look like
while (atomic_cmpxchg(...))
which is also not that good.

Moreover - in RSS accounting patches I perform page list manipulations under this lock, so this also saves one atomic op.

Quoted 127 lines

Quoted 127 lines

include/linux/res_counter.h +22 -0

+static inline void res_counter_uncharge_locked(struct res_counter *cnt,
+        unsigned long val)
+{
+    if (unlikely(cnt->usage < val)) {
+        WARN_ON(1);
+        val = cnt->usage;
+    }
+
+    cnt->usage -= val;
+}
+
+static inline void res_counter_uncharge(struct res_counter *cnt,
+        unsigned long val)
+{
+    unsigned long flags;
+
+    spin_lock_irqsave(&cnt->lock, flags);
+    res_counter_uncharge_locked(cnt, val);
+    spin_unlock_irqrestore(&cnt->lock, flags);
+}
+
+#endif

diff -upr linux-2.6.20.orig/init/Kconfig linux-2.6.20-0/init/Kconfig

init/Kconfig 2007-03-06 13:33:28.000000000 +0300 +4 -0

@@ -265,6 +265,10 @@ config CPUSETS
 
       Say N if unsure.
 
+config RESOURCE_COUNTERS
+    bool
+    select CONTAINERS
+
 config SYSFS_DEPRECATED
     bool "Create deprecated sysfs files"
     default y

diff -upr linux-2.6.20.orig/kernel/Makefile
linux-2.6.20-0/kernel/Makefile
--- linux-2.6.20.orig/kernel/Makefile 2007-03-06 13:33:28.000000000
+0300
+++ linux-2.6.20-0/kernel/Makefile 2007-03-06 13:33:28.000000000 +0300

init/Kconfig 2007-03-06 13:33:28.000000000 +0300 +1 -1

@@ -51,6 +51,7 @@ obj-$(CONFIG_RELAY) += relay.o
 obj-$(CONFIG_UTS_NS) += utsname.o
 obj-$(CONFIG_TASK_DELAY_ACCT) += delayacct.o
 obj-$(CONFIG_TASKSTATS) += taskstats.o tsacct.o
+obj-$(CONFIG_RESOURCE_COUNTERS) += res_counter.o
 
 ifneq ($(CONFIG_SCHED_NO_NO_OMIT_FRAME_POINTER),y)
 # According to Alan Modra <[email protected]>, the
-fno-omit-frame-pointer is

diff -upr linux-2.6.20.orig/kernel/res_counter.c
linux-2.6.20-0/kernel/res_counter.c
--- linux-2.6.20.orig/kernel/res_counter.c 2007-03-06
13:39:17.000000000 +0300
+++ linux-2.6.20-0/kernel/res_counter.c 2007-03-06
13:33:28.000000000 +0300

init/Kconfig 2007-03-06 13:33:28.000000000 +0300 +15 -0

@@ -0,0 +1,72 @@
+/*
+ * resource containers
+ *
+ * Copyright 2007 OpenVZ SWsoft Inc
+ *
+ * Author: Pavel Emelianov <[email protected]>
+ *
+ */
+
+#include <linux/parser.h>
+#include <linux/fs.h>
+#include <linux/res_counter.h>
+#include <asm/uaccess.h>
+
+static inline unsigned long *res_counter_member(struct res_counter

*cnt, int member)

init/Kconfig 2007-03-06 13:33:28.000000000 +0300 +54 -0

+{
+    switch (member) {
+    case RES_USAGE:
+        return &cnt->usage;
+    case RES_LIMIT:
+        return &cnt->limit;
+    case RES_FAILCNT:
+        return &cnt->failcnt;
+    };
+
+    BUG();
+    return NULL;
+}
+
+ssize_t res_counter_read(struct res_counter *cnt, int member,
+        const char __user *userbuf, size_t nbytes, loff_t *pos)
+{
+    unsigned long *val;
+    char buf[64], *s;
+
+    s = buf;
+    val = res_counter_member(cnt, member);
+    s += sprintf(s, "%lu\n", *val);
+    return simple_read_from_buffer((void __user *)userbuf, nbytes,
+            pos, buf, s - buf);
+}
+
+ssize_t res_counter_write(struct res_counter *cnt, int member,
+        const char __user *userbuf, size_t nbytes, loff_t *pos)
+{
+    int ret;
+    char *buf, *end;
+    unsigned long tmp, *val;
+
+    buf = kmalloc(nbytes + 1, GFP_KERNEL);
+    ret = -ENOMEM;
+    if (buf == NULL)
+        goto out;
+
+    buf[nbytes] = 0;
+    ret = -EFAULT;
+    if (copy_from_user(buf, userbuf, nbytes))
+        goto out_free;
+
+    ret = -EINVAL;
+    tmp = simple_strtoul(buf, &end, 10);
+    if (*end != '\0')
+        goto out_free;
+
+    val = res_counter_member(cnt, member);
+    *val = tmp;
+    ret = nbytes;
+out_free:
+    kfree(buf);

Quoted 9 lines

init/Kconfig 2007-03-06 13:33:28.000000000 +0300

Quoted 4 lines

init/Kconfig 2007-03-06 13:33:28.000000000 +0300 +3 -0

+out:
+    return ret;
+}

These bits look a little out of sync, with no users for these routines in this patch. Won't you get a compiler warning, compiling this bit alone?

Nope - when you have a non-static function without users in a file no compiler warning produced.
_______________________________________________
Containers mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/containers

mm/rss_container.c line 28

Quoted 11 lines · On Tue, 06 Mar 2007 17:55:29 +0300 Pavel Emelianov <[email protected]> wrote:

mm/rss_container.c +11 -0

+struct rss_container {
+	struct res_counter res;
+	struct list_head page_list;
+	struct container_subsys_state css;
+};
+
+struct page_container {
+	struct page *page;
+	struct rss_container *cnt;
+	struct list_head list;
+};

ah. This looks good. I'll find a hunk of time to go through this work and through Paul's patches. It'd be good to get both patchsets lined up in -mm within a couple of weeks. But..

We need to decide whether we want to do per-container memory limitation via these data structures, or whether we do it via a physical scan of some software zone, possibly based on Mel's patches.

_______________________________________________
Containers mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/containers

Quoted 5 lines · Pavel Emelianov

The idea is:

Task may be "the entity that allocates the resources" and "the entity that is a resource allocated".

Quoted 8 lines

When task is the first entity it may move across containers (that is implemented in your patches). When task is a resource it shouldn't move across containers like files or pages do.

More generally - allocated resources hold reference to original container till they die. No resource migration is performed.

Did I express my idea cleanly?

Yes, but I disagree with the premise. The title of your patch is "Account for the number of tasks within container", but that's not what the subsystem does, it accounts for the number of forks within the container that aren't directly accompanied by an exit.

Ideally, resources like files and pages would be able to follow tasks as well. The reason that files and pages aren't easily migrated from one container to another is that there could be sharing involved; figuring out the sharing can be expensive, and it's not clear what to do if two users are in different containers.

But in the case of a task count, there are no such issues with sharing, so it seems to me to be more sensible (and more efficient) to just limit the number of tasks in a container.

i.e. when moving a task into a container or forking a task within a container, increment the count; when moving a task out of a container or when it exits, decrement the count.

With your approach, if you were to set the task limit of an empty container A to 1, and then move a process P from B into A, P would be able to fork a new child, since the "task count" would be 0 (as P was being charged to B still). Surely the fact that there's 1 process in A should prevent P from forking?

Paul

Hi, Pavel,

Please find my patch to add LRU behaviour to your latest RSS controller.

Balbir Singh
Linux Technology Center
IBM, ISTL

------------------------------------------------------------------------

Add LRU behaviour to the RSS controller patches posted by Pavel Emelianov

	http://lkml.org/lkml/2007/3/6/198

which was in turn similar to the RSS controller posted by me

	http://lkml.org/lkml/2007/2/26/8

Pavel's patches have a per container list of pages, which helps reduce reclaim time of the RSS controller but the per container list of pages is in FIFO order. I've implemented active and inactive lists per container to help select the right set of pages to reclaim when the container is under memory pressure.

I've tested these patches on a ppc64 machine and they work fine for the minimal testing I've done.

Pavel would you please include these patches in your next iteration.

Comments, suggestions and further improvements are as always welcome!

Signed-off-by <[email protected]>

---

 include/linux/rss_container.h |    1 
 mm/rss_container.c            |   47 +++++++++++++++++++++++++++++++-----------
 mm/swap.c                     |    5 ++++
 mm/vmscan.c                   |    3 ++
 4 files changed, 44 insertions(+), 12 deletions(-)

diff -puN include/linux/rss_container.h~rss-container-lru2 include/linux/rss_container.h

include/linux/rss_container.h renamed from include/linux/rss_container.h~rss-container-lru2 +1 -0

@@ -19,6 +19,7 @@ int container_rss_prepare(struct page *,
 void container_rss_add(struct page_container *);
 void container_rss_del(struct page_container *);
 void container_rss_release(struct page_container *);
+void container_rss_move_lists(struct page *pg, bool active);
 
 int mm_init_container(struct mm_struct *mm, struct task_struct *tsk);
 void mm_free_container(struct mm_struct *mm);

diff -puN mm/rss_container.c~rss-container-lru2 mm/rss_container.c

mm/rss_container.c renamed from mm/rss_container.c~rss-container-lru2 +35 -12

@@ -17,7 +17,8 @@ static struct container_subsys rss_subsy
 
 struct rss_container {
 	struct res_counter res;
-	struct list_head page_list;
+	struct list_head inactive_list;
+	struct list_head active_list;
 	struct container_subsys_state css;
 };
 

@@ -96,6 +97,26 @@ void container_rss_release(struct page_c
 	kfree(pc);
 }
 
+void container_rss_move_lists(struct page *pg, bool active)
+{
+	struct rss_container *rss;
+	struct page_container *pc;
+
+	if (!page_mapped(pg))
+		return;
+
+	pc = page_container(pg);
+	BUG_ON(!pc);
+	rss = pc->cnt;
+
+	spin_lock_irq(&rss->res.lock);
+	if (active)
+		list_move(&pc->list, &rss->active_list);
+	else
+		list_move(&pc->list, &rss->inactive_list);
+	spin_unlock_irq(&rss->res.lock);
+}
+
 void container_rss_add(struct page_container *pc)
 {
 	struct page *pg;

@@ -105,7 +126,7 @@ void container_rss_add(struct page_conta
 	rss = pc->cnt;
 
 	spin_lock(&rss->res.lock);
-	list_add(&pc->list, &rss->page_list);
+	list_add(&pc->list, &rss->active_list);
 	spin_unlock(&rss->res.lock);
 
 	page_container(pg) = pc;

@@ -141,7 +162,10 @@ unsigned long container_isolate_pages(un
 	struct zone *z;
 
 	spin_lock_irq(&rss->res.lock);
-	src = &rss->page_list;
+	if (active)
+		src = &rss->active_list;
+	else
+		src = &rss->inactive_list;
 
 	for (scan = 0; scan < nr_to_scan && !list_empty(src); scan++) {
 		pc = list_entry(src->prev, struct page_container, list);

@@ -152,13 +176,10 @@ unsigned long container_isolate_pages(un
 
 		spin_lock(&z->lru_lock);
 		if (PageLRU(page)) {
-			if ((active && PageActive(page)) ||
-					(!active && !PageActive(page))) {
-				if (likely(get_page_unless_zero(page))) {
-					ClearPageLRU(page);
-					nr_taken++;
-					list_move(&page->lru, dst);
-				}
+			if (likely(get_page_unless_zero(page))) {
+				ClearPageLRU(page);
+				nr_taken++;
+				list_move(&page->lru, dst);
 			}
 		}
 		spin_unlock(&z->lru_lock);

@@ -212,7 +233,8 @@ static int rss_create(struct container_s
 		return -ENOMEM;
 
 	res_counter_init(&rss->res);
-	INIT_LIST_HEAD(&rss->page_list);
+	INIT_LIST_HEAD(&rss->inactive_list);
+	INIT_LIST_HEAD(&rss->active_list);
 	cont->subsys[rss_subsys.subsys_id] = &rss->css;
 	return 0;
 }

@@ -284,7 +306,8 @@ static __init int rss_create_early(struc
 
 	rss = &init_rss_container;
 	res_counter_init(&rss->res);
-	INIT_LIST_HEAD(&rss->page_list);
+	INIT_LIST_HEAD(&rss->inactive_list);
+	INIT_LIST_HEAD(&rss->active_list);
 	cont->subsys[rss_subsys.subsys_id] = &rss->css;
 	ss->create = rss_create;
 	return 0;

diff -puN mm/vmscan.c~rss-container-lru2 mm/vmscan.c

mm/vmscan.c renamed from mm/vmscan.c~rss-container-lru2 +3 -0

@@ -1142,6 +1142,7 @@ static unsigned long container_shrink_pa
 			else
 				add_page_to_inactive_list(z, page);
 			spin_unlock_irq(&z->lru_lock);
+			container_rss_move_lists(page, false);
 
 			put_page(page);
 		}

@@ -1191,6 +1192,7 @@ static void container_shrink_pages_activ
 		list_move(&page->lru, &z->inactive_list);
 		z->nr_inactive++;
 		spin_unlock_irq(&z->lru_lock);
+		container_rss_move_lists(page, false);
 
 		put_page(page);
 	}

@@ -1206,6 +1208,7 @@ static void container_shrink_pages_activ
 		list_move(&page->lru, &z->active_list);
 		z->nr_active++;
 		spin_unlock_irq(&z->lru_lock);
+		container_rss_move_lists(page, true);
 
 		put_page(page);
 	}

diff -puN mm/swap.c~rss-container-lru2 mm/swap.c

mm/swap.c renamed from mm/swap.c~rss-container-lru2 +5 -0

@@ -30,6 +30,7 @@
 #include <linux/cpu.h>
 #include <linux/notifier.h>
 #include <linux/init.h>
+#include <linux/rss_container.h>
 
 /* How many pages do we try to swap or page in/out together? */
 int page_cluster;

@@ -140,6 +141,7 @@ int rotate_reclaimable_page(struct page 
 void fastcall activate_page(struct page *page)
 {
 	struct zone *zone = page_zone(page);
+	bool moved = false;
 
 	spin_lock_irq(&zone->lru_lock);
 	if (PageLRU(page) && !PageActive(page)) {

@@ -147,8 +149,11 @@ void fastcall activate_page(struct page 
 		SetPageActive(page);
 		add_page_to_active_list(zone, page);
 		__count_vm_event(PGACTIVATE);
+		moved = true;
 	}
 	spin_unlock_irq(&zone->lru_lock);
+	if (moved)
+		container_rss_move_lists(page, true);
 }
 
 /*

_

Quoted 57 lines · Pavel Emelianov

Quoted 56 lines · Balbir Singh

Quoted 44 lines · Pavel Emelianov

Introduce generic structures and routines for
resource accounting.

Each resource accounting container is supposed to
aggregate it, container_subsystem_state and its
resource-specific members within.

------------------------------------------------------------------------

diff -upr linux-2.6.20.orig/include/linux/res_counter.h linux-2.6.20-0/include/linux/res_counter.h
--- linux-2.6.20.orig/include/linux/res_counter.h 2007-03-06
13:39:17.000000000 +0300
+++ linux-2.6.20-0/include/linux/res_counter.h 2007-03-06
13:33:28.000000000 +0300

include/linux/res_counter.h +27 -0

@@ -0,0 +1,83 @@
+#ifndef __RES_COUNTER_H__
+#define __RES_COUNTER_H__
+/*
+ * resource counters
+ *
+ * Copyright 2007 OpenVZ SWsoft Inc
+ *
+ * Author: Pavel Emelianov <[email protected]>
+ *
+ */
+
+#include <linux/container.h>
+
+struct res_counter {
+    unsigned long usage;
+    unsigned long limit;
+    unsigned long failcnt;
+    spinlock_t lock;
+};
+
+enum {
+    RES_USAGE,
+    RES_LIMIT,
+    RES_FAILCNT,
+};
+
+ssize_t res_counter_read(struct res_counter *cnt, int member,

include/linux/res_counter.h line 37

Quoted 10 lines

include/linux/res_counter.h +10 -0

+        const char __user *buf, size_t nbytes, loff_t *pos);
+ssize_t res_counter_write(struct res_counter *cnt, int member,
+        const char __user *buf, size_t nbytes, loff_t *pos);
+
+static inline void res_counter_init(struct res_counter *cnt)
+{
+    spin_lock_init(&cnt->lock);
+    cnt->limit = (unsigned long)LONG_MAX;
+}
+

Is there any way to indicate that there are no limits on this container.

Quoted 2 lines

Yes - LONG_MAX is essentially a "no limit" value as no container will ever have such many files :)

-1 or ~0 is a viable choice for userspace to communicate 'infinite' or 'unlimited'

Quoted 29 lines

Quoted 29 lines

LONG_MAX is quite huge, but still when the administrator wants to configure a container to *un-limited usage*, it becomes hard for the administrator.

Quoted 14 lines

include/linux/res_counter.h +14 -0

+static inline int res_counter_charge_locked(struct res_counter *cnt,
+        unsigned long val)
+{
+    if (cnt->usage <= cnt->limit - val) {
+        cnt->usage += val;
+        return 0;
+    }
+
+    cnt->failcnt++;
+    return -ENOMEM;
+}
+
+static inline int res_counter_charge(struct res_counter *cnt,
+        unsigned long val)

include/linux/res_counter.h line 61

Quoted 10 lines

include/linux/res_counter.h +10 -0

+{
+    int ret;
+    unsigned long flags;
+
+    spin_lock_irqsave(&cnt->lock, flags);
+    ret = res_counter_charge_locked(cnt, val);
+    spin_unlock_irqrestore(&cnt->lock, flags);
+    return ret;
+}
+

Will atomic counters help here.

Quoted 3 lines

I'm afraid no. We have to atomically check for limit and alter one of usage or failcnt depending on the checking result. Making this with atomic_xxx ops will require at least two ops.

Linux-VServer does the accounting with atomic counters, so that works quite fine, just do the checks at the beginning of whatever resource allocation and the accounting once the resource is acquired ...

From the time you checked for resource till you really

Quoted 6 lines

If we'll remove failcnt this would look like
while (atomic_cmpxchg(...))
which is also not that good.

Moreover - in RSS accounting patches I perform page list manipulations under this lock, so this also saves one atomic op.

it still hasn't been shown that this kind of RSS limit doesn't add big time overhead to normal operations (inside and outside of such a resource container)

note that the 'usual' memory accounting is much more lightweight and serves similar purposes ...

best,
Herbert

Quoted 141 lines

Quoted 126 lines

Quoted 126 lines

include/linux/res_counter.h +22 -0

+static inline void res_counter_uncharge_locked(struct res_counter *cnt,
+        unsigned long val)
+{
+    if (unlikely(cnt->usage < val)) {
+        WARN_ON(1);
+        val = cnt->usage;
+    }
+
+    cnt->usage -= val;
+}
+
+static inline void res_counter_uncharge(struct res_counter *cnt,
+        unsigned long val)
+{
+    unsigned long flags;
+
+    spin_lock_irqsave(&cnt->lock, flags);
+    res_counter_uncharge_locked(cnt, val);
+    spin_unlock_irqrestore(&cnt->lock, flags);
+}
+
+#endif

diff -upr linux-2.6.20.orig/init/Kconfig linux-2.6.20-0/init/Kconfig

init/Kconfig 2007-03-06 13:33:28.000000000 +0300 +4 -0

@@ -265,6 +265,10 @@ config CPUSETS
 
       Say N if unsure.
 
+config RESOURCE_COUNTERS
+    bool
+    select CONTAINERS
+
 config SYSFS_DEPRECATED
     bool "Create deprecated sysfs files"
     default y

diff -upr linux-2.6.20.orig/kernel/Makefile
linux-2.6.20-0/kernel/Makefile
--- linux-2.6.20.orig/kernel/Makefile 2007-03-06 13:33:28.000000000
+0300
+++ linux-2.6.20-0/kernel/Makefile 2007-03-06 13:33:28.000000000 +0300

init/Kconfig 2007-03-06 13:33:28.000000000 +0300 +1 -1

@@ -51,6 +51,7 @@ obj-$(CONFIG_RELAY) += relay.o
 obj-$(CONFIG_UTS_NS) += utsname.o
 obj-$(CONFIG_TASK_DELAY_ACCT) += delayacct.o
 obj-$(CONFIG_TASKSTATS) += taskstats.o tsacct.o
+obj-$(CONFIG_RESOURCE_COUNTERS) += res_counter.o
 
 ifneq ($(CONFIG_SCHED_NO_NO_OMIT_FRAME_POINTER),y)
 # According to Alan Modra <[email protected]>, the
-fno-omit-frame-pointer is

diff -upr linux-2.6.20.orig/kernel/res_counter.c
linux-2.6.20-0/kernel/res_counter.c
--- linux-2.6.20.orig/kernel/res_counter.c 2007-03-06
13:39:17.000000000 +0300
+++ linux-2.6.20-0/kernel/res_counter.c 2007-03-06
13:33:28.000000000 +0300

init/Kconfig 2007-03-06 13:33:28.000000000 +0300 +15 -0

@@ -0,0 +1,72 @@
+/*
+ * resource containers
+ *
+ * Copyright 2007 OpenVZ SWsoft Inc
+ *
+ * Author: Pavel Emelianov <[email protected]>
+ *
+ */
+
+#include <linux/parser.h>
+#include <linux/fs.h>
+#include <linux/res_counter.h>
+#include <asm/uaccess.h>
+
+static inline unsigned long *res_counter_member(struct res_counter

*cnt, int member)

init/Kconfig 2007-03-06 13:33:28.000000000 +0300 +53 -0

+{
+    switch (member) {
+    case RES_USAGE:
+        return &cnt->usage;
+    case RES_LIMIT:
+        return &cnt->limit;
+    case RES_FAILCNT:
+        return &cnt->failcnt;
+    };
+
+    BUG();
+    return NULL;
+}
+
+ssize_t res_counter_read(struct res_counter *cnt, int member,
+        const char __user *userbuf, size_t nbytes, loff_t *pos)
+{
+    unsigned long *val;
+    char buf[64], *s;
+
+    s = buf;
+    val = res_counter_member(cnt, member);
+    s += sprintf(s, "%lu\n", *val);
+    return simple_read_from_buffer((void __user *)userbuf, nbytes,
+            pos, buf, s - buf);
+}
+
+ssize_t res_counter_write(struct res_counter *cnt, int member,
+        const char __user *userbuf, size_t nbytes, loff_t *pos)
+{
+    int ret;
+    char *buf, *end;
+    unsigned long tmp, *val;
+
+    buf = kmalloc(nbytes + 1, GFP_KERNEL);
+    ret = -ENOMEM;
+    if (buf == NULL)
+        goto out;
+
+    buf[nbytes] = 0;
+    ret = -EFAULT;
+    if (copy_from_user(buf, userbuf, nbytes))
+        goto out_free;
+
+    ret = -EINVAL;
+    tmp = simple_strtoul(buf, &end, 10);
+    if (*end != '\0')
+        goto out_free;
+
+    val = res_counter_member(cnt, member);
+    *val = tmp;
+    ret = nbytes;
+out_free:

Quoted 9 lines

init/Kconfig 2007-03-06 13:33:28.000000000 +0300

Quoted 5 lines

init/Kconfig 2007-03-06 13:33:28.000000000 +0300 +4 -0

+    kfree(buf);
+out:
+    return ret;
+}

These bits look a little out of sync, with no users for these routines in this patch. Won't you get a compiler warning, compiling this bit alone?

Nope - when you have a non-static function without users in a file no compiler warning produced.
_______________________________________________
Containers mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/containers

Quoted 17 lines · Andrew Morton

mm/rss_container.c line 28

Quoted 11 lines · On Tue, 06 Mar 2007 17:55:29 +0300 Pavel Emelianov <[email protected]> wrote:

mm/rss_container.c +11 -0

+struct rss_container {
+	struct res_counter res;
+	struct list_head page_list;
+	struct container_subsys_state css;
+};
+
+struct page_container {
+	struct page *page;
+	struct rss_container *cnt;
+	struct list_head list;
+};

ah. This looks good. I'll find a hunk of time to go through this work and through Paul's patches. It'd be good to get both patchsets lined up in -mm within a couple of weeks. But..

doesn't look so good for me, mainly becaus of the
additional per page data and per page processing

on 4GB memory, with 100 guests, 50% shared for each guest, this basically means ~1mio pages, 500k shared and 1500k x sizeof(page_container) entries, which roughly boils down to ~25MB of wasted memory ...

increase the amount of shared pages and it starts getting worse, but maybe I'm missing something here

Quoted 3 lines

We need to decide whether we want to do per-container memory limitation via these data structures, or whether we do it via a physical scan of some software zone, possibly based on Mel's patches.

why not do simple page accounting (as done currently in Linux) and use that for the limits, without keeping the reference from container to page?

best,
Herbert

Quoted 4 lines

_______________________________________________
Containers mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/containers

-
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/rss_container.c line 28

Quoted 11 lines · On Tue, 06 Mar 2007 17:55:29 +0300 Pavel Emelianov <[email protected]> wrote:

mm/rss_container.c +11 -0

+struct rss_container {
+	struct res_counter res;
+	struct list_head page_list;
+	struct container_subsys_state css;
+};
+
+struct page_container {
+	struct page *page;
+	struct rss_container *cnt;
+	struct list_head list;
+};

ah. This looks good. I'll find a hunk of time to go through this work and through Paul's patches. It'd be good to get both patchsets lined up in -mm within a couple of weeks. But..

We need to decide whether we want to do per-container memory limitation via these data structures, or whether we do it via a physical scan of some software zone, possibly based on Mel's patches.

Quoted 21 lines · Andrew Morton

mm/rss_container.c line 28

Quoted 11 lines · On Tue, 06 Mar 2007 17:55:29 +0300 Pavel Emelianov <[email protected]> wrote:

mm/rss_container.c +11 -0

+struct rss_container {
+	struct res_counter res;
+	struct list_head page_list;
+	struct container_subsys_state css;
+};
+
+struct page_container {
+	struct page *page;
+	struct rss_container *cnt;
+	struct list_head list;
+};

ah. This looks good. I'll find a hunk of time to go through this work and through Paul's patches. It'd be good to get both patchsets lined up in -mm within a couple of weeks. But..

Quoted 3 lines

We need to decide whether we want to do per-container memory limitation via these data structures, or whether we do it via a physical scan of some software zone, possibly based on Mel's patches.

i.e. a separate memzone for each container?

imho memzone approach is inconvinient for pages sharing and shares accounting. it also makes memory management more strict, forbids overcommiting per-container etc.

Maybe you have some ideas how we can decide on this?

Quoted 22 lines · Herbert Poetzl

Quoted 17 lines · Andrew Morton

mm/rss_container.c line 28

Quoted 11 lines · On Tue, 06 Mar 2007 17:55:29 +0300 Pavel Emelianov <[email protected]> wrote:

mm/rss_container.c +11 -0

+struct rss_container {
+	struct res_counter res;
+	struct list_head page_list;
+	struct container_subsys_state css;
+};
+
+struct page_container {
+	struct page *page;
+	struct rss_container *cnt;
+	struct list_head list;
+};

ah. This looks good. I'll find a hunk of time to go through this work and through Paul's patches. It'd be good to get both patchsets lined up in -mm within a couple of weeks. But..

doesn't look so good for me, mainly becaus of the
additional per page data and per page processing

Quoted 7 lines

on 4GB memory, with 100 guests, 50% shared for each guest, this basically means ~1mio pages, 500k shared and 1500k x sizeof(page_container) entries, which roughly boils down to ~25MB of wasted memory ...

increase the amount of shared pages and it starts getting worse, but maybe I'm missing something here

You are. Each page has only one page_container associated with it despite the number of containers it is shared between.

Quoted 5 lines

Quoted 4 lines

We need to decide whether we want to do per-container memory limitation via these data structures, or whether we do it via a physical scan of some software zone, possibly based on Mel's patches.

Quoted 3 lines

why not do simple page accounting (as done currently in Linux) and use that for the limits, without keeping the reference from container to page?

As I've already answered in my previous letter simple limiting w/o per-container reclamation and per-container oom killer isn't a good memory management. It doesn't allow to handle resource shortage gracefully.

This patchset provides more grace way to handle this, but full memory management includes accounting of VMA-length as well (returning ENOMEM from system call) but we've decided to start with RSS.

Quoted 13 lines

best,
Herbert

Quoted 4 lines

_______________________________________________
Containers mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/containers

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