* [PATCH v2] mm/memcontrol: use kstrtobool for swapaccount param parsing
@ 2022-09-09 8:46 Liu Shixin
2022-09-09 21:53 ` Andrew Morton
2022-09-13 10:34 ` [PATCH v2] " Muchun Song
0 siblings, 2 replies; 5+ messages in thread
From: Liu Shixin @ 2022-09-09 8:46 UTC (permalink / raw)
To: Jonathan Corbet, Johannes Weiner, Michal Hocko, Roman Gushchin,
Shakeel Butt, Muchun Song, Andrew Morton
Cc: linux-doc, linux-kernel, cgroups, linux-mm, Liu Shixin, Kefeng Wang
Use kstrtobool which is more powerful to handle all kinds of parameters
like 'Yy1Nn0' or [oO][NnFf] for "on" and "off".
Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
Documentation/admin-guide/kernel-parameters.txt | 7 ++++---
mm/memcontrol.c | 8 ++++----
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 2434c6729f1c..5f8531e39bc9 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -6037,10 +6037,11 @@
Execution Facility on pSeries.
swapaccount= [KNL]
- Format: [0|1]
+ Format: { [oO][Nn]/Y/y/1 | [oO][Ff]/N/n/0 }
Enable accounting of swap in memory resource
- controller if no parameter or 1 is given or disable
- it if 0 is given (See Documentation/admin-guide/cgroup-v1/memory.rst)
+ controller if no parameter or [oO][Nn]/Y/y/1 is given
+ or disable it if [oO][Ff]/N/n/0 is given
+ (See Documentation/admin-guide/cgroup-v1/memory.rst)
swiotlb= [ARM,IA-64,PPC,MIPS,X86]
Format: { <int> [,<int>] | force | noforce }
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 0a1a8a846870..5511c0c120d9 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -7434,10 +7434,10 @@ bool mem_cgroup_swap_full(struct folio *folio)
static int __init setup_swap_account(char *s)
{
- if (!strcmp(s, "1"))
- cgroup_memory_noswap = false;
- else if (!strcmp(s, "0"))
- cgroup_memory_noswap = true;
+ bool res;
+
+ if (!kstrtobool(s, &res))
+ cgroup_memory_noswap = !res;
return 1;
}
__setup("swapaccount=", setup_swap_account);
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] mm/memcontrol: use kstrtobool for swapaccount param parsing
2022-09-09 8:46 [PATCH v2] mm/memcontrol: use kstrtobool for swapaccount param parsing Liu Shixin
@ 2022-09-09 21:53 ` Andrew Morton
2022-09-13 7:13 ` [PATCH v3] " Liu Shixin
2022-09-13 10:34 ` [PATCH v2] " Muchun Song
1 sibling, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2022-09-09 21:53 UTC (permalink / raw)
To: Liu Shixin
Cc: Jonathan Corbet, Johannes Weiner, Michal Hocko, Roman Gushchin,
Shakeel Butt, Muchun Song, linux-doc, linux-kernel, cgroups,
linux-mm, Kefeng Wang
On Fri, 9 Sep 2022 16:46:47 +0800 Liu Shixin <liushixin2@huawei.com> wrote:
> Use kstrtobool which is more powerful to handle all kinds of parameters
> like 'Yy1Nn0' or [oO][NnFf] for "on" and "off".
>
> ...
>
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -6037,10 +6037,11 @@
> Execution Facility on pSeries.
>
> swapaccount= [KNL]
> - Format: [0|1]
> + Format: { [oO][Nn]/Y/y/1 | [oO][Ff]/N/n/0 }
> Enable accounting of swap in memory resource
> - controller if no parameter or 1 is given or disable
> - it if 0 is given (See Documentation/admin-guide/cgroup-v1/memory.rst)
> + controller if no parameter or [oO][Nn]/Y/y/1 is given
> + or disable it if [oO][Ff]/N/n/0 is given
> + (See Documentation/admin-guide/cgroup-v1/memory.rst)
>
> swiotlb= [ARM,IA-64,PPC,MIPS,X86]
> Format: { <int> [,<int>] | force | noforce }
mhocko suggested dropping this change as well.
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 0a1a8a846870..5511c0c120d9 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -7434,10 +7434,10 @@ bool mem_cgroup_swap_full(struct folio *folio)
>
> static int __init setup_swap_account(char *s)
> {
> - if (!strcmp(s, "1"))
> - cgroup_memory_noswap = false;
> - else if (!strcmp(s, "0"))
> - cgroup_memory_noswap = true;
> + bool res;
> +
> + if (!kstrtobool(s, &res))
> + cgroup_memory_noswap = !res;
> return 1;
> }
> __setup("swapaccount=", setup_swap_account);
And I agree. See, the risk with this patch is that someone will
develop userspace code which relies upon the new behaviour. Then when
someone tries to use that code on an older kernel, whoops, it doesn't
work. In other words, we're encouraging development of
non-backward-compatible userspace code.
Leaving the documentation as it was (just "0|1") will help to prevent
that situation.
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3] mm/memcontrol: use kstrtobool for swapaccount param parsing
2022-09-09 21:53 ` Andrew Morton
@ 2022-09-13 7:13 ` Liu Shixin
2022-09-19 15:01 ` Michal Hocko
0 siblings, 1 reply; 5+ messages in thread
From: Liu Shixin @ 2022-09-13 7:13 UTC (permalink / raw)
To: Andrew Morton
Cc: Jonathan Corbet, Johannes Weiner, Michal Hocko, Roman Gushchin,
Shakeel Butt, Muchun Song, cgroups, linux-doc, linux-mm,
linux-kernel, Liu Shixin, Kefeng Wang
Use kstrtobool which is more powerful to handle all kinds of parameters
like 'Yy1Nn0' or [oO][NnFf] for "on" and "off".
Signed-off-by: Liu Shixin <liushixin2@huawei.com>
---
mm/memcontrol.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index 0a1a8a846870..5511c0c120d9 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -7434,10 +7434,10 @@ bool mem_cgroup_swap_full(struct folio *folio)
static int __init setup_swap_account(char *s)
{
- if (!strcmp(s, "1"))
- cgroup_memory_noswap = false;
- else if (!strcmp(s, "0"))
- cgroup_memory_noswap = true;
+ bool res;
+
+ if (!kstrtobool(s, &res))
+ cgroup_memory_noswap = !res;
return 1;
}
__setup("swapaccount=", setup_swap_account);
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2] mm/memcontrol: use kstrtobool for swapaccount param parsing
2022-09-09 8:46 [PATCH v2] mm/memcontrol: use kstrtobool for swapaccount param parsing Liu Shixin
2022-09-09 21:53 ` Andrew Morton
@ 2022-09-13 10:34 ` Muchun Song
1 sibling, 0 replies; 5+ messages in thread
From: Muchun Song @ 2022-09-13 10:34 UTC (permalink / raw)
To: Liu Shixin
Cc: Jonathan Corbet, Johannes Weiner, Michal Hocko, Roman Gushchin,
Shakeel Butt, Muchun Song, Andrew Morton, linux-doc,
linux-kernel, cgroups, linux-mm, Kefeng Wang
> On Sep 9, 2022, at 16:46, Liu Shixin <liushixin2@huawei.com> wrote:
>
> Use kstrtobool which is more powerful to handle all kinds of parameters
> like 'Yy1Nn0' or [oO][NnFf] for "on" and "off".
>
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
With Andrew and Michal’s suggestion.
Reviewed-by: Muchun Song <songmuchun@bytedance.com>
Thanks.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] mm/memcontrol: use kstrtobool for swapaccount param parsing
2022-09-13 7:13 ` [PATCH v3] " Liu Shixin
@ 2022-09-19 15:01 ` Michal Hocko
0 siblings, 0 replies; 5+ messages in thread
From: Michal Hocko @ 2022-09-19 15:01 UTC (permalink / raw)
To: Liu Shixin
Cc: Andrew Morton, Jonathan Corbet, Johannes Weiner, Roman Gushchin,
Shakeel Butt, Muchun Song, cgroups, linux-doc, linux-mm,
linux-kernel, Kefeng Wang
On Tue 13-09-22 15:13:58, Liu Shixin wrote:
> Use kstrtobool which is more powerful to handle all kinds of parameters
> like 'Yy1Nn0' or [oO][NnFf] for "on" and "off".
the main usefulness is with the code reusability of library functions
rather than ad-hoc stuff.
> Signed-off-by: Liu Shixin <liushixin2@huawei.com>
Acked-by: Michal Hocko <mhocko@suse.com>
Thanks!
> ---
> mm/memcontrol.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index 0a1a8a846870..5511c0c120d9 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -7434,10 +7434,10 @@ bool mem_cgroup_swap_full(struct folio *folio)
>
> static int __init setup_swap_account(char *s)
> {
> - if (!strcmp(s, "1"))
> - cgroup_memory_noswap = false;
> - else if (!strcmp(s, "0"))
> - cgroup_memory_noswap = true;
> + bool res;
> +
> + if (!kstrtobool(s, &res))
> + cgroup_memory_noswap = !res;
> return 1;
> }
> __setup("swapaccount=", setup_swap_account);
> --
> 2.25.1
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-09-19 15:01 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-09 8:46 [PATCH v2] mm/memcontrol: use kstrtobool for swapaccount param parsing Liu Shixin
2022-09-09 21:53 ` Andrew Morton
2022-09-13 7:13 ` [PATCH v3] " Liu Shixin
2022-09-19 15:01 ` Michal Hocko
2022-09-13 10:34 ` [PATCH v2] " Muchun Song
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).