* [RFC PATCH] Make swap accounting default behavior configurable @ 2010-11-10 12:51 Michal Hocko 2010-11-11 0:46 ` Daisuke Nishimura 0 siblings, 1 reply; 12+ messages in thread From: Michal Hocko @ 2010-11-10 12:51 UTC (permalink / raw) To: linux-mm; +Cc: KAMEZAWA Hiroyuki, linux-kernel Hi, could you consider the patch bellow? It basically changes the default swap accounting behavior (when it is turned on in configuration) to be configurable as well. The rationale is described in the patch but in short it makes it much more easier to enable this feature in distribution kernels as the functionality can be provided in the general purpose kernel (with the option disabled) without any drawbacks and interested users can enable it. This is not possible currently. I am aware that boot command line parameter name change is not ideal but the original semantic wasn't good enough and I don't like noswapaccount=yes|no very much. If we really have to stick to it I can rework the patch to keep the name and just add the yes|no logic, though. Or we can keep the original one and add swapaccount paramete which would mean the oposite as the other one. The patch is based on the current Linus tree. Any thoughts? --- >From c874f2c1ff1493e49611f19308434e564c2d37c6 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) and changes the original noswapaccount parameter to swapaccount=true|false which provides a more fine grained way to control this feature. 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> --- init/Kconfig | 13 +++++++++++++ mm/memcontrol.c | 19 ++++++++++++++----- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/init/Kconfig b/init/Kconfig index 88c1046..61d55a7 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=true 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 swapaccount=false does the trick). menuconfig CGROUP_SCHED bool "Group CPU scheduler" diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 9a99cfa..7c699b3 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,11 +4916,13 @@ struct cgroup_subsys mem_cgroup_subsys = { }; #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP - -static int __init disable_swap_account(char *s) +static int __init enable_swap_account(char *s) { - really_do_swap_account = 0; + if (!s || !strcmp(s, "true")) + really_do_swap_account = 1; + else if (!strcmp(s, "false")) + really_do_swap_account = 0; return 1; } -__setup("noswapaccount", disable_swap_account); +__setup("swapaccount", enable_swap_account); #endif -- 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] 12+ messages in thread
* Re: [RFC PATCH] Make swap accounting default behavior configurable 2010-11-10 12:51 [RFC PATCH] Make swap accounting default behavior configurable Michal Hocko @ 2010-11-11 0:46 ` Daisuke Nishimura 2010-11-11 9:31 ` Michal Hocko 0 siblings, 1 reply; 12+ messages in thread From: Daisuke Nishimura @ 2010-11-11 0:46 UTC (permalink / raw) To: Michal Hocko; +Cc: linux-mm, KAMEZAWA Hiroyuki, linux-kernel, Daisuke Nishimura On Wed, 10 Nov 2010 13:51:54 +0100 Michal Hocko <mhocko@suse.cz> wrote: > Hi, > could you consider the patch bellow? It basically changes the default > swap accounting behavior (when it is turned on in configuration) to be > configurable as well. > > The rationale is described in the patch but in short it makes it much > more easier to enable this feature in distribution kernels as the > functionality can be provided in the general purpose kernel (with the > option disabled) without any drawbacks and interested users can enable > it. This is not possible currently. > > I am aware that boot command line parameter name change is not ideal but > the original semantic wasn't good enough and I don't like > noswapaccount=yes|no very much. > > If we really have to stick to it I can rework the patch to keep the name > and just add the yes|no logic, though. Or we can keep the original one > and add swapaccount paramete which would mean the oposite as the other > one. > hmm, I agree that current parameter name(noswapaccount) is not desirable for yes|no, but IMHO changing the user interface(iow, making what worked before unusable) is worse. Although I'm not sure how many people are using this parameter, I vote for using "noswapaccount[=(yes|no)]". And you should update Documentation/kernel-parameters.txt too. Thanks, Daisuke Nishimura. > The patch is based on the current Linus tree. > > Any thoughts? > --- > > From c874f2c1ff1493e49611f19308434e564c2d37c6 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) and changes the original > noswapaccount parameter to swapaccount=true|false which provides > a more fine grained way to control this feature. > > 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> > --- > init/Kconfig | 13 +++++++++++++ > mm/memcontrol.c | 19 ++++++++++++++----- > 2 files changed, 27 insertions(+), 5 deletions(-) > > diff --git a/init/Kconfig b/init/Kconfig > index 88c1046..61d55a7 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=true 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 swapaccount=false does the trick). > > menuconfig CGROUP_SCHED > bool "Group CPU scheduler" > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 9a99cfa..7c699b3 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,11 +4916,13 @@ struct cgroup_subsys mem_cgroup_subsys = { > }; > > #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP > - > -static int __init disable_swap_account(char *s) > +static int __init enable_swap_account(char *s) > { > - really_do_swap_account = 0; > + if (!s || !strcmp(s, "true")) > + really_do_swap_account = 1; > + else if (!strcmp(s, "false")) > + really_do_swap_account = 0; > return 1; > } > -__setup("noswapaccount", disable_swap_account); > +__setup("swapaccount", enable_swap_account); > #endif > -- > 1.7.2.3 > > > -- > Michal Hocko > L3 team > SUSE LINUX s.r.o. > Lihovarska 1060/12 > 190 00 Praha 9 > Czech Republic > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majordomo@kvack.org. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/ > Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH] Make swap accounting default behavior configurable 2010-11-11 0:46 ` Daisuke Nishimura @ 2010-11-11 9:31 ` Michal Hocko 2010-11-12 0:41 ` Daisuke Nishimura 0 siblings, 1 reply; 12+ messages in thread From: Michal Hocko @ 2010-11-11 9:31 UTC (permalink / raw) To: Daisuke Nishimura; +Cc: linux-mm, KAMEZAWA Hiroyuki, linux-kernel On Thu 11-11-10 09:46:13, Daisuke Nishimura wrote: > On Wed, 10 Nov 2010 13:51:54 +0100 > Michal Hocko <mhocko@suse.cz> wrote: > > > Hi, > > could you consider the patch bellow? It basically changes the default > > swap accounting behavior (when it is turned on in configuration) to be > > configurable as well. > > > > The rationale is described in the patch but in short it makes it much > > more easier to enable this feature in distribution kernels as the > > functionality can be provided in the general purpose kernel (with the > > option disabled) without any drawbacks and interested users can enable > > it. This is not possible currently. > > > > I am aware that boot command line parameter name change is not ideal but > > the original semantic wasn't good enough and I don't like > > noswapaccount=yes|no very much. > > > > If we really have to stick to it I can rework the patch to keep the name > > and just add the yes|no logic, though. Or we can keep the original one > > and add swapaccount paramete which would mean the oposite as the other > > one. > > > hmm, I agree that current parameter name(noswapaccount) is not desirable > for yes|no, but IMHO changing the user interface(iow, making what worked before > unusable) is worse. > > Although I'm not sure how many people are using this parameter, I vote for > using "noswapaccount[=(yes|no)]". Isn't a new swapaccount parameter better than that? I know we don't want to have too many parameters but having a something with a clear meaning is better IMO (noswapaccount=no doesn't sound very intuitive to me). > And you should update Documentation/kernel-parameters.txt too. Yes, I am aware of that and will do that once there is an agreement on the patch itself. At this stage, I just wanted to have a feadback about the change. > > Thanks, > Daisuke Nishimura. > > > The patch is based on the current Linus tree. > > > > Any thoughts? > > --- > > > > From c874f2c1ff1493e49611f19308434e564c2d37c6 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) and changes the original > > noswapaccount parameter to swapaccount=true|false which provides > > a more fine grained way to control this feature. > > > > 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> > > --- > > init/Kconfig | 13 +++++++++++++ > > mm/memcontrol.c | 19 ++++++++++++++----- > > 2 files changed, 27 insertions(+), 5 deletions(-) > > > > diff --git a/init/Kconfig b/init/Kconfig > > index 88c1046..61d55a7 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=true 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 swapaccount=false does the trick). > > > > menuconfig CGROUP_SCHED > > bool "Group CPU scheduler" > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > > index 9a99cfa..7c699b3 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,11 +4916,13 @@ struct cgroup_subsys mem_cgroup_subsys = { > > }; > > > > #ifdef CONFIG_CGROUP_MEM_RES_CTLR_SWAP > > - > > -static int __init disable_swap_account(char *s) > > +static int __init enable_swap_account(char *s) > > { > > - really_do_swap_account = 0; > > + if (!s || !strcmp(s, "true")) > > + really_do_swap_account = 1; > > + else if (!strcmp(s, "false")) > > + really_do_swap_account = 0; > > return 1; > > } > > -__setup("noswapaccount", disable_swap_account); > > +__setup("swapaccount", enable_swap_account); > > #endif > > -- > > 1.7.2.3 > > > > > > -- > > Michal Hocko > > L3 team > > SUSE LINUX s.r.o. > > Lihovarska 1060/12 > > 190 00 Praha 9 > > Czech Republic > > > > -- > > To unsubscribe, send a message with 'unsubscribe linux-mm' in > > the body to majordomo@kvack.org. For more info on Linux MM, > > see: http://www.linux-mm.org/ . > > Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/ > > Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majordomo@kvack.org. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/ > Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> -- Michal Hocko L3 team SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH] Make swap accounting default behavior configurable 2010-11-11 9:31 ` Michal Hocko @ 2010-11-12 0:41 ` Daisuke Nishimura 2010-11-12 8:31 ` [RFC PATCH] Make swap accounting default behavior configurable v2 Michal Hocko 0 siblings, 1 reply; 12+ messages in thread From: Daisuke Nishimura @ 2010-11-12 0:41 UTC (permalink / raw) To: Michal Hocko; +Cc: linux-mm, KAMEZAWA Hiroyuki, linux-kernel, Daisuke Nishimura On Thu, 11 Nov 2010 10:31:55 +0100 Michal Hocko <mhocko@suse.cz> wrote: > On Thu 11-11-10 09:46:13, Daisuke Nishimura wrote: > > On Wed, 10 Nov 2010 13:51:54 +0100 > > Michal Hocko <mhocko@suse.cz> wrote: > > > > > Hi, > > > could you consider the patch bellow? It basically changes the default > > > swap accounting behavior (when it is turned on in configuration) to be > > > configurable as well. > > > > > > The rationale is described in the patch but in short it makes it much > > > more easier to enable this feature in distribution kernels as the > > > functionality can be provided in the general purpose kernel (with the > > > option disabled) without any drawbacks and interested users can enable > > > it. This is not possible currently. > > > > > > I am aware that boot command line parameter name change is not ideal but > > > the original semantic wasn't good enough and I don't like > > > noswapaccount=yes|no very much. > > > > > > If we really have to stick to it I can rework the patch to keep the name > > > and just add the yes|no logic, though. Or we can keep the original one > > > and add swapaccount paramete which would mean the oposite as the other > > > one. > > > > > hmm, I agree that current parameter name(noswapaccount) is not desirable > > for yes|no, but IMHO changing the user interface(iow, making what worked before > > unusable) is worse. > > > > Although I'm not sure how many people are using this parameter, I vote for > > using "noswapaccount[=(yes|no)]". > > Isn't a new swapaccount parameter better than that? I know we don't want > to have too many parameters but having a something with a clear meaning > is better IMO (noswapaccount=no doesn't sound very intuitive to me). > Fair enough. It's just an trade-off between compatibility and understandability. > > And you should update Documentation/kernel-parameters.txt too. > > Yes, I am aware of that and will do that once there is an agreement on > the patch itself. At this stage, I just wanted to have a feadback about > the change. > I'll ack your patch when it's been released with documentation update. Thanks, Daisuke Nishimura. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH] Make swap accounting default behavior configurable v2 2010-11-12 0:41 ` Daisuke Nishimura @ 2010-11-12 8:31 ` Michal Hocko 2010-11-15 1:13 ` Daisuke Nishimura 0 siblings, 1 reply; 12+ messages in thread From: Michal Hocko @ 2010-11-12 8:31 UTC (permalink / raw) To: Daisuke Nishimura; +Cc: linux-mm, KAMEZAWA Hiroyuki, linux-kernel On Fri 12-11-10 09:41:18, Daisuke Nishimura wrote: > On Thu, 11 Nov 2010 10:31:55 +0100 > Michal Hocko <mhocko@suse.cz> wrote: > > > On Thu 11-11-10 09:46:13, Daisuke Nishimura wrote: > > > On Wed, 10 Nov 2010 13:51:54 +0100 > > > Michal Hocko <mhocko@suse.cz> wrote: > > > > > > > Hi, > > > > could you consider the patch bellow? It basically changes the default > > > > swap accounting behavior (when it is turned on in configuration) to be > > > > configurable as well. > > > > > > > > The rationale is described in the patch but in short it makes it much > > > > more easier to enable this feature in distribution kernels as the > > > > functionality can be provided in the general purpose kernel (with the > > > > option disabled) without any drawbacks and interested users can enable > > > > it. This is not possible currently. > > > > > > > > I am aware that boot command line parameter name change is not ideal but > > > > the original semantic wasn't good enough and I don't like > > > > noswapaccount=yes|no very much. > > > > > > > > If we really have to stick to it I can rework the patch to keep the name > > > > and just add the yes|no logic, though. Or we can keep the original one > > > > and add swapaccount paramete which would mean the oposite as the other > > > > one. > > > > > > > hmm, I agree that current parameter name(noswapaccount) is not desirable > > > for yes|no, but IMHO changing the user interface(iow, making what worked before > > > unusable) is worse. > > > > > > Although I'm not sure how many people are using this parameter, I vote for > > > using "noswapaccount[=(yes|no)]". > > > > Isn't a new swapaccount parameter better than that? I know we don't want > > to have too many parameters but having a something with a clear meaning > > is better IMO (noswapaccount=no doesn't sound very intuitive to me). > > > Fair enough. It's just an trade-off between compatibility and understandability. > > > > And you should update Documentation/kernel-parameters.txt too. > > > > Yes, I am aware of that and will do that once there is an agreement on > > the patch itself. At this stage, I just wanted to have a feadback about > > the change. > > > I'll ack your patch when it's been released with documentation update. Changes since v1: * do not remove noswapaccount parameter and add swapaccount parameter instead * Documentation/kernel-parameters.txt updated --- >From c60e3d76702cd9b4b5b13c01be335da31d03bc1c 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> --- Documentation/kernel-parameters.txt | 2 ++ init/Kconfig | 13 +++++++++++++ mm/memcontrol.c | 15 ++++++++++++++- 3 files changed, 29 insertions(+), 1 deletions(-) diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt index ed45e98..7077148 100644 --- a/Documentation/kernel-parameters.txt +++ b/Documentation/kernel-parameters.txt @@ -1752,6 +1752,8 @@ and is between 256 and 4096 characters. It is defined in the file noswapaccount [KNL] Disable accounting of swap in memory resource controller. (See Documentation/cgroups/memory.txt) + swapaccount [KNL] Enable accounting of swap in memory resource + controller. (See Documentation/cgroups/memory.txt) nosync [HW,M68K] Disables sync negotiation for all devices. 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 ^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [RFC PATCH] Make swap accounting default behavior configurable v2 2010-11-12 8:31 ` [RFC PATCH] Make swap accounting default behavior configurable v2 Michal Hocko @ 2010-11-15 1:13 ` Daisuke Nishimura 2010-11-15 2:03 ` Balbir Singh 2010-11-15 8:35 ` [RFC PATCH] Make swap accounting default behavior configurable v3 Michal Hocko 0 siblings, 2 replies; 12+ messages in thread From: Daisuke Nishimura @ 2010-11-15 1:13 UTC (permalink / raw) To: Michal Hocko Cc: linux-mm, KAMEZAWA Hiroyuki, linux-kernel, Andrew Morton, balbir, Daisuke Nishimura On Fri, 12 Nov 2010 09:31:03 +0100 Michal Hocko <mhocko@suse.cz> wrote: > On Fri 12-11-10 09:41:18, Daisuke Nishimura wrote: > > On Thu, 11 Nov 2010 10:31:55 +0100 > > Michal Hocko <mhocko@suse.cz> wrote: > > > > > On Thu 11-11-10 09:46:13, Daisuke Nishimura wrote: > > > > On Wed, 10 Nov 2010 13:51:54 +0100 > > > > Michal Hocko <mhocko@suse.cz> wrote: > > > > > > > > > Hi, > > > > > could you consider the patch bellow? It basically changes the default > > > > > swap accounting behavior (when it is turned on in configuration) to be > > > > > configurable as well. > > > > > > > > > > The rationale is described in the patch but in short it makes it much > > > > > more easier to enable this feature in distribution kernels as the > > > > > functionality can be provided in the general purpose kernel (with the > > > > > option disabled) without any drawbacks and interested users can enable > > > > > it. This is not possible currently. > > > > > > > > > > I am aware that boot command line parameter name change is not ideal but > > > > > the original semantic wasn't good enough and I don't like > > > > > noswapaccount=yes|no very much. > > > > > > > > > > If we really have to stick to it I can rework the patch to keep the name > > > > > and just add the yes|no logic, though. Or we can keep the original one > > > > > and add swapaccount paramete which would mean the oposite as the other > > > > > one. > > > > > > > > > hmm, I agree that current parameter name(noswapaccount) is not desirable > > > > for yes|no, but IMHO changing the user interface(iow, making what worked before > > > > unusable) is worse. > > > > > > > > Although I'm not sure how many people are using this parameter, I vote for > > > > using "noswapaccount[=(yes|no)]". > > > > > > Isn't a new swapaccount parameter better than that? I know we don't want > > > to have too many parameters but having a something with a clear meaning > > > is better IMO (noswapaccount=no doesn't sound very intuitive to me). > > > > > Fair enough. It's just an trade-off between compatibility and understandability. > > > > > > And you should update Documentation/kernel-parameters.txt too. > > > > > > Yes, I am aware of that and will do that once there is an agreement on > > > the patch itself. At this stage, I just wanted to have a feadback about > > > the change. > > > > > I'll ack your patch when it's been released with documentation update. > > Changes since v1: > * do not remove noswapaccount parameter and add swapaccount parameter > instead > * Documentation/kernel-parameters.txt updated > > --- > From c60e3d76702cd9b4b5b13c01be335da31d03bc1c 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> > --- > Documentation/kernel-parameters.txt | 2 ++ > init/Kconfig | 13 +++++++++++++ > mm/memcontrol.c | 15 ++++++++++++++- > 3 files changed, 29 insertions(+), 1 deletions(-) > > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt > index ed45e98..7077148 100644 > --- a/Documentation/kernel-parameters.txt > +++ b/Documentation/kernel-parameters.txt > @@ -1752,6 +1752,8 @@ and is between 256 and 4096 characters. It is defined in the file > > noswapaccount [KNL] Disable accounting of swap in memory resource > controller. (See Documentation/cgroups/memory.txt) > + swapaccount [KNL] Enable accounting of swap in memory resource > + controller. (See Documentation/cgroups/memory.txt) > > nosync [HW,M68K] Disables sync negotiation for all devices. > (I've add Andrew and Balbir to CC-list.) It seems that almost all parameters are listed in alphabetic order in the document, so I think it would be better to obey the rule. Thanks, Daisuke Nishimura. > 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 > ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH] Make swap accounting default behavior configurable v2 2010-11-15 1:13 ` Daisuke Nishimura @ 2010-11-15 2:03 ` Balbir Singh 2010-11-15 2:31 ` Daisuke Nishimura 2010-11-15 8:35 ` [RFC PATCH] Make swap accounting default behavior configurable v3 Michal Hocko 1 sibling, 1 reply; 12+ messages in thread From: Balbir Singh @ 2010-11-15 2:03 UTC (permalink / raw) To: Daisuke Nishimura Cc: Michal Hocko, linux-mm, KAMEZAWA Hiroyuki, linux-kernel, Andrew Morton * nishimura@mxp.nes.nec.co.jp <nishimura@mxp.nes.nec.co.jp> [2010-11-15 10:13:35]: Thanks Nishimura-San It seems like the motivation for the patch is to allow distros to enable memory cgroups and swap control, but to have swap control turned off by default (because we provide default on today) - is my understanding correct? -- Three Cheers, Balbir ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH] Make swap accounting default behavior configurable v2 2010-11-15 2:03 ` Balbir Singh @ 2010-11-15 2:31 ` Daisuke Nishimura 0 siblings, 0 replies; 12+ messages in thread From: Daisuke Nishimura @ 2010-11-15 2:31 UTC (permalink / raw) To: balbir Cc: Michal Hocko, linux-mm, KAMEZAWA Hiroyuki, linux-kernel, Andrew Morton, Daisuke Nishimura On Mon, 15 Nov 2010 07:33:30 +0530 Balbir Singh <balbir@linux.vnet.ibm.com> wrote: > * nishimura@mxp.nes.nec.co.jp <nishimura@mxp.nes.nec.co.jp> [2010-11-15 10:13:35]: > > Thanks Nishimura-San > > It seems like the motivation for the patch is to allow distros to > enable memory cgroups and swap control, but to have swap control > turned off by default (because we provide default on today) > - is my understanding correct? > I think you're right. This patch make the default behavior configurable by .config and let users turn swap control on even if it's disabled by .config. It will give distros and users wider choice. Thanks, Daisuke Nishimura. ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH] Make swap accounting default behavior configurable v3 2010-11-15 1:13 ` Daisuke Nishimura 2010-11-15 2:03 ` Balbir Singh @ 2010-11-15 8:35 ` Michal Hocko 2010-11-16 4:48 ` Daisuke Nishimura 1 sibling, 1 reply; 12+ messages in thread From: Michal Hocko @ 2010-11-15 8:35 UTC (permalink / raw) To: Daisuke Nishimura Cc: linux-mm, KAMEZAWA Hiroyuki, linux-kernel, Andrew Morton, balbir On Mon 15-11-10 10:13:35, Daisuke Nishimura wrote: > On Fri, 12 Nov 2010 09:31:03 +0100 [...] > > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt > > index ed45e98..7077148 100644 > > --- a/Documentation/kernel-parameters.txt > > +++ b/Documentation/kernel-parameters.txt > > @@ -1752,6 +1752,8 @@ and is between 256 and 4096 characters. It is defined in the file > > > > noswapaccount [KNL] Disable accounting of swap in memory resource > > controller. (See Documentation/cgroups/memory.txt) > > + swapaccount [KNL] Enable accounting of swap in memory resource > > + controller. (See Documentation/cgroups/memory.txt) > > > > nosync [HW,M68K] Disables sync negotiation for all devices. > > > (I've add Andrew and Balbir to CC-list.) > It seems that almost all parameters are listed in alphabetic order in the document, > so I think it would be better to obey the rule. You are right. The header of the file says: " The following is a consolidated list of the kernel parameters as implemented (mostly) by the __setup() macro and sorted into English Dictionary order (defined as ignoring all punctuation and sorting digits before letters in a case insensitive manner), and with descriptions where known." Updated patch follows bellow. > > Thanks, > Daisuke Nishimura. 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 21df3801e2b65f47a2807534487ebb353dad6340 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> --- 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] 12+ messages in thread
* Re: [RFC PATCH] Make swap accounting default behavior configurable v3 2010-11-15 8:35 ` [RFC PATCH] Make swap accounting default behavior configurable v3 Michal Hocko @ 2010-11-16 4:48 ` Daisuke Nishimura 2010-11-16 8:15 ` Michal Hocko 0 siblings, 1 reply; 12+ messages in thread From: Daisuke Nishimura @ 2010-11-16 4:48 UTC (permalink / raw) To: Michal Hocko Cc: linux-mm, KAMEZAWA Hiroyuki, linux-kernel, Andrew Morton, balbir, Daisuke Nishimura Acked-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> Thank you for your work. Daisuke Nishimura. On Mon, 15 Nov 2010 09:35:40 +0100 Michal Hocko <mhocko@suse.cz> wrote: > On Mon 15-11-10 10:13:35, Daisuke Nishimura wrote: > > On Fri, 12 Nov 2010 09:31:03 +0100 > [...] > > > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt > > > index ed45e98..7077148 100644 > > > --- a/Documentation/kernel-parameters.txt > > > +++ b/Documentation/kernel-parameters.txt > > > @@ -1752,6 +1752,8 @@ and is between 256 and 4096 characters. It is defined in the file > > > > > > noswapaccount [KNL] Disable accounting of swap in memory resource > > > controller. (See Documentation/cgroups/memory.txt) > > > + swapaccount [KNL] Enable accounting of swap in memory resource > > > + controller. (See Documentation/cgroups/memory.txt) > > > > > > nosync [HW,M68K] Disables sync negotiation for all devices. > > > > > (I've add Andrew and Balbir to CC-list.) > > It seems that almost all parameters are listed in alphabetic order in the document, > > so I think it would be better to obey the rule. > > You are right. The header of the file says: > > " The following is a consolidated list of the kernel parameters as > implemented (mostly) by the __setup() macro and sorted into English > Dictionary order (defined as ignoring all punctuation and sorting digits > before letters in a case insensitive manner), and with descriptions > where known." > > Updated patch follows bellow. > > > > > Thanks, > > Daisuke Nishimura. > > 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 21df3801e2b65f47a2807534487ebb353dad6340 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> > --- > 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 [flat|nested] 12+ messages in thread
* Re: [RFC PATCH] Make swap accounting default behavior configurable v3 2010-11-16 4:48 ` Daisuke Nishimura @ 2010-11-16 8:15 ` Michal Hocko 2010-11-16 10:03 ` Daisuke Nishimura 0 siblings, 1 reply; 12+ messages in thread From: Michal Hocko @ 2010-11-16 8:15 UTC (permalink / raw) To: Daisuke Nishimura Cc: linux-mm, KAMEZAWA Hiroyuki, linux-kernel, Andrew Morton, balbir On Tue 16-11-10 13:48:00, Daisuke Nishimura wrote: > Acked-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> Thank you. What should be next steps? Waiting for other ACKs or push it through Andrew? Btw. is this a stable tree material? I guess that distribution would like to have this patch and the stable is the easiest way how to deliver this. > > Thank you for your work. > Daisuke Nishimura. > > On Mon, 15 Nov 2010 09:35:40 +0100 > Michal Hocko <mhocko@suse.cz> wrote: > > > On Mon 15-11-10 10:13:35, Daisuke Nishimura wrote: > > > On Fri, 12 Nov 2010 09:31:03 +0100 > > [...] > > > > diff --git a/Documentation/kernel-parameters.txt b/Documentation/kernel-parameters.txt > > > > index ed45e98..7077148 100644 > > > > --- a/Documentation/kernel-parameters.txt > > > > +++ b/Documentation/kernel-parameters.txt > > > > @@ -1752,6 +1752,8 @@ and is between 256 and 4096 characters. It is defined in the file > > > > > > > > noswapaccount [KNL] Disable accounting of swap in memory resource > > > > controller. (See Documentation/cgroups/memory.txt) > > > > + swapaccount [KNL] Enable accounting of swap in memory resource > > > > + controller. (See Documentation/cgroups/memory.txt) > > > > > > > > nosync [HW,M68K] Disables sync negotiation for all devices. > > > > > > > (I've add Andrew and Balbir to CC-list.) > > > It seems that almost all parameters are listed in alphabetic order in the document, > > > so I think it would be better to obey the rule. > > > > You are right. The header of the file says: > > > > " The following is a consolidated list of the kernel parameters as > > implemented (mostly) by the __setup() macro and sorted into English > > Dictionary order (defined as ignoring all punctuation and sorting digits > > before letters in a case insensitive manner), and with descriptions > > where known." > > > > Updated patch follows bellow. > > > > > > > > Thanks, > > > Daisuke Nishimura. > > > > 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 21df3801e2b65f47a2807534487ebb353dad6340 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> > > --- > > 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 > > -- > To unsubscribe, send a message with 'unsubscribe linux-mm' in > the body to majordomo@kvack.org. For more info on Linux MM, > see: http://www.linux-mm.org/ . > Fight unfair telecom policy in Canada: sign http://dissolvethecrtc.ca/ > Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a> -- Michal Hocko L3 team SUSE LINUX s.r.o. Lihovarska 1060/12 190 00 Praha 9 Czech Republic ^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [RFC PATCH] Make swap accounting default behavior configurable v3 2010-11-16 8:15 ` Michal Hocko @ 2010-11-16 10:03 ` Daisuke Nishimura 0 siblings, 0 replies; 12+ messages in thread From: Daisuke Nishimura @ 2010-11-16 10:03 UTC (permalink / raw) To: Michal Hocko Cc: linux-mm, KAMEZAWA Hiroyuki, linux-kernel, Andrew Morton, balbir, Daisuke Nishimura On Tue, 16 Nov 2010 09:15:44 +0100 Michal Hocko <mhocko@suse.cz> wrote: > On Tue 16-11-10 13:48:00, Daisuke Nishimura wrote: > > Acked-by: Daisuke Nishimura <nishimura@mxp.nes.nec.co.jp> > > Thank you. What should be next steps? Waiting for other ACKs or push it > through Andrew? I recommend you to resend the patch removing "RFC" in a clean patch format (i.e. without any quotes). > Btw. is this a stable tree material? I guess that distribution would > like to have this patch and the stable is the easiest way how to deliver > this. > It's not stable material. This is not a BUG or anything that meets the conditions described in Documentation/stable_kernel_rules.txt. Thanks, Daisuke Nishimura. ^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2010-11-16 10:06 UTC | newest] Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-11-10 12:51 [RFC PATCH] Make swap accounting default behavior configurable Michal Hocko 2010-11-11 0:46 ` Daisuke Nishimura 2010-11-11 9:31 ` Michal Hocko 2010-11-12 0:41 ` Daisuke Nishimura 2010-11-12 8:31 ` [RFC PATCH] Make swap accounting default behavior configurable v2 Michal Hocko 2010-11-15 1:13 ` Daisuke Nishimura 2010-11-15 2:03 ` Balbir Singh 2010-11-15 2:31 ` Daisuke Nishimura 2010-11-15 8:35 ` [RFC PATCH] Make swap accounting default behavior configurable v3 Michal Hocko 2010-11-16 4:48 ` Daisuke Nishimura 2010-11-16 8:15 ` Michal Hocko 2010-11-16 10:03 ` Daisuke Nishimura
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).