Re: swapaccount= commandline option

[Yosry Ahmed <yosryahmed@google.com>]

On Tue, Feb 13, 2024 at 12:06 AM Michal Hocko <mhocko@suse.com> wrote:
>
> On Tue 13-02-24 08:01:37, Narcis Garcia wrote:
> > El 12/2/24 a les 23:37, Yosry Ahmed ha escrit:
> > > On Sun, Feb 11, 2024 at 1:33 AM Narcis Garcia <debianlists@actiu.net> wrote:
> > > >
> > > > Hello,
> > > >
> > > > I read this warning on Linux 6.1 boot:
> > > > The swapaccount= commandline option is deprecated. Please report your
> > > > usecase to linux-mm@kvack.org if you depend on this functionality.
> > > >
> > > > And I want to know if this impacts on LinuX Containers (LXC) swap
> > > > accounting.
> > >
> > > swapaccount= was deprecated in the sense that it is always on now
> > > (i.e. you cannot do swapaccount=0 anymore). I believe you are using
> > > swapaccount=1 and want to make sure that swap accounting will still
> > > work with v6.1, right?
> > >
> > > This is the second question we have had about this message [1]. I
> > > think we need to clarify the warning (and perhaps backport to stable
> > > kernels).
> > >
> > > [1]https://lore.kernel.org/linux-mm/2593755.Xrv9ptJ69j@sinistra.local/
> >
> >
> > Thank you for answering this question, and sure this clarification is needed
> > both in documentation and warning message.
> > I've found deprecation message was commited on 26 Sep 2022.
> > From what Linux version release, this deprecation is included?
> >
> > I suggest this for setup_swap_account() code:
> > pr_warn_once("The swapaccount= commandline option is deprecated. "
> >       "Swap accounting is always on. "
> >       "Please report your usecase to linux-mm@kvack.org if you "
> >       "depend on this functionality.\n");
>
> Would it make more sense to warn only if somebody tries to disable the
> swap accounting?
> ---
> diff --git a/mm/memcontrol.c b/mm/memcontrol.c
> index e4c8735e7c85..dec75eedf634 100644
> --- a/mm/memcontrol.c
> +++ b/mm/memcontrol.c
> @@ -7936,7 +7936,10 @@ 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. "
> +       bool res;
> +
> +       if (kstrtobool(s, &res) || !res)
> +               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");
>         return 1;
> --

Yes, this makes sense to me, but I still think explicitly specifying
that swap accounting is always on now would make things even clearer.