RSS controller based on process containers (v3)

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.

Signed-off-by Pavel Emelianov <[email protected]>

---

diff -upr linux-2.6.22-rc2-mm1.orig/include/linux/res_counter.h linux-2.6.22-rc2-mm1-0/include/linux/res_counter.h

include/linux/res_counter.h +9 -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/res_counter.h +45 -0

+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);
+
+static inline void res_counter_init(struct res_counter *cnt)
+{
+	spin_lock_init(&cnt->lock);
+	cnt->limit = (unsigned long)LONG_MAX;
+}
+
+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;
+
+	spin_lock_irqsave(&cnt->lock, flags);
+	ret = res_counter_charge_locked(cnt, val);
+	spin_unlock_irqrestore(&cnt->lock, flags);
+	return ret;
+}
+
+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;
+}

include/linux/res_counter.h +1 -0

+#endif

diff -upr linux-2.6.22-rc2-mm1.orig/init/Kconfig linux-2.6.22-rc2-mm1-0/init/Kconfig

init/Kconfig +4 -0

@@ -328,6 +328,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.22-rc2-mm1.orig/kernel/Makefile linux-2.6.22-rc2-mm1-0/kernel/Makefile

kernel/Makefile +1 -0

@@ -56,6 +56,7 @@ obj-$(CONFIG_SYSCTL) += utsname_sysctl.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.22-rc2-mm1.orig/kernel/res_counter.c linux-2.6.22-rc2-mm1-0/kernel/res_counter.c

kernel/res_counter.c +4 -0

@@ -0,0 +1,73 @@
+/*
+ * resource containers
+ *
+ * Copyright 2007 OpenVZ SWsoft Inc

kernel/res_counter.c +48 -0

+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')

Naturally mm_struct determines the resource consumer in memory accounting. So each mm_struct should have a pointer on container it belongs to. When a new task is created its mm_struct is assigned to the container this task belongs to.

diff -upr linux-2.6.22-rc2-mm1.orig/include/linux/sched.h linux-2.6.22-rc2-mm1-0/include/linux/sched.h

include/linux/sched.h +3 -0

@@ -390,6 +390,9 @@ struct mm_struct {
 	/* aio bits */
 	rwlock_t		ioctx_list_lock;
 	struct kioctx		*ioctx_list;
