| Message ID | 20220908083452.2844125-1-liushixin2@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/memcontrol: use kstrtobool for swapaccount param parsing | expand |
On Thu, 8 Sep 2022 16:34:52 +0800 Liu Shixin <liushixin2@huawei.com> wrote: > --- a/mm/swap_cgroup.c > +++ b/mm/swap_cgroup.c > @@ -194,7 +194,7 @@ int swap_cgroup_swapon(int type, unsigned long max_pages) > return 0; > nomem: > pr_info("couldn't allocate enough memory for swap_cgroup\n"); > - pr_info("swap_cgroup can be disabled by swapaccount=0 boot option\n"); > + pr_info("swap_cgroup can be disabled by swapaccount=[oO][Ff]/N/n/0 boot option\n"); I'm not sure this really needed changing. "=0" was OK and the message now looks rather silly. But whatever.
On Thu 08-09-22 15:02:48, Andrew Morton wrote: > On Thu, 8 Sep 2022 16:34:52 +0800 Liu Shixin <liushixin2@huawei.com> wrote: > > > --- a/mm/swap_cgroup.c > > +++ b/mm/swap_cgroup.c > > @@ -194,7 +194,7 @@ int swap_cgroup_swapon(int type, unsigned long max_pages) > > return 0; > > nomem: > > pr_info("couldn't allocate enough memory for swap_cgroup\n"); > > - pr_info("swap_cgroup can be disabled by swapaccount=0 boot option\n"); > > + pr_info("swap_cgroup can be disabled by swapaccount=[oO][Ff]/N/n/0 boot option\n"); > > I'm not sure this really needed changing. "=0" was OK and the message > now looks rather silly. Agreed. While swapaccount=0 is clear the extended form, even if correct, might just confuse some admins.
On 2022/9/9 15:37, Michal Hocko wrote: > On Thu 08-09-22 15:02:48, Andrew Morton wrote: >> On Thu, 8 Sep 2022 16:34:52 +0800 Liu Shixin <liushixin2@huawei.com> wrote: >> >>> --- a/mm/swap_cgroup.c >>> +++ b/mm/swap_cgroup.c >>> @@ -194,7 +194,7 @@ int swap_cgroup_swapon(int type, unsigned long max_pages) >>> return 0; >>> nomem: >>> pr_info("couldn't allocate enough memory for swap_cgroup\n"); >>> - pr_info("swap_cgroup can be disabled by swapaccount=0 boot option\n"); >>> + pr_info("swap_cgroup can be disabled by swapaccount=[oO][Ff]/N/n/0 boot option\n"); >> I'm not sure this really needed changing. "=0" was OK and the message >> now looks rather silly. > Agreed. While swapaccount=0 is clear the extended form, even if correct, > might just confuse some admins. Thanks, the previous info is more appropriate, I will update the patch.
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); diff --git a/mm/swap_cgroup.c b/mm/swap_cgroup.c index 5a9442979a18..107dd0e94b01 100644 --- a/mm/swap_cgroup.c +++ b/mm/swap_cgroup.c @@ -194,7 +194,7 @@ int swap_cgroup_swapon(int type, unsigned long max_pages) return 0; nomem: pr_info("couldn't allocate enough memory for swap_cgroup\n"); - pr_info("swap_cgroup can be disabled by swapaccount=0 boot option\n"); + pr_info("swap_cgroup can be disabled by swapaccount=[oO][Ff]/N/n/0 boot option\n"); return -ENOMEM; }
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 ++++---- mm/swap_cgroup.c | 2 +- 3 files changed, 9 insertions(+), 8 deletions(-)