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: <20230105101844.1893104-1-jthoughton@google.com>
 <20230105101844.1893104-35-jthoughton@google.com> <Y7g6ihsCTIC765CO@x1n>
In-Reply-To: <Y7g6ihsCTIC765CO@x1n>
From:   James Houghton <jthoughton@google.com>
Date:   Tue, 10 Jan 2023 09:50:59 -0500
Message-ID: <CADrL8HXbTzX8T-S57mUQ8ggJ=DGMcQAxJapiQCFndqjPNLV=-Q@mail.gmail.com>
Subject: Re: [PATCH 34/46] hugetlb: userfaultfd: when using MADV_SPLIT, round
 addresses to PAGE_SIZE
To:     Peter Xu <peterx@redhat.com>
Cc:     Mike Kravetz <mike.kravetz@oracle.com>,
        Muchun Song <songmuchun@bytedance.com>,
        David Hildenbrand <david@redhat.com>,
        David Rientjes <rientjes@google.com>,
        Axel Rasmussen <axelrasmussen@google.com>,
        Mina Almasry <almasrymina@google.com>,
        "Zach O'Keefe" <zokeefe@google.com>,
        Manish Mishra <manish.mishra@nutanix.com>,
        Naoya Horiguchi <naoya.horiguchi@nec.com>,
        "Dr . David Alan Gilbert" <dgilbert@redhat.com>,
        "Matthew Wilcox (Oracle)" <willy@infradead.org>,
        Vlastimil Babka <vbabka@suse.cz>,
        Baolin Wang <baolin.wang@linux.alibaba.com>,
        Miaohe Lin <linmiaohe@huawei.com>,
        Yang Shi <shy828301@gmail.com>,
        Andrew Morton <akpm@linux-foundation.org>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, Jan 6, 2023 at 10:13 AM Peter Xu <peterx@redhat.com> wrote:
>
> On Thu, Jan 05, 2023 at 10:18:32AM +0000, James Houghton wrote:
> > MADV_SPLIT enables HugeTLB HGM which allows for UFFDIO_CONTINUE in
> > PAGE_SIZE chunks. If a huge-page-aligned address were to be provided,
> > userspace would be completely unable to take advantage of HGM. That
> > would then require userspace to know to provide
> > UFFD_FEATURE_EXACT_ADDRESS.
> >
> > This patch would make it harder to make a mistake. Instead of requiring
> > userspace to provide UFFD_FEATURE_EXACT_ADDRESS, always provide a usabl=
e
> > address.
> >
> > Signed-off-by: James Houghton <jthoughton@google.com>
> > ---
> >  mm/hugetlb.c | 31 +++++++++++++++----------------
> >  1 file changed, 15 insertions(+), 16 deletions(-)
> >
> > diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> > index 5af6db52f34e..5b6215e03fe1 100644
> > --- a/mm/hugetlb.c
> > +++ b/mm/hugetlb.c
> > @@ -5936,28 +5936,27 @@ static inline vm_fault_t hugetlb_handle_userfau=
lt(struct vm_area_struct *vma,
> >                                                 unsigned long addr,
> >                                                 unsigned long reason)
> >  {
> > +     u32 hash;
> > +     struct vm_fault vmf;
> > +
> >       /*
> >        * Don't use the hpage-aligned address if the user has explicitly
> >        * enabled HGM.
> >        */
> >       if (hugetlb_hgm_advised(vma) && reason =3D=3D VM_UFFD_MINOR)
> > -             haddr =3D address & PAGE_MASK;
> > -
> > -     u32 hash;
> > -     struct vm_fault vmf =3D {
> > -             .vma =3D vma,
> > -             .address =3D haddr,
> > -             .real_address =3D addr,
> > -             .flags =3D flags,
> > +             haddr =3D addr & PAGE_MASK;
> >
> > -             /*
> > -              * Hard to debug if it ends up being
> > -              * used by a callee that assumes
> > -              * something about the other
> > -              * uninitialized fields... same as in
> > -              * memory.c
> > -              */
> > -     };
> > +     vmf.vma =3D vma;
> > +     vmf.address =3D haddr;
> > +     vmf.real_address =3D addr;
> > +     vmf.flags =3D flags;
>
> Const fields here:
>
> mm/hugetlb.c: In function =E2=80=98hugetlb_handle_userfault=E2=80=99:
> mm/hugetlb.c:5961:17: error: assignment of member =E2=80=98vma=E2=80=99 i=
n read-only object
>  5961 |         vmf.vma =3D vma;
>       |                 ^
> mm/hugetlb.c:5962:21: error: assignment of member =E2=80=98address=E2=80=
=99 in read-only object
>  5962 |         vmf.address =3D haddr;
>       |                     ^
> mm/hugetlb.c:5963:26: error: assignment of member =E2=80=98real_address=
=E2=80=99 in read-only object
>  5963 |         vmf.real_address =3D addr;

Thanks Peter for this and your other findings. Not sure why my
compiler (clang) let me do this. :/ Will send a v2 soon with this +
the other problems fixed.