All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] mm/memcontrol: use kstrtobool for swapaccount param parsing
@ 2022-09-09  8:46 ` Liu Shixin
  0 siblings, 0 replies; 9+ 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] 9+ messages in thread
* [PATCH v2] mm/memcontrol: use kstrtobool for swapaccount param parsing
@ 2022-09-09  8:46 ` Liu Shixin
  0 siblings, 0 replies; 9+ 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] 9+ messages in thread
* Re: [PATCH v2] mm/memcontrol: use kstrtobool for swapaccount param parsing
  2022-09-09  8:46 ` Liu Shixin
@ 2022-09-09 21:53     ` Andrew Morton
  -1 siblings, 0 replies; 9+ 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-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA,
	cgroups-u79uwXL29TY76Z2rM5mHXA, linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
	Kefeng Wang

On Fri, 9 Sep 2022 16:46:47 +0800 Liu Shixin <liushixin2-hv44wF8Li93QT0dZR+AlfA@public.gmane.org> 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] 9+ messages in thread
* Re: [PATCH v2] mm/memcontrol: use kstrtobool for swapaccount param parsing
@ 2022-09-09 21:53     ` Andrew Morton
  0 siblings, 0 replies; 9+ 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] 9+ 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
  -1 siblings, 0 replies; 9+ 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] 9+ messages in thread
* [PATCH v3] mm/memcontrol: use kstrtobool for swapaccount param parsing
@ 2022-09-13  7:13       ` Liu Shixin
  0 siblings, 0 replies; 9+ 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] 9+ messages in thread
* Re: [PATCH v2] mm/memcontrol: use kstrtobool for swapaccount param parsing
  2022-09-09  8:46 ` Liu Shixin
@ 2022-09-13 10:34   ` Muchun Song
  -1 siblings, 0 replies; 9+ 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] 9+ messages in thread
* Re: [PATCH v2] mm/memcontrol: use kstrtobool for swapaccount param parsing
@ 2022-09-13 10:34   ` Muchun Song
  0 siblings, 0 replies; 9+ 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] 9+ messages in thread
* Re: [PATCH v3] mm/memcontrol: use kstrtobool for swapaccount param parsing
  2022-09-13  7:13       ` Liu Shixin
  (?)
@ 2022-09-19 15:01       ` Michal Hocko
  -1 siblings, 0 replies; 9+ 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] 9+ messages in thread
end of thread, other threads:[~2022-09-19 15:01 UTC | newest]

Thread overview: 9+ 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  8:46 ` Liu Shixin
     [not found] ` <20220909084647.3598299-1-liushixin2-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
2022-09-09 21:53   ` Andrew Morton
2022-09-09 21:53     ` Andrew Morton
2022-09-13  7:13     ` [PATCH v3] " Liu Shixin
2022-09-13  7:13       ` Liu Shixin
2022-09-19 15:01       ` Michal Hocko
2022-09-13 10:34 ` [PATCH v2] " Muchun Song
2022-09-13 10:34   ` Muchun Song

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.