linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] Make swap accounting default behavior configurable
@ 2010-11-16 10:17 Michal Hocko
  2010-11-16 20:46 ` Andrew Morton
  0 siblings, 1 reply; 16+ messages in thread
From: Michal Hocko @ 2010-11-16 10:17 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-mm, KAMEZAWA Hiroyuki, linux-kernel, balbir, Daisuke Nishimura

Hi Andrew,
could you consider the following patch for the Linus tree, please?
The discussion took place in this email thread 
http://lkml.org/lkml/2010/11/10/114.
The patch is based on top of 151f52f09c572 commit in the Linus tree.

Please let me know if there I should route this patch through somebody
else.

Thanks!

---
>From 30238aaec758988493af793939f14b0ba83dc4b3 Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@suse.cz>
Date: Wed, 10 Nov 2010 13:30:04 +0100
Subject: [PATCH] Make swap accounting default behavior configurable

Swap accounting can be configured by CONFIG_CGROUP_MEM_RES_CTLR_SWAP
configuration option and then it is turned on by default. There is
a boot option (noswapaccount) which can disable this feature.

This makes it hard for distributors to enable the configuration option
as this feature leads to a bigger memory consumption and this is a no-go
for general purpose distribution kernel. On the other hand swap
accounting may be very usuful for some workloads.

This patch adds a new configuration option which controls the default
behavior (CGROUP_MEM_RES_CTLR_SWAP_ENABLED). If the option is selected
then the feature is turned on by default.

It also adds a new boot parameter swapaccount which (contrary to
noswapaccount) enables the feature. (I would consider swapaccount=yes|no
semantic with removed noswapaccount parameter much better but this
parameter is kind of API which might be in use and unexpected breakage
is no-go.)

The default behavior is unchanged (if CONFIG_CGROUP_MEM_RES_CTLR_SWAP is
enabled then CONFIG_CGROUP_MEM_RES_CTLR_SWAP_ENABLED is enabled as well)

Signed-off-by: Michal Hocko <mhocko@suse.cz>
Acked-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
---
 Documentation/kernel-parameters.txt |    3 +++
 init/Kconfig                        |   13 +++++++++++++
 mm/memcontrol.c                     |   15 ++++++++++++++-
 3 files changed, 30 insertions(+), 1 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index ed45e98..14eafa5 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2385,6 +2385,9 @@ and is between 256 and 4096 characters. It is defined in the file
 			improve throughput, but will also increase the
 			amount of memory reserved for use by the client.
 
+	swapaccount	[KNL] Enable accounting of swap in memory resource
+			controller. (See Documentation/cgroups/memory.txt)
+
 	swiotlb=	[IA-64] Number of I/O TLB slabs
 
 	switches=	[HW,M68k]
diff --git a/init/Kconfig b/init/Kconfig
index 88c1046..c972899 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -613,6 +613,19 @@ config CGROUP_MEM_RES_CTLR_SWAP
 	  if boot option "noswapaccount" is set, swap will not be accounted.
 	  Now, memory usage of swap_cgroup is 2 bytes per entry. If swap page
 	  size is 4096bytes, 512k per 1Gbytes of swap.
+config CGROUP_MEM_RES_CTLR_SWAP_ENABLED
+	bool "Memory Resource Controller Swap Extension enabled by default"
+	depends on CGROUP_MEM_RES_CTLR_SWAP
+	default y
+	help
+	  Memory Resource Controller Swap Extension comes with its price in
+	  a bigger memory consumption. General purpose distribution kernels
+	  which want to enable the feautre but keep it disabled by default
+	  and let the user enable it by swapaccount boot command line
+	  parameter should have this option unselected.
+	  For those who want to have the feature enabled by default should
+	  select this option (if, for some reason, they need to disable it
+	  then noswapaccount does the trick).
 
 menuconfig CGROUP_SCHED
 	bool "Group CPU scheduler"
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 9a99cfa..4f479fe 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -61,7 +61,14 @@ struct mem_cgroup *root_mem_cgroup __read_mostly;
 #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
 /* Turned on only when memory cgroup is enabled && really_do_swap_account = 1 */
 int do_swap_account __read_mostly;
-static int really_do_swap_account __initdata = 1; /* for remember boot option*/
+
+/* for remember boot option*/
+#ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP_ENABLED
+static int really_do_swap_account __initdata = 1;
+#else
+static int really_do_swap_account __initdata = 0;
+#endif
+
 #else
 #define do_swap_account		(0)
 #endif
@@ -4909,6 +4916,12 @@ struct cgroup_subsys mem_cgroup_subsys = {
 };
 
 #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
+static int __init enable_swap_account(char *s)
+{
+	really_do_swap_account = 1;
+	return 1;
+}
+__setup("swapaccount", enable_swap_account);
 
 static int __init disable_swap_account(char *s)
 {
-- 
1.7.2.3

-- 
Michal Hocko
L3 team 
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

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

* Re: [PATCH] Make swap accounting default behavior configurable
  2010-11-16 10:17 [PATCH] Make swap accounting default behavior configurable Michal Hocko
@ 2010-11-16 20:46 ` Andrew Morton
  2010-11-16 21:21   ` [stable] " Greg KH
  2010-11-17  0:23   ` Daisuke Nishimura
  0 siblings, 2 replies; 16+ messages in thread
From: Andrew Morton @ 2010-11-16 20:46 UTC (permalink / raw)
  To: Michal Hocko
  Cc: linux-mm, KAMEZAWA Hiroyuki, linux-kernel, balbir,
	Daisuke Nishimura, stable

On Tue, 16 Nov 2010 11:17:26 +0100
Michal Hocko <mhocko@suse.cz> wrote:

> Hi Andrew,
> could you consider the following patch for the Linus tree, please?
> The discussion took place in this email thread 
> http://lkml.org/lkml/2010/11/10/114.
> The patch is based on top of 151f52f09c572 commit in the Linus tree.
> 
> Please let me know if there I should route this patch through somebody
> else.
> 
> Thanks!
> 
> ---
> >From 30238aaec758988493af793939f14b0ba83dc4b3 Mon Sep 17 00:00:00 2001
> From: Michal Hocko <mhocko@suse.cz>
> Date: Wed, 10 Nov 2010 13:30:04 +0100
> Subject: [PATCH] Make swap accounting default behavior configurable
> 
> Swap accounting can be configured by CONFIG_CGROUP_MEM_RES_CTLR_SWAP
> configuration option and then it is turned on by default. There is
> a boot option (noswapaccount) which can disable this feature.
> 
> This makes it hard for distributors to enable the configuration option
> as this feature leads to a bigger memory consumption and this is a no-go
> for general purpose distribution kernel. On the other hand swap
> accounting may be very usuful for some workloads.

