Re: [PATCH v5 5/7] selftests: mm: Add a test for remapping to area immediately after existing mapping

[Joel Fernandes]

On Sun, Aug 27, 2023 at 10:42:44AM +0100, Lorenzo Stoakes wrote:
> On Tue, Aug 22, 2023 at 01:54:58AM +0000, Joel Fernandes (Google) wrote:
> > This patch adds support for verifying that we correctly handle the
> > situation where something is already mapped before the destination of the remap.
> >
> > Any realignment of destination address and PMD-copy will destroy that
> > existing mapping. In such cases, we need to avoid doing the optimization.
> >
> > To test this, we map an area called the preamble before the remap
> > region. Then we verify after the mremap operation that this region did not get
> > corrupted.
> >
> > Putting some prints in the kernel, I verified that we optimize
> > correctly in different situations:
> >
> > Optimize when there is alignment and no previous mapping (this is tested
> > by previous patch).
> > <prints>
> > can_align_down(old_vma->vm_start=2900000, old_addr=2900000, mask=-2097152): 0
> > can_align_down(new_vma->vm_start=2f00000, new_addr=2f00000, mask=-2097152): 0
> > === Starting move_page_tables ===
> > Doing PUD move for 2800000 -> 2e00000 of extent=200000 <-- Optimization
> > Doing PUD move for 2a00000 -> 3000000 of extent=200000
> > Doing PUD move for 2c00000 -> 3200000 of extent=200000
> > </prints>
> >
> > Don't optimize when there is alignment but there is previous mapping
> > (this is tested by this patch).
> > Notice that can_align_down() returns 1 for the destination mapping
> > as we detected there is something there.
> > <prints>
> > can_align_down(old_vma->vm_start=2900000, old_addr=2900000, mask=-2097152): 0
> > can_align_down(new_vma->vm_start=5700000, new_addr=5700000, mask=-2097152): 1
> > === Starting move_page_tables ===
> > Doing move_ptes for 2900000 -> 5700000 of extent=100000 <-- Unoptimized
> > Doing PUD move for 2a00000 -> 5800000 of extent=200000
> > Doing PUD move for 2c00000 -> 5a00000 of extent=200000
> > </prints>
> >
> 
> Have you additionally tested this by changing the code to be intentionally
> broken then running the test and observing it fail?

Yes I did! Because while developing the patch, it was broken many times and
the test failed during those times. ;-)

> Looks good to me,
> Reviewed-by: Lorenzo Stoakes <lstoakes@xxxxxxxxx>

Thanks!

 - Joel