Re: [PATCH 1/1] fixup for mm/debug: print vm_refcnt state when dumping the vma

[Suren Baghdasaryan <surenb@google.com>]

On Tue, Jan 7, 2025 at 10:50 AM Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
>
> * Suren Baghdasaryan <surenb@google.com> [241226 15:03]:
> > vma->vm_refcnt does not exist when CONFIG_PER_VMA_LOCK=n, therefore
> > when CONFIG_PER_VMA_LOCK=n and CONFIG_DEBUG_VM=y we need to avoid its
> > usage in dump_vma().
>
> It might be worth having a function that decodes the information in the
> vma lock and outputs a string that spells out what is going on,

You mean something like "attached, 2 reader(s), 0 writer(s)" instead
of "refcnt 0x3" ?

> and
> another that just outputs "not supported" or something.

Do you mean we should report "not supported" when CONFIG_PER_VMA_LOCK=n?

>
> This has the benefit of spelling out the state and only having one place
> to update in future dump_vma() changes.
>
> >
> > Reported-by: kernel test robot <lkp@intel.com>
> > Closes: https://lore.kernel.org/oe-kbuild-all/202412270328.HiJX1mRt-lkp@intel.com/
> > Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> > ---
> > Fixes https://lore.kernel.org/all/20241226170710.1159679-14-surenb@google.com/
> > posted over mm-unstable.
> >
> >  mm/debug.c | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/mm/debug.c b/mm/debug.c
> > index 68b3ba3cf603..9c4cbc3733b0 100644
> > --- a/mm/debug.c
> > +++ b/mm/debug.c
> > @@ -178,6 +178,7 @@ EXPORT_SYMBOL(dump_page);
> >
> >  void dump_vma(const struct vm_area_struct *vma)
> >  {
> > +#ifdef CONFIG_PER_VMA_LOCK
> >       pr_emerg("vma %px start %px end %px mm %px\n"
> >               "prot %lx anon_vma %px vm_ops %px\n"
> >               "pgoff %lx file %px private_data %px\n"
> > @@ -187,6 +188,17 @@ void dump_vma(const struct vm_area_struct *vma)
> >               vma->anon_vma, vma->vm_ops, vma->vm_pgoff,
> >               vma->vm_file, vma->vm_private_data,
> >               vma->vm_flags, &vma->vm_flags, refcount_read(&vma->vm_refcnt));
> > +#else
> > +     pr_emerg("vma %px start %px end %px mm %px\n"
> > +             "prot %lx anon_vma %px vm_ops %px\n"
> > +             "pgoff %lx file %px private_data %px\n"
> > +             "flags: %#lx(%pGv)\n",
> > +             vma, (void *)vma->vm_start, (void *)vma->vm_end, vma->vm_mm,
> > +             (unsigned long)pgprot_val(vma->vm_page_prot),
> > +             vma->anon_vma, vma->vm_ops, vma->vm_pgoff,
> > +             vma->vm_file, vma->vm_private_data,
> > +             vma->vm_flags, &vma->vm_flags);
> > +#endif
> >  }
> >  EXPORT_SYMBOL(dump_vma);
> >
> > --
> > 2.47.1.613.gc27f4b7a9f-goog
> >