Xen devel - Nov 2012 - [PATCH] xen: tmem: selfballooning should be enabled when xen tmem is enabled

Konrad: Any chance this can get in for the upcoming window?
(Or is it enough of a bug fix that it can go in at an -rcN?)

It was just pointed out to me that some kernels have
cleancache and frontswap and xen_tmem enabled but NOT
xen_selfballooning!  While this configuration should be
possible, nearly all kernels that have CONFIG_XEN_TMEM=y should
also have CONFIG_XEN_SELFBALLOONING=y, since Transcendent
Memory (tmem) for Xen has very limited value without
selfballooning.

This is probably a result of a Kconfig mistake fixed I think
by the patch below.  Note that the year-old Oracle UEK2 kernel
distro has both CONFIG_XEN_TMEM and CONFIG_XEN_SELFBALLOONING
enabled, as does a Fedora 17 kernel update (3.6.6-1.fc17), so
the combination should be well tested.  Also, Xen tmem (and thus
selfballooning) are currently only enabled when a kernel boot
parameter is supplied so there is no runtime impact without
that boot parameter.

Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>

diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
index d4dffcd..b5f02f3 100644
--- a/drivers/xen/Kconfig
+++ b/drivers/xen/Kconfig
@@ -10,9 +10,9 @@ config XEN_BALLOON
 	  return unneeded memory to the system.
 
 config XEN_SELFBALLOONING
-	bool "Dynamically self-balloon kernel memory to target"
-	depends on XEN && XEN_BALLOON && CLEANCACHE && SWAP
&& XEN_TMEM
-	default n
+	bool
+	depends on XEN_BALLOON && SWAP
+	default y if XEN_TMEM
 	help
 	  Self-ballooning dynamically balloons available kernel memory driven
 	  by the current usage of anonymous memory ("committed AS") and

On Tue, Nov 20, 2012 at 02:42:18PM -0800, Dan Magenheimer
wrote:
> Konrad: Any chance this can get in for the upcoming window?
> (Or is it enough of a bug fix that it can go in at an -rcN?)
Lets do it in the next merge window. About now I am comfortable
only with regression fixes.
> 
> It was just pointed out to me that some kernels have
> cleancache and frontswap and xen_tmem enabled but NOT
> xen_selfballooning!  While this configuration should be
> possible, nearly all kernels that have CONFIG_XEN_TMEM=y should
> also have CONFIG_XEN_SELFBALLOONING=y, since Transcendent
> Memory (tmem) for Xen has very limited value without
> selfballooning.
> 
> This is probably a result of a Kconfig mistake fixed I think
> by the patch below.  Note that the year-old Oracle UEK2 kernel
> distro has both CONFIG_XEN_TMEM and CONFIG_XEN_SELFBALLOONING
> enabled, as does a Fedora 17 kernel update (3.6.6-1.fc17), so
> the combination should be well tested.  Also, Xen tmem (and thus
> selfballooning) are currently only enabled when a kernel boot
> parameter is supplied so there is no runtime impact without
> that boot parameter.
> 
> Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>
> 
> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> index d4dffcd..b5f02f3 100644
> --- a/drivers/xen/Kconfig
> +++ b/drivers/xen/Kconfig
> @@ -10,9 +10,9 @@ config XEN_BALLOON
>  	  return unneeded memory to the system.
>  
>  config XEN_SELFBALLOONING
> -	bool "Dynamically self-balloon kernel memory to target"
> -	depends on XEN && XEN_BALLOON && CLEANCACHE &&
SWAP && XEN_TMEM
> -	default n
> +	bool
> +	depends on XEN_BALLOON && SWAP
> +	default y if XEN_TMEM
>  	help
>  	  Self-ballooning dynamically balloons available kernel memory driven
>  	  by the current usage of anonymous memory ("committed AS") and

