Re: [PATCH v4] [linux-3.0+ for xen] tmem: self-ballooning and frontswap-selfshrinking

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

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;