vm_refcnt encodes a number of useful states:
- whether vma is attached or detached
- the number of current vma readers
- presence of a vma writer
Let's include it in the vma dump.
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
Acked-by: Vlastimil Babka <vbabka@suse.cz>
---
mm/debug.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/mm/debug.c b/mm/debug.c
index 8d2acf432385..325d7bf22038 100644
--- a/mm/debug.c
+++ b/mm/debug.c
@@ -178,6 +178,17 @@ 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"
+ "flags: %#lx(%pGv) refcnt %x\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, 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"
@@ -187,6 +198,7 @@ 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);
+#endif
}
EXPORT_SYMBOL(dump_vma);
--
2.47.1.613.gc27f4b7a9f-googOn Fri, Jan 10, 2025 at 08:26:00PM -0800, Suren Baghdasaryan wrote:
> vm_refcnt encodes a number of useful states:
> - whether vma is attached or detached
> - the number of current vma readers
> - presence of a vma writer
> Let's include it in the vma dump.
>
> Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> Acked-by: Vlastimil Babka <vbabka@suse.cz>
> ---
> mm/debug.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/mm/debug.c b/mm/debug.c
> index 8d2acf432385..325d7bf22038 100644
> --- a/mm/debug.c
> +++ b/mm/debug.c
> @@ -178,6 +178,17 @@ 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"
> + "flags: %#lx(%pGv) refcnt %x\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, 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"
> @@ -187,6 +198,7 @@ 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);
> +#endif
> }
This is pretty horribly duplicative and not in line with how this kind of
thing is done in the rest of the file. You're just adding one entry, so why
not:
void dump_vma(const struct vm_area_struct *vma)
{
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"
#ifdef CONFIG_PER_VMA_LOCK
"refcnt %x\n"
#endif
"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,
#ifdef CONFIG_PER_VMA_LOCK
refcount_read(&vma->vm_refcnt),
#endif
&vma->vm_flags);
}
?
> EXPORT_SYMBOL(dump_vma);
>
> --
> 2.47.1.613.gc27f4b7a9f-goog
>
* Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [250113 11:21]:
> On Fri, Jan 10, 2025 at 08:26:00PM -0800, Suren Baghdasaryan wrote:
> > vm_refcnt encodes a number of useful states:
> > - whether vma is attached or detached
> > - the number of current vma readers
> > - presence of a vma writer
> > Let's include it in the vma dump.
> >
> > Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> > Acked-by: Vlastimil Babka <vbabka@suse.cz>
> > ---
> > mm/debug.c | 12 ++++++++++++
> > 1 file changed, 12 insertions(+)
> >
> > diff --git a/mm/debug.c b/mm/debug.c
> > index 8d2acf432385..325d7bf22038 100644
> > --- a/mm/debug.c
> > +++ b/mm/debug.c
> > @@ -178,6 +178,17 @@ 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"
> > + "flags: %#lx(%pGv) refcnt %x\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, 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"
> > @@ -187,6 +198,7 @@ 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);
> > +#endif
> > }
>
> This is pretty horribly duplicative and not in line with how this kind of
> thing is done in the rest of the file. You're just adding one entry, so why
> not:
>
> void dump_vma(const struct vm_area_struct *vma)
> {
> 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"
> #ifdef CONFIG_PER_VMA_LOCK
> "refcnt %x\n"
> #endif
> "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,
> #ifdef CONFIG_PER_VMA_LOCK
> refcount_read(&vma->vm_refcnt),
> #endif
> &vma->vm_flags);
> }
right, I had an issue with this as well.
Another option would be:
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",
<big mess here>);
dump_vma_refcnt();
pr_emerg("flags:...", vma_vm_flags);
Then dump_vma_refcnt() either dumps the refcnt or does nothing,
depending on the config option.
Either way is good with me. Lorenzo's suggestion is in line with the
file and it's clear as to why the refcnt might be missing, but I don't
really see this being an issue in practice.
Thanks,
Liam
On Mon, Jan 13, 2025 at 8:35 AM Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
>
> * Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [250113 11:21]:
> > On Fri, Jan 10, 2025 at 08:26:00PM -0800, Suren Baghdasaryan wrote:
> > > vm_refcnt encodes a number of useful states:
> > > - whether vma is attached or detached
> > > - the number of current vma readers
> > > - presence of a vma writer
> > > Let's include it in the vma dump.
> > >
> > > Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> > > Acked-by: Vlastimil Babka <vbabka@suse.cz>
> > > ---
> > > mm/debug.c | 12 ++++++++++++
> > > 1 file changed, 12 insertions(+)
> > >
> > > diff --git a/mm/debug.c b/mm/debug.c
> > > index 8d2acf432385..325d7bf22038 100644
> > > --- a/mm/debug.c
> > > +++ b/mm/debug.c
> > > @@ -178,6 +178,17 @@ 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"
> > > + "flags: %#lx(%pGv) refcnt %x\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, 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"
> > > @@ -187,6 +198,7 @@ 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);
> > > +#endif
> > > }
> >
> > This is pretty horribly duplicative and not in line with how this kind of
> > thing is done in the rest of the file. You're just adding one entry, so why
> > not:
> >
> > void dump_vma(const struct vm_area_struct *vma)
> > {
> > 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"
> > #ifdef CONFIG_PER_VMA_LOCK
> > "refcnt %x\n"
> > #endif
> > "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,
> > #ifdef CONFIG_PER_VMA_LOCK
> > refcount_read(&vma->vm_refcnt),
> > #endif
> > &vma->vm_flags);
> > }
>
> right, I had an issue with this as well.
>
> Another option would be:
>
> 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",
> <big mess here>);
> dump_vma_refcnt();
> pr_emerg("flags:...", vma_vm_flags);
>
>
> Then dump_vma_refcnt() either dumps the refcnt or does nothing,
> depending on the config option.
>
> Either way is good with me. Lorenzo's suggestion is in line with the
> file and it's clear as to why the refcnt might be missing, but I don't
> really see this being an issue in practice.
Thanks for clarifying! Lorenzo's suggestion LGTM too. I'll adopt it. Thanks!
>
> Thanks,
> Liam
>
On Mon, Jan 13, 2025 at 09:57:54AM -0800, Suren Baghdasaryan wrote:
> On Mon, Jan 13, 2025 at 8:35 AM Liam R. Howlett <Liam.Howlett@oracle.com> wrote:
> >
> > * Lorenzo Stoakes <lorenzo.stoakes@oracle.com> [250113 11:21]:
> > > On Fri, Jan 10, 2025 at 08:26:00PM -0800, Suren Baghdasaryan wrote:
> > > > vm_refcnt encodes a number of useful states:
> > > > - whether vma is attached or detached
> > > > - the number of current vma readers
> > > > - presence of a vma writer
> > > > Let's include it in the vma dump.
> > > >
> > > > Signed-off-by: Suren Baghdasaryan <surenb@google.com>
> > > > Acked-by: Vlastimil Babka <vbabka@suse.cz>
> > > > ---
> > > > mm/debug.c | 12 ++++++++++++
> > > > 1 file changed, 12 insertions(+)
> > > >
> > > > diff --git a/mm/debug.c b/mm/debug.c
> > > > index 8d2acf432385..325d7bf22038 100644
> > > > --- a/mm/debug.c
> > > > +++ b/mm/debug.c
> > > > @@ -178,6 +178,17 @@ 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"
> > > > + "flags: %#lx(%pGv) refcnt %x\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, 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"
> > > > @@ -187,6 +198,7 @@ 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);
> > > > +#endif
> > > > }
> > >
> > > This is pretty horribly duplicative and not in line with how this kind of
> > > thing is done in the rest of the file. You're just adding one entry, so why
> > > not:
> > >
> > > void dump_vma(const struct vm_area_struct *vma)
> > > {
> > > 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"
> > > #ifdef CONFIG_PER_VMA_LOCK
> > > "refcnt %x\n"
> > > #endif
> > > "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,
> > > #ifdef CONFIG_PER_VMA_LOCK
> > > refcount_read(&vma->vm_refcnt),
> > > #endif
> > > &vma->vm_flags);
> > > }
> >
> > right, I had an issue with this as well.
> >
> > Another option would be:
> >
> > 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",
> > <big mess here>);
> > dump_vma_refcnt();
> > pr_emerg("flags:...", vma_vm_flags);
> >
> >
> > Then dump_vma_refcnt() either dumps the refcnt or does nothing,
> > depending on the config option.
> >
> > Either way is good with me. Lorenzo's suggestion is in line with the
> > file and it's clear as to why the refcnt might be missing, but I don't
> > really see this being an issue in practice.
>
> Thanks for clarifying! Lorenzo's suggestion LGTM too. I'll adopt it. Thanks!
>
Cheers guys!
> >
> > Thanks,
> > Liam
> >
© 2016 - 2026 Red Hat, Inc.