| Message ID | 20240213081634.3652326-1-hannes@cmpxchg.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm: memcontrol: clarify swapaccount=0 deprecation warning | expand |
On Tue, Feb 13, 2024 at 12:16 AM Johannes Weiner <hannes@cmpxchg.org> wrote: > > The swapaccount deprecation warning is throwing false positives. Since > we deprecated the knob and defaulted to enabling, the only reports > we've been getting are from folks that set swapaccount=1. While this > is a nice affirmation that always-enabling was the right choice, we > certainly don't want to warn when users request the supported mode. > > Only warn when disabling is requested, and clarify the warning. > > Fixes: b25806dcd3d5 ("mm: memcontrol: deprecate swapaccounting=0 mode") > Cc: stable@vger.kernel.org > Reported-by: "Jonas Schäfer" <jonas@wielicki.name> > Reported-by: Narcis Garcia <debianlists@actiu.net> > Suggested-by: Yosry Ahmed <yosryahmed@google.com> > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org> Reviewed-by: Yosry Ahmed <yosryahmed@google.com> > --- > mm/memcontrol.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 1ed40f9d3a27..107ec5d36819 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -7971,9 +7971,13 @@ bool mem_cgroup_swap_full(struct folio *folio) > > static int __init setup_swap_account(char *s) > { > - pr_warn_once("The swapaccount= commandline option is deprecated. " > - "Please report your usecase to linux-mm@kvack.org if you " > - "depend on this functionality.\n"); > + bool res; > + > + if (!kstrtobool(s, &res) && !res) > + pr_warn_once("The swapaccount=0 commdandline option is deprecated " > + "in favor of configuring swap control via cgroupfs. " > + "Please report your usecase to linux-mm@kvack.org if you " > + "depend on this functionality.\n"); This line is surely getting long, but I see other similar instances so I guess that's okay. > return 1; > } > __setup("swapaccount=", setup_swap_account); > -- > 2.43.0 >
On Tue 13-02-24 03:16:34, Johannes Weiner wrote: > The swapaccount deprecation warning is throwing false positives. Since > we deprecated the knob and defaulted to enabling, the only reports > we've been getting are from folks that set swapaccount=1. While this > is a nice affirmation that always-enabling was the right choice, we > certainly don't want to warn when users request the supported mode. > > Only warn when disabling is requested, and clarify the warning. > > Fixes: b25806dcd3d5 ("mm: memcontrol: deprecate swapaccounting=0 mode") > Cc: stable@vger.kernel.org > Reported-by: "Jonas Schäfer" <jonas@wielicki.name> > Reported-by: Narcis Garcia <debianlists@actiu.net> > Suggested-by: Yosry Ahmed <yosryahmed@google.com> > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org> Acked-by: Michal Hocko <mhocko@suse.com> Thanks! > --- > mm/memcontrol.c | 10 +++++++--- > 1 file changed, 7 insertions(+), 3 deletions(-) > > diff --git a/mm/memcontrol.c b/mm/memcontrol.c > index 1ed40f9d3a27..107ec5d36819 100644 > --- a/mm/memcontrol.c > +++ b/mm/memcontrol.c > @@ -7971,9 +7971,13 @@ bool mem_cgroup_swap_full(struct folio *folio) > > static int __init setup_swap_account(char *s) > { > - pr_warn_once("The swapaccount= commandline option is deprecated. " > - "Please report your usecase to linux-mm@kvack.org if you " > - "depend on this functionality.\n"); > + bool res; > + > + if (!kstrtobool(s, &res) && !res) > + pr_warn_once("The swapaccount=0 commdandline option is deprecated " > + "in favor of configuring swap control via cgroupfs. " > + "Please report your usecase to linux-mm@kvack.org if you " > + "depend on this functionality.\n"); > return 1; > } > __setup("swapaccount=", setup_swap_account); > -- > 2.43.0
On Tue, Feb 13, 2024 at 12:16 AM Johannes Weiner <hannes@cmpxchg.org> wrote: > > The swapaccount deprecation warning is throwing false positives. Since > we deprecated the knob and defaulted to enabling, the only reports > we've been getting are from folks that set swapaccount=1. While this > is a nice affirmation that always-enabling was the right choice, we > certainly don't want to warn when users request the supported mode. > > Only warn when disabling is requested, and clarify the warning. > > Fixes: b25806dcd3d5 ("mm: memcontrol: deprecate swapaccounting=0 mode") > Cc: stable@vger.kernel.org > Reported-by: "Jonas Schäfer" <jonas@wielicki.name> > Reported-by: Narcis Garcia <debianlists@actiu.net> > Suggested-by: Yosry Ahmed <yosryahmed@google.com> > Signed-off-by: Johannes Weiner <hannes@cmpxchg.org> Acked-by: Shakeel Butt <shakeelb@google.com>
On Tue, Feb 13, 2024 at 03:16:34AM -0500, Johannes Weiner <hannes@cmpxchg.org> wrote: > The swapaccount deprecation warning is throwing false positives. Since > we deprecated the knob and defaulted to enabling, the only reports > we've been getting are from folks that set swapaccount=1. While this > is a nice affirmation that always-enabling was the right choice, we > certainly don't want to warn when users request the supported mode. But shouldn't such users be still warned about effectively unused option? I think `return 0;` from the param handler should ensure that. > + if (!kstrtobool(s, &res) && !res) > + pr_warn_once("The swapaccount=0 commdandline option is deprecated " commandline Regards, Michal
diff --git a/mm/memcontrol.c b/mm/memcontrol.c index 1ed40f9d3a27..107ec5d36819 100644 --- a/mm/memcontrol.c +++ b/mm/memcontrol.c @@ -7971,9 +7971,13 @@ bool mem_cgroup_swap_full(struct folio *folio) static int __init setup_swap_account(char *s) { - pr_warn_once("The swapaccount= commandline option is deprecated. " - "Please report your usecase to linux-mm@kvack.org if you " - "depend on this functionality.\n"); + bool res; + + if (!kstrtobool(s, &res) && !res) + pr_warn_once("The swapaccount=0 commdandline option is deprecated " + "in favor of configuring swap control via cgroupfs. " + "Please report your usecase to linux-mm@kvack.org if you " + "depend on this functionality.\n"); return 1; } __setup("swapaccount=", setup_swap_account);
The swapaccount deprecation warning is throwing false positives. Since we deprecated the knob and defaulted to enabling, the only reports we've been getting are from folks that set swapaccount=1. While this is a nice affirmation that always-enabling was the right choice, we certainly don't want to warn when users request the supported mode. Only warn when disabling is requested, and clarify the warning. Fixes: b25806dcd3d5 ("mm: memcontrol: deprecate swapaccounting=0 mode") Cc: stable@vger.kernel.org Reported-by: "Jonas Schäfer" <jonas@wielicki.name> Reported-by: Narcis Garcia <debianlists@actiu.net> Suggested-by: Yosry Ahmed <yosryahmed@google.com> Signed-off-by: Johannes Weiner <hannes@cmpxchg.org> --- mm/memcontrol.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)