Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230330155707.3106228-1-peterx@redhat.com> <20230330160714.3106999-1-peterx@redhat.com>
 <20230331183726.GD12460@monkey>
In-Reply-To: <20230331183726.GD12460@monkey>
From:   Axel Rasmussen <axelrasmussen@google.com>
Date:   Fri, 31 Mar 2023 18:57:41 -0700
Message-ID: <CAJHvVcjOqShPeu3mYk2Xu1ZyMfFLuPCUp8+8nQ+CUyCj4nZVqA@mail.gmail.com>
Subject: Re: [PATCH 10/29] selftests/mm: Test UFFDIO_ZEROPAGE only when !hugetlb
To:     Mike Kravetz <mike.kravetz@oracle.com>
Cc:     Peter Xu <peterx@redhat.com>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, David Hildenbrand <david@redhat.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Andrea Arcangeli <aarcange@redhat.com>,
        Leonardo Bras Soares Passos <lsoaresp@redhat.com>,
        Mike Rapoport <rppt@linux.vnet.ibm.com>,
        Nadav Amit <nadav.amit@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, Mar 31, 2023 at 11:37=E2=80=AFAM Mike Kravetz <mike.kravetz@oracle.=
com> wrote:
>
> On 03/30/23 12:07, Peter Xu wrote:
> > Make the check as simple as "test_type =3D=3D TEST_HUGETLB" because tha=
t's the
> > only mem that doesn't support ZEROPAGE.
> >
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >  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/s=
elftests/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 lo=
ng offset, bool retry)
> >  {
> >       struct uffdio_zeropage uffdio_zeropage;
> >       int ret;
> > -     bool has_zeropage =3D get_expected_ioctls(0) & (1 << _UFFDIO_ZERO=
PAGE);
> > +     bool has_zeropage =3D !(test_type =3D=3D TEST_HUGETLB);
>
> It is true that hugetlb is the only mem type that does not support zeropa=
ge.
> So, the change is correct.
>
> However, I actually prefer the explicit check that is there today.  It se=
ems
> 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