diff mbox series

[18/34] mm: Update vma_iter_store() to use MAS_WARN_ON()

Message ID 20230425140955.3834476-19-Liam.Howlett@oracle.com (mailing list archive)
State New
Headers show
Series Maple tree mas_{next,prev}_range() and cleanup | expand

Commit Message

Liam R. Howlett April 25, 2023, 2:09 p.m. UTC
MAS_WARN_ON() will provide more information on the maple state and can
be more useful for debugging.  Use this version of WARN_ON() in the
debugging code when storing to the tree.

Signed-off-by: Liam R. Howlett <Liam.Howlett@oracle.com>
---
 mm/internal.h | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

Comments

Sergey Senozhatsky April 27, 2023, 1:07 a.m. UTC | #1
Cc-ing Petr

On (23/04/25 10:09), Liam R. Howlett wrote:
> +	if (MAS_WARN_ON(&vmi->mas, vmi->mas.node != MAS_START &&
> +			vmi->mas.index > vma->vm_start)) {
> +		printk("%lx > %lx\n", vmi->mas.index, vma->vm_start);
> +		printk("store of vma %lx-%lx", vma->vm_start, vma->vm_end);
> +		printk("into slot    %lx-%lx", vmi->mas.index, vmi->mas.last);
>  	}

[..]

> +	if (MAS_WARN_ON(&vmi->mas, vmi->mas.node != MAS_START &&
> +			vmi->mas.last <  vma->vm_start)) {
> +		printk("%lx < %lx\n", vmi->mas.last, vma->vm_start);
> +		printk("store of vma %lx-%lx", vma->vm_start, vma->vm_end);
> +		printk("into slot    %lx-%lx", vmi->mas.index, vmi->mas.last);
>  	}

Any reason for "store of vma" and "into slot" to be two separate
printk()-s? It's not guaranteed that these will be printed as a
continuous line. A single printk should work fine:

	pr_foo("store of vma %lx-%lx into slot    %lx-%lx", ...);

The line probably needs to be terminated with \n unless you expect
more pr_cont() printks after this, which doesn't look like a case.
Additionally, do you want to pass a specific loglevel? E.g. pr_debug()?
Liam R. Howlett April 27, 2023, 1:17 a.m. UTC | #2
* Sergey Senozhatsky <senozhatsky@chromium.org> [230426 21:07]:
> Cc-ing Petr
> 
> On (23/04/25 10:09), Liam R. Howlett wrote:
> > +	if (MAS_WARN_ON(&vmi->mas, vmi->mas.node != MAS_START &&
> > +			vmi->mas.index > vma->vm_start)) {
> > +		printk("%lx > %lx\n", vmi->mas.index, vma->vm_start);
> > +		printk("store of vma %lx-%lx", vma->vm_start, vma->vm_end);
> > +		printk("into slot    %lx-%lx", vmi->mas.index, vmi->mas.last);
> >  	}
> 
> [..]
> 
> > +	if (MAS_WARN_ON(&vmi->mas, vmi->mas.node != MAS_START &&
> > +			vmi->mas.last <  vma->vm_start)) {
> > +		printk("%lx < %lx\n", vmi->mas.last, vma->vm_start);
> > +		printk("store of vma %lx-%lx", vma->vm_start, vma->vm_end);
> > +		printk("into slot    %lx-%lx", vmi->mas.index, vmi->mas.last);
> >  	}
> 
> Any reason for "store of vma" and "into slot" to be two separate
> printk()-s? It's not guaranteed that these will be printed as a
> continuous line. A single printk should work fine:
> 
> 	pr_foo("store of vma %lx-%lx into slot    %lx-%lx", ...);
> 

Really, just for readability.  I'll split the string literal instead.

> The line probably needs to be terminated with \n unless you expect
> more pr_cont() printks after this, which doesn't look like a case.
> Additionally, do you want to pass a specific loglevel? E.g. pr_debug()?

Yes, I should do both of these, thanks.
diff mbox series

Patch

diff --git a/mm/internal.h b/mm/internal.h
index 8d1a8bd001247..76612a860e58e 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -1047,18 +1047,17 @@  static inline void vma_iter_store(struct vma_iterator *vmi,
 {
 
 #if defined(CONFIG_DEBUG_VM_MAPLE_TREE)
-	if (WARN_ON(vmi->mas.node != MAS_START && vmi->mas.index > vma->vm_start)) {
-		printk("%lu > %lu\n", vmi->mas.index, vma->vm_start);
-		printk("store of vma %lu-%lu", vma->vm_start, vma->vm_end);
-		printk("into slot    %lu-%lu", vmi->mas.index, vmi->mas.last);
-		vma_iter_dump_tree(vmi);
+	if (MAS_WARN_ON(&vmi->mas, vmi->mas.node != MAS_START &&
+			vmi->mas.index > vma->vm_start)) {
+		printk("%lx > %lx\n", vmi->mas.index, vma->vm_start);
+		printk("store of vma %lx-%lx", vma->vm_start, vma->vm_end);
+		printk("into slot    %lx-%lx", vmi->mas.index, vmi->mas.last);
 	}
-	if (WARN_ON(vmi->mas.node != MAS_START && vmi->mas.last <  vma->vm_start)) {
-		printk("%lu < %lu\n", vmi->mas.last, vma->vm_start);
-		printk("store of vma %lu-%lu", vma->vm_start, vma->vm_end);
-		printk("into slot    %lu-%lu", vmi->mas.index, vmi->mas.last);
-		mt_dump(vmi->mas.tree, mt_dump_hex);
-		vma_iter_dump_tree(vmi);
+	if (MAS_WARN_ON(&vmi->mas, vmi->mas.node != MAS_START &&
+			vmi->mas.last <  vma->vm_start)) {
+		printk("%lx < %lx\n", vmi->mas.last, vma->vm_start);
+		printk("store of vma %lx-%lx", vma->vm_start, vma->vm_end);
+		printk("into slot    %lx-%lx", vmi->mas.index, vmi->mas.last);
 	}
 #endif