traversing vma on nommu

[Hajime Tazaki]

On Thu, 07 Nov 2024 23:22:51 +0900,
Liam R. Howlett wrote:

> > after adding mt_validate() across various places, I found
> > vma_iter_clear() in nommu.c triggers this issue on nommu UML.
> > 
> > I'm not totally understanding but if I changed the part with
> > vma_iter_clear_gfp(), the issue (and validation error reports) are
> > disappeared.
> > 
> > diff --git a/mm/nommu.c b/mm/nommu.c
> > index 385b0c15add8..b5c11bbd69de 100644
> > --- a/mm/nommu.c
> > +++ b/mm/nommu.c
> > @@ -581,7 +581,8 @@ static int delete_vma_from_mm(struct vm_area_struct *vma)
> >  	cleanup_vma_from_mm(vma);
> >  
> >  	/* remove from the MM's tree and list */
> > -	vma_iter_clear(&vmi);
> > +	vma_iter_clear_gfp(&vmi, vma->vm_start, vma->vm_end, GFP_KERNEL);
> > +	mt_validate(&current->mm->mm_mt);
> >  	return 0;
> >  }
> > 
> > do you think this is an appropriate fix ?
> 
> No.  Something is going wrong in regards to the vma iterator setup.  If
> the values are incorrect on the preallocation then you may not have
> enough memory to do the store.  You may end up in a situation where the
> vma remains in the tree but the vm_file is removed from the interval
> tree and that doesn't seem like a good idea.

thanks for the information.

> Can you check vma_iter_addr(vmi) and vma_iter_end(vmi) before and after
> the preallocation call?  I would expect them to have the vma start/end
> addresses.
> 
> Actually, if you can provide the maple state again during this process
> then it would be very helpful.

--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -573,15 +573,39 @@ static int delete_vma_from_mm(struct vm_area_struct *vma)
        VMA_ITERATOR(vmi, vma->vm_mm, vma->vm_start);
 
        vma_iter_config(&vmi, vma->vm_start, vma->vm_end);
+       if (!strcmp(current->comm, "apk")) {
+               pr_warn(">>>>>>>>>>>>>>> vma_iter_addr()=%lx, vma_iter_end()=%lx",
+                       vma_iter_addr(&vmi), vma_iter_end(&vmi));
+               pr_warn("               vma->start=%lx, vma->end=%lx",
+                       vma->vm_start, vma->vm_end);
+               mas_dump(&vmi.mas);
+       }
+
        if (vma_iter_prealloc(&vmi, vma)) {
                pr_warn("Allocation of vma tree for process %d failed\n",
                       current->pid);
                return -ENOMEM;
        }
+
+       if (!strcmp(current->comm, "apk")) {
+               pr_warn("<<<<<<<<<<<<<<< vma_iter_addr()=%lx, vma_iter_end()=%lx",
+                       vma_iter_addr(&vmi), vma_iter_end(&vmi));
+               pr_warn("               vma->start=%lx, vma->end=%lx",
+                       vma->vm_start, vma->vm_end);
+               mas_dump(&vmi.mas);
+       }
        cleanup_vma_from_mm(vma);
 
        /* remove from the MM's tree and list */
        vma_iter_clear(&vmi);
+
+       if (!strcmp(current->comm, "apk")) {
+               pr_warn("<<<<<<<<<<<<<<< vma_iter_addr()=%lx, vma_iter_end()=%lx",
+                       vma_iter_addr(&vmi), vma_iter_end(&vmi));
+               mas_dump(&vmi.mas);
+               mt_validate(&current->mm->mm_mt);
+               pr_warn("===========================================================");
+       }
        return 0;
 }
 /*

I got the output:

https://gist.github.com/thehajime/08306c3d36c06f3dfb48390ca2b929f0

# again, it's long even if suppressing with the process name...

I'm really not sure if this is what you requested.
Let me know if this isn't sufficient for you.

-- Hajime