This patch is needed by distros, and distros use the -stable tree, I
assume.  Do you see reasons why this patch should be backported into
-stable, so distros don't need to patch it themselves?  If so, any
particular kernel versions?  2.6.37?

> This patch adds a new configuration option which controls the default
> behavior (CGROUP_MEM_RES_CTLR_SWAP_ENABLED). If the option is selected
> then the feature is turned on by default.
> 
> It also adds a new boot parameter swapaccount which (contrary to
> noswapaccount) enables the feature. (I would consider swapaccount=yes|no
> semantic with removed noswapaccount parameter much better but this
> parameter is kind of API which might be in use and unexpected breakage
> is no-go.)
> 
> The default behavior is unchanged (if CONFIG_CGROUP_MEM_RES_CTLR_SWAP is
> enabled then CONFIG_CGROUP_MEM_RES_CTLR_SWAP_ENABLED is enabled as well)
> 
> Signed-off-by: Michal Hocko <mhocko@suse.cz>
> Acked-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> ---
>  Documentation/kernel-parameters.txt |    3 +++
>  init/Kconfig                        |   13 +++++++++++++
>  mm/memcontrol.c                     |   15 ++++++++++++++-
>  3 files changed, 30 insertions(+), 1 deletions(-)
> 
> diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> index ed45e98..14eafa5 100644
> --- a/Documentation/kernel-parameters.txt
> +++ b/Documentation/kernel-parameters.txt
> @@ -2385,6 +2385,9 @@ and is between 256 and 4096 characters. It is defined in the file
>  			improve throughput, but will also increase the
>  			amount of memory reserved for use by the client.
>  
> +	swapaccount	[KNL] Enable accounting of swap in memory resource
> +			controller. (See Documentation/cgroups/memory.txt)

So we have swapaccount and noswapaccount.  Ho hum, "swapaccount=[1|0]"
would have been better.


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

* Re: [stable] [PATCH] Make swap accounting default behavior configurable
  2010-11-16 20:46 ` Andrew Morton
@ 2010-11-16 21:21   ` Greg KH
  2010-11-18  8:21     ` Michal Hocko
  2010-11-17  0:23   ` Daisuke Nishimura
  1 sibling, 1 reply; 16+ messages in thread
From: Greg KH @ 2010-11-16 21:21 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Michal Hocko, Daisuke Nishimura, linux-kernel, linux-mm, stable,
	KAMEZAWA Hiroyuki, balbir

On Tue, Nov 16, 2010 at 12:46:15PM -0800, Andrew Morton wrote:
> On Tue, 16 Nov 2010 11:17:26 +0100
> Michal Hocko <mhocko@suse.cz> wrote:
> 
> > Hi Andrew,
> > could you consider the following patch for the Linus tree, please?
> > The discussion took place in this email thread 
> > http://lkml.org/lkml/2010/11/10/114.
> > The patch is based on top of 151f52f09c572 commit in the Linus tree.
> > 
> > Please let me know if there I should route this patch through somebody
> > else.
> > 
> > Thanks!
> > 
> > ---
> > >From 30238aaec758988493af793939f14b0ba83dc4b3 Mon Sep 17 00:00:00 2001
> > From: Michal Hocko <mhocko@suse.cz>
> > Date: Wed, 10 Nov 2010 13:30:04 +0100
> > Subject: [PATCH] Make swap accounting default behavior configurable
> > 
> > Swap accounting can be configured by CONFIG_CGROUP_MEM_RES_CTLR_SWAP
> > configuration option and then it is turned on by default. There is
> > a boot option (noswapaccount) which can disable this feature.
> > 
> > This makes it hard for distributors to enable the configuration option
> > as this feature leads to a bigger memory consumption and this is a no-go
> > for general purpose distribution kernel. On the other hand swap
> > accounting may be very usuful for some workloads.
> 
> This patch is needed by distros, and distros use the -stable tree, I
> assume.  Do you see reasons why this patch should be backported into
> -stable, so distros don't need to patch it themselves?  If so, any
> particular kernel versions?  2.6.37?

Sorry, I really don't want to start backporting features to stable
kernels if at all possible.  Distros can pick them up on their own if
they determine it is needed.

thanks,

greg k-h

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

* Re: [PATCH] Make swap accounting default behavior configurable
  2010-11-16 20:46 ` Andrew Morton
  2010-11-16 21:21   ` [stable] " Greg KH
