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

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

[Suren Baghdasaryan]

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().

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

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

[Liam R. Howlett]

* 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, and
another that just outputs "not supported" or something.

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
> 

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

[Suren Baghdasaryan]

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
> >

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

[Suren Baghdasaryan]

On Tue, Jan 7, 2025 at 10:57 AM Suren Baghdasaryan <surenb@google.com> wrote:
>
> 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?

Hi Liam,
I have v8 ready for posting except for this patch which I'm not clear
how you wanted to change. I thought printing the raw vm_refcnt for
debugging would be consistent with how we output other fields, like
vm_flags for example (without interpreting each bit).
 Could you please clarify, preferably with a sample output, how you
would like to see vm_refcnt printed? I can also keep this patch out of
the series for now if we are not sure yet how it should be displayed.
It's not critical and can be added later.
Thanks,
Suren.

>
> >
> > 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
> > >