| Message ID | 20230105101844.1893104-35-jthoughton@google.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Based on latest mm-unstable (85b44c25cd1e). | expand |
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@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_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; > + /* > + * 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 > + */ PS: I think we can drop this along the way. > > /* > * vma_lock and hugetlb_fault_mutex must be dropped before handling > -- > 2.39.0.314.g84b9a713c41-goog >
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 usable > > 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_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.
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; + /* + * 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 + */ /* * vma_lock and hugetlb_fault_mutex must be dropped before handling
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@google.com> --- mm/hugetlb.c | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-)