@ 2010-11-17  0:23   ` Daisuke Nishimura
  2010-11-17  0:57     ` KAMEZAWA Hiroyuki
  2010-11-17  1:12     ` Andrew Morton
  1 sibling, 2 replies; 16+ messages in thread
From: Daisuke Nishimura @ 2010-11-17  0:23 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Michal Hocko, linux-mm, KAMEZAWA Hiroyuki, linux-kernel, balbir,
	stable, Daisuke Nishimura

On Tue, 16 Nov 2010 12:46:15 -0800
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Tue, 16 Nov 2010 11:17:26 +0100
> Michal Hocko <mhocko@suse.cz> wrote:
> 
> > Hi Andrew,
> > could you consider the following patch for the Linus tree, please?
> > The discussion took place in this email thread 
> > http://lkml.org/lkml/2010/11/10/114.
> > The patch is based on top of 151f52f09c572 commit in the Linus tree.
> > 
> > Please let me know if there I should route this patch through somebody
> > else.
> > 
> > Thanks!
> > 
> > ---
> > >From 30238aaec758988493af793939f14b0ba83dc4b3 Mon Sep 17 00:00:00 2001
> > From: Michal Hocko <mhocko@suse.cz>
> > Date: Wed, 10 Nov 2010 13:30:04 +0100
> > Subject: [PATCH] Make swap accounting default behavior configurable
> > 
> > Swap accounting can be configured by CONFIG_CGROUP_MEM_RES_CTLR_SWAP
> > configuration option and then it is turned on by default. There is
> > a boot option (noswapaccount) which can disable this feature.
> > 
> > This makes it hard for distributors to enable the configuration option
> > as this feature leads to a bigger memory consumption and this is a no-go
> > for general purpose distribution kernel. On the other hand swap
> > accounting may be very usuful for some workloads.
> 
> This patch is needed by distros, and distros use the -stable tree, I
> assume.  Do you see reasons why this patch should be backported into
> -stable, so distros don't need to patch it themselves?  If so, any
> particular kernel versions?  2.6.37?
> 
> > This patch adds a new configuration option which controls the default
> > behavior (CGROUP_MEM_RES_CTLR_SWAP_ENABLED). If the option is selected
> > then the feature is turned on by default.
> > 
> > It also adds a new boot parameter swapaccount which (contrary to
> > noswapaccount) enables the feature. (I would consider swapaccount=yes|no
> > semantic with removed noswapaccount parameter much better but this
> > parameter is kind of API which might be in use and unexpected breakage
> > is no-go.)
> > 
> > The default behavior is unchanged (if CONFIG_CGROUP_MEM_RES_CTLR_SWAP is
> > enabled then CONFIG_CGROUP_MEM_RES_CTLR_SWAP_ENABLED is enabled as well)
> > 
> > Signed-off-by: Michal Hocko <mhocko@suse.cz>
> > Acked-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> > ---
> >  Documentation/kernel-parameters.txt |    3 +++
> >  init/Kconfig                        |   13 +++++++++++++
> >  mm/memcontrol.c                     |   15 ++++++++++++++-
> >  3 files changed, 30 insertions(+), 1 deletions(-)
> > 
> > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> > index ed45e98..14eafa5 100644
> > --- a/Documentation/kernel-parameters.txt
> > +++ b/Documentation/kernel-parameters.txt
> > @@ -2385,6 +2385,9 @@ and is between 256 and 4096 characters. It is defined in the file
> >  			improve throughput, but will also increase the
> >  			amount of memory reserved for use by the client.
> >  
> > +	swapaccount	[KNL] Enable accounting of swap in memory resource
> > +			controller. (See Documentation/cgroups/memory.txt)
> 
> So we have swapaccount and noswapaccount.  Ho hum, "swapaccount=[1|0]"
> would have been better.
> 
I suggested to keep "noswapaccount" for compatibility.
If you and other guys don't like having two parameters, I don't stick to
the old parameter.

Thanks,
Daisuke Nishimura.

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

* Re: [PATCH] Make swap accounting default behavior configurable
  2010-11-17  0:23   ` Daisuke Nishimura
@ 2010-11-17  0:57     ` KAMEZAWA Hiroyuki
  2010-11-17  1:12     ` Andrew Morton
  1 sibling, 0 replies; 16+ messages in thread
From: KAMEZAWA Hiroyuki @ 2010-11-17  0:57 UTC (permalink / raw)
  To: Daisuke Nishimura
  Cc: Andrew Morton, Michal Hocko, linux-mm, linux-kernel, balbir, stable

On Wed, 17 Nov 2010 09:23:39 +0900
Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:

> On Tue, 16 Nov 2010 12:46:15 -0800
> Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Tue, 16 Nov 2010 11:17:26 +0100
> > Michal Hocko <mhocko@suse.cz> wrote:
> > 
> > > Hi Andrew,
> > > could you consider the following patch for the Linus tree, please?
> > > The discussion took place in this email thread 
> > > http://lkml.org/lkml/2010/11/10/114.
> > > The patch is based on top of 151f52f09c572 commit in the Linus tree.
> > > 
> > > Please let me know if there I should route this patch through somebody
> > > else.
> > > 
> > > Thanks!
> > > 
> > > ---
> > > >From 30238aaec758988493af793939f14b0ba83dc4b3 Mon Sep 17 00:00:00 2001
> > > From: Michal Hocko <mhocko@suse.cz>
> > > Date: Wed, 10 Nov 2010 13:30:04 +0100
> > > Subject: [PATCH] Make swap accounting default behavior configurable
> > > 
> > > Swap accounting can be configured by CONFIG_CGROUP_MEM_RES_CTLR_SWAP
> > > configuration option and then it is turned on by default. There is
> > > a boot option (noswapaccount) which can disable this feature.
> > > 
> > > This makes it hard for distributors to enable the configuration option
> > > as this feature leads to a bigger memory consumption and this is a no-go
> > > for general purpose distribution kernel. On the other hand swap
> > > accounting may be very usuful for some workloads.
> > 
> > This patch is needed by distros, and distros use the -stable tree, I
> > assume.  Do you see reasons why this patch should be backported into
> > -stable, so distros don't need to patch it themselves?  If so, any
> > particular kernel versions?  2.6.37?
> > 
> > > This patch adds a new configuration option which controls the default
> > > behavior (CGROUP_MEM_RES_CTLR_SWAP_ENABLED). If the option is selected
> > > then the feature is turned on by default.
> > > 
> > > It also adds a new boot parameter swapaccount which (contrary to
> > > noswapaccount) enables the feature. (I would consider swapaccount=yes|no
> > > semantic with removed noswapaccount parameter much better but this
> > > parameter is kind of API which might be in use and unexpected breakage
> > > is no-go.)
> > > 
> > > The default behavior is unchanged (if CONFIG_CGROUP_MEM_RES_CTLR_SWAP is
> > > enabled then CONFIG_CGROUP_MEM_RES_CTLR_SWAP_ENABLED is enabled as well)
> > > 
> > > Signed-off-by: Michal Hocko <mhocko@suse.cz>
> > > Acked-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp>
> > > ---
> > >  Documentation/kernel-parameters.txt |    3 +++
> > >  init/Kconfig                        |   13 +++++++++++++
> > >  mm/memcontrol.c                     |   15 ++++++++++++++-
> > >  3 files changed, 30 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> > > index ed45e98..14eafa5 100644
> > > --- a/Documentation/kernel-parameters.txt
> > > +++ b/Documentation/kernel-parameters.txt
> > > @@ -2385,6 +2385,9 @@ and is between 256 and 4096 characters. It is defined in the file
> > >  			improve throughput, but will also increase the
> > >  			amount of memory reserved for use by the client.
> > >  
> > > +	swapaccount	[KNL] Enable accounting of swap in memory resource
> > > +			controller. (See Documentation/cgroups/memory.txt)
> > 
> > So we have swapaccount and noswapaccount.  Ho hum, "swapaccount=[1|0]"
> > would have been better.
> > 
> I suggested to keep "noswapaccount" for compatibility.
> If you and other guys don't like having two parameters, I don't stick to
> the old parameter.
> 

