Re: [PATCH v2] selftests/mm: add simple VM_PFNMAP tests based on mmap'ing /dev/mem

[Lorenzo Stoakes <lorenzo.stoakes@oracle.com>]

On Mon, May 12, 2025 at 10:18:05AM +0200, David Hildenbrand wrote:
> On 09.05.25 17:55, Lorenzo Stoakes wrote:
> > On Fri, May 09, 2025 at 05:30:32PM +0200, David Hildenbrand wrote:
> > > Let's test some basic functionality using /dev/mem. These tests will
> > > implicitly cover some PAT (Page Attribute Handling) handling on x86.
> > >
> > > These tests will only run when /dev/mem access to the first two pages
> > > in physical address space is possible and allowed; otherwise, the tests
> > > are skipped.
> > >
> > > On current x86-64 with PAT inside a VM, all tests pass:
> > >
> > > 	TAP version 13
> > > 	1..6
> > > 	# Starting 6 tests from 1 test cases.
> > > 	#  RUN           pfnmap.madvise_disallowed ...
> > > 	#            OK  pfnmap.madvise_disallowed
> > > 	ok 1 pfnmap.madvise_disallowed
> > > 	#  RUN           pfnmap.munmap_split ...
> > > 	#            OK  pfnmap.munmap_split
> > > 	ok 2 pfnmap.munmap_split
> > > 	#  RUN           pfnmap.mremap_fixed ...
> > > 	#            OK  pfnmap.mremap_fixed
> > > 	ok 3 pfnmap.mremap_fixed
> > > 	#  RUN           pfnmap.mremap_shrink ...
> > > 	#            OK  pfnmap.mremap_shrink
> > > 	ok 4 pfnmap.mremap_shrink
> > > 	#  RUN           pfnmap.mremap_expand ...
> > > 	#            OK  pfnmap.mremap_expand
> > > 	ok 5 pfnmap.mremap_expand
> > > 	#  RUN           pfnmap.fork ...
> > > 	#            OK  pfnmap.fork
> > > 	ok 6 pfnmap.fork
> > > 	# PASSED: 6 / 6 tests passed.
> > > 	# Totals: pass:6 fail:0 xfail:0 xpass:0 skip:0 error:0
> > >
> > > However, we are able to trigger:
> > >
> > > [   27.888251] x86/PAT: pfnmap:1790 freeing invalid memtype [mem 0x00000000-0x00000fff]
> > >
> > > There are probably more things worth testing in the future, such as
> > > MAP_PRIVATE handling. But this set of tests is sufficient to cover most of
> > > the things we will rework regarding PAT handling.
> > >
> > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > Cc: Shuah Khan <shuah@kernel.org>
> > > Cc: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
> > > Cc: Ingo Molnar <mingo@redhat.com>
> > > Cc: Peter Xu <peterx@redhat.com>
> > > Cc: Dev Jain <dev.jain@arm.com>
> > > Signed-off-by: David Hildenbrand <david@redhat.com>
> >
> > Nice, big improvement!
> >
> > Reviewed-by: Lorenzo Stoakes <lorenzo.stoakes@oracle.com>
>
> Thanks! It was worth spending the time on using the harness.
>
> The FIXTURE_TEARDOWN() stuff is really confusing. It's not actually required
> to teardown most stuff (unless you create files in setup etc), because all
> tests are executed in a fork'ed child, where fd's, mappings, ... will go
> away immediately afterwards during the exit().

Yeah, it's maybe not always necessary, but stil handy, and at least allows for
strict cleanup/separation between tests.

And having things structured this way makes life much easier in many other
regards, as you have one place for it, you don't have to manually fiddle with
test counts etc. etc.

Overall I think it's a big win :)

>
> I still implemented FIXTURE_TEARDOWN (like everybody else), because maybe
> the manual teardown can find other issues not triggered during exit().

Ack!

>
> --
> Cheers,
>
> David / dhildenb
>

Cheers, Lorenzo