[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CADrL8HXbTzX8T-S57mUQ8ggJ=DGMcQAxJapiQCFndqjPNLV=-Q@mail.gmail.com>
Date: Tue, 10 Jan 2023 09:50:59 -0500
From: James Houghton <jthoughton@...gle.com>
To: Peter Xu <peterx@...hat.com>
Cc: Mike Kravetz <mike.kravetz@...cle.com>,
Muchun Song <songmuchun@...edance.com>,
David Hildenbrand <david@...hat.com>,
David Rientjes <rientjes@...gle.com>,
Axel Rasmussen <axelrasmussen@...gle.com>,
Mina Almasry <almasrymina@...gle.com>,
"Zach O'Keefe" <zokeefe@...gle.com>,
Manish Mishra <manish.mishra@...anix.com>,
Naoya Horiguchi <naoya.horiguchi@....com>,
"Dr . David Alan Gilbert" <dgilbert@...hat.com>,
"Matthew Wilcox (Oracle)" <willy@...radead.org>,
Vlastimil Babka <vbabka@...e.cz>,
Baolin Wang <baolin.wang@...ux.alibaba.com>,
Miaohe Lin <linmiaohe@...wei.com>,
Yang Shi <shy828301@...il.com>,
Andrew Morton <akpm@...ux-foundation.org>, linux-mm@...ck.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 34/46] hugetlb: userfaultfd: when using MADV_SPLIT, round
addresses to PAGE_SIZE
On Fri, Jan 6, 2023 at 10:13 AM Peter Xu <peterx@...hat.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 usable
> > address.
> >
> > Signed-off-by: James Houghton <jthoughton@...gle.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_userfault(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 == VM_UFFD_MINOR)
> > - haddr = address & PAGE_MASK;
> > -
> > - u32 hash;
> > - struct vm_fault vmf = {
> > - .vma = vma,
> > - .address = haddr,
> > - .real_address = addr,
> > - .flags = flags,
> > + haddr = 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 = vma;
> > + vmf.address = haddr;
> > + vmf.real_address = addr;
> > + vmf.flags = flags;
>
> Const fields here:
>
> mm/hugetlb.c: In function ‘hugetlb_handle_userfault’:
> mm/hugetlb.c:5961:17: error: assignment of member ‘vma’ in read-only object
> 5961 | vmf.vma = vma;
> | ^
> mm/hugetlb.c:5962:21: error: assignment of member ‘address’ in read-only object
> 5962 | vmf.address = haddr;
> | ^
> mm/hugetlb.c:5963:26: error: assignment of member ‘real_address’ in read-only object
> 5963 | vmf.real_address = 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.
Powered by blists - more mailing lists