linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] xen: tmem: selfballooning should be enabled when xen tmem is enabled
@ 2012-11-20 22:42 Dan Magenheimer
  2012-11-21  2:47 ` Konrad Rzeszutek Wilk
  2012-11-21  8:21 ` [Xen-devel] " Jan Beulich
  0 siblings, 2 replies; 6+ messages in thread
From: Dan Magenheimer @ 2012-11-20 22:42 UTC (permalink / raw)
  To: Konrad Wilk; +Cc: linux-kernel, xen-devel

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

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] xen: tmem: selfballooning should be enabled when xen tmem is enabled
  2012-11-20 22:42 [PATCH] xen: tmem: selfballooning should be enabled when xen tmem is enabled Dan Magenheimer
@ 2012-11-21  2:47 ` Konrad Rzeszutek Wilk
  2012-11-21 15:26   ` Dan Magenheimer
  2012-11-21  8:21 ` [Xen-devel] " Jan Beulich
  1 sibling, 1 reply; 6+ messages in thread
From: Konrad Rzeszutek Wilk @ 2012-11-21  2:47 UTC (permalink / raw)
  To: Dan Magenheimer; +Cc: linux-kernel, xen-devel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Xen-devel] [PATCH] xen: tmem: selfballooning should be enabled when xen tmem is enabled
  2012-11-20 22:42 [PATCH] xen: tmem: selfballooning should be enabled when xen tmem is enabled Dan Magenheimer
  2012-11-21  2:47 ` Konrad Rzeszutek Wilk
@ 2012-11-21  8:21 ` Jan Beulich
  2012-11-21 15:42   ` Dan Magenheimer
  1 sibling, 1 reply; 6+ messages in thread
From: Jan Beulich @ 2012-11-21  8:21 UTC (permalink / raw)
  To: Dan Magenheimer; +Cc: xen-devel, Konrad Wilk, linux-kernel

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [PATCH] xen: tmem: selfballooning should be enabled when xen tmem is enabled
  2012-11-21  2:47 ` Konrad Rzeszutek Wilk
@ 2012-11-21 15:26   ` Dan Magenheimer
  0 siblings, 0 replies; 6+ messages in thread
From: Dan Magenheimer @ 2012-11-21 15:26 UTC (permalink / raw)
  To: Konrad Wilk; +Cc: linux-kernel, xen-devel

> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [Xen-devel] [PATCH] xen: tmem: selfballooning should be enabled when xen tmem is enabled
  2012-11-21  8:21 ` [Xen-devel] " Jan Beulich
@ 2012-11-21 15:42   ` Dan Magenheimer
  2012-11-21 15:54     ` Jan Beulich
  0 siblings, 1 reply; 6+ messages in thread
From: Dan Magenheimer @ 2012-11-21 15:42 UTC (permalink / raw)
  To: Jan Beulich; +Cc: xen-devel, Konrad Wilk, linux-kernel

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* RE: [Xen-devel] [PATCH] xen: tmem: selfballooning should be enabled when xen tmem is enabled
  2012-11-21 15:42   ` Dan Magenheimer
@ 2012-11-21 15:54     ` Jan Beulich
  0 siblings, 0 replies; 6+ messages in thread
From: Jan Beulich @ 2012-11-21 15:54 UTC (permalink / raw)
  To: Dan Magenheimer; +Cc: xen-devel, Konrad Wilk, linux-kernel

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-11-21 15:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-11-20 22:42 [PATCH] xen: tmem: selfballooning should be enabled when xen tmem is enabled Dan Magenheimer
2012-11-21  2:47 ` Konrad Rzeszutek Wilk
2012-11-21 15:26   ` Dan Magenheimer
2012-11-21  8:21 ` [Xen-devel] " Jan Beulich
2012-11-21 15:42   ` Dan Magenheimer
2012-11-21 15:54     ` Jan Beulich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).