* [PATCH] mm: memcontrol: fix handling of swapaccount parameter
@ 2013-08-14 13:21 Gergely Risko
2013-08-14 18:36 ` Michal Hocko
0 siblings, 1 reply; 6+ messages in thread
From: Gergely Risko @ 2013-08-14 13:21 UTC (permalink / raw)
To: cgroups, linux-mm, linux-kernel; +Cc: torvalds, Gergely Risko
Fixed swap accounting option parsing to enable if called without argument.
Signed-off-by: Gergely Risko <gergely@risko.hu>
---
mm/memcontrol.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index c290a1c..8ec2507 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -6970,13 +6970,13 @@ struct cgroup_subsys mem_cgroup_subsys = {
static int __init enable_swap_account(char *s)
{
/* consider enabled if no parameter or 1 is given */
- if (!strcmp(s, "1"))
+ if (*s++ != '=' || !*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);
+__setup("swapaccount", enable_swap_account);
static void __init memsw_file_init(void)
{
--
1.8.3.2
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] mm: memcontrol: fix handling of swapaccount parameter
2013-08-14 13:21 [PATCH] mm: memcontrol: fix handling of swapaccount parameter Gergely Risko
@ 2013-08-14 18:36 ` Michal Hocko
2013-08-14 18:49 ` Michal Hocko
0 siblings, 1 reply; 6+ messages in thread
From: Michal Hocko @ 2013-08-14 18:36 UTC (permalink / raw)
To: Gergely Risko; +Cc: cgroups, linux-mm, linux-kernel, torvalds
On Wed 14-08-13 15:21:35, Gergely Risko wrote:
> Fixed swap accounting option parsing to enable if called without argument.
We used to have [no]swapaccount but that one has been removed by a2c8990a
(memsw: remove noswapaccount kernel parameter) so I do not think that
swapaccount without any given value makes much sense these days.
> Signed-off-by: Gergely Risko <gergely@risko.hu>
> ---
> mm/memcontrol.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index c290a1c..8ec2507 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -6970,13 +6970,13 @@ struct cgroup_subsys mem_cgroup_subsys = {
> static int __init enable_swap_account(char *s)
> {
> /* consider enabled if no parameter or 1 is given */
> - if (!strcmp(s, "1"))
> + if (*s++ != '=' || !*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);
> +__setup("swapaccount", enable_swap_account);
>
> static void __init memsw_file_init(void)
> {
> --
> 1.8.3.2
>
> --
> 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/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
--
Michal Hocko
SUSE Labs
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] mm: memcontrol: fix handling of swapaccount parameter
2013-08-14 18:36 ` Michal Hocko
@ 2013-08-14 18:49 ` Michal Hocko
2013-08-14 21:22 ` Gergely Risko
0 siblings, 1 reply; 6+ messages in thread
From: Michal Hocko @ 2013-08-14 18:49 UTC (permalink / raw)
To: Gergely Risko; +Cc: cgroups, linux-mm, linux-kernel, torvalds
On Wed 14-08-13 20:36:04, Michal Hocko wrote:
> On Wed 14-08-13 15:21:35, Gergely Risko wrote:
> > Fixed swap accounting option parsing to enable if called without argument.
>
> We used to have [no]swapaccount but that one has been removed by a2c8990a
> (memsw: remove noswapaccount kernel parameter) so I do not think that
> swapaccount without any given value makes much sense these days.
Now that I am reading your changelog again it says this is a fix. Have
you experienced any troubles because of the parameter semantic change?
> > Signed-off-by: Gergely Risko <gergely@risko.hu>
> > ---
> > mm/memcontrol.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> > index c290a1c..8ec2507 100644
> > --- a/mm/memcontrol.c
> > +++ b/mm/memcontrol.c
> > @@ -6970,13 +6970,13 @@ struct cgroup_subsys mem_cgroup_subsys = {
> > static int __init enable_swap_account(char *s)
> > {
> > /* consider enabled if no parameter or 1 is given */
> > - if (!strcmp(s, "1"))
> > + if (*s++ != '=' || !*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);
> > +__setup("swapaccount", enable_swap_account);
> >
> > static void __init memsw_file_init(void)
> > {
> > --
> > 1.8.3.2
> >
> > --
> > 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/ .
> > Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
>
> --
> Michal Hocko
> SUSE Labs
>
> --
> 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/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
--
Michal Hocko
SUSE Labs
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] mm: memcontrol: fix handling of swapaccount parameter
2013-08-14 18:49 ` Michal Hocko
@ 2013-08-14 21:22 ` Gergely Risko
2013-08-15 7:47 ` Michal Hocko
0 siblings, 1 reply; 6+ messages in thread
From: Gergely Risko @ 2013-08-14 21:22 UTC (permalink / raw)
To: Michal Hocko; +Cc: cgroups, linux-mm, linux-kernel, torvalds
On Wed, 14 Aug 2013 20:49:56 +0200, Michal Hocko <mhocko@suse.cz> writes:
> On Wed 14-08-13 20:36:04, Michal Hocko wrote:
>> On Wed 14-08-13 15:21:35, Gergely Risko wrote:
>> > Fixed swap accounting option parsing to enable if called without argument.
>>
>> We used to have [no]swapaccount but that one has been removed by a2c8990a
>> (memsw: remove noswapaccount kernel parameter) so I do not think that
>> swapaccount without any given value makes much sense these days.
>
> Now that I am reading your changelog again it says this is a fix. Have
> you experienced any troubles because of the parameter semantic change?
Yeah, I experienced trouble, I was new to all of this containers +
cgroups + namespaces thingies and while trying out stuff it was totally
impossible for me to enable swap accounting and I didn't understand why.
In Debian swap accounting is off by default, even when you
cgroup_enable=memory. So you have to explicitly enable swapaccounting.
I've found the following documentation snippets all pointing to enable
swap accounting by just simply adding "swapaccount" to the kernel
command line. They all state that "swapaccount" is enough, no need for
"swapaccount=1" (actually some of them don't even mention =1 at all):
- make menuconfig documentation for swap accounting,
- /usr/share/doc/lxc/README.Debian from the lxc package,
- Documentation/kernel-parameters.txt:
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),
- the comment in the source code just above the line ("consider enabled
if no parameter or 1 is given").
And of course it's a trivial thing for the user to try swapaccount=1
when simply swapaccount doesn't work, but it's still a very bad
experience, because the documentation seems to be clear and every
command line change requires a reboot.
It's OK for me if we fix the documentation instead of the code. But
notice that the code is trivial to fix and the documentation has already
spread out to various debian packages, internet forums, bug reports,
etc. So it seems to be less hassle to actually implement the
documentation than to document the code.
Gergely
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] mm: memcontrol: fix handling of swapaccount parameter
2013-08-14 21:22 ` Gergely Risko
@ 2013-08-15 7:47 ` Michal Hocko
2013-08-15 9:01 ` Gergely Risko
0 siblings, 1 reply; 6+ messages in thread
From: Michal Hocko @ 2013-08-15 7:47 UTC (permalink / raw)
To: Gergely Risko; +Cc: cgroups, linux-mm, linux-kernel, torvalds, Andrew Morton
[Let's CC Andrew]
On Wed 14-08-13 23:22:23, Gergely Risko wrote:
> On Wed, 14 Aug 2013 20:49:56 +0200, Michal Hocko <mhocko@suse.cz> writes:
>
> > On Wed 14-08-13 20:36:04, Michal Hocko wrote:
> >> On Wed 14-08-13 15:21:35, Gergely Risko wrote:
> >> > Fixed swap accounting option parsing to enable if called without argument.
> >>
> >> We used to have [no]swapaccount but that one has been removed by a2c8990a
> >> (memsw: remove noswapaccount kernel parameter) so I do not think that
> >> swapaccount without any given value makes much sense these days.
> >
> > Now that I am reading your changelog again it says this is a fix. Have
> > you experienced any troubles because of the parameter semantic change?
>
> Yeah, I experienced trouble, I was new to all of this containers +
> cgroups + namespaces thingies and while trying out stuff it was totally
> impossible for me to enable swap accounting and I didn't understand why.
>
> In Debian swap accounting is off by default, even when you
> cgroup_enable=memory. So you have to explicitly enable swapaccounting.
>
> I've found the following documentation snippets all pointing to enable
> swap accounting by just simply adding "swapaccount" to the kernel
> command line. They all state that "swapaccount" is enough, no need for
> "swapaccount=1" (actually some of them don't even mention =1 at all):
> - make menuconfig documentation for swap accounting,
> - /usr/share/doc/lxc/README.Debian from the lxc package,
I've submitted a report with patch
(http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=719774)
> - Documentation/kernel-parameters.txt:
> 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),
> - the comment in the source code just above the line ("consider enabled
> if no parameter or 1 is given").
Ohh, I have totally missed those left-overs. I would rather fix the doc
than reintroduce the handling without any value.
---
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] mm: memcontrol: fix handling of swapaccount parameter
2013-08-15 7:47 ` Michal Hocko
@ 2013-08-15 9:01 ` Gergely Risko
0 siblings, 0 replies; 6+ messages in thread
From: Gergely Risko @ 2013-08-15 9:01 UTC (permalink / raw)
To: Michal Hocko; +Cc: cgroups, linux-mm, linux-kernel, torvalds, Andrew Morton
On Thu, 15 Aug 2013 09:47:14 +0200, Michal Hocko <mhocko@suse.cz> writes:
> Ohh, I have totally missed those left-overs. I would rather fix the doc
> than reintroduce the handling without any value.
Okay, fine with me, thanks for fixing these left-overs!
Gergely
--
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/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-08-15 9:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-14 13:21 [PATCH] mm: memcontrol: fix handling of swapaccount parameter Gergely Risko
2013-08-14 18:36 ` Michal Hocko
2013-08-14 18:49 ` Michal Hocko
2013-08-14 21:22 ` Gergely Risko
2013-08-15 7:47 ` Michal Hocko
2013-08-15 9:01 ` Gergely Risko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox