Dan Magenheimer
2011-Jun-29 20:15 UTC
[Xen-devel] [PATCH v3] [linux-3.0+ for xen] tmem: self-ballooning and frontswap-selfshrinking
Hi Konrad (and other reviewers) --
Here is version 3 of the Xen tmem selfballooning/frontswap-selfshrinking
patch. Many thanks to the reviewers of V1 and V2 for your excellent
comments and feedback. I hope V3 is ready for merging as I''m
hoping to get this merged for the upcoming 3.1 window.
If it is satisfactory, you may wish to check the commit in my tree:
git://git.kernel.org/pub/scm/linux/kernel/git/djm/tmem.git
#xen-tmem-selfballoon-v3
If all looks good and there are no issues raised in the next
day or two, I will post an official git pull request to you
and cc lkml.
Note that this patch has a slight build dependency on last week''s:
afec6e04922d0c8c7e244be2e544bac5e7e36294 xen: prepare tmem shim to handle
frontswap
in your devel/next-3.0 branch (because tmem_enabled is EXPORT_SYM''ed
in that commit and extern''ed in a file in this commit). I
don''t
think this is a problem as you''ve already pushed that commit
to linux-next, but am noting the dependency in case some kind
of commit reordering were to occur.
==>From: Dan Magenheimer <dan.magenheimer@oracle.com>
Subject: xen: tmem: self-ballooning and frontswap-selfshrinking
This patch introduces two in-kernel drivers for Xen transcendent memory
("tmem") functionality that complement cleancache and frontswap. Both
use control theory to dynamically adjust and optimize memory utilization.
Selfballooning controls the in-kernel Xen balloon driver, targeting a goal
value (vm_committed_as), thus pushing less frequently used clean
page cache pages (through the cleancache code) into Xen tmem where
Xen can balance needs across all VMs residing on the physical machine.
Frontswap-selfshrinking controls the number of pages in frontswap,
driving it towards zero (effectively doing a partial swapoff) when
in-kernel memory pressure subsides, freeing up RAM for other VMs.
More detail is provided in the header comment of xen-selfballooning.c.
Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>
[v3: konrad.wilk@oracle.com: fix potential divides-by-zero]
[v3: konrad.wilk@oracle.com: add many more comments, fix nits]
[v2: rebased to linux-3.0-rc1]
[v2: Ian.Campbell@citrix.com: reorganize as new file (xen-selfballoon.c)]
[v2: dkiper@net-space.pl: proper access to vm_committed_as]
[v2: dkiper@net-space.pl: accounting fixes]
Cc: Jan Beulich <JBeulich@novell.com>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>
Cc: <xen-devel@lists.xensource.com>
---
Diffstat:
drivers/xen/Kconfig | 12
drivers/xen/Makefile | 1
drivers/xen/xen-balloon.c | 2
drivers/xen/xen-selfballoon.c | 429 +++++++++++++++++++++
include/xen/balloon.h | 10
include/xen/tmem.h | 5
6 files changed, 459 insertions(+)
diff -Napur -X linux-3.0-rc1/Documentation/dontdiff
linux-3.0-rc1-frontswap/drivers/xen/Kconfig
linux-3.0-rc1-frontswap-selfballoon/drivers/xen/Kconfig
--- linux-3.0-rc1-frontswap/drivers/xen/Kconfig 2011-05-31 17:09:07.606910896
-0600
+++ linux-3.0-rc1-frontswap-selfballoon/drivers/xen/Kconfig 2011-06-29
10:52:39.834899532 -0600
@@ -9,6 +9,18 @@ config XEN_BALLOON
the system to expand the domain''s memory allocation, or
alternatively
return unneeded memory to the system.
+config XEN_SELFBALLOONING
+ bool "dynamically self-balloon kernel memory to target"
+ depends on XEN && XEN_BALLOON && CLEANCACHE
+ default n
+ help
+ Self-ballooning dynamically balloons available kernel memory driven
+ by the current usage of anonymous memory ("committed AS") and
+ controlled by various sysfs-settable parameters. May be overridden
+ by the noselfballooning kernel boot parameter. If FRONTSWAP
+ is configured, also enables frontswap-selfshrinking, which can be
+ overridden by the noselfshrink kernel boot paramater.
+
config XEN_SCRUB_PAGES
bool "Scrub pages before returning them to system"
depends on XEN_BALLOON
diff -Napur -X linux-3.0-rc1/Documentation/dontdiff
linux-3.0-rc1-frontswap/drivers/xen/Makefile
linux-3.0-rc1-frontswap-selfballoon/drivers/xen/Makefile
--- linux-3.0-rc1-frontswap/drivers/xen/Makefile 2011-05-31 17:09:57.006875306
-0600
+++ linux-3.0-rc1-frontswap-selfballoon/drivers/xen/Makefile 2011-06-16
11:39:06.123852289 -0600
@@ -8,6 +8,7 @@ obj-$(CONFIG_BLOCK) += biomerge.o
obj-$(CONFIG_HOTPLUG_CPU) += cpu_hotplug.o
obj-$(CONFIG_XEN_XENCOMM) += xencomm.o
obj-$(CONFIG_XEN_BALLOON) += xen-balloon.o
+obj-$(CONFIG_XEN_SELFBALLOONING) += xen-selfballoon.o
obj-$(CONFIG_XEN_DEV_EVTCHN) += xen-evtchn.o
obj-$(CONFIG_XEN_GNTDEV) += xen-gntdev.o
obj-$(CONFIG_XEN_GRANT_DEV_ALLOC) += xen-gntalloc.o
diff -Napur -X linux-3.0-rc1/Documentation/dontdiff
linux-3.0-rc1-frontswap/drivers/xen/xen-balloon.c
linux-3.0-rc1-frontswap-selfballoon/drivers/xen/xen-balloon.c
--- linux-3.0-rc1-frontswap/drivers/xen/xen-balloon.c 2011-05-29
18:43:36.000000000 -0600
+++ linux-3.0-rc1-frontswap-selfballoon/drivers/xen/xen-balloon.c 2011-06-20
15:37:30.405798859 -0600
@@ -98,6 +98,8 @@ static int __init balloon_init(void)
register_balloon(&balloon_sysdev);
+ register_xen_selfballooning(&balloon_sysdev);
+
target_watch.callback = watch_target;
xenstore_notifier.notifier_call = balloon_init_watcher;
diff -Napur -X linux-3.0-rc1/Documentation/dontdiff
linux-3.0-rc1-frontswap/drivers/xen/xen-selfballoon.c
linux-3.0-rc1-frontswap-selfballoon/drivers/xen/xen-selfballoon.c
--- linux-3.0-rc1-frontswap/drivers/xen/xen-selfballoon.c 1969-12-31
17:00:00.000000000 -0700
+++ linux-3.0-rc1-frontswap-selfballoon/drivers/xen/xen-selfballoon.c 2011-06-29
11:44:44.067872414 -0600
@@ -0,0 +1,429 @@
+/******************************************************************************
+ * Xen selfballoon driver (and optional frontswap self-shrinking driver)
+ *
+ * Copyright (c) 2009-2011, Dan Magenheimer, Oracle Corp.
+ *
+ * This code complements the cleancache and frontswap patchsets to optimize
+ * support for Xen Transcendent Memory ("tmem"). The policy it
implements
+ * is rudimentary and will likely improve over time, but it does work well
+ * enough today.
+ *
+ * Two functionalities are implemented here which both use "control
theory"
+ * (feedback) to optimize memory utilization. In a virtualized environment
+ * such as Xen, RAM is often a scarce resource and we would like to ensure
+ * that each of a possibly large number of virtual machines is using RAM
+ * efficiently, i.e. using as little as possible when under light load
+ * and obtaining as much as possible when memory demands are high.
+ * Since RAM needs vary highly dynamically and sometimes dramatically,
+ * "hysteresis" is used, that is, memory target is determined not
just
+ * on current data but also on past data stored in the system.
+ *
+ * "Selfballooning" creates memory pressure by managing the Xen
balloon
+ * driver to decrease and increase available kernel memory, driven
+ * largely by the target value of "Committed_AS" (see /proc/meminfo).
+ * Since Committed_AS does not account for clean mapped pages (i.e. pages
+ * in RAM that are identical to pages on disk), selfballooning has the
+ * affect of pushing less frequently used clean pagecache pages out of
+ * kernel RAM and, presumably using cleancache, into Xen tmem where
+ * Xen can more efficiently optimize RAM utilization for such pages.
+ *
+ * When kernel memory demand unexpectedly increases faster than Xen, via
+ * the selfballoon driver, is able to (or chooses to) provide usable RAM,
+ * the kernel may invoke swapping. In most cases, frontswap is able
+ * to absorb this swapping into Xen tmem. However, due to the fact
+ * that the kernel swap subsystem assumes swapping occurs to a disk,
+ * swapped pages may sit on the disk for a very long time; even if
+ * the kernel knows the page will never be used again. This is because
+ * the disk space costs very little and can be overwritten when
+ * necessary. When such stale pages are in frontswap, however, they
+ * are taking up valuable real estate. "Frontswap selfshrinking"
works
+ * to resolve this: When frontswap activity is otherwise stable
+ * and the guest kernel is not under memory pressure, the frontswap
+ * selfshrinking" accounts for this by providing pressure to remove some
+ * pages from frontswap and return them to kernel memory.
+ *
+ * For both "selfballooning" and "frontswap-selfshrinking",
worker
+ * threads are used and sysfs tunables are provided to adjust the frequency
+ * and rate of adjustments to achieve the goal, as well as to disable one
+ * or both functions independently.
+ *
+ * While some argue that this functionality can and should be implemented
+ * in userspace, it has been observed that bad things happen (e.g. OOMs).
+ *
+ */
+
+#include <linux/kernel.h>
+#include <linux/mm.h>
+#include <linux/mman.h>
+
+#include <xen/balloon.h>
+
+#ifdef CONFIG_FRONTSWAP
+#include <linux/frontswap.h>
+#endif
+#include <xen/tmem.h>
+
+/* enable/disable with sysfs */
+static int xen_selfballooning_enabled __read_mostly;
+
+/* enable/disable with kernel boot option */
+static bool __initdata use_selfballooning = true;
+
+/*
+ * controls rate at which memory target (this iteration) approaches
+ * ultimate goal when memory need is increasing (up-hysteresis) or
+ * decreasing (down-hysteresis). higher values of hysteresis cause
+ * slower increases/decreases
+ */
+static unsigned int selfballoon_downhysteresis __read_mostly;
+static unsigned int selfballoon_uphysteresis __read_mostly;
+
+/* in HZ, controls frequency of worker invocation */
+static unsigned int selfballoon_interval __read_mostly;
+
+static void selfballoon_process(struct work_struct *work);
+static DECLARE_DELAYED_WORK(selfballoon_worker, selfballoon_process);
+
+#ifdef CONFIG_FRONTSWAP
+/* enable/disable with sysfs */
+static bool frontswap_selfshrinking __read_mostly;
+
+/* enable/disable with kernel boot option */
+static bool __initdata use_frontswap_selfshrink = true;
+
+/* control rate for frontswap shrinking. higher hysteresis is slower */
+static unsigned int frontswap_hysteresis __read_mostly;
+
+/*
+ * number of selfballoon worker invocations to wait before observing that
+ * frontswap selfshrinking should commence. (Note that selfshrinking does
+ * not use a separate worker thread.)
+ */
+static unsigned int frontswap_inertia __read_mostly;
+
+/* countdown to next invocation of frontswap_shrink() */
+static unsigned long frontswap_inertia_counter;
+
+/*
+ * Invoked by the selfballoon worker thread, uses current number of pages
+ * in frontswap (frontswap_curr_pages()), previous status, and control
+ * values (hysteresis and inertia) to determine if frontswap should be
+ * shrunk and what the new frontswap size should be. Note that
+ * frontswap_shrink is essentially a partial swapoff that immediately
+ * transfers pages from the "swap device" (frontswap) back into
kernel
+ * RAM; despite the name, frontswap "shrinking" is very different
from
+ * the "shrinker" interface used by the kernel MM subsystem to
reclaim
+ * memory.
+ */
+static void frontswap_selfshrink(void)
+{
+ static unsigned long cur_frontswap_pages;
+ static unsigned long last_frontswap_pages;
+ static unsigned long tgt_frontswap_pages;
+
+ if (!frontswap_selfshrinking)
+ return;
+ last_frontswap_pages = cur_frontswap_pages;
+ cur_frontswap_pages = frontswap_curr_pages();
+ if (!cur_frontswap_pages ||
+ (cur_frontswap_pages > last_frontswap_pages)) {
+ frontswap_inertia_counter = frontswap_inertia;
+ return;
+ }
+ if (frontswap_inertia_counter && --frontswap_inertia_counter)
+ return;
+ if (cur_frontswap_pages <= frontswap_hysteresis)
+ tgt_frontswap_pages = 0;
+ else
+ tgt_frontswap_pages = cur_frontswap_pages -
+ (cur_frontswap_pages / frontswap_hysteresis);
+ frontswap_shrink(tgt_frontswap_pages);
+}
+
+static int __init no_frontswap_selfshrink_setup(char *s)
+{
+ use_frontswap_selfshrink = false;
+ return 1;
+}
+
+__setup("noselfshrink", no_frontswap_selfshrink_setup);
+#endif
+
+/*
+ * use current balloon size, the goal (vm_committed_as), and hysteresis
+ * parameters to set a new target balloon size
+ */
+static void selfballoon_process(struct work_struct *work)
+{
+ unsigned long cur_pages, goal_pages, tgt_pages;
+ bool reset_timer = false;
+
+ if (xen_selfballooning_enabled) {
+ cur_pages = balloon_stats.current_pages;
+ tgt_pages = cur_pages; /* default is no change */
+ goal_pages = percpu_counter_read_positive(&vm_committed_as) +
+ balloon_stats.current_pages - totalram_pages;
+ if (cur_pages > goal_pages)
+ tgt_pages = cur_pages -
+ ((cur_pages - goal_pages) /
+ selfballoon_downhysteresis);
+ else if (cur_pages < goal_pages)
+ tgt_pages = cur_pages +
+ ((goal_pages - cur_pages) /
+ selfballoon_uphysteresis);
+ /* else if cur_pages == goal_pages, no change */
+ balloon_set_new_target(tgt_pages);
+ reset_timer = true;
+ }
+#ifdef CONFIG_FRONTSWAP
+ if (frontswap_selfshrinking) {
+ frontswap_selfshrink();
+ reset_timer = true;
+ }
+#endif
+ if (reset_timer)
+ schedule_delayed_work(&selfballoon_worker,
+ selfballoon_interval * HZ);
+}
+
+#ifdef CONFIG_SYSFS
+
+#include <linux/sysdev.h>
+#include <linux/capability.h>
+
+#define SELFBALLOON_SHOW(name, format, args...) \
+ static ssize_t show_##name(struct sys_device *dev, \
+ struct sysdev_attribute *attr, \
+ char *buf) \
+ { \
+ return sprintf(buf, format, ##args); \
+ }
+
+SELFBALLOON_SHOW(selfballooning, "%d\n", xen_selfballooning_enabled);
+
+static ssize_t store_selfballooning(struct sys_device *dev,
+ struct sysdev_attribute *attr,
+ const char *buf,
+ size_t count)
+{
+ char *endchar;
+ bool was_enabled = xen_selfballooning_enabled;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ xen_selfballooning_enabled = !!memparse(buf, &endchar);
+
+ if (!was_enabled && xen_selfballooning_enabled)
+ schedule_delayed_work(&selfballoon_worker,
+ selfballoon_interval * HZ);
+
+ return count;
+}
+
+static SYSDEV_ATTR(selfballooning, S_IRUGO | S_IWUSR,
+ show_selfballooning, store_selfballooning);
+
+SELFBALLOON_SHOW(selfballoon_interval, "%d\n", selfballoon_interval);
+
+static ssize_t store_selfballoon_interval(struct sys_device *dev,
+ struct sysdev_attribute *attr,
+ const char *buf,
+ size_t count)
+{
+ char *endchar;
+ unsigned int val;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+ val = memparse(buf, &endchar);
+ if (val != 0)
+ selfballoon_interval = val;
+ return count;
+}
+
+static SYSDEV_ATTR(selfballoon_interval, S_IRUGO | S_IWUSR,
+ show_selfballoon_interval, store_selfballoon_interval);
+
+SELFBALLOON_SHOW(selfballoon_downhys, "%d\n",
selfballoon_downhysteresis);
+
+static ssize_t store_selfballoon_downhys(struct sys_device *dev,
+ struct sysdev_attribute *attr,
+ const char *buf,
+ size_t count)
+{
+ char *endchar;
+ unsigned int val;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+ val = memparse(buf, &endchar);
+ if (val != 0)
+ selfballoon_downhysteresis = val;
+ return count;
+}
+
+static SYSDEV_ATTR(selfballoon_downhysteresis, S_IRUGO | S_IWUSR,
+ show_selfballoon_downhys, store_selfballoon_downhys);
+
+
+SELFBALLOON_SHOW(selfballoon_uphys, "%d\n",
selfballoon_uphysteresis);
+
+static ssize_t store_selfballoon_uphys(struct sys_device *dev,
+ struct sysdev_attribute *attr,
+ const char *buf,
+ size_t count)
+{
+ char *endchar;
+ unsigned int val;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+ val = memparse(buf, &endchar);
+ if (val != 0)
+ selfballoon_uphysteresis = val;
+ return count;
+}
+
+static SYSDEV_ATTR(selfballoon_uphysteresis, S_IRUGO | S_IWUSR,
+ show_selfballoon_uphys, store_selfballoon_uphys);
+
+#ifdef CONFIG_FRONTSWAP
+SELFBALLOON_SHOW(frontswap_selfshrinking, "%d\n",
frontswap_selfshrinking);
+
+static ssize_t store_frontswap_selfshrinking(struct sys_device *dev,
+ struct sysdev_attribute *attr,
+ const char *buf,
+ size_t count)
+{
+ char *endchar;
+ bool was_enabled = frontswap_selfshrinking;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+
+ frontswap_selfshrinking = !!memparse(buf, &endchar);
+
+ if (!was_enabled && !xen_selfballooning_enabled &&
+ frontswap_selfshrinking)
+ schedule_delayed_work(&selfballoon_worker,
+ selfballoon_interval * HZ);
+
+ return count;
+}
+
+static SYSDEV_ATTR(frontswap_selfshrinking, S_IRUGO | S_IWUSR,
+ show_frontswap_selfshrinking, store_frontswap_selfshrinking);
+
+SELFBALLOON_SHOW(frontswap_inertia, "%d\n", frontswap_inertia);
+
+static ssize_t store_frontswap_inertia(struct sys_device *dev,
+ struct sysdev_attribute *attr,
+ const char *buf,
+ size_t count)
+{
+ char *endchar;
+ unsigned long val;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+ val = memparse(buf, &endchar);
+ if (val != 0) {
+ frontswap_inertia = val;
+ frontswap_inertia_counter = val;
+ }
+ return count;
+}
+
+static SYSDEV_ATTR(frontswap_inertia, S_IRUGO | S_IWUSR,
+ show_frontswap_inertia, store_frontswap_inertia);
+
+SELFBALLOON_SHOW(frontswap_hysteresis, "%d\n", frontswap_hysteresis);
+
+static ssize_t store_frontswap_hysteresis(struct sys_device *dev,
+ struct sysdev_attribute *attr,
+ const char *buf,
+ size_t count)
+{
+ char *endchar;
+ unsigned int val;
+
+ if (!capable(CAP_SYS_ADMIN))
+ return -EPERM;
+ val = memparse(buf, &endchar);
+ if (val != 0)
+ frontswap_hysteresis = val;
+ return count;
+}
+
+static SYSDEV_ATTR(frontswap_hysteresis, S_IRUGO | S_IWUSR,
+ show_frontswap_hysteresis, store_frontswap_hysteresis);
+
+#endif /* CONFIG_FRONTSWAP */
+
+static struct attribute *selfballoon_attrs[] = {
+ &attr_selfballooning.attr,
+ &attr_selfballoon_interval.attr,
+ &attr_selfballoon_downhysteresis.attr,
+ &attr_selfballoon_uphysteresis.attr,
+#ifdef CONFIG_FRONTSWAP
+ &attr_frontswap_selfshrinking.attr,
+ &attr_frontswap_hysteresis.attr,
+ &attr_frontswap_inertia.attr,
+#endif
+};
+
+static struct attribute_group selfballoon_group = {
+ .name = "selfballoon",
+ .attrs = selfballoon_attrs
+};
+#endif
+
+int register_xen_selfballooning(struct sys_device *sysdev)
+{
+ int error = -1;
+
+#ifdef CONFIG_SYSFS
+ error = sysfs_create_group(&sysdev->kobj, &selfballoon_group);
+#endif
+ return error;
+}
+EXPORT_SYMBOL(register_xen_selfballooning);
+
+static int __init noselfballooning_setup(char *s)
+{
+ use_selfballooning = false;
+ return 1;
+}
+
+__setup("noselfballooning", noselfballooning_setup);
+
+/*
+ * the default values for the various parameters were deemed reasonable
+ * by experimentation, may be workload-dependent, and can all be
+ * adjusted via sysfs
+ */
+static int __init xen_selfballoon_init(void)
+{
+ if (!xen_domain())
+ return -ENODEV;
+
+ pr_info("xen/balloon: Initializing Xen selfballooning driver.\n");
+ xen_selfballooning_enabled = tmem_enabled && use_selfballooning;
+ selfballoon_interval = 5;
+ selfballoon_downhysteresis = 8;
+ selfballoon_uphysteresis = 1;
+#ifdef CONFIG_FRONTSWAP
+ pr_info("xen/balloon: Initializing frontswap selfshrinking
driver.\n");
+ frontswap_selfshrinking = use_frontswap_selfshrink &&
frontswap_enabled;
+ frontswap_hysteresis = 20;
+ frontswap_inertia = 3;
+#endif
+ schedule_delayed_work(&selfballoon_worker, selfballoon_interval * HZ);
+
+ return 0;
+}
+
+subsys_initcall(xen_selfballoon_init);
+
+MODULE_LICENSE("GPL");
diff -Napur -X linux-3.0-rc1/Documentation/dontdiff
linux-3.0-rc1-frontswap/include/xen/balloon.h
linux-3.0-rc1-frontswap-selfballoon/include/xen/balloon.h
--- linux-3.0-rc1-frontswap/include/xen/balloon.h 2011-05-29 18:43:36.000000000
-0600
+++ linux-3.0-rc1-frontswap-selfballoon/include/xen/balloon.h 2011-06-20
14:58:24.975176230 -0600
@@ -23,3 +23,13 @@ void balloon_set_new_target(unsigned lon
int alloc_xenballooned_pages(int nr_pages, struct page** pages);
void free_xenballooned_pages(int nr_pages, struct page** pages);
+
+struct sys_device;
+#ifdef CONFIG_XEN_SELFBALLOONING
+extern int register_xen_selfballooning(struct sys_device *sysdev);
+#else
+static inline int register_xen_selfballooning(struct sys_device *sysdev)
+{
+ return -1;
+}
+#endif
diff -Napur -X linux-3.0-rc1/Documentation/dontdiff
linux-3.0-rc1-frontswap/include/xen/tmem.h
linux-3.0-rc1-frontswap-selfballoon/include/xen/tmem.h
--- linux-3.0-rc1-frontswap/include/xen/tmem.h 1969-12-31 17:00:00.000000000
-0700
+++ linux-3.0-rc1-frontswap-selfballoon/include/xen/tmem.h 2011-06-29
10:30:11.113929299 -0600
@@ -0,0 +1,5 @@
+#ifndef _XEN_TMEM_H
+#define _XEN_TMEM_H
+/* defined in drivers/xen/tmem.c */
+extern int tmem_enabled;
+#endif /* _XEN_TMEM_H */
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel
Konrad Rzeszutek Wilk
2011-Jun-30 21:50 UTC
[Xen-devel] Re: [PATCH v3] [linux-3.0+ for xen] tmem: self-ballooning and frontswap-selfshrinking
> ("tmem") functionality that complement cleancache and frontswap. Both > use control theory to dynamically adjust and optimize memory utilization. > Selfballooning controls the in-kernel Xen balloon driver, targeting a goal > value (vm_committed_as), thus pushing less frequently used clean > page cache pages (through the cleancache code) into Xen tmem where > Xen can balance needs across all VMs residing on the physical machine.Can this be used by KVM or HyperV code? Can it be made generic? If so, why not? If only Xen can use this, what would it entail for other balloon drivers to use this? Or is it that they really don''t need to b/c none of them use the clean cache code?> Frontswap-selfshrinking controls the number of pages in frontswap, > driving it towards zero (effectively doing a partial swapoff) when > in-kernel memory pressure subsides, freeing up RAM for other VMs._______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Dan Magenheimer
2011-Jun-30 22:08 UTC
[Xen-devel] RE: [PATCH v3] [linux-3.0+ for xen] tmem: self-ballooning and frontswap-selfshrinking
> From: Konrad Rzeszutek Wilk > Sent: Thursday, June 30, 2011 3:50 PM > To: Dan Magenheimer > Cc: xen-devel@lists.xensource.com; JBeulich@novell.com; jeremy@goop.org; Ian Campbell; Daniel Kiper > Subject: Re: [PATCH v3] [linux-3.0+ for xen] tmem: self-ballooning and frontswap-selfshrinking > > > ("tmem") functionality that complement cleancache and frontswap. Both > > use control theory to dynamically adjust and optimize memory utilization. > > Selfballooning controls the in-kernel Xen balloon driver, targeting a goal > > value (vm_committed_as), thus pushing less frequently used clean > > page cache pages (through the cleancache code) into Xen tmem where > > Xen can balance needs across all VMs residing on the physical machine. > > Can this be used by KVM or HyperV code? Can it be made generic? If so, why not? > If only Xen can use this, what would it entail for other balloon drivers > to use this? Or is it that they really don''t need to b/c none of them > use the clean cache code?I''m not an expert on KVM nor on HyperV. If either becomes capable of supporting tmem and if the KVM/HyperV equivalent of balloon drivers are suitable, concepts similar to selfballooning and frontswap-selfshrinking are likely useful, though it would require quite a bit more study to try to guess at how one might make the proposed code generic. I expect to talk to folk at the upcoming KVM Forum to consider next steps, and another proposed patch (https://lkml.org/lkml/2011/6/30/354) moves the in-kernel tmem support one step closer to supporting KVM. It''s been awhile since I''ve communicated with Ky about tmem so I will re-open that conversation. But it will probably be months/years before generic''izing this proposed patch is feasible. Seems best to cross that bridge later. Dan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Konrad Rzeszutek Wilk
2011-Jun-30 22:19 UTC
[Xen-devel] Re: [PATCH v3] [linux-3.0+ for xen] tmem: self-ballooning and frontswap-selfshrinking
On Thu, Jun 30, 2011 at 03:08:41PM -0700, Dan Magenheimer wrote:> > From: Konrad Rzeszutek Wilk > > Sent: Thursday, June 30, 2011 3:50 PM > > To: Dan Magenheimer > > Cc: xen-devel@lists.xensource.com; JBeulich@novell.com; jeremy@goop.org; Ian Campbell; Daniel Kiper > > Subject: Re: [PATCH v3] [linux-3.0+ for xen] tmem: self-ballooning and frontswap-selfshrinking > > > > > ("tmem") functionality that complement cleancache and frontswap. Both > > > use control theory to dynamically adjust and optimize memory utilization. > > > Selfballooning controls the in-kernel Xen balloon driver, targeting a goal > > > value (vm_committed_as), thus pushing less frequently used clean > > > page cache pages (through the cleancache code) into Xen tmem where > > > Xen can balance needs across all VMs residing on the physical machine. > > > > Can this be used by KVM or HyperV code? Can it be made generic? If so, why not? > > If only Xen can use this, what would it entail for other balloon drivers > > to use this? Or is it that they really don''t need to b/c none of them > > use the clean cache code? > > I''m not an expert on KVM nor on HyperV. If either becomes capable > of supporting tmem and if the KVM/HyperV equivalent of balloon drivers > are suitable, concepts similar to selfballooning and frontswap-selfshrinking > are likely useful, though it would require quite a bit more study to > try to guess at how one might make the proposed code generic. > > I expect to talk to folk at the upcoming KVM Forum to consider > next steps, and another proposed patch (https://lkml.org/lkml/2011/6/30/354) > moves the in-kernel tmem support one step closer to supporting KVM. > > It''s been awhile since I''ve communicated with Ky about tmem so I will > re-open that conversation. > > But it will probably be months/years before generic''izing this proposed > patch is feasible. Seems best to cross that bridge later.The mechanism this patch employs to "balloon" is quite generic - it uses the shrinker API. I am trying to understand from a technical perspective why this code cannot be outside Xen as generic code? _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Dan Magenheimer
2011-Jun-30 22:43 UTC
[Xen-devel] RE: [PATCH v3] [linux-3.0+ for xen] tmem: self-ballooning and frontswap-selfshrinking
> > > Can this be used by KVM or HyperV code? Can it be made generic? If so, why not? > > > If only Xen can use this, what would it entail for other balloon drivers > > > to use this? Or is it that they really don''t need to b/c none of them > > > use the clean cache code? > > > > I''m not an expert on KVM nor on HyperV. If either becomes capable > > of supporting tmem and if the KVM/HyperV equivalent of balloon drivers > > are suitable, concepts similar to selfballooning and frontswap-selfshrinking > > are likely useful, though it would require quite a bit more study to > > try to guess at how one might make the proposed code generic. > > > > I expect to talk to folk at the upcoming KVM Forum to consider > > next steps, and another proposed patch (https://lkml.org/lkml/2011/6/30/354) > > moves the in-kernel tmem support one step closer to supporting KVM. > > > > It''s been awhile since I''ve communicated with Ky about tmem so I will > > re-open that conversation. > > > > But it will probably be months/years before generic''izing this proposed > > patch is feasible. Seems best to cross that bridge later. > > The mechanism this patch employs to "balloon" is quite generic - it uses the > shrinker API. I am trying to understand from a technical perspective why this > code cannot be outside Xen as generic code?No it doesn''t use the kernel shrinker API. I added a couple of comments to explain that (since you had assumed that in your earlier review). The kernel shrinker API calls shrinker function hooks in other subsystems to request they surrender otherwise unused memory so that the kernel can reclaim space when under memory pressure. The frontswap shrinker causes dirty anonymous pages held in transcendent memory to be forced back into real kernel RAM when the kernel is NOT under memory pressure. Both are doing shrinking, by the English definition, just shrinking very different things with very different objectives which requires very different interfaces and mechanisms. Perhaps overloading the term "shrinker" is confusing, but I think I''ve already coined enough new words for tmem, don''t you? ;-) And, for completeness, self-ballooning is also very different than the kernel shrinker API... a similar concept might be implemented using the kernel shrinker API (and Daniel Kiper and I discussed that once on-list), but the kernel "policy" for calling shrinker functions is a bit obscure, has very different objectives, and would only work "one direction" (reclaiming memory from the balloon driver, not providing it). Yes, the core concept of using control theory to manage memory supply vs. demand is potentially generic and eventually other tmem-capable or tmem-ish environments might want to do something similar. But I do think that''s likely to be months/years away and trying to generic''ize the proposed code (and especially getting it upstream under these circumstances) will likely be an exercise in frustration until other in-kernel users try to use it first. (The code implementing the core concept is extremely small in any case... the vast majority of the code in this patch is sysfs, initialization, and work management overhead, which is unlikely to be generic anyway.) Just my opinion... Dan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Daniel Kiper
2011-Jul-04 13:31 UTC
[Xen-devel] Re: [PATCH v3] [linux-3.0+ for xen] tmem: self-ballooning and frontswap-selfshrinking
On Wed, Jun 29, 2011 at 01:15:24PM -0700, Dan Magenheimer wrote:> Hi Konrad (and other reviewers) --Sorry for late reply, however, I am very busy now.> Here is version 3 of the Xen tmem selfballooning/frontswap-selfshrinking > patch. Many thanks to the reviewers of V1 and V2 for your excellent > comments and feedback. I hope V3 is ready for merging as I''m > hoping to get this merged for the upcoming 3.1 window. > > If it is satisfactory, you may wish to check the commit in my tree: > > git://git.kernel.org/pub/scm/linux/kernel/git/djm/tmem.git #xen-tmem-selfballoon-v3 > > If all looks good and there are no issues raised in the next > day or two, I will post an official git pull request to you > and cc lkml. > > Note that this patch has a slight build dependency on last week''s: > > afec6e04922d0c8c7e244be2e544bac5e7e36294 xen: prepare tmem shim to handle frontswap > > in your devel/next-3.0 branch (because tmem_enabled is EXPORT_SYM''ed > in that commit and extern''ed in a file in this commit). I don''t > think this is a problem as you''ve already pushed that commit > to linux-next, but am noting the dependency in case some kind > of commit reordering were to occur. > > ==> >From: Dan Magenheimer <dan.magenheimer@oracle.com> > Subject: xen: tmem: self-ballooning and frontswap-selfshrinking > > This patch introduces two in-kernel drivers for Xen transcendent memory > ("tmem") functionality that complement cleancache and frontswap. Both > use control theory to dynamically adjust and optimize memory utilization. > Selfballooning controls the in-kernel Xen balloon driver, targeting a goal > value (vm_committed_as), thus pushing less frequently used clean > page cache pages (through the cleancache code) into Xen tmem where > Xen can balance needs across all VMs residing on the physical machine. > Frontswap-selfshrinking controls the number of pages in frontswap, > driving it towards zero (effectively doing a partial swapoff) when > in-kernel memory pressure subsides, freeing up RAM for other VMs. > > More detail is provided in the header comment of xen-selfballooning.c. > > Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com> > > [v3: konrad.wilk@oracle.com: fix potential divides-by-zero] > [v3: konrad.wilk@oracle.com: add many more comments, fix nits] > [v2: rebased to linux-3.0-rc1] > [v2: Ian.Campbell@citrix.com: reorganize as new file (xen-selfballoon.c)] > [v2: dkiper@net-space.pl: proper access to vm_committed_as] > [v2: dkiper@net-space.pl: accounting fixes] > Cc: Jan Beulich <JBeulich@novell.com> > Cc: Jeremy Fitzhardinge <jeremy@goop.org> > Cc: <xen-devel@lists.xensource.com> > > --- > > Diffstat: > drivers/xen/Kconfig | 12 > drivers/xen/Makefile | 1 > drivers/xen/xen-balloon.c | 2 > drivers/xen/xen-selfballoon.c | 429 +++++++++++++++++++++ > include/xen/balloon.h | 10 > include/xen/tmem.h | 5 > 6 files changed, 459 insertions(+) > > diff -Napur -X linux-3.0-rc1/Documentation/dontdiff linux-3.0-rc1-frontswap/drivers/xen/Kconfig linux-3.0-rc1-frontswap-selfballoon/drivers/xen/Kconfig > --- linux-3.0-rc1-frontswap/drivers/xen/Kconfig 2011-05-31 17:09:07.606910896 -0600 > +++ linux-3.0-rc1-frontswap-selfballoon/drivers/xen/Kconfig 2011-06-29 10:52:39.834899532 -0600 > @@ -9,6 +9,18 @@ config XEN_BALLOON > the system to expand the domain''s memory allocation, or alternatively > return unneeded memory to the system. > > +config XEN_SELFBALLOONING > + bool "dynamically self-balloon kernel memory to target" > + depends on XEN && XEN_BALLOON && CLEANCACHE > + default n > + help > + Self-ballooning dynamically balloons available kernel memory driven > + by the current usage of anonymous memory ("committed AS") and > + controlled by various sysfs-settable parameters. May be overridden > + by the noselfballooning kernel boot parameter. If FRONTSWAP > + is configured, also enables frontswap-selfshrinking, which can be > + overridden by the noselfshrink kernel boot paramater. > + > config XEN_SCRUB_PAGES > bool "Scrub pages before returning them to system" > depends on XEN_BALLOON > diff -Napur -X linux-3.0-rc1/Documentation/dontdiff linux-3.0-rc1-frontswap/drivers/xen/Makefile linux-3.0-rc1-frontswap-selfballoon/drivers/xen/Makefile > --- linux-3.0-rc1-frontswap/drivers/xen/Makefile 2011-05-31 17:09:57.006875306 -0600 > +++ linux-3.0-rc1-frontswap-selfballoon/drivers/xen/Makefile 2011-06-16 11:39:06.123852289 -0600 > @@ -8,6 +8,7 @@ obj-$(CONFIG_BLOCK) += biomerge.o > obj-$(CONFIG_HOTPLUG_CPU) += cpu_hotplug.o > obj-$(CONFIG_XEN_XENCOMM) += xencomm.o > obj-$(CONFIG_XEN_BALLOON) += xen-balloon.o > +obj-$(CONFIG_XEN_SELFBALLOONING) += xen-selfballoon.o > obj-$(CONFIG_XEN_DEV_EVTCHN) += xen-evtchn.o > obj-$(CONFIG_XEN_GNTDEV) += xen-gntdev.o > obj-$(CONFIG_XEN_GRANT_DEV_ALLOC) += xen-gntalloc.o > diff -Napur -X linux-3.0-rc1/Documentation/dontdiff linux-3.0-rc1-frontswap/drivers/xen/xen-balloon.c linux-3.0-rc1-frontswap-selfballoon/drivers/xen/xen-balloon.c > --- linux-3.0-rc1-frontswap/drivers/xen/xen-balloon.c 2011-05-29 18:43:36.000000000 -0600 > +++ linux-3.0-rc1-frontswap-selfballoon/drivers/xen/xen-balloon.c 2011-06-20 15:37:30.405798859 -0600 > @@ -98,6 +98,8 @@ static int __init balloon_init(void) > > register_balloon(&balloon_sysdev); > > + register_xen_selfballooning(&balloon_sysdev); > + > target_watch.callback = watch_target; > xenstore_notifier.notifier_call = balloon_init_watcher; > > diff -Napur -X linux-3.0-rc1/Documentation/dontdiff linux-3.0-rc1-frontswap/drivers/xen/xen-selfballoon.c linux-3.0-rc1-frontswap-selfballoon/drivers/xen/xen-selfballoon.c > --- linux-3.0-rc1-frontswap/drivers/xen/xen-selfballoon.c 1969-12-31 17:00:00.000000000 -0700 > +++ linux-3.0-rc1-frontswap-selfballoon/drivers/xen/xen-selfballoon.c 2011-06-29 11:44:44.067872414 -0600 > @@ -0,0 +1,429 @@ > +/****************************************************************************** > + * Xen selfballoon driver (and optional frontswap self-shrinking driver) > + * > + * Copyright (c) 2009-2011, Dan Magenheimer, Oracle Corp. > + * > + * This code complements the cleancache and frontswap patchsets to optimize > + * support for Xen Transcendent Memory ("tmem"). The policy it implements > + * is rudimentary and will likely improve over time, but it does work well > + * enough today. > + * > + * Two functionalities are implemented here which both use "control theory" > + * (feedback) to optimize memory utilization. In a virtualized environment > + * such as Xen, RAM is often a scarce resource and we would like to ensure > + * that each of a possibly large number of virtual machines is using RAM > + * efficiently, i.e. using as little as possible when under light load > + * and obtaining as much as possible when memory demands are high. > + * Since RAM needs vary highly dynamically and sometimes dramatically, > + * "hysteresis" is used, that is, memory target is determined not just > + * on current data but also on past data stored in the system. > + * > + * "Selfballooning" creates memory pressure by managing the Xen balloon > + * driver to decrease and increase available kernel memory, driven > + * largely by the target value of "Committed_AS" (see /proc/meminfo). > + * Since Committed_AS does not account for clean mapped pages (i.e. pages > + * in RAM that are identical to pages on disk), selfballooning has the > + * affect of pushing less frequently used clean pagecache pages out of > + * kernel RAM and, presumably using cleancache, into Xen tmem where > + * Xen can more efficiently optimize RAM utilization for such pages. > + * > + * When kernel memory demand unexpectedly increases faster than Xen, via > + * the selfballoon driver, is able to (or chooses to) provide usable RAM, > + * the kernel may invoke swapping. In most cases, frontswap is able > + * to absorb this swapping into Xen tmem. However, due to the fact > + * that the kernel swap subsystem assumes swapping occurs to a disk, > + * swapped pages may sit on the disk for a very long time; even if > + * the kernel knows the page will never be used again. This is because > + * the disk space costs very little and can be overwritten when > + * necessary. When such stale pages are in frontswap, however, they > + * are taking up valuable real estate. "Frontswap selfshrinking" works > + * to resolve this: When frontswap activity is otherwise stable > + * and the guest kernel is not under memory pressure, the frontswap > + * selfshrinking" accounts for this by providing pressure to remove some > + * pages from frontswap and return them to kernel memory. > + * > + * For both "selfballooning" and "frontswap-selfshrinking", worker > + * threads are used and sysfs tunables are provided to adjust the frequency > + * and rate of adjustments to achieve the goal, as well as to disable one > + * or both functions independently. > + * > + * While some argue that this functionality can and should be implemented > + * in userspace, it has been observed that bad things happen (e.g. OOMs). > + * > + */ > + > +#include <linux/kernel.h> > +#include <linux/mm.h> > +#include <linux/mman.h> > + > +#include <xen/balloon.h> > + > +#ifdef CONFIG_FRONTSWAP > +#include <linux/frontswap.h> > +#endifPlease move this condition to linux/frontswap.h and include this header file (linux/frontswap.h) unconditionally.> +#include <xen/tmem.h> > + > +/* enable/disable with sysfs */ > +static int xen_selfballooning_enabled __read_mostly; > + > +/* enable/disable with kernel boot option */ > +static bool __initdata use_selfballooning = true;static bool use_selfballooning __initdata = true; Please look into include/linux/init.h for details.> + > +/* > + * controls rate at which memory target (this iteration) approaches > + * ultimate goal when memory need is increasing (up-hysteresis) or > + * decreasing (down-hysteresis). higher values of hysteresis cause > + * slower increases/decreases > + */ > +static unsigned int selfballoon_downhysteresis __read_mostly; > +static unsigned int selfballoon_uphysteresis __read_mostly; > + > +/* in HZ, controls frequency of worker invocation */ > +static unsigned int selfballoon_interval __read_mostly;Could you create a struct selfballoon ??? I think it will be more readable.> + > +static void selfballoon_process(struct work_struct *work); > +static DECLARE_DELAYED_WORK(selfballoon_worker, selfballoon_process); > + > +#ifdef CONFIG_FRONTSWAP > +/* enable/disable with sysfs */ > +static bool frontswap_selfshrinking __read_mostly; > + > +/* enable/disable with kernel boot option */ > +static bool __initdata use_frontswap_selfshrink = true;Ditto.> + > +/* control rate for frontswap shrinking. higher hysteresis is slower */ > +static unsigned int frontswap_hysteresis __read_mostly; > + > +/* > + * number of selfballoon worker invocations to wait before observing that > + * frontswap selfshrinking should commence. (Note that selfshrinking does > + * not use a separate worker thread.) > + */ > +static unsigned int frontswap_inertia __read_mostly; > + > +/* countdown to next invocation of frontswap_shrink() */ > +static unsigned long frontswap_inertia_counter;struct frontswap ???> + > +/* > + * Invoked by the selfballoon worker thread, uses current number of pages > + * in frontswap (frontswap_curr_pages()), previous status, and control > + * values (hysteresis and inertia) to determine if frontswap should be > + * shrunk and what the new frontswap size should be. Note that > + * frontswap_shrink is essentially a partial swapoff that immediately > + * transfers pages from the "swap device" (frontswap) back into kernel > + * RAM; despite the name, frontswap "shrinking" is very different from > + * the "shrinker" interface used by the kernel MM subsystem to reclaim > + * memory. > + */ > +static void frontswap_selfshrink(void) > +{ > + static unsigned long cur_frontswap_pages; > + static unsigned long last_frontswap_pages; > + static unsigned long tgt_frontswap_pages;If you create struct frontswap then move it to this struct.> + > + if (!frontswap_selfshrinking) > + return; > + last_frontswap_pages = cur_frontswap_pages; > + cur_frontswap_pages = frontswap_curr_pages(); > + if (!cur_frontswap_pages || > + (cur_frontswap_pages > last_frontswap_pages)) { > + frontswap_inertia_counter = frontswap_inertia; > + return; > + } > + if (frontswap_inertia_counter && --frontswap_inertia_counter) > + return; > + if (cur_frontswap_pages <= frontswap_hysteresis) > + tgt_frontswap_pages = 0; > + else > + tgt_frontswap_pages = cur_frontswap_pages - > + (cur_frontswap_pages / frontswap_hysteresis); > + frontswap_shrink(tgt_frontswap_pages); > +} > + > +static int __init no_frontswap_selfshrink_setup(char *s) > +{ > + use_frontswap_selfshrink = false; > + return 1; > +} > + > +__setup("noselfshrink", no_frontswap_selfshrink_setup); > +#endif > + > +/* > + * use current balloon size, the goal (vm_committed_as), and hysteresis > + * parameters to set a new target balloon size > + */ > +static void selfballoon_process(struct work_struct *work) > +{ > + unsigned long cur_pages, goal_pages, tgt_pages; > + bool reset_timer = false; > + > + if (xen_selfballooning_enabled) { > + cur_pages = balloon_stats.current_pages; > + tgt_pages = cur_pages; /* default is no change */ > + goal_pages = percpu_counter_read_positive(&vm_committed_as) + > + balloon_stats.current_pages - totalram_pages; > + if (cur_pages > goal_pages) > + tgt_pages = cur_pages - > + ((cur_pages - goal_pages) / > + selfballoon_downhysteresis); > + else if (cur_pages < goal_pages) > + tgt_pages = cur_pages + > + ((goal_pages - cur_pages) / > + selfballoon_uphysteresis); > + /* else if cur_pages == goal_pages, no change */ > + balloon_set_new_target(tgt_pages); > + reset_timer = true; > + } > +#ifdef CONFIG_FRONTSWAP > + if (frontswap_selfshrinking) { > + frontswap_selfshrink(); > + reset_timer = true; > + } > +#endif > + if (reset_timer) > + schedule_delayed_work(&selfballoon_worker, > + selfballoon_interval * HZ); > +} > + > +#ifdef CONFIG_SYSFS > + > +#include <linux/sysdev.h> > +#include <linux/capability.h> > + > +#define SELFBALLOON_SHOW(name, format, args...) \ > + static ssize_t show_##name(struct sys_device *dev, \ > + struct sysdev_attribute *attr, \ > + char *buf) \ > + { \ > + return sprintf(buf, format, ##args); \ > + } > + > +SELFBALLOON_SHOW(selfballooning, "%d\n", xen_selfballooning_enabled); > + > +static ssize_t store_selfballooning(struct sys_device *dev, > + struct sysdev_attribute *attr, > + const char *buf, > + size_t count) > +{ > + char *endchar; > + bool was_enabled = xen_selfballooning_enabled; > + > + if (!capable(CAP_SYS_ADMIN)) > + return -EPERM; > + > + xen_selfballooning_enabled = !!memparse(buf, &endchar);Replace memparse() by strict_strtoul().> + > + if (!was_enabled && xen_selfballooning_enabled) > + schedule_delayed_work(&selfballoon_worker, > + selfballoon_interval * HZ); > + > + return count; > +} > + > +static SYSDEV_ATTR(selfballooning, S_IRUGO | S_IWUSR, > + show_selfballooning, store_selfballooning); > + > +SELFBALLOON_SHOW(selfballoon_interval, "%d\n", selfballoon_interval); > + > +static ssize_t store_selfballoon_interval(struct sys_device *dev, > + struct sysdev_attribute *attr, > + const char *buf, > + size_t count) > +{ > + char *endchar; > + unsigned int val; > + > + if (!capable(CAP_SYS_ADMIN)) > + return -EPERM; > + val = memparse(buf, &endchar);Ditto.> + if (val != 0) > + selfballoon_interval = val; > + return count; > +} > + > +static SYSDEV_ATTR(selfballoon_interval, S_IRUGO | S_IWUSR, > + show_selfballoon_interval, store_selfballoon_interval); > + > +SELFBALLOON_SHOW(selfballoon_downhys, "%d\n", selfballoon_downhysteresis); > + > +static ssize_t store_selfballoon_downhys(struct sys_device *dev, > + struct sysdev_attribute *attr, > + const char *buf, > + size_t count) > +{ > + char *endchar; > + unsigned int val; > + > + if (!capable(CAP_SYS_ADMIN)) > + return -EPERM; > + val = memparse(buf, &endchar);Ditto.> + if (val != 0) > + selfballoon_downhysteresis = val; > + return count; > +} > + > +static SYSDEV_ATTR(selfballoon_downhysteresis, S_IRUGO | S_IWUSR, > + show_selfballoon_downhys, store_selfballoon_downhys); > + > + > +SELFBALLOON_SHOW(selfballoon_uphys, "%d\n", selfballoon_uphysteresis); > + > +static ssize_t store_selfballoon_uphys(struct sys_device *dev, > + struct sysdev_attribute *attr, > + const char *buf, > + size_t count) > +{ > + char *endchar; > + unsigned int val; > + > + if (!capable(CAP_SYS_ADMIN)) > + return -EPERM; > + val = memparse(buf, &endchar);Ditto.> + if (val != 0) > + selfballoon_uphysteresis = val; > + return count; > +} > + > +static SYSDEV_ATTR(selfballoon_uphysteresis, S_IRUGO | S_IWUSR, > + show_selfballoon_uphys, store_selfballoon_uphys); > + > +#ifdef CONFIG_FRONTSWAP > +SELFBALLOON_SHOW(frontswap_selfshrinking, "%d\n", frontswap_selfshrinking); > + > +static ssize_t store_frontswap_selfshrinking(struct sys_device *dev, > + struct sysdev_attribute *attr, > + const char *buf, > + size_t count) > +{ > + char *endchar; > + bool was_enabled = frontswap_selfshrinking; > + > + if (!capable(CAP_SYS_ADMIN)) > + return -EPERM; > + > + frontswap_selfshrinking = !!memparse(buf, &endchar);Ditto.> + > + if (!was_enabled && !xen_selfballooning_enabled && > + frontswap_selfshrinking) > + schedule_delayed_work(&selfballoon_worker, > + selfballoon_interval * HZ);I think it is not worth to wrap lines which only have langht slightly above 80 characters limit. In this case two lines are more readable.> + > + return count; > +} > + > +static SYSDEV_ATTR(frontswap_selfshrinking, S_IRUGO | S_IWUSR, > + show_frontswap_selfshrinking, store_frontswap_selfshrinking); > + > +SELFBALLOON_SHOW(frontswap_inertia, "%d\n", frontswap_inertia); > + > +static ssize_t store_frontswap_inertia(struct sys_device *dev, > + struct sysdev_attribute *attr, > + const char *buf, > + size_t count) > +{ > + char *endchar; > + unsigned long val; > + > + if (!capable(CAP_SYS_ADMIN)) > + return -EPERM; > + val = memparse(buf, &endchar);Ditto.> + if (val != 0) { > + frontswap_inertia = val; > + frontswap_inertia_counter = val; > + } > + return count; > +} > + > +static SYSDEV_ATTR(frontswap_inertia, S_IRUGO | S_IWUSR, > + show_frontswap_inertia, store_frontswap_inertia); > + > +SELFBALLOON_SHOW(frontswap_hysteresis, "%d\n", frontswap_hysteresis); > + > +static ssize_t store_frontswap_hysteresis(struct sys_device *dev, > + struct sysdev_attribute *attr, > + const char *buf, > + size_t count) > +{ > + char *endchar; > + unsigned int val; > + > + if (!capable(CAP_SYS_ADMIN)) > + return -EPERM; > + val = memparse(buf, &endchar);Ditto.> + if (val != 0) > + frontswap_hysteresis = val; > + return count; > +} > + > +static SYSDEV_ATTR(frontswap_hysteresis, S_IRUGO | S_IWUSR, > + show_frontswap_hysteresis, store_frontswap_hysteresis); > + > +#endif /* CONFIG_FRONTSWAP */ > + > +static struct attribute *selfballoon_attrs[] = { > + &attr_selfballooning.attr, > + &attr_selfballoon_interval.attr, > + &attr_selfballoon_downhysteresis.attr, > + &attr_selfballoon_uphysteresis.attr, > +#ifdef CONFIG_FRONTSWAP > + &attr_frontswap_selfshrinking.attr, > + &attr_frontswap_hysteresis.attr, > + &attr_frontswap_inertia.attr, > +#endif > +}; > + > +static struct attribute_group selfballoon_group = { > + .name = "selfballoon", > + .attrs = selfballoon_attrs > +}; > +#endif > + > +int register_xen_selfballooning(struct sys_device *sysdev) > +{ > + int error = -1;int error = -ENOSYS;> + > +#ifdef CONFIG_SYSFS > + error = sysfs_create_group(&sysdev->kobj, &selfballoon_group); > +#endif > + return error; > +} > +EXPORT_SYMBOL(register_xen_selfballooning); > + > +static int __init noselfballooning_setup(char *s) > +{ > + use_selfballooning = false; > + return 1; > +} > + > +__setup("noselfballooning", noselfballooning_setup); > + > +/* > + * the default values for the various parameters were deemed reasonable > + * by experimentation, may be workload-dependent, and can all be > + * adjusted via sysfs > + */ > +static int __init xen_selfballoon_init(void) > +{ > + if (!xen_domain()) > + return -ENODEV; > + > + pr_info("xen/balloon: Initializing Xen selfballooning driver.\n"); > + xen_selfballooning_enabled = tmem_enabled && use_selfballooning; > + selfballoon_interval = 5; > + selfballoon_downhysteresis = 8; > + selfballoon_uphysteresis = 1; > +#ifdef CONFIG_FRONTSWAP > + pr_info("xen/balloon: Initializing frontswap selfshrinking driver.\n"); > + frontswap_selfshrinking = use_frontswap_selfshrink && frontswap_enabled; > + frontswap_hysteresis = 20; > + frontswap_inertia = 3; > +#endif > + schedule_delayed_work(&selfballoon_worker, selfballoon_interval * HZ); > + > + return 0; > +} > + > +subsys_initcall(xen_selfballoon_init); > + > +MODULE_LICENSE("GPL"); > diff -Napur -X linux-3.0-rc1/Documentation/dontdiff linux-3.0-rc1-frontswap/include/xen/balloon.h linux-3.0-rc1-frontswap-selfballoon/include/xen/balloon.h > --- linux-3.0-rc1-frontswap/include/xen/balloon.h 2011-05-29 18:43:36.000000000 -0600 > +++ linux-3.0-rc1-frontswap-selfballoon/include/xen/balloon.h 2011-06-20 14:58:24.975176230 -0600 > @@ -23,3 +23,13 @@ void balloon_set_new_target(unsigned lon > > int alloc_xenballooned_pages(int nr_pages, struct page** pages); > void free_xenballooned_pages(int nr_pages, struct page** pages); > + > +struct sys_device; > +#ifdef CONFIG_XEN_SELFBALLOONING > +extern int register_xen_selfballooning(struct sys_device *sysdev); > +#else > +static inline int register_xen_selfballooning(struct sys_device *sysdev) > +{ > + return -1;return -ENOSYS; or return 0;> +} > +#endif > diff -Napur -X linux-3.0-rc1/Documentation/dontdiff linux-3.0-rc1-frontswap/include/xen/tmem.h linux-3.0-rc1-frontswap-selfballoon/include/xen/tmem.h > --- linux-3.0-rc1-frontswap/include/xen/tmem.h 1969-12-31 17:00:00.000000000 -0700 > +++ linux-3.0-rc1-frontswap-selfballoon/include/xen/tmem.h 2011-06-29 10:30:11.113929299 -0600 > @@ -0,0 +1,5 @@ > +#ifndef _XEN_TMEM_H > +#define _XEN_TMEM_H > +/* defined in drivers/xen/tmem.c */ > +extern int tmem_enabled; > +#endif /* _XEN_TMEM_H */Daniel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Dan Magenheimer
2011-Jul-04 20:06 UTC
[Xen-devel] RE: [PATCH v3] [linux-3.0+ for xen] tmem: self-ballooning and frontswap-selfshrinking
> From: Daniel Kiper [mailto:dkiper@net-space.pl] > Subject: Re: [PATCH v3] [linux-3.0+ for xen] tmem: self-ballooning and frontswap-selfshrinking > > On Wed, Jun 29, 2011 at 01:15:24PM -0700, Dan Magenheimer wrote: > > Hi Konrad (and other reviewers) -- > > Sorry for late reply, however, I am very busy now.Hi Daniel -- Thanks for taking the time to reply. I''ve made some of the changes you suggested and disagree with a couple of others. See below. Konrad, I will prepare a v4 and a git tree, but if you have already pulled, the changes for v4 are all syntactic and have no functional impact.> > +#ifdef CONFIG_FRONTSWAP > > +#include <linux/frontswap.h> > > +#endif > > Please move this condition to linux/frontswap.h and include > this header file (linux/frontswap.h) unconditionally.Sorry, this resolves a chicken-and-egg problem as it is. If the frontswap patch is not present, there is no file called include/linux/frontswap.h. The ifdef can be removed later when we are sure the frontswap patch is upstream.> > +static bool __initdata use_selfballooning = true; > > static bool use_selfballooning __initdata = true; > > Please look into include/linux/init.h for details.OK, there''s lots of examples that do it the other way throughout the kernel, but your reference looks like it should be authoritative so I made both changes.> > +static unsigned int selfballoon_downhysteresis __read_mostly; > > +static unsigned int selfballoon_uphysteresis __read_mostly; > > + > > +/* in HZ, controls frequency of worker invocation */ > > +static unsigned int selfballoon_interval __read_mostly; > > Could you create a struct selfballoon ??? > I think it will be more readable.Hmmmm... I guess I disagree. A struct is useful if, for example, you are going to pass a reference to a group of variables. These are all static and all start with the same prefix (due to Jeremy''s input some time ago that statics should still have unique names for debug purposes), so I don''t think grouping them will make it more readable. Maybe you are just used to seeing the struct in balloon.c? (same for the other similar comment)> > + xen_selfballooning_enabled = !!memparse(buf, &endchar); > > Replace memparse() by strict_strtoul().Again, there are many examples in the kernel that use memparse, but it appears that newer code does use strict_strtoul, so I made the changes throughout.> > + if (!was_enabled && !xen_selfballooning_enabled && > > + frontswap_selfshrinking) > > + schedule_delayed_work(&selfballoon_worker, > > + selfballoon_interval * HZ); > > I think it is not worth to wrap lines which only have langht > slightly above 80 characters limit. In this case two lines > are more readable.While I would tend to agree, if checkpatch doesn''t like it, someone is going to complain so I''d rather ensure the 80 character limit is preserved.> > +struct sys_device; > > +#ifdef CONFIG_XEN_SELFBALLOONING > > +extern int register_xen_selfballooning(struct sys_device *sysdev); > > +#else > > +static inline int register_xen_selfballooning(struct sys_device *sysdev) > > +{ > > + return -1; > > return -ENOSYS; > > or > > return 0;This is a bit of a nit, since the return value (like all the other register functions) is ignored, but I made the change anyway. Thanks again for looking it over, Daniel! Dan _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Daniel Kiper
2011-Jul-05 17:55 UTC
[Xen-devel] Re: [PATCH v3] [linux-3.0+ for xen] tmem: self-ballooning and frontswap-selfshrinking
On Mon, Jul 04, 2011 at 01:06:33PM -0700, Dan Magenheimer wrote:> > > +#ifdef CONFIG_FRONTSWAP > > > +#include <linux/frontswap.h> > > > +#endif > > > > Please move this condition to linux/frontswap.h and include > > this header file (linux/frontswap.h) unconditionally. > > Sorry, this resolves a chicken-and-egg problem as it is. If > the frontswap patch is not present, there is no file called > include/linux/frontswap.h. The ifdef can be removed later > when we are sure the frontswap patch is upstream.Hmmm... I think that in this situation it should be moved to frontswap patch.> > > +static bool __initdata use_selfballooning = true; > > > > static bool use_selfballooning __initdata = true; > > > > Please look into include/linux/init.h for details. > > OK, there''s lots of examples that do it the other way > throughout the kernel, but your reference looks like > it should be authoritative so I made both changes.Thanks.> > > +static unsigned int selfballoon_downhysteresis __read_mostly; > > > +static unsigned int selfballoon_uphysteresis __read_mostly; > > > + > > > +/* in HZ, controls frequency of worker invocation */ > > > +static unsigned int selfballoon_interval __read_mostly; > > > > Could you create a struct selfballoon ??? > > I think it will be more readable. > > Hmmmm... I guess I disagree. A struct is useful if, for example, > you are going to pass a reference to a group of variables. These > are all static and all start with the same prefix (due to Jeremy''s > input some time ago that statics should still have unique names > for debug purposes), so I don''t think grouping them will make > it more readable. Maybe you are just used to seeing the > struct in balloon.c? > > (same for the other similar comment)I do not fully agree, however, I do not insist on changing that.> > > + xen_selfballooning_enabled = !!memparse(buf, &endchar); > > > > Replace memparse() by strict_strtoul(). > > Again, there are many examples in the kernel that use memparse, > but it appears that newer code does use strict_strtoul, > so I made the changes throughout.As I saw it was designed to read memory size from kernel command line and module options (lib/cmdline.c). It is mostly used in that context. Additionally, you are using memparse() for parsing values which are not memory sizes. It could be misleading. That is why I asked you to change that to strict_strtoul() (it is generic).> > > + if (!was_enabled && !xen_selfballooning_enabled && > > > + frontswap_selfshrinking) > > > + schedule_delayed_work(&selfballoon_worker, > > > + selfballoon_interval * HZ); > > > > I think it is not worth to wrap lines which only have langht > > slightly above 80 characters limit. In this case two lines > > are more readable. > > While I would tend to agree, if checkpatch doesn''t like it, > someone is going to complain so I''d rather ensure the 80 > character limit is preserved.Line lengths overlimits are marked as warnings. If they are sane then kernel developers do not complain. Daniel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Dan Magenheimer
2011-Jul-05 19:38 UTC
[Xen-devel] RE: [PATCH v3] [linux-3.0+ for xen] tmem: self-ballooning and frontswap-selfshrinking
Hi Daniel -- Thanks for taking the time to reply again. Although you didn''t say it explicitly, I think you are now OK with V4. True? I need to get this wrapped up so I can re-purpose a test machine for another use. Thanks, Dan> > > Please move this condition to linux/frontswap.h and include > > > this header file (linux/frontswap.h) unconditionally. > > > > Sorry, this resolves a chicken-and-egg problem as it is. If > > the frontswap patch is not present, there is no file called > > include/linux/frontswap.h. The ifdef can be removed later > > when we are sure the frontswap patch is upstream. > > Hmmm... I think that in this situation > it should be moved to frontswap patch.You prefer the egg-before-the-chicken, I prefer the chicken-before-the-egg. :-) This approach demonstrates Xen''s clear use for frontswap, and allows trees both with frontswap (linux-next) and without frontswap (linux-3.0) to properly build.> As I saw it was designed to read memory size from kernel > command line and module options (lib/cmdline.c). It is > mostly used in that context. Additionally, you are using > memparse() for parsing values which are not memory sizes. > It could be misleading. That is why I asked you to > change that to strict_strtoul() (it is generic).I agree that strict_strtoul is the better of two very similar ways of doing a very similar thing in the kernel. Changed.> > While I would tend to agree, if checkpatch doesn''t like it, > > someone is going to complain so I''d rather ensure the 80 > > character limit is preserved. > > Line lengths overlimits are marked as warnings. If they are sane > then kernel developers do not complain.That''s not my experience... it seems to be a personal preference and some people have an allergic reaction to longer-than-80 lines. So I prefer to err on the side of a clean checkpatch. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Daniel Kiper
2011-Jul-06 09:39 UTC
[Xen-devel] Re: [PATCH v3] [linux-3.0+ for xen] tmem: self-ballooning and frontswap-selfshrinking
On Tue, Jul 05, 2011 at 12:38:03PM -0700, Dan Magenheimer wrote:> Hi Daniel -- > > Thanks for taking the time to reply again. Although you didn''t > say it explicitly, I think you are now OK with V4. True?Sorry, I forgot about it. If you change(d) what we agreed it is OK. However, please look below...> > > > Please move this condition to linux/frontswap.h and include > > > > this header file (linux/frontswap.h) unconditionally. > > > > > > Sorry, this resolves a chicken-and-egg problem as it is. If > > > the frontswap patch is not present, there is no file called > > > include/linux/frontswap.h. The ifdef can be removed later > > > when we are sure the frontswap patch is upstream. > > > > Hmmm... I think that in this situation > > it should be moved to frontswap patch. > > You prefer the egg-before-the-chicken, I prefer the > chicken-before-the-egg. :-) This approach demonstrates > Xen''s clear use for frontswap, and allows trees both with > frontswap (linux-next) and without frontswap (linux-3.0) > to properly build.Your soultion introduce code into linux-3.0 which could not be enabled (compiled and used) and could confuse others for what it is for if it could not be enabled. I still think that this patch should be splited into two independent (to some extent) entities (selfballooning and frontswap). Later one/both of them could be applied to appriopriate tree introducing only code which could be enabled and used.> > As I saw it was designed to read memory size from kernel > > command line and module options (lib/cmdline.c). It is > > mostly used in that context. Additionally, you are using > > memparse() for parsing values which are not memory sizes. > > It could be misleading. That is why I asked you to > > change that to strict_strtoul() (it is generic). > > I agree that strict_strtoul is the better of two very similar > ways of doing a very similar thing in the kernel. Changed.Thanks.> > > While I would tend to agree, if checkpatch doesn''t like it, > > > someone is going to complain so I''d rather ensure the 80 > > > character limit is preserved. > > > > Line lengths overlimits are marked as warnings. If they are sane > > then kernel developers do not complain. > > That''s not my experience... it seems to be a personal > preference and some people have an allergic reaction to > longer-than-80 lines. So I prefer to err on the side of > a clean checkpatch.OK. Daniel _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel