Re: [PATCH 10/29] selftests/mm: Test UFFDIO_ZEROPAGE only when !hugetlb

[David Hildenbrand]

On 01.04.23 03:57, Axel Rasmussen wrote:
> On Fri, Mar 31, 2023 at 11:37 AM Mike Kravetz <mike.kravetz@xxxxxxxxxx> wrote:
> > On 03/30/23 12:07, Peter Xu wrote:
> > > Make the check as simple as "test_type == TEST_HUGETLB" because that's the
> > > only mem that doesn't support ZEROPAGE.
> > >
> > > Signed-off-by: Peter Xu <peterx@xxxxxxxxxx>
> > > ---
> > >   tools/testing/selftests/mm/userfaultfd.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/tools/testing/selftests/mm/userfaultfd.c b/tools/testing/selftests/mm/userfaultfd.c
> > > index 795fbc4d84f8..d724f1c78847 100644
> > > --- a/tools/testing/selftests/mm/userfaultfd.c
> > > +++ b/tools/testing/selftests/mm/userfaultfd.c
> > > @@ -1118,7 +1118,7 @@ static int __uffdio_zeropage(int ufd, unsigned long offset, bool retry)
> > >   {
> > >        struct uffdio_zeropage uffdio_zeropage;
> > >        int ret;
> > > -     bool has_zeropage = get_expected_ioctls(0) & (1 << _UFFDIO_ZEROPAGE);
> > > +     bool has_zeropage = !(test_type == TEST_HUGETLB);
> >
> > It is true that hugetlb is the only mem type that does not support zeropage.
> > So, the change is correct.
> >
> > However, I actually prefer the explicit check that is there today.  It seems
> > more like a test of the API.  And, is more future proof is code changes.
> >
> > Just my opinion/thoughts, not a strong objection.
>
> I agree. The existing code is more robust to future changes where we
> might support or stop supporting this ioctl in some cases. It also
> proves that the ioctl works, any time the API reports that it is
> supported / ought to work, independent of when the *test* thinks it
> should be supported.
>
> Then again, I think this is unlikely to change in the future, so I
> also agree with Mike that it's not the biggest deal.

As there were already discussions on eventually supporting 
UFFDIO_ZEROPAGE that doesn't place the shared zeropage but ... a fresh 
zeropage, it might make sense to keep it as is.

--
Thanks,

David / dhildenb