From mboxrd@z Thu Jan  1 00:00:00 1970
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Subject: Re: [PATCH v4] [linux-3.0+ for xen] tmem:
 self-ballooning and frontswap-selfshrinking
Date: Wed, 6 Jul 2011 09:42:30 -0400
Message-ID: <20110706134230.GB4138@dumpdata.com>
References: <068f4a4e-45e1-4968-9419-5b2e2c0b661f@default>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Return-path: <xen-devel-bounces@lists.xensource.com>
Content-Disposition: inline
In-Reply-To: <068f4a4e-45e1-4968-9419-5b2e2c0b661f@default>
List-Unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xensource.com>
List-Help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-Subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
Sender: xen-devel-bounces@lists.xensource.com
Errors-To: xen-devel-bounces@lists.xensource.com
To: Dan Magenheimer <dan.magenheimer@oracle.com>
Cc: jeremy@goop.org, xen-devel@lists.xensource.com, Ian Campbell <Ian.Campbell@citrix.com>, JBeulich@novell.com
List-Id: xen-devel@lists.xenproject.org

On Tue, Jul 05, 2011 at 09:29:00AM -0700, Dan Magenheimer wrote:
> Hi Konrad (and other reviewers) --

Hey Dan,

Only got two comments.

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

You are still missing the 'NULL' here.

> +};
> +
> +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;

I think those hysteris values can be set at the top of the file.

> +#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;