Re: [PATCH v2 3/3] Turn off test_uffdio_wp if CONFIG_PTE_MARKER_UFFD_WP is not configured.

[Peter Xu]

On Mon, Jun 24, 2024 at 09:53:57AM -0400, Audra Mitchell wrote:
> On Fri, Jun 21, 2024 at 05:27:43PM -0400, Peter Xu wrote:
> > On Fri, Jun 21, 2024 at 02:12:24PM -0400, Audra Mitchell wrote:
> > > If CONFIG_PTE_MARKER_UFFD_WP is disabled, then testing with test_uffdio_up
> > 
> > Here you're talking about pte markers, then..
> > 
> > > enables calling uffdio_regsiter with the flag UFFDIO_REGISTER_MODE_WP. The
> > > kernel ensures in vma_can_userfault() that if CONFIG_PTE_MARKER_UFFD_WP
> > > is disabled, only allow the VM_UFFD_WP on anonymous vmas.
> > > 
> > > Signed-off-by: Audra Mitchell <audra@xxxxxxxxxx>
> > > ---
> > >  tools/testing/selftests/mm/uffd-stress.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/tools/testing/selftests/mm/uffd-stress.c b/tools/testing/selftests/mm/uffd-stress.c
> > > index b9b6d858eab8..2601c9dfadd6 100644
> > > --- a/tools/testing/selftests/mm/uffd-stress.c
> > > +++ b/tools/testing/selftests/mm/uffd-stress.c
> > > @@ -419,6 +419,9 @@ static void parse_test_type_arg(const char *raw_type)
> > >  	test_uffdio_wp = test_uffdio_wp &&
> > >  		(features & UFFD_FEATURE_PAGEFAULT_FLAG_WP);
> > >  
> > > +	if (test_type != TEST_ANON && !(features & UFFD_FEATURE_WP_UNPOPULATED))
> > > +		test_uffdio_wp = false;
> > 
> > ... here you're checking against wp_unpopulated.  I'm slightly confused.
> > 
> > Are you running this test over shmem/hugetlb when the WP feature isn't
> > supported?
> >
> > I'm wondering whether you're looking for UFFD_FEATURE_WP_HUGETLBFS_SHMEM
> > instead.
> 
> I can confirm, its all really confusing... So in userfaultfd_api, we disable
> three features if CONFIG_PTE_MARKER_UFFD_WP is not enabled- including 
> UFFD_FEATURE_WP_UNPOPULATED:
> 
> #ifndef CONFIG_PTE_MARKER_UFFD_WP
>         uffdio_api.features &= ~UFFD_FEATURE_WP_HUGETLBFS_SHMEM;
>         uffdio_api.features &= ~UFFD_FEATURE_WP_UNPOPULATED;
>         uffdio_api.features &= ~UFFD_FEATURE_WP_ASYNC;
> #endif
> 
> If you run the userfaultfd selftests with the run_vmtests script we get
> several failures stemming from trying to call uffdio_regsiter with the flag 
> UFFDIO_REGISTER_MODE_WP. However, the kernel ensures in vma_can_userfault() 
> that if CONFIG_PTE_MARKER_UFFD_WP is disabled, only allow the VM_UFFD_WP -
> which is set when you pass the UFFDIO_REGISTER_MODE_WP flag - on 
> anonymous vmas.
> 
> In parse_test_type_arg() I added the features check against 
> UFFD_FEATURE_WP_UNPOPULATED as it seemed the most well know feature/flag. I'm 
> more than happy to take any suggestions and adapt them if you have any! 

There're documents for these features in the headers:

	 * UFFD_FEATURE_WP_HUGETLBFS_SHMEM indicates that userfaultfd
	 * write-protection mode is supported on both shmem and hugetlbfs.
	 *
	 * UFFD_FEATURE_WP_UNPOPULATED indicates that userfaultfd
	 * write-protection mode will always apply to unpopulated pages
	 * (i.e. empty ptes).  This will be the default behavior for shmem
	 * & hugetlbfs, so this flag only affects anonymous memory behavior
	 * when userfault write-protection mode is registered.

While in this context ("test_type != TEST_ANON") IIUC the accurate feature
to check is UFFD_FEATURE_WP_HUGETLBFS_SHMEM.

In most kernels they should behave the same indeed, but note that since
UNPOPULATED was introduced later than shmem/hugetlb support, it means on
some kernel the result of checking these two features will be different.

Thanks,

-- 
Peter Xu