>>> On 20.11.12 at 23:42, Dan Magenheimer
<dan.magenheimer@oracle.com> wrote:
> Konrad: Any chance this can get in for the upcoming window?
> (Or is it enough of a bug fix that it can go in at an -rcN?)
> 
> It was just pointed out to me that some kernels have
> cleancache and frontswap and xen_tmem enabled but NOT
> xen_selfballooning!  While this configuration should be
> possible, nearly all kernels that have CONFIG_XEN_TMEM=y should
> also have CONFIG_XEN_SELFBALLOONING=y, since Transcendent
> Memory (tmem) for Xen has very limited value without
> selfballooning.
> 
> This is probably a result of a Kconfig mistake fixed I think
> by the patch below.  Note that the year-old Oracle UEK2 kernel
> distro has both CONFIG_XEN_TMEM and CONFIG_XEN_SELFBALLOONING
> enabled, as does a Fedora 17 kernel update (3.6.6-1.fc17), so
> the combination should be well tested.  Also, Xen tmem (and thus
> selfballooning) are currently only enabled when a kernel boot
> parameter is supplied so there is no runtime impact without
> that boot parameter.
> 
> Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>
> 
> diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> index d4dffcd..b5f02f3 100644
> --- a/drivers/xen/Kconfig
> +++ b/drivers/xen/Kconfig
> @@ -10,9 +10,9 @@ config XEN_BALLOON
>  	  return unneeded memory to the system.
>  
>  config XEN_SELFBALLOONING
> -	bool "Dynamically self-balloon kernel memory to target"
Why would you want to take away the configurability of this?
You wanting it always on in your use case doesn''t mean everyone
agrees. This would be the right way only when the option being
off despite all its dependencies being enabled is actively wrong.
> -	depends on XEN && XEN_BALLOON && CLEANCACHE &&
SWAP && XEN_TMEM
> -	default n
> +	bool
> +	depends on XEN_BALLOON && SWAP
> +	default y if XEN_TMEM
Changing the default, otoh, is certainly acceptable. However, this
should imo be (assuming that you dropped the CLEANCACHE
dependency for an unrelated [and unexplained] reason),

	depends on XEN_BALLOON && SWAP && XEN_TMEM
	default XEN_TMEM

i.e. the default selection can be simplified, but if you indeed
have a good reason to drop the prompt, the
dependencies should continue to include the symbol referenced
by the default directive, as otherwise you may end up with a
.config pointlessly having

# CONFIG_XEN_SELFBALLOONING is disabled. This is particularly
annoying when subsequently this gets a prompt re-added, since
at that point a "make oldconfig" won''t ask for the item to
get
possibly enabled as there is a value known for it already.
>  	help
>  	  Self-ballooning dynamically balloons available kernel memory driven
>  	  by the current usage of anonymous memory ("committed AS") and
If you take away the prompt, keeping the help text isn''t useful
either.

Jan

> From: Konrad Rzeszutek Wilk
> Sent: Tuesday, November 20, 2012 7:48 PM
> To: Dan Magenheimer
> Cc: linux-kernel@vger.kernel.org; xen-devel@lists.xen.org
> Subject: Re: [PATCH] xen: tmem: selfballooning should be enabled when xen
tmem is enabled
> 
> On Tue, Nov 20, 2012 at 02:42:18PM -0800, Dan Magenheimer wrote:
> > Konrad: Any chance this can get in for the upcoming window?
> > (Or is it enough of a bug fix that it can go in at an -rcN?)
> 
> Lets do it in the next merge window. About now I am comfortable
> only with regression fixes.
OK, thanks!
> > It was just pointed out to me that some kernels have
> > cleancache and frontswap and xen_tmem enabled but NOT
> > xen_selfballooning!  While this configuration should be
> > possible, nearly all kernels that have CONFIG_XEN_TMEM=y should
> > also have CONFIG_XEN_SELFBALLOONING=y, since Transcendent
> > Memory (tmem) for Xen has very limited value without
> > selfballooning.
> >
> > This is probably a result of a Kconfig mistake fixed I think
> > by the patch below.  Note that the year-old Oracle UEK2 kernel
> > distro has both CONFIG_XEN_TMEM and CONFIG_XEN_SELFBALLOONING
> > enabled, as does a Fedora 17 kernel update (3.6.6-1.fc17), so
> > the combination should be well tested.  Also, Xen tmem (and thus
> > selfballooning) are currently only enabled when a kernel boot
> > parameter is supplied so there is no runtime impact without
> > that boot parameter.
> >
> > Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>
> >
> > diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> > index d4dffcd..b5f02f3 100644
> > --- a/drivers/xen/Kconfig
> > +++ b/drivers/xen/Kconfig
> > @@ -10,9 +10,9 @@ config XEN_BALLOON
> >  	  return unneeded memory to the system.
> >
> >  config XEN_SELFBALLOONING
> > -	bool "Dynamically self-balloon kernel memory to target"
> > -	depends on XEN && XEN_BALLOON && CLEANCACHE
&& SWAP && XEN_TMEM
> > -	default n
> > +	bool
> > +	depends on XEN_BALLOON && SWAP
> > +	default y if XEN_TMEM
> >  	help
> >  	  Self-ballooning dynamically balloons available kernel memory
driven
> >  	  by the current usage of anonymous memory ("committed
AS") and

> From: Jan Beulich [mailto:JBeulich@suse.com]
> Sent: Wednesday, November 21, 2012 1:21 AM
> To: Dan Magenheimer
> Cc: xen-devel@lists.xen.org; Konrad Wilk; linux-kernel@vger.kernel.org
> Subject: Re: [Xen-devel] [PATCH] xen: tmem: selfballooning should be
enabled when xen tmem is enabled
> 
> >>> On 20.11.12 at 23:42, Dan Magenheimer
<dan.magenheimer@oracle.com> wrote:
> > Konrad: Any chance this can get in for the upcoming window?
> > (Or is it enough of a bug fix that it can go in at an -rcN?)
> >
> > It was just pointed out to me that some kernels have
> > cleancache and frontswap and xen_tmem enabled but NOT
> > xen_selfballooning!  While this configuration should be
> > possible, nearly all kernels that have CONFIG_XEN_TMEM=y should
> > also have CONFIG_XEN_SELFBALLOONING=y, since Transcendent
> > Memory (tmem) for Xen has very limited value without
> > selfballooning.
> >
> > This is probably a result of a Kconfig mistake fixed I think
> > by the patch below.  Note that the year-old Oracle UEK2 kernel
> > distro has both CONFIG_XEN_TMEM and CONFIG_XEN_SELFBALLOONING
> > enabled, as does a Fedora 17 kernel update (3.6.6-1.fc17), so
> > the combination should be well tested.  Also, Xen tmem (and thus
> > selfballooning) are currently only enabled when a kernel boot
> > parameter is supplied so there is no runtime impact without
> > that boot parameter.
> >
> > Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>
> >
> > diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
> > index d4dffcd..b5f02f3 100644
> > --- a/drivers/xen/Kconfig
> > +++ b/drivers/xen/Kconfig
> > @@ -10,9 +10,9 @@ config XEN_BALLOON
> >  	  return unneeded memory to the system.
> >
> >  config XEN_SELFBALLOONING
> > -	bool "Dynamically self-balloon kernel memory to target"
> 
> Why would you want to take away the configurability of this?
> You wanting it always on in your use case doesn''t mean everyone
> agrees. This would be the right way only when the option being
> off despite all its dependencies being enabled is actively wrong.
I''d like it to be configurable, but my config steps
(e.g. yes "" | make oldconfig, after enabling CLEANCACHE and
FRONTSWAP and XEN_TMEM) failed to enable it.  Removing the
prompt is the only thing that worked.
 
> > -	depends on XEN && XEN_BALLOON && CLEANCACHE
&& SWAP && XEN_TMEM
> > -	default n
> > +	bool
> > +	depends on XEN_BALLOON && SWAP
> > +	default y if XEN_TMEM
> 
> Changing the default, otoh, is certainly acceptable. However, this
> should imo be (assuming that you dropped the CLEANCACHE
> dependency for an unrelated [and unexplained] reason),
> 
> 	depends on XEN_BALLOON && SWAP && XEN_TMEM
> 	default XEN_TMEM
> 
> i.e. the default selection can be simplified, but if you indeed
> have a good reason to drop the prompt, the
> dependencies should continue to include the symbol referenced
> by the default directive, as otherwise you may end up with a
> .config pointlessly having
> 
> # CONFIG_XEN_SELFBALLOONING is disabled. This is particularly
> annoying when subsequently this gets a prompt re-added, since
> at that point a "make oldconfig" won''t ask for the item
to get
> possibly enabled as there is a value known for it already.
> 
> >  	help
> >  	  Self-ballooning dynamically balloons available kernel memory
driven
> >  	  by the current usage of anonymous memory ("committed
AS") and
> 
> If you take away the prompt, keeping the help text isn''t useful
> either.
Thanks for the feedback.  I''m an idiot with Kconfig and have
to play with it until it works as I expect.  Since Konrad
isn''t going to accept it for the upcoming window anyway,
I''ll play with it some more, but if the answer is obvious,
please spoon feed it to me. :-)

Dan

P.S. I removed the CLEANCACHE dependency because that dependency
is present for XEN_TMEM and I assumed dependencies are transitive.

>>> On 21.11.12 at 16:42, Dan Magenheimer
<dan.magenheimer@oracle.com> wrote:
>>  From: Jan Beulich [mailto:JBeulich@suse.com]
>> Sent: Wednesday, November 21, 2012 1:21 AM
>> To: Dan Magenheimer
>> Cc: xen-devel@lists.xen.org; Konrad Wilk; linux-kernel@vger.kernel.org 
>> Subject: Re: [Xen-devel] [PATCH] xen: tmem: selfballooning should be
enabled
> when xen tmem is enabled
>> 
>> >>> On 20.11.12 at 23:42, Dan Magenheimer
<dan.magenheimer@oracle.com> wrote:
>> > Konrad: Any chance this can get in for the upcoming window?
>> > (Or is it enough of a bug fix that it can go in at an -rcN?)
>> >
>> > It was just pointed out to me that some kernels have
>> > cleancache and frontswap and xen_tmem enabled but NOT
>> > xen_selfballooning!  While this configuration should be
>> > possible, nearly all kernels that have CONFIG_XEN_TMEM=y should
>> > also have CONFIG_XEN_SELFBALLOONING=y, since Transcendent
>> > Memory (tmem) for Xen has very limited value without
>> > selfballooning.
>> >
>> > This is probably a result of a Kconfig mistake fixed I think
>> > by the patch below.  Note that the year-old Oracle UEK2 kernel
>> > distro has both CONFIG_XEN_TMEM and CONFIG_XEN_SELFBALLOONING
>> > enabled, as does a Fedora 17 kernel update (3.6.6-1.fc17), so
>> > the combination should be well tested.  Also, Xen tmem (and thus
>> > selfballooning) are currently only enabled when a kernel boot
>> > parameter is supplied so there is no runtime impact without
>> > that boot parameter.
>> >
>> > Signed-off-by: Dan Magenheimer <dan.magenheimer@oracle.com>
>> >
>> > diff --git a/drivers/xen/Kconfig b/drivers/xen/Kconfig
>> > index d4dffcd..b5f02f3 100644
>> > --- a/drivers/xen/Kconfig
>> > +++ b/drivers/xen/Kconfig
>> > @@ -10,9 +10,9 @@ config XEN_BALLOON
>> >  	  return unneeded memory to the system.
>> >
>> >  config XEN_SELFBALLOONING
>> > -	bool "Dynamically self-balloon kernel memory to
target"
>> 
>> Why would you want to take away the configurability of this?
>> You wanting it always on in your use case doesn''t mean
everyone
>> agrees. This would be the right way only when the option being
>> off despite all its dependencies being enabled is actively wrong.
> 
> I''d like it to be configurable, but my config steps
> (e.g. yes "" | make oldconfig, after enabling CLEANCACHE and
> FRONTSWAP and XEN_TMEM) failed to enable it.  Removing the
> prompt is the only thing that worked.
If all of the dependencies are enabled, then I can''t see why the
prompt wouldn''t show up.
> P.S. I removed the CLEANCACHE dependency because that dependency
> is present for XEN_TMEM and I assumed dependencies are transitive.
That''s correct.

Jan