I don't think "noswapaccount" is important if "enable is default" when 
proper configuration is used ....because it's rarelly used.
 
BTW, memory usage of swap_cgroup is really important ? It consumes 
1 Mbytes per 2G of swap.

Off topic.
I wonder I'll be happy if we can have default config template for all
others as recent "Add Kconfig option for default swappiness" discuss.

Thanks,
-Kame


Thanks,
-Kame




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

* Re: [PATCH] Make swap accounting default behavior configurable
  2010-11-17  0:23   ` Daisuke Nishimura
  2010-11-17  0:57     ` KAMEZAWA Hiroyuki
@ 2010-11-17  1:12     ` Andrew Morton
  2010-11-17  3:28       ` Daisuke Nishimura
  1 sibling, 1 reply; 16+ messages in thread
From: Andrew Morton @ 2010-11-17  1:12 UTC (permalink / raw)
  To: Daisuke Nishimura
  Cc: Michal Hocko, linux-mm, KAMEZAWA Hiroyuki, linux-kernel, balbir, stable

On Wed, 17 Nov 2010 09:23:39 +0900 Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:

> > > 
> > > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> > > index ed45e98..14eafa5 100644
> > > --- a/Documentation/kernel-parameters.txt
> > > +++ b/Documentation/kernel-parameters.txt
> > > @@ -2385,6 +2385,9 @@ and is between 256 and 4096 characters. It is defined in the file
> > >  			improve throughput, but will also increase the
> > >  			amount of memory reserved for use by the client.
> > >  
> > > +	swapaccount	[KNL] Enable accounting of swap in memory resource
> > > +			controller. (See Documentation/cgroups/memory.txt)
> > 
> > So we have swapaccount and noswapaccount.  Ho hum, "swapaccount=[1|0]"
> > would have been better.
> > 
> I suggested to keep "noswapaccount" for compatibility.
> If you and other guys don't like having two parameters, I don't stick to
> the old parameter.
> 

Yes, we're stuck with the old one now.

But we should note that "foo=[0|1]" is superior to "foo" and "nofoo". 
Even if we didn't initially intend to add "nofoo".


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

* Re: [PATCH] Make swap accounting default behavior configurable
  2010-11-17  1:12     ` Andrew Morton
@ 2010-11-17  3:28       ` Daisuke Nishimura
  2010-11-18  8:23         ` Michal Hocko
  0 siblings, 1 reply; 16+ messages in thread
From: Daisuke Nishimura @ 2010-11-17  3:28 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Andrew Morton, linux-mm, KAMEZAWA Hiroyuki, linux-kernel, balbir,
	stable, Daisuke Nishimura

On Tue, 16 Nov 2010 17:12:25 -0800
Andrew Morton <akpm@linux-foundation.org> wrote:

> On Wed, 17 Nov 2010 09:23:39 +0900 Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:
> 
> > > > 
> > > > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> > > > index ed45e98..14eafa5 100644
> > > > --- a/Documentation/kernel-parameters.txt
> > > > +++ b/Documentation/kernel-parameters.txt
> > > > @@ -2385,6 +2385,9 @@ and is between 256 and 4096 characters. It is defined in the file
> > > >  			improve throughput, but will also increase the
> > > >  			amount of memory reserved for use by the client.
> > > >  
> > > > +	swapaccount	[KNL] Enable accounting of swap in memory resource
> > > > +			controller. (See Documentation/cgroups/memory.txt)
> > > 
> > > So we have swapaccount and noswapaccount.  Ho hum, "swapaccount=[1|0]"
> > > would have been better.
> > > 
> > I suggested to keep "noswapaccount" for compatibility.
> > If you and other guys don't like having two parameters, I don't stick to
> > the old parameter.
> > 
> 
> Yes, we're stuck with the old one now.
> 
> But we should note that "foo=[0|1]" is superior to "foo" and "nofoo". 
> Even if we didn't initially intend to add "nofoo".
> 
I see.

Michal-san, could you update your patch to use "swapaccount=[1|0]" ?

Thanks,
Daisuke Nishimura.

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

* Re: [stable] [PATCH] Make swap accounting default behavior configurable
  2010-11-16 21:21   ` [stable] " Greg KH
@ 2010-11-18  8:21     ` Michal Hocko
  2010-11-18 16:36       ` Greg KH
  0 siblings, 1 reply; 16+ messages in thread
From: Michal Hocko @ 2010-11-18  8:21 UTC (permalink / raw)
  To: Greg KH
  Cc: Andrew Morton, Daisuke Nishimura, linux-kernel, linux-mm, stable,
	KAMEZAWA Hiroyuki, balbir

On Tue 16-11-10 13:21:57, Greg KH wrote:
> On Tue, Nov 16, 2010 at 12:46:15PM -0800, Andrew Morton wrote:
> > On Tue, 16 Nov 2010 11:17:26 +0100
> > Michal Hocko <mhocko@suse.cz> wrote:
> > 
> > > Hi Andrew,
> > > could you consider the following patch for the Linus tree, please?
> > > The discussion took place in this email thread 
> > > http://lkml.org/lkml/2010/11/10/114.
> > > The patch is based on top of 151f52f09c572 commit in the Linus tree.
> > > 
> > > Please let me know if there I should route this patch through somebody
> > > else.
> > > 
> > > Thanks!
> > > 
> > > ---
> > > >From 30238aaec758988493af793939f14b0ba83dc4b3 Mon Sep 17 00:00:00 2001
> > > From: Michal Hocko <mhocko@suse.cz>
> > > Date: Wed, 10 Nov 2010 13:30:04 +0100
> > > Subject: [PATCH] Make swap accounting default behavior configurable
> > > 
> > > Swap accounting can be configured by CONFIG_CGROUP_MEM_RES_CTLR_SWAP
> > > configuration option and then it is turned on by default. There is
> > > a boot option (noswapaccount) which can disable this feature.
> > > 
> > > This makes it hard for distributors to enable the configuration option
> > > as this feature leads to a bigger memory consumption and this is a no-go
> > > for general purpose distribution kernel. On the other hand swap
> > > accounting may be very usuful for some workloads.
> > 
> > This patch is needed by distros, and distros use the -stable tree, I
> > assume.  Do you see reasons why this patch should be backported into
> > -stable, so distros don't need to patch it themselves?  If so, any
> > particular kernel versions?  2.6.37?

