| Message ID | 20220909084647.3598299-1-liushixin2@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v2] mm/memcontrol: use kstrtobool for swapaccount param parsing | expand |
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.
> 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.
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);
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(-)