linux-kernel - Re: [PATCH 10/29] selftests/mm: Test UFFDIO

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 31 Mar 2023 18:57:41 -0700
From:   Axel Rasmussen <axelrasmussen@...gle.com>
To:     Mike Kravetz <mike.kravetz@...cle.com>
Cc:     Peter Xu <peterx@...hat.com>, linux-mm@...ck.org,
        linux-kernel@...r.kernel.org, David Hildenbrand <david@...hat.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Andrea Arcangeli <aarcange@...hat.com>,
        Leonardo Bras Soares Passos <lsoaresp@...hat.com>,
        Mike Rapoport <rppt@...ux.vnet.ibm.com>,
        Nadav Amit <nadav.amit@...il.com>
Subject: Re: [PATCH 10/29] selftests/mm: Test UFFDIO_ZEROPAGE only when !hugetlb

On Fri, Mar 31, 2023 at 11:37 AM Mike Kravetz <mike.kravetz@...cle.com> 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@...hat.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/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