I have suggested pushing to the stable in the original thread as well. 
I was told that this is not a bug fix.

I do not care much in which particular version to push this but I
guess this doesn't qualify as "regression fix only Linus policy" so it
is probably too late for .37.
Nevertheless, if we reconsider -stable then .37 would be much really
helpful to get it into stable ASAP.

> 
> Sorry, I really don't want to start backporting features to stable
> kernels if at all possible.  Distros can pick them up on their own if
> they determine it is needed.

I really do agree with the part about features. But isn't this patch
basically for distros (to help them to provide the swapaccounting feature
without the cost of higher memory consumption in default configuration)?
If this doesn't go to the stable then all (interested) of them would
need to maintain the patch. Otherwise the change would come directly
from the upstream.

Moreover, it is not a new feature it just consolidates the default
behavior of the already existing functionality.

> 
> thanks,
> 
> greg k-h

Thanks
-- 
Michal Hocko
L3 team 
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

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

* Re: [PATCH] Make swap accounting default behavior configurable
  2010-11-17  3:28       ` Daisuke Nishimura
@ 2010-11-18  8:23         ` Michal Hocko
  2010-11-18  8:46           ` Daisuke Nishimura
  0 siblings, 1 reply; 16+ messages in thread
From: Michal Hocko @ 2010-11-18  8:23 UTC (permalink / raw)
  To: Daisuke Nishimura
  Cc: Andrew Morton, linux-mm, KAMEZAWA Hiroyuki, linux-kernel, balbir, stable

On Wed 17-11-10 12:28:01, Daisuke Nishimura wrote:
> On Tue, 16 Nov 2010 17:12:25 -0800
> Andrew Morton <akpm@linux-foundation.org> wrote:
> 
> > On Wed, 17 Nov 2010 09:23:39 +0900 Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:
> > 
> > > > > 
> > > > > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> > > > > index ed45e98..14eafa5 100644
> > > > > --- a/Documentation/kernel-parameters.txt
> > > > > +++ b/Documentation/kernel-parameters.txt
> > > > > @@ -2385,6 +2385,9 @@ and is between 256 and 4096 characters. It is defined in the file
> > > > >  			improve throughput, but will also increase the
> > > > >  			amount of memory reserved for use by the client.
> > > > >  
> > > > > +	swapaccount	[KNL] Enable accounting of swap in memory resource
> > > > > +			controller. (See Documentation/cgroups/memory.txt)
> > > > 
> > > > So we have swapaccount and noswapaccount.  Ho hum, "swapaccount=[1|0]"
> > > > would have been better.
> > > > 
> > > I suggested to keep "noswapaccount" for compatibility.
> > > If you and other guys don't like having two parameters, I don't stick to
> > > the old parameter.
> > > 
> > 
> > Yes, we're stuck with the old one now.
> > 
> > But we should note that "foo=[0|1]" is superior to "foo" and "nofoo". 
> > Even if we didn't initially intend to add "nofoo".
> > 
> I see.
> 
> Michal-san, could you update your patch to use "swapaccount=[1|0]" ?

I have noticed that Andrew has already taken the last version of the
patch for -mm tree. Should I still rework it to change swapaccount to
swapaccount=0|1 resp. true|false?

> 
> Thanks,
> Daisuke Nishimura.
-- 
Michal Hocko
L3 team 
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

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

* Re: [PATCH] Make swap accounting default behavior configurable
  2010-11-18  8:23         ` Michal Hocko
@ 2010-11-18  8:46           ` Daisuke Nishimura
  2010-11-18  8:53             ` KAMEZAWA Hiroyuki
  0 siblings, 1 reply; 16+ messages in thread
From: Daisuke Nishimura @ 2010-11-18  8:46 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Andrew Morton, linux-mm, KAMEZAWA Hiroyuki, linux-kernel, balbir,
	stable, Daisuke Nishimura

On Thu, 18 Nov 2010 09:23:32 +0100
Michal Hocko <mhocko@suse.cz> wrote:

> On Wed 17-11-10 12:28:01, Daisuke Nishimura wrote:
> > On Tue, 16 Nov 2010 17:12:25 -0800
> > Andrew Morton <akpm@linux-foundation.org> wrote:
> > 
> > > On Wed, 17 Nov 2010 09:23:39 +0900 Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:
> > > 
> > > > > > 
> > > > > > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> > > > > > index ed45e98..14eafa5 100644
> > > > > > --- a/Documentation/kernel-parameters.txt
> > > > > > +++ b/Documentation/kernel-parameters.txt
> > > > > > @@ -2385,6 +2385,9 @@ and is between 256 and 4096 characters. It is defined in the file
> > > > > >  			improve throughput, but will also increase the
> > > > > >  			amount of memory reserved for use by the client.
> > > > > >  
> > > > > > +	swapaccount	[KNL] Enable accounting of swap in memory resource
> > > > > > +			controller. (See Documentation/cgroups/memory.txt)
> > > > > 
> > > > > So we have swapaccount and noswapaccount.  Ho hum, "swapaccount=[1|0]"
> > > > > would have been better.
> > > > > 
> > > > I suggested to keep "noswapaccount" for compatibility.
> > > > If you and other guys don't like having two parameters, I don't stick to
> > > > the old parameter.
> > > > 
> > > 
> > > Yes, we're stuck with the old one now.
> > > 
> > > But we should note that "foo=[0|1]" is superior to "foo" and "nofoo". 
> > > Even if we didn't initially intend to add "nofoo".
> > > 
> > I see.
> > 
> > Michal-san, could you update your patch to use "swapaccount=[1|0]" ?
> 
> I have noticed that Andrew has already taken the last version of the
> patch for -mm tree. Should I still rework it to change swapaccount to
> swapaccount=0|1 resp. true|false?
> 
It's usual to update a patch into more sophisticated one while it is in -mm tree.
So, I think you'd better to do it(btw, I prefer 0|1 to true|false.
Reading kernel-parameters.txt, 0|1 is more commonly used.).

Thanks,
Daisuke Nishimura.

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

* Re: [PATCH] Make swap accounting default behavior configurable
  2010-11-18  8:46           ` Daisuke Nishimura
@ 2010-11-18  8:53             ` KAMEZAWA Hiroyuki
  2010-11-18  9:56               ` [PATCH] Make swap accounting default behavior configurable v4 Michal Hocko
  0 siblings, 1 reply; 16+ messages in thread
From: KAMEZAWA Hiroyuki @ 2010-11-18  8:53 UTC (permalink / raw)
  To: Daisuke Nishimura
  Cc: Michal Hocko, Andrew Morton, linux-mm, linux-kernel, balbir, stable

On Thu, 18 Nov 2010 17:46:54 +0900
Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:

> On Thu, 18 Nov 2010 09:23:32 +0100
> Michal Hocko <mhocko@suse.cz> wrote:
> 
> > On Wed 17-11-10 12:28:01, Daisuke Nishimura wrote:
> > > On Tue, 16 Nov 2010 17:12:25 -0800
> > > Andrew Morton <akpm@linux-foundation.org> wrote:
> > > 
> > > > On Wed, 17 Nov 2010 09:23:39 +0900 Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:
> > > > 
> > > > > > > 
> > > > > > > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
> > > > > > > index ed45e98..14eafa5 100644
> > > > > > > --- a/Documentation/kernel-parameters.txt
> > > > > > > +++ b/Documentation/kernel-parameters.txt
> > > > > > > @@ -2385,6 +2385,9 @@ and is between 256 and 4096 characters. It is defined in the file
> > > > > > >  			improve throughput, but will also increase the
> > > > > > >  			amount of memory reserved for use by the client.
> > > > > > >  
> > > > > > > +	swapaccount	[KNL] Enable accounting of swap in memory resource
> > > > > > > +			controller. (See Documentation/cgroups/memory.txt)
> > > > > > 
> > > > > > So we have swapaccount and noswapaccount.  Ho hum, "swapaccount=[1|0]"
> > > > > > would have been better.
> > > > > > 
> > > > > I suggested to keep "noswapaccount" for compatibility.
> > > > > If you and other guys don't like having two parameters, I don't stick to
> > > > > the old parameter.
> > > > > 
> > > > 
> > > > Yes, we're stuck with the old one now.
> > > > 
> > > > But we should note that "foo=[0|1]" is superior to "foo" and "nofoo". 
> > > > Even if we didn't initially intend to add "nofoo".
> > > > 
> > > I see.
> > > 
> > > Michal-san, could you update your patch to use "swapaccount=[1|0]" ?
> > 
> > I have noticed that Andrew has already taken the last version of the
> > patch for -mm tree. Should I still rework it to change swapaccount to
> > swapaccount=0|1 resp. true|false?
> > 
> It's usual to update a patch into more sophisticated one while it is in -mm tree.
> So, I think you'd better to do it(btw, I prefer 0|1 to true|false.
> Reading kernel-parameters.txt, 0|1 is more commonly used.).
> 

I vote for 0|1

Thanks,
-Kame


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

* Re: [PATCH] Make swap accounting default behavior configurable v4
  2010-11-18  8:53             ` KAMEZAWA Hiroyuki
@ 2010-11-18  9:56               ` Michal Hocko
  2010-11-18 10:14                 ` Daisuke Nishimura
  0 siblings, 1 reply; 16+ messages in thread
From: Michal Hocko @ 2010-11-18  9:56 UTC (permalink / raw)
  To: Daisuke Nishimura
  Cc: Andrew Morton, linux-mm, linux-kernel, balbir, stable, KAMEZAWA Hiroyuki

On Thu 18-11-10 17:53:34, KAMEZAWA Hiroyuki wrote:
> On Thu, 18 Nov 2010 17:46:54 +0900
> Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:
> 
> > On Thu, 18 Nov 2010 09:23:32 +0100
> > Michal Hocko <mhocko@suse.cz> wrote:
> > 
> > > On Wed 17-11-10 12:28:01, Daisuke Nishimura wrote:
> > > > On Tue, 16 Nov 2010 17:12:25 -0800
> > > > Andrew Morton <akpm@linux-foundation.org> wrote:
[...]
> > > > > Yes, we're stuck with the old one now.
> > > > > 
> > > > > But we should note that "foo=[0|1]" is superior to "foo" and "nofoo". 
> > > > > Even if we didn't initially intend to add "nofoo".
> > > > > 
> > > > I see.
> > > > 
> > > > Michal-san, could you update your patch to use "swapaccount=[1|0]" ?
> > > 
> > > I have noticed that Andrew has already taken the last version of the
> > > patch for -mm tree. Should I still rework it to change swapaccount to
> > > swapaccount=0|1 resp. true|false?
> > > 
> > It's usual to update a patch into more sophisticated one while it is in -mm tree.
> > So, I think you'd better to do it(btw, I prefer 0|1 to true|false.
> > Reading kernel-parameters.txt, 0|1 is more commonly used.).
> > 
> 
> I vote for 0|1

Changes since v3:
* add 0|1 parameter values handling

Changes since v2:
* put the new parameter description to the proper (alphabetically
* sorted)
  place in Documentation/kernel-parameters.txt
  
Changes since v1:
* do not remove noswapaccount parameter and add swapaccount parameter
* instead
* Documentation/kernel-parameters.txt updated)

--- 
>From 72c003f6586c092c1a69f5482cd102e97d7ef40f Mon Sep 17 00:00:00 2001
From: Michal Hocko <mhocko@suse.cz>
Date: Wed, 10 Nov 2010 13:30:04 +0100
Subject: [PATCH] Make swap accounting default behavior configurable

Swap accounting can be configured by CONFIG_CGROUP_MEM_RES_CTLR_SWAP
configuration option and then it is turned on by default. There is
a boot option (noswapaccount) which can disable this feature.

This makes it hard for distributors to enable the configuration option
as this feature leads to a bigger memory consumption and this is a no-go
for general purpose distribution kernel. On the other hand swap
accounting may be very usuful for some workloads.

This patch adds a new configuration option which controls the default
behavior (CGROUP_MEM_RES_CTLR_SWAP_ENABLED). If the option is selected
then the feature is turned on by default.

It also adds a new boot parameter swapaccount[=1|0] which enhances the
original noswapaccount parameter semantic by means of enable/disable
logic (defaults to 1 if no value is provided to be still consistent with
noswapaccount).

The default behavior is unchanged (if CONFIG_CGROUP_MEM_RES_CTLR_SWAP is
enabled then CONFIG_CGROUP_MEM_RES_CTLR_SWAP_ENABLED is enabled as well)

Signed-off-by: Michal Hocko <mhocko@suse.cz>
---
 Documentation/kernel-parameters.txt |    5 +++++
 init/Kconfig                        |   13 +++++++++++++
 mm/memcontrol.c                     |   21 +++++++++++++++++++--
 3 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt
index ed45e98..98c4902 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -2385,6 +2385,11 @@ and is between 256 and 4096 characters. It is defined in the file
 			improve throughput, but will also increase the
 			amount of memory reserved for use by the client.
 
+	swapaccount[=0|1]
+			[KNL] Enable accounting of swap in memory resource
+			controller if no parameter or 1 is given or disable
+			it if 0 is given (See Documentation/cgroups/memory.txt)
+
 	swiotlb=	[IA-64] Number of I/O TLB slabs
 
 	switches=	[HW,M68k]
diff --git a/init/Kconfig b/init/Kconfig
index 88c1046..c972899 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -613,6 +613,19 @@ config CGROUP_MEM_RES_CTLR_SWAP
 	  if boot option "noswapaccount" is set, swap will not be accounted.
 	  Now, memory usage of swap_cgroup is 2 bytes per entry. If swap page
 	  size is 4096bytes, 512k per 1Gbytes of swap.
+config CGROUP_MEM_RES_CTLR_SWAP_ENABLED
+	bool "Memory Resource Controller Swap Extension enabled by default"
+	depends on CGROUP_MEM_RES_CTLR_SWAP
+	default y
+	help
+	  Memory Resource Controller Swap Extension comes with its price in
+	  a bigger memory consumption. General purpose distribution kernels
+	  which want to enable the feautre but keep it disabled by default
+	  and let the user enable it by swapaccount boot command line
+	  parameter should have this option unselected.
+	  For those who want to have the feature enabled by default should
+	  select this option (if, for some reason, they need to disable it
+	  then noswapaccount does the trick).
 
 menuconfig CGROUP_SCHED
 	bool "Group CPU scheduler"
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 9a99cfa..bed98b6 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -61,7 +61,14 @@ struct mem_cgroup *root_mem_cgroup __read_mostly;
 #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
 /* Turned on only when memory cgroup is enabled && really_do_swap_account = 1 */
 int do_swap_account __read_mostly;
-static int really_do_swap_account __initdata = 1; /* for remember boot option*/
+
+/* for remember boot option*/
+#ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP_ENABLED
+static int really_do_swap_account __initdata = 1;
+#else
+static int really_do_swap_account __initdata = 0;
+#endif
+
 #else
 #define do_swap_account		(0)
 #endif
@@ -4909,10 +4916,20 @@ struct cgroup_subsys mem_cgroup_subsys = {
 };
 
 #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP
+static int __init enable_swap_account(char *s)
+{
+	/* consider enabled if no parameter or 1 is given */
+	if (!s || !strcmp(s, "1"))
+		really_do_swap_account = 1;
+	else if (!strcmp(s, "0"))
+		really_do_swap_account = 0;
+	return 1;
+}
+__setup("swapaccount", enable_swap_account);
 
 static int __init disable_swap_account(char *s)
 {
-	really_do_swap_account = 0;
+	enable_swap_account("0");
 	return 1;
 }
 __setup("noswapaccount", disable_swap_account);
-- 
1.7.2.3


-- 
Michal Hocko
L3 team 
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

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

* Re: [PATCH] Make swap accounting default behavior configurable v4
  2010-11-18  9:56               ` [PATCH] Make swap accounting default behavior configurable v4 Michal Hocko
@ 2010-11-18 10:14                 ` Daisuke Nishimura
  2010-11-18 10:23                   ` Michal Hocko
  0 siblings, 1 reply; 16+ messages in thread
From: Daisuke Nishimura @ 2010-11-18 10:14 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Andrew Morton, linux-mm, linux-kernel, balbir, stable,
	KAMEZAWA Hiroyuki, Daisuke Nishimura

On Thu, 18 Nov 2010 10:56:07 +0100
Michal Hocko <mhocko@suse.cz> wrote:

> On Thu 18-11-10 17:53:34, KAMEZAWA Hiroyuki wrote:
> > On Thu, 18 Nov 2010 17:46:54 +0900
> > Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:
> > 
> > > On Thu, 18 Nov 2010 09:23:32 +0100
> > > Michal Hocko <mhocko@suse.cz> wrote:
> > > 
> > > > On Wed 17-11-10 12:28:01, Daisuke Nishimura wrote:
> > > > > On Tue, 16 Nov 2010 17:12:25 -0800
> > > > > Andrew Morton <akpm@linux-foundation.org> wrote:
> [...]
> > > > > > Yes, we're stuck with the old one now.
> > > > > > 
> > > > > > But we should note that "foo=[0|1]" is superior to "foo" and "nofoo". 
> > > > > > Even if we didn't initially intend to add "nofoo".
> > > > > > 
> > > > > I see.
> > > > > 
> > > > > Michal-san, could you update your patch to use "swapaccount=[1|0]" ?
> > > > 
> > > > I have noticed that Andrew has already taken the last version of the
> > > > patch for -mm tree. Should I still rework it to change swapaccount to
> > > > swapaccount=0|1 resp. true|false?
> > > > 
> > > It's usual to update a patch into more sophisticated one while it is in -mm tree.
> > > So, I think you'd better to do it(btw, I prefer 0|1 to true|false.
> > > Reading kernel-parameters.txt, 0|1 is more commonly used.).
> > > 
> > 
> > I vote for 0|1
> 
> Changes since v3:
> * add 0|1 parameter values handling
> 
> Changes since v2:
> * put the new parameter description to the proper (alphabetically
> * sorted)
>   place in Documentation/kernel-parameters.txt
>   
> Changes since v1:
> * do not remove noswapaccount parameter and add swapaccount parameter
> * instead
> * Documentation/kernel-parameters.txt updated)
> 

I'm sorry again and again, but I think removing "noswapaccount" completely
would be better, as Andrew said first:
> So we have swapaccount and noswapaccount.  Ho hum, "swapaccount=[1|0]"
> would have been better.


Thanks,
Daisuke Nishimura.

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

* Re: [PATCH] Make swap accounting default behavior configurable v4
  2010-11-18 10:14                 ` Daisuke Nishimura
@ 2010-11-18 10:23                   ` Michal Hocko
  2010-11-18 17:52                     ` Andrew Morton
  0 siblings, 1 reply; 16+ messages in thread
From: Michal Hocko @ 2010-11-18 10:23 UTC (permalink / raw)
  To: Daisuke Nishimura
  Cc: Andrew Morton, linux-mm, linux-kernel, balbir, KAMEZAWA Hiroyuki

[I am removing stable from CC - to shield them off the discussion]

On Thu 18-11-10 19:14:27, Daisuke Nishimura wrote:
> On Thu, 18 Nov 2010 10:56:07 +0100
> Michal Hocko <mhocko@suse.cz> wrote:
> 
> > On Thu 18-11-10 17:53:34, KAMEZAWA Hiroyuki wrote:
> > > On Thu, 18 Nov 2010 17:46:54 +0900
> > > Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> wrote:
> > > 
> > > > On Thu, 18 Nov 2010 09:23:32 +0100
> > > > Michal Hocko <mhocko@suse.cz> wrote:
> > > > 
> > > > > On Wed 17-11-10 12:28:01, Daisuke Nishimura wrote:
> > > > > > On Tue, 16 Nov 2010 17:12:25 -0800
> > > > > > Andrew Morton <akpm@linux-foundation.org> wrote:
> > [...]
> > > > > > > Yes, we're stuck with the old one now.
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

> > > > > > > 
> > > > > > > But we should note that "foo=[0|1]" is superior to "foo" and "nofoo". 
> > > > > > > Even if we didn't initially intend to add "nofoo".
> > > > > > > 
> > > > > > I see.
> > > > > > 
> > > > > > Michal-san, could you update your patch to use "swapaccount=[1|0]" ?
> > > > > 
> > > > > I have noticed that Andrew has already taken the last version of the
> > > > > patch for -mm tree. Should I still rework it to change swapaccount to
> > > > > swapaccount=0|1 resp. true|false?
> > > > > 
> > > > It's usual to update a patch into more sophisticated one while it is in -mm tree.
> > > > So, I think you'd better to do it(btw, I prefer 0|1 to true|false.
> > > > Reading kernel-parameters.txt, 0|1 is more commonly used.).
> > > > 
> > > 
> > > I vote for 0|1
> > 
> > Changes since v3:
> > * add 0|1 parameter values handling
> > 
> > Changes since v2:
> > * put the new parameter description to the proper (alphabetically
> > * sorted)
> >   place in Documentation/kernel-parameters.txt
> >   
> > Changes since v1:
> > * do not remove noswapaccount parameter and add swapaccount parameter
> > * instead
> > * Documentation/kernel-parameters.txt updated)
> > 
> 
> I'm sorry again and again, but I think removing "noswapaccount" completely
> would be better, as Andrew said first:

I read the above Andrew's statement that we really should stick with the
old parameter.

> > So we have swapaccount and noswapaccount.  Ho hum, "swapaccount=[1|0]"
> > would have been better.
> 
> 
> Thanks,
> Daisuke Nishimura.

-- 
Michal Hocko
L3 team 
SUSE LINUX s.r.o.
Lihovarska 1060/12
190 00 Praha 9    
Czech Republic

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

* Re: [stable] [PATCH] Make swap accounting default behavior configurable
  2010-11-18  8:21     ` Michal Hocko
@ 2010-11-18 16:36       ` Greg KH
  0 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2010-11-18 16:36 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Andrew Morton, Daisuke Nishimura, linux-kernel, linux-mm, stable,
	KAMEZAWA Hiroyuki, balbir

On Thu, Nov 18, 2010 at 09:21:32AM +0100, Michal Hocko wrote:
> > Sorry, I really don't want to start backporting features to stable
> > kernels if at all possible.  Distros can pick them up on their own if
> > they determine it is needed.
> 
> I really do agree with the part about features. But isn't this patch
> basically for distros (to help them to provide the swapaccounting feature
> without the cost of higher memory consumption in default configuration)?

Then if the distros want it, they can pick it up themselves.

> If this doesn't go to the stable then all (interested) of them would
> need to maintain the patch. Otherwise the change would come directly
> from the upstream.

They can cherry-pick from upstream like they do for everything else, no
real change here.

> Moreover, it is not a new feature it just consolidates the default
> behavior of the already existing functionality.

Again, it doesn't match up with the stable kernel rules, sorry, no, this
is not a bugfix or regression.

thanks,

greg k-h

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

* Re: [PATCH] Make swap accounting default behavior configurable v4
  2010-11-18 10:23                   ` Michal Hocko
@ 2010-11-18 17:52                     ` Andrew Morton
  0 siblings, 0 replies; 16+ messages in thread
From: Andrew Morton @ 2010-11-18 17:52 UTC (permalink / raw)
  To: Michal Hocko
  Cc: Daisuke Nishimura, linux-mm, linux-kernel, balbir, KAMEZAWA Hiroyuki

On Thu, 18 Nov 2010 11:23:49 +0100 Michal Hocko <mhocko@suse.cz> wrote:

> > I'm sorry again and again, but I think removing "noswapaccount" completely
> > would be better, as Andrew said first:
> 
> I read the above Andrew's statement that we really should stick with the
> old parameter.

yup.  We shouldn't remove the existing parameter, which people might be
using already.

> > > So we have swapaccount and noswapaccount.  Ho hum, "swapaccount=[1|0]"
> > > would have been better.

What I meant was that it was a mistake to add the "noswapaccount" in
the first place.  We should have made it "swapaccount=0", because that
would leave open the later option of reversing the default, and
enabling "swapaccount=1".

It also give us the option of adding "swapaccount=2"!  Perhaps to
enable alternative swap accounting behaviour.

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

end of thread, other threads:[~2010-11-18 17:57 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-16 10:17 [PATCH] Make swap accounting default behavior configurable Michal Hocko
2010-11-16 20:46 ` Andrew Morton
2010-11-16 21:21   ` [stable] " Greg KH
2010-11-18  8:21     ` Michal Hocko
2010-11-18 16:36       ` Greg KH
2010-11-17  0:23   ` Daisuke Nishimura
2010-11-17  0:57     ` KAMEZAWA Hiroyuki
2010-11-17  1:12     ` Andrew Morton
2010-11-17  3:28       ` Daisuke Nishimura
2010-11-18  8:23         ` Michal Hocko
2010-11-18  8:46           ` Daisuke Nishimura
2010-11-18  8:53             ` KAMEZAWA Hiroyuki
2010-11-18  9:56               ` [PATCH] Make swap accounting default behavior configurable v4 Michal Hocko
2010-11-18 10:14                 ` Daisuke Nishimura
2010-11-18 10:23                   ` Michal Hocko
2010-11-18 17:52                     ` Andrew Morton

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