+#ifdef CONFIG_RSS_CONTAINER
+	struct rss_container	*rss_container;
+#endif
 };
 
 struct sighand_struct {

diff -upr linux-2.6.22-rc2-mm1.orig/kernel/fork.c linux-2.6.22-rc2-mm1-0/kernel/fork.c

kernel/fork.c +3 -1

@@ -57,6 +57,8 @@
 #include <asm/cacheflush.h>
 #include <asm/tlbflush.h>
 
+#include <linux/rss_container.h>
+
 /*
  * Protected counters by write_lock_irq(&tasklist_lock)
  */

@@ -329,7 +331,7 @@ static inline void mm_free_pgd(struct mm
 
 #include <linux/init_task.h>
 
-static struct mm_struct * mm_init(struct mm_struct * mm)
+static struct mm_struct * mm_init(struct mm_struct *mm, struct task_struct *tsk)
 {
 	atomic_set(&mm->mm_users, 1);
 	atomic_set(&mm->mm_count, 1);

The core routines for tracking the page ownership, registration of RSS subsystem in the containers and the definition of rss_container as container subsystem
combined with resource counter.

include/linux/rss_container.h +19 -0

@@ -0,0 +1,64 @@
+#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 *);

include/linux/rss_container.h +21 -0

+	return 0;
+}
 
...

@@ -0,0 +1,271 @@
+/*
+ * 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/swap.h>
+#include <linux/res_counter.h>
+#include <linux/rss_container.h>
+
+struct rss_container {
+	struct res_counter res;
+	struct list_head inactive_list;

include/linux/rss_container.h +7 -0

+	struct rss_container *rss;
+	struct page_container *pc;
+
+	rcu_read_lock();
+	rss = rcu_dereference(vma->vm_mm->rss_container);
+	css_get(&rss->css);
+	rcu_read_unlock();

include/linux/rss_container.h +45 -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;
+
+	pg = pc->page;
+	rss = pc->cnt;
+
+	spin_lock_irq(&rss->res.lock);
+	list_add(&pc->list, &rss->active_list);
+	spin_unlock_irq(&rss->res.lock);
+
+	page_container(pg) = pc;
+}
+
+void container_rss_del(struct page_container *pc)
+{
+	struct rss_container *rss;

include/linux/rss_container.h +6 -0

+static void rss_move_task(struct container_subsys *ss,
+		struct container *cont,
+		struct container *old_cont,
+		struct task_struct *p)
+{
+	struct mm_struct *mm;

include/linux/rss_container.h +56 -0

+	css_get(&rss->css);
+	rcu_assign_pointer(mm->rss_container, rss);
+	css_put(&old_rss->css);
+
+out_put:
+	mmput(mm);
+out:
+	return;
+}
+
+static struct rss_container init_rss_container;
+
+
+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 ssize_t rss_read_reclaimed(struct container *cont, struct cftype *cft,
+		struct file *file, char __user *userbuf,
+		size_t nbytes, loff_t *ppos)
+{
+	char buf[64], *s;
+
+	s = buf;
+	s += sprintf(s, "%d\n",
+			atomic_read(&rss_from_cont(cont)->rss_reclaimed));
+	return simple_read_from_buffer((void __user *)userbuf, nbytes,
+			ppos, buf, s - buf);
+}
+
+
+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,
+};
+

include/linux/rss_container.h line 35

Quoted 5 lines

include/linux/rss_container.h +5 -0

+#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 */

eh? What is gcc's problem here?

Quoted 5 lines

include/linux/rss_container.h +2 -0

+	return 0;
+}
 
...

mm/rss_container.c line 258

Quoted 13 lines

mm/rss_container.c +13 -0

+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;
+	if ((rc = container_add_file(cont, &rss_reclaimed)) < 0)
+		return rc;

If we fail partway through here, do the thus-far-created fiels get cleaned up?

Quoted 6 lines

mm/rss_container.c +6 -0

+	return 0;
+}
+
+struct container_subsys rss_subsys = {
+	.name = "rss",
+	.subsys_id = rss_subsys_id,

mm/rss_container.c line 271

Quoted 6 lines

mm/rss_container.c +6 -0

+	.create = rss_create,
+	.destroy = rss_destroy,
+	.populate = rss_populate,
+	.attach = rss_move_task,
+	.early_init = 1,
+};

Did this need kernel-wide scope?

As described above, pages are charged to their first touchers. The first toucher is determined using pages' _mapcount manipulations in rmap calls.

Page is charged in two stages:
1. preparation, in which the resource availability is checked. This stage may lead to page reclamation, thus it is performed in a "might-sleep" places;
2. the container assignment to page. This is done in an atomic code that handles races between multiple touchers.

Quoted 10 lines · On Wed, 30 May 2007 19:34:18 +0400 Pavel Emelianov <[email protected]> wrote:

As described above, pages are charged to their first touchers. The first toucher is determined using pages' _mapcount manipulations in rmap calls.

Page is charged in two stages:
1. preparation, in which the resource availability is checked. This stage may lead to page reclamation, thus it is performed in a "might-sleep" places;
2. the container assignment to page. This is done in an atomic code that handles races between multiple touchers.

I suppose we need to think about what to do about higher-order pages, and compound pages, and hugetlb memory.

The core change is that the isolate_lru_pages() call is replaced with struct scan_controll->isolate_pages() call.

Other changes include exporting __isolate_lru_page() for per-container isolator and handling variable-to-pointer changes in try_to_free_pages().

This makes possible to use different isolation routines for per-container page reclamation. This will be used by the following patch.

...

include/linux/swap.h +3 -0

+struct rss_container;
+extern unsigned long try_to_free_pages_in_container(struct rss_container *);
+int __isolate_lru_page(struct page *page, int mode);
 
 
 extern unsigned long shrink_all_memory(unsigned long nr_pages);
 extern int vm_swappiness;
 extern int remove_mapping(struct address_space *mapping, struct page *page);

diff -upr linux-2.6.22-rc2-mm1.orig/mm/vmscan.c linux-2.6.22-rc2-mm1-0/mm/vmscan.c

mm/vmscan.c +2 -0

@@ -47,6 +47,8 @@
 
 #include "internal.h"
 
+#include <linux/rss_container.h>
+
 struct scan_control {
 	/* Incremented by the number of inactive pages that were scanned */
 	unsigned long nr_scanned;

mm/vmscan.c unsigned long try_to_free_pages(struct z · line 1199

Quoted 4 lines

mm/vmscan.c +3 -0

+		nr_reclaimed += shrink_zones(priority, zones, sc);
+		if (sc->cnt == NULL)
+			shrink_slab(sc->nr_scanned, gfp_mask, lru_pages);

We don't we shrink slab if called to shrink a container.

This is a fundamental design decision, and a design shortcoming. A full discussion of this is absolutely appropriate to the patch changelog. Please don't just hide stuff like this in the patch and leave people wondering, or ignorant.

Quoted 5 lines · Andrew Morton

mm/vmscan.c unsigned long try_to_free_pages(struct z · line 1199

Quoted 3 lines

mm/vmscan.c +3 -0

+		nr_reclaimed += shrink_zones(priority, zones, sc);
+		if (sc->cnt == NULL)
+			shrink_slab(sc->nr_scanned, gfp_mask, lru_pages);

We don't we shrink slab if called to shrink a container.

Quoted 4 lines

This is a fundamental design decision, and a design shortcoming. A full discussion of this is absolutely appropriate to the patch changelog. Please don't just hide stuff like this in the patch and leave people wondering, or ignorant.

Yes, we don't because we do not account for slab usage right now. We account only for memory allocated to user space. A good fat comment will help here.

Quoted 6 lines · Balbir Singh

Quoted 5 lines · Andrew Morton

mm/vmscan.c unsigned long try_to_free_pages(struct z · line 1199

Quoted 3 lines

mm/vmscan.c +3 -0

+		nr_reclaimed += shrink_zones(priority, zones, sc);
+		if (sc->cnt == NULL)
+			shrink_slab(sc->nr_scanned, gfp_mask, lru_pages);

We don't we shrink slab if called to shrink a container.

Quoted 8 lines

Quoted 4 lines

This is a fundamental design decision, and a design shortcoming. A full discussion of this is absolutely appropriate to the patch changelog. Please don't just hide stuff like this in the patch and leave people wondering, or ignorant.

Yes, we don't because we do not account for slab usage right now. We account only for memory allocated to user space. A good fat comment will help here.

I have already added the comment. But the problem is not in that we do not account for kernel memory. Shrinking slabs won't unmap any pages from user-space and thus won't help user to charge more. This will only make kernel suffer from re-creation of objects.

Implement try_to_free_pages_in_container() to free the pages in container that has run out of memory.

The scan_control->isolate_pages() function is set to isolate_pages_in_container() that isolates the container pages only. The exported __isolate_lru_page() call makes things look simpler than in the previous version.

Includes fix from Balbir Singh <[email protected]>

}

mm/rss_container.c +11 -0

+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);
+	if (pc == NULL)
+		return;

mm/rss_container.c +5 -0

+static unsigned long isolate_container_pages(unsigned long nr_to_scan,
+		struct list_head *src, struct list_head *dst,
+		unsigned long *scanned, struct zone *zone, int mode)
+{
+	unsigned long nr_taken = 0;

mm/rss_container.c +48 -0

+		list_move(&pc->list, &pc_list);
+
+		if (__isolate_lru_page(page, mode) == 0) {
+			list_move(&page->lru, dst);
+			nr_taken++;
+		}
+	}
+
+	list_splice(&pc_list, src);
+
+	*scanned = scan;
+	return nr_taken;
+}
+
+unsigned long isolate_pages_in_container(unsigned long nr_to_scan,
+		struct list_head *dst, unsigned long *scanned,
+		int order, int mode, struct zone *zone,
+		struct rss_container *rss, int active)
+{
+	unsigned long ret;
+
+	spin_lock(&rss->res.lock);
+	if (active)
+		ret = isolate_container_pages(nr_to_scan, &rss->active_list,
+				dst, scanned, zone, mode);
+	else
+		ret = isolate_container_pages(nr_to_scan, &rss->inactive_list,
+				dst, scanned, zone, mode);
+	spin_unlock(&rss->res.lock);
+	return ret;
+}
+
 void container_rss_add(struct page_container *pc)
 {
 	struct page *pg;
 
...
 
+#ifdef CONFIG_RSS_CONTAINER
+unsigned long try_to_free_pages_in_container(struct rss_container *cnt)
+{
+	struct scan_control sc = {
+		.gfp_mask = GFP_KERNEL,
+		.may_writepage = 1,
+		.swap_cluster_max = 1,
+		.may_swap = 1,
+		.swappiness = vm_swappiness,
+		.order = 0, /* in this case we wanted one page only */
+		.cnt = cnt,
+		.isolate_pages = isolate_pages_in_container,
+	};
+	int node;
+	struct zone **zones;
+

mm/vmscan.c +4 -0

+	return 0;
+}
+#endif
+
 unsigned long try_to_free_pages(struct zone **zones, int order, gfp_t gfp_mask)
 {
 	struct scan_control sc = {

On Wed, 30 May 2007 19:42:26 +0400
Pavel Emelianov <[email protected]> wrote:

Quoted 22 lines

mm/vmscan.c +21 -0

+#ifdef CONFIG_RSS_CONTAINER
+unsigned long try_to_free_pages_in_container(struct rss_container *cnt)
+{
+	struct scan_control sc = {
+		.gfp_mask = GFP_KERNEL,
+		.may_writepage = 1,
+		.swap_cluster_max = 1,
+		.may_swap = 1,
+		.swappiness = vm_swappiness,
+		.order = 0, /* in this case we wanted one page only */
+		.cnt = cnt,
+		.isolate_pages = isolate_pages_in_container,
+	};
+	int node;
+	struct zone **zones;
+
+	for_each_online_node(node) {
+#ifdef CONFIG_HIGHMEM
+		zones = NODE_DATA(node)->node_zonelists[ZONE_HIGHMEM].zones;
+		if (do_try_to_free_pages(zones, sc.gfp_mask, &sc))
+			return 1;

mm/vmscan.c out: · line 1276

Quoted 5 lines

mm/vmscan.c +5 -0

+#endif
+		zones = NODE_DATA(node)->node_zonelists[ZONE_NORMAL].zones;
+		if (do_try_to_free_pages(zones, sc.gfp_mask, &sc))
+			return 1;
+	}

Definitely need to handle ZONE_DMA32 and ZONE_DMA (some architectures put all memory into ZONE_DMA (or they used to))

mm/vmscan.c out: · line 1283

Quoted 7 lines

mm/vmscan.c +4 -0

+	return 0;
+}
+#endif
+
 unsigned long try_to_free_pages(struct zone **zones, int order, gfp_t gfp_mask)
 {
 	struct scan_control sc = {

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

Quoted 3 lines · Andrew Morton

On Wed, 30 May 2007 19:42:26 +0400
Pavel Emelianov <[email protected]> wrote:

[snip]

Quoted 20 lines

Quoted 20 lines

mm/vmscan.c +20 -0

+#ifdef CONFIG_RSS_CONTAINER
+unsigned long try_to_free_pages_in_container(struct rss_container *cnt)
+{
+	struct scan_control sc = {
+		.gfp_mask = GFP_KERNEL,
+		.may_writepage = 1,
+		.swap_cluster_max = 1,
+		.may_swap = 1,
+		.swappiness = vm_swappiness,
+		.order = 0, /* in this case we wanted one page only */
+		.cnt = cnt,
+		.isolate_pages = isolate_pages_in_container,
+	};
+	int node;
+	struct zone **zones;
+
+	for_each_online_node(node) {
+#ifdef CONFIG_HIGHMEM
+		zones = NODE_DATA(node)->node_zonelists[ZONE_HIGHMEM].zones;
+		if (do_try_to_free_pages(zones, sc.gfp_mask, &sc))

Quoted 8 lines

mm/vmscan.c out: · line 1276

Quoted 6 lines

mm/vmscan.c +6 -0

+			return 1;
+#endif
+		zones = NODE_DATA(node)->node_zonelists[ZONE_NORMAL].zones;
+		if (do_try_to_free_pages(zones, sc.gfp_mask, &sc))
+			return 1;
+	}

Definitely need to handle ZONE_DMA32 and ZONE_DMA (some architectures put all memory into ZONE_DMA (or they used to))

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

mm/vmscan.c +4 -0

+	for_each_online_node(node) {
+#ifdef CONFIG_HIGHMEM
+		zones = NODE_DATA(node)->node_zonelists[ZONE_HIGHMEM].zones;
+		if (do_try_to_free_pages(zones, sc.gfp_mask, &sc))

Quoted 8 lines

Quoted 8 lines

mm/vmscan.c out: · line 1276

Quoted 6 lines

mm/vmscan.c +6 -0

+			return 1;
+#endif
+		zones = NODE_DATA(node)->node_zonelists[ZONE_NORMAL].zones;
+		if (do_try_to_free_pages(zones, sc.gfp_mask, &sc))
+			return 1;
+	}

Definitely need to handle ZONE_DMA32 and ZONE_DMA (some architectures put all memory into ZONE_DMA (or they used to))

Won't the node_zonelists[ZONE_NORMAL].zones contain all lower zones (including ZONE_DMA and ZONE_DMA32) from various nodes organized by cost?

I need to go lookup the node_zonelists creation.

Balbir Singh wrote:

[snip]

Quoted 12 lines

Quoted 4 lines

Quoted 4 lines

Quoted 4 lines

mm/vmscan.c +4 -0

+	for_each_online_node(node) {
+#ifdef CONFIG_HIGHMEM
+		zones = NODE_DATA(node)->node_zonelists[ZONE_HIGHMEM].zones;
+		if (do_try_to_free_pages(zones, sc.gfp_mask, &sc))

Quoted 8 lines

Quoted 8 lines

mm/vmscan.c out: · line 1276

Quoted 6 lines

mm/vmscan.c +6 -0

+			return 1;
+#endif
+		zones = NODE_DATA(node)->node_zonelists[ZONE_NORMAL].zones;
+		if (do_try_to_free_pages(zones, sc.gfp_mask, &sc))
+			return 1;
+	}

Definitely need to handle ZONE_DMA32 and ZONE_DMA (some architectures put all memory into ZONE_DMA (or they used to))

Quoted 4 lines

Won't the node_zonelists[ZONE_NORMAL].zones contain all lower zones (including ZONE_DMA and ZONE_DMA32) from various nodes organized by cost?

I need to go lookup the node_zonelists creation.

OK, I've got it. The node's zonelist indexed with ZONE_XXX contains the zone specified and all the zones beyond it. Thus shrinking from ZONE_HIGHMEM (ZONE_NORMAL if the latter does not exist) is enough.

Thanks,
Pavel

Quoted 23 lines · On Wed, 30 May 2007 19:42:26 +0400 Pavel Emelianov <[email protected]> wrote:

Implement try_to_free_pages_in_container() to free the pages in container that has run out of memory.

The scan_control->isolate_pages() function is set to isolate_pages_in_container() that isolates the container pages only. The exported __isolate_lru_page() call makes things look simpler than in the previous version.

Includes fix from Balbir Singh <[email protected]>

}

mm/rss_container.c +11 -0

+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);
+	if (pc == NULL)
+		return;

mm/rss_container.c line 158

Quoted 10 lines

mm/rss_container.c +10 -0

+
+	rss = pc->cnt;
+
+	spin_lock(&rss->res.lock);
+	if (active)
+		list_move(&pc->list, &rss->active_list);
+	else
+		list_move(&pc->list, &rss->inactive_list);
+	spin_unlock(&rss->res.lock);
+}

This is an interesting-looking function. Please document it?

I'm inferring that the rss container has an active and inactive list and that this basically follows the same operation as the traditional per-zone lists?

Would I be correct in guessing that pages which are on the per-rss-container lists are also eligible for reclaim off the traditional page LRUs? If so, how does that work? When a page gets freed off the

per-zone LRUs does it also get removed from the per-rss_container LRU? But how can this be right?

Pages can get taken off the LRU and freed at interrupt time, and this code isn't interrupt-safe.

I note that this lock is not irq-safe, whereas the lru locks are irq-safe. So we don't perform the rotate_reclaimable_page() operation within the RSS container? I think we could do so. I wonder if this was considered.

A description of how all this code works would help a lot.

Quoted 5 lines

mm/rss_container.c +5 -0

+static unsigned long isolate_container_pages(unsigned long nr_to_scan,
+		struct list_head *src, struct list_head *dst,
+		unsigned long *scanned, struct zone *zone, int mode)
+{
+	unsigned long nr_taken = 0;

mm/rss_container.c line 174

Quoted 10 lines

mm/rss_container.c +10 -0

+	struct page *page;
+	struct page_container *pc;
+	unsigned long scan;
+	LIST_HEAD(pc_list);
+
+	for (scan = 0; scan < nr_to_scan && !list_empty(src); scan++) {
+		pc = list_entry(src->prev, struct page_container, list);
+		page = pc->page;
+		if (page_zone(page) != zone)
+			continue;

That page_zone() check is interesting. What's going on here?

I'm suspecting that we have a problem here: if there are a lot of pages on *src which are in the wrong zone, we can suffer reclaim distress leading to omm-killings, or excessive CPU consumption?

Quoted 3 lines

Would I be correct in guessing that pages which are on the per-rss-container lists are also eligible for reclaim off the traditional page LRUs? If so, how does that work? When a page gets freed off the

Yes. All the pages are accessible from booth - global and per-container LRU lists and reclamation can be performed from booth.

Quoted 2 lines

per-zone LRUs does it also get removed from the per-rss_container LRU? But how can this be right?

I don't get your idea here.

Quoted 8 lines · On Thu, 31 May 2007 14:35:43 +0400 Pavel Emelianov <[email protected]> wrote:

Quoted 3 lines

Would I be correct in guessing that pages which are on the per-rss-container lists are also eligible for reclaim off the traditional page LRUs? If so, how does that work? When a page gets freed off the

Yes. All the pages are accessible from booth - global and per-container LRU lists and reclamation can be performed from booth.

Quoted 2 lines

per-zone LRUs does it also get removed from the per-rss_container LRU? But how can this be right?

Quoted 1 line

I don't get your idea here.

If we have a page which is on the zone LRU with refcount=1 and someone does put_page() on it, we will take that page off the zone LRU and then actually free the page.

I am assuming that your patches change that logic so that we will also remove that page from the per-container LRU at the same time?

If so, the problem which I see is that, under rare circumstances, that final put_page() will occur from interrupt context. Hence we would be trying to remove the page from the per-container LRU at interrupt time. But the locks which you have in there are not suited for taking from interrupt context.

Quoted 17 lines · Andrew Morton

Quoted 9 lines · On Thu, 31 May 2007 14:35:43 +0400 Pavel Emelianov <[email protected]> wrote:

Quoted 3 lines

Would I be correct in guessing that pages which are on the per-rss-container lists are also eligible for reclaim off the traditional page LRUs? If so, how does that work? When a page gets freed off the

Yes. All the pages are accessible from booth - global and per-container LRU lists and reclamation can be performed from booth.

Quoted 2 lines

per-zone LRUs does it also get removed from the per-rss_container LRU? But how can this be right?

I don't get your idea here.

If we have a page which is on the zone LRU with refcount=1 and someone does put_page() on it, we will take that page off the zone LRU and then actually free the page.

Quoted 2 lines

I am assuming that your patches change that logic so that we will also remove that page from the per-container LRU at the same time?

Oh, I see. No that will work another way. Page stays in per-container LRU lists as long as it is mapped. When the last process is unmapping the page, it it is removed, but stays in global LRU till its refcount drops to 0.

Quoted 27 lines · On Wed, 30 May 2007 19:42:26 +0400 Pavel Emelianov <[email protected]> wrote:

Implement try_to_free_pages_in_container() to free the pages in container that has run out of memory.

The scan_control->isolate_pages() function is set to isolate_pages_in_container() that isolates the container pages only. The exported __isolate_lru_page() call makes things look simpler than in the previous version.

Includes fix from Balbir Singh <[email protected]>

}

mm/rss_container.c +15 -0

+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);
+	if (pc == NULL)
+		return;
+
+	rss = pc->cnt;
+
+	spin_lock(&rss->res.lock);

mm/rss_container.c line 158

Quoted 6 lines

mm/rss_container.c +6 -0

+	if (active)
+		list_move(&pc->list, &rss->active_list);
+	else
+		list_move(&pc->list, &rss->inactive_list);
+	spin_unlock(&rss->res.lock);
+}

This is an interesting-looking function. Please document it?

I'm inferring that the rss container has an active and inactive list and that this basically follows the same operation as the traditional per-zone lists?

Would I be correct in guessing that pages which are on the per-rss-container lists are also eligible for reclaim off the traditional page LRUs? If so, how does that work? When a page gets freed off the per-zone LRUs does it also get removed from the per-rss_container LRU? But how can this be right? Pages can get taken off the LRU and freed at interrupt time, and this code isn't interrupt-safe.

I note that this lock is not irq-safe, whereas the lru locks are irq-safe. So we don't perform the rotate_reclaimable_page() operation within the RSS container? I think we could do so. I wonder if this was considered.

A description of how all this code works would help a lot.

Quoted 5 lines

mm/rss_container.c +5 -0

+static unsigned long isolate_container_pages(unsigned long nr_to_scan,
+		struct list_head *src, struct list_head *dst,
+		unsigned long *scanned, struct zone *zone, int mode)
+{
+	unsigned long nr_taken = 0;

mm/rss_container.c line 174

Quoted 10 lines

mm/rss_container.c +10 -0

+	struct page *page;
+	struct page_container *pc;
+	unsigned long scan;
+	LIST_HEAD(pc_list);
+
+	for (scan = 0; scan < nr_to_scan && !list_empty(src); scan++) {
+		pc = list_entry(src->prev, struct page_container, list);
+		page = pc->page;
+		if (page_zone(page) != zone)
+			continue;

That page_zone() check is interesting. What's going on here?

I'm suspecting that we have a problem here: if there are a lot of pages on *src which are in the wrong zone, we can suffer reclaim distress leading to omm-killings, or excessive CPU consumption?

Quoted 5 lines

mm/vmscan.c +5 -0

+	for_each_online_node(node) {
+#ifdef CONFIG_HIGHMEM
+		zones = NODE_DATA(node)->node_zonelists[ZONE_HIGHMEM].zones;
+		if (do_try_to_free_pages(zones, sc.gfp_mask, &sc))
+			return 1;

mm/vmscan.c out: · line 1276

Quoted 5 lines

mm/vmscan.c +5 -0

+#endif
+		zones = NODE_DATA(node)->node_zonelists[ZONE_NORMAL].zones;
+		if (do_try_to_free_pages(zones, sc.gfp_mask, &sc))
+			return 1;
+	}

Definitely need to handle ZONE_DMA32 and ZONE_DMA (some architectures put all memory into ZONE_DMA (or they used to))

Quoted 29 lines · Andrew Morton

Quoted 26 lines · On Wed, 30 May 2007 19:42:26 +0400 Pavel Emelianov <[email protected]> wrote:

Implement try_to_free_pages_in_container() to free the pages in container that has run out of memory.

The scan_control->isolate_pages() function is set to isolate_pages_in_container() that isolates the container pages only. The exported __isolate_lru_page() call makes things look simpler than in the previous version.

Includes fix from Balbir Singh <[email protected]>

}

mm/rss_container.c +14 -0

+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);
+	if (pc == NULL)
+		return;
+
+	rss = pc->cnt;
+

Quoted 9 lines

mm/rss_container.c line 158

Quoted 7 lines

mm/rss_container.c +7 -0

+	spin_lock(&rss->res.lock);
+	if (active)
+		list_move(&pc->list, &rss->active_list);
+	else
+		list_move(&pc->list, &rss->inactive_list);
+	spin_unlock(&rss->res.lock);
+}

This is an interesting-looking function. Please document it?

Will do. This function is called when we want to move a page in the LRU list. This could happen when a page is activated or when reclaim finds that a particular page cannot be reclaimed right now.

Quoted 4 lines

I'm inferring that the rss container has an active and inactive list and that this basically follows the same operation as the traditional per-zone lists?

Yes, correct.

Quoted 7 lines

Would I be correct in guessing that pages which are on the per-rss-container lists are also eligible for reclaim off the traditional page LRUs? If so, how does that work? When a page gets freed off the per-zone LRUs does it also get removed from the per-rss_container LRU? But how can this be right? Pages can get taken off the LRU and freed at interrupt time, and this code isn't interrupt-safe.

Yes, before a page is reclaimed from the global LRU list, we go through page_remove_rmap() in try_to_unmap(). Pages are removed from the container LRU before they are reclaimed.

Quoted 4 lines

I note that this lock is not irq-safe, whereas the lru locks are irq-safe. So we don't perform the rotate_reclaimable_page() operation within the RSS container? I think we could do so. I wonder if this was considered.

The lock needs to be interrupt safe.

Quoted 19 lines

A description of how all this code works would help a lot.

Quoted 5 lines

mm/rss_container.c +5 -0

+static unsigned long isolate_container_pages(unsigned long nr_to_scan,
+		struct list_head *src, struct list_head *dst,
+		unsigned long *scanned, struct zone *zone, int mode)
+{
+	unsigned long nr_taken = 0;

mm/rss_container.c line 174

Quoted 10 lines

mm/rss_container.c +10 -0

+	struct page *page;
+	struct page_container *pc;
+	unsigned long scan;
+	LIST_HEAD(pc_list);
+
+	for (scan = 0; scan < nr_to_scan && !list_empty(src); scan++) {
+		pc = list_entry(src->prev, struct page_container, list);
+		page = pc->page;
+		if (page_zone(page) != zone)
+			continue;

That page_zone() check is interesting. What's going on here?

Quoted 4 lines

I'm suspecting that we have a problem here: if there are a lot of pages on *src which are in the wrong zone, we can suffer reclaim distress leading to omm-killings, or excessive CPU consumption?

We discussed this on lkml. Basically, now for every zone we try to reclaim pages from the container, it increases CPU utilization if we choose the wrong zone to reclaim from, but it provides the following benefit

Code reuse (shrink_zone* is reused along with helper functions). I am not sure if Pavel had any other benefits in mind like benefits on a NUMA box.

Quoted 4 lines

Quoted 4 lines

mm/vmscan.c +4 -0

+	for_each_online_node(node) {
+#ifdef CONFIG_HIGHMEM
+		zones = NODE_DATA(node)->node_zonelists[ZONE_HIGHMEM].zones;
+		if (do_try_to_free_pages(zones, sc.gfp_mask, &sc))

Quoted 9 lines

mm/vmscan.c out: · line 1276

Quoted 6 lines

mm/vmscan.c +6 -0

+			return 1;
+#endif
+		zones = NODE_DATA(node)->node_zonelists[ZONE_NORMAL].zones;
+		if (do_try_to_free_pages(zones, sc.gfp_mask, &sc))
+			return 1;
+	}

Definitely need to handle ZONE_DMA32 and ZONE_DMA (some architectures put all memory into ZONE_DMA (or they used to))

node_zonelists[ZONE_NORMAL].zones should contain ZONE_DMA and ZONE_DMA32 right?