Re: [PATCH v8 14/21] mm/mmap: Avoid zeroing vma tree in mmap_region()

[Lorenzo Stoakes]

On Wed, Oct 02, 2024 at 09:39:27AM GMT, Lorenzo Stoakes wrote:
> On Tue, Oct 01, 2024 at 07:01:41PM GMT, Lorenzo Stoakes wrote:
> > On Tue, Oct 01, 2024 at 06:43:35PM GMT, Bert Karwatzki wrote:
> > [snip]
> > > I applied this patch to linux-next-20240110 (it applied cleany) and got the same
> > > error again (Andrew Morton asked on bugzilla me to put the logs into mails):
>
> Hi Bert,
>
> Could you possibly apply the below hacky patch (again using the 1st oct next
> tree as a base), and share the dmesg output?
>
> Hopefully this should give us some more information and test some theories.
>
> Thanks, again appreciate all your help with this!
>
> Cheers, Lorenzo
>
> ----8<----
> From 36c5c148580c5aaf93f0f689c2c3de36ff62f1ba Mon Sep 17 00:00:00 2001
> From: Lorenzo Stoakes <lorenzo.stoakes@xxxxxxxxxx>
> Date: Wed, 2 Oct 2024 09:19:28 +0100
> Subject: [PATCH] hack: mm: see if we can get some more information
>
> Add some dreadful printk() hacks so we can try to get some more information
> on what's going on.
> ---
>  mm/internal.h | 15 +++++++++++++++
>  mm/mmap.c     | 20 ++++++++++++++++++++
>  mm/vma.c      | 11 +++++++++++
>  3 files changed, 46 insertions(+)
>
> diff --git a/mm/internal.h b/mm/internal.h
> index 93083bbeeefa..cd9414b4651d 100644
> --- a/mm/internal.h
> +++ b/mm/internal.h
> @@ -1443,4 +1443,19 @@ static inline void accept_page(struct page *page)
>  }
>  #endif /* CONFIG_UNACCEPTED_MEMORY */
>
> +static inline bool check_interesting(unsigned long start, unsigned long end)
> +{
> +	const unsigned long interesting_start = 0x1740000;
> +	/* Include off-by-one on purpose. */
> +	const unsigned long interesting_end = 0x68000000 + 1;

Sorry to be a pain, could you update 0x68000000 to 0x798b1000 here?

Thanks!

> +
> +	/*  interesting_start            interesting_end
> +	 *          |--------------------------|
> +	 *           ============================> end
> +	 *        <=============================   start
> +	 */
> +	return end > interesting_start && /* after or overlaps... */
> +		start < interesting_end;  /* ...overlaps. */
> +}
> +
>  #endif	/* __MM_INTERNAL_H */
> diff --git a/mm/mmap.c b/mm/mmap.c
> index dd4b35a25aeb..0ed27e558ebb 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1341,6 +1341,18 @@ struct vm_area_struct *expand_stack(struct mm_struct *mm, unsigned long addr)
>  	return vma;
>  }
>
> +static void ljs_dump(struct mm_struct *mm,
> +		     unsigned long addr, unsigned long len,
> +		     vm_flags_t vm_flags, bool is_unmap)
> +{
> +	if (!check_interesting(addr, addr + len))
> +		return;
> +
> +	pr_err("LJS: %s mm=%p [0x%lx, 0x%lx) [vm_flags=%lu]\n",
> +	       is_unmap ? "munmap" : "mmap", mm, addr, addr + len,
> +		vm_flags);
> +}
> +
>  /* do_munmap() - Wrapper function for non-maple tree aware do_munmap() calls.
>   * @mm: The mm_struct
>   * @start: The start address to munmap
> @@ -1354,6 +1366,8 @@ int do_munmap(struct mm_struct *mm, unsigned long start, size_t len,
>  {
>  	VMA_ITERATOR(vmi, mm, start);
>
> +	ljs_dump(mm, start, len, 0, true);
> +
>  	return do_vmi_munmap(&vmi, mm, start, len, uf, false);
>  }
>
> @@ -1375,11 +1389,17 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
>  	VMA_ITERATOR(vmi, mm, addr);
>  	VMG_STATE(vmg, mm, &vmi, addr, end, vm_flags, pgoff);
>
> +	ljs_dump(mm, addr, len, vm_flags, false);
> +
>  	vmg.file = file;
>  	/* Find the first overlapping VMA */
>  	vma = vma_find(&vmi, end);
>  	init_vma_munmap(&vms, &vmi, vma, addr, end, uf, /* unlock = */ false);
>  	if (vma) {
> +		if (check_interesting(addr, addr + len))
> +			pr_err("LJS: mm=%p First VMA we unmap is [%lx, %lx)\n",
> +			       vma->vm_mm, vma->vm_start, vma->vm_end);
> +
>  		mt_init_flags(&mt_detach, vmi.mas.tree->ma_flags & MT_FLAGS_LOCK_MASK);
>  		mt_on_stack(mt_detach);
>  		mas_init(&mas_detach, &mt_detach, /* addr = */ 0);
> diff --git a/mm/vma.c b/mm/vma.c
> index 4737afcb064c..989ea3ce366d 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -1202,6 +1202,11 @@ int vms_gather_munmap_vmas(struct vma_munmap_struct *vms,
>  			goto start_split_failed;
>  		}
>
> +		if (check_interesting(vms->vma->vm_start, vms->vma->vm_end))
> +			pr_err("LJS: mm=%p vms=[%lx, %lx) split START of [%lx, %lx)\n",
> +			       vms->vma->vm_mm, vms->start, vms->end,
> +			       vms->vma->vm_start, vms->vma->vm_end);
> +
>  		error = __split_vma(vms->vmi, vms->vma, vms->start, 1);
>  		if (error)
>  			goto start_split_failed;
> @@ -1223,6 +1228,12 @@ int vms_gather_munmap_vmas(struct vma_munmap_struct *vms,
>  		}
>  		/* Does it split the end? */
>  		if (next->vm_end > vms->end) {
> +
> +			if (check_interesting(next->vm_start, next->vm_end))
> +				pr_err("LJS: mm=%p vms=[%lx, %lx) split END of [%lx, %lx)\n",
> +				       next->vm_mm, vms->start, vms->end,
> +				       next->vm_start, next->vm_end);
> +
>  			error = __split_vma(vms->vmi, next, vms->end, 0);
>  			if (error)
>  				goto end_split_failed;
> --
> 2.46.2