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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

> --
> Mike Kravetz





[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [eCos]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux