On Wed, Jun 02, 2021 at 10:30:13AM +0200, Christian König wrote: > We discussed if that is really the right approach for quite a while now, but > digging deeper into a bug report on arm turned out that this is actually > horrible broken right now. > > The reason for this is that vmf_insert_mixed_prot() always tries to grab > a reference to the underlaying page on architectures without > ARCH_HAS_PTE_SPECIAL and as far as I can see also enabled GUP. > > So nuke using VM_MIXEDMAP here and use VM_PFNMAP instead. > > Also set VM_SHARED, not 100% sure if that is needed with VM_PFNMAP, but better > save than sorry. > > Signed-off-by: Christian König <christian.koenig@xxxxxxx> > Bugs: https://gitlab.freedesktop.org/drm/amd/-/issues/1606#note_936174 I thought we still have the same issue open for ttm_bo_vm_insert_huge()? Or at least a potentially pretty big bug, because our current huge entries don't stop gup (because there's no pmd_mkspecial right now in the kernel). So I think if you want to close this for good then we also need to (temporarily at least) disable the huge entry code? -Daniel > --- > drivers/gpu/drm/ttm/ttm_bo_vm.c | 29 +++++++---------------------- > 1 file changed, 7 insertions(+), 22 deletions(-) > > diff --git a/drivers/gpu/drm/ttm/ttm_bo_vm.c b/drivers/gpu/drm/ttm/ttm_bo_vm.c > index 9bd15cb39145..bf86ae849340 100644 > --- a/drivers/gpu/drm/ttm/ttm_bo_vm.c > +++ b/drivers/gpu/drm/ttm/ttm_bo_vm.c > @@ -359,12 +359,7 @@ vm_fault_t ttm_bo_vm_fault_reserved(struct vm_fault *vmf, > * at arbitrary times while the data is mmap'ed. > * See vmf_insert_mixed_prot() for a discussion. > */ > - if (vma->vm_flags & VM_MIXEDMAP) > - ret = vmf_insert_mixed_prot(vma, address, > - __pfn_to_pfn_t(pfn, PFN_DEV), > - prot); > - else > - ret = vmf_insert_pfn_prot(vma, address, pfn, prot); > + ret = vmf_insert_pfn_prot(vma, address, pfn, prot); > > /* Never error on prefaulted PTEs */ > if (unlikely((ret & VM_FAULT_ERROR))) { > @@ -411,15 +406,9 @@ vm_fault_t ttm_bo_vm_dummy_page(struct vm_fault *vmf, pgprot_t prot) > pfn = page_to_pfn(page); > > /* Prefault the entire VMA range right away to avoid further faults */ > - for (address = vma->vm_start; address < vma->vm_end; address += PAGE_SIZE) { > - > - if (vma->vm_flags & VM_MIXEDMAP) > - ret = vmf_insert_mixed_prot(vma, address, > - __pfn_to_pfn_t(pfn, PFN_DEV), > - prot); > - else > - ret = vmf_insert_pfn_prot(vma, address, pfn, prot); > - } > + for (address = vma->vm_start; address < vma->vm_end; > + address += PAGE_SIZE) > + ret = vmf_insert_pfn_prot(vma, address, pfn, prot); > > return ret; > } > @@ -576,14 +565,10 @@ static void ttm_bo_mmap_vma_setup(struct ttm_buffer_object *bo, struct vm_area_s > > vma->vm_private_data = bo; > > - /* > - * We'd like to use VM_PFNMAP on shared mappings, where > - * (vma->vm_flags & VM_SHARED) != 0, for performance reasons, > - * but for some reason VM_PFNMAP + x86 PAT + write-combine is very > - * bad for performance. Until that has been sorted out, use > - * VM_MIXEDMAP on all mappings. See freedesktop.org bug #75719 > + /* Enforce VM_SHARED here since no driver backend actually supports COW > + * on TTM buffer object mappings. > */ > - vma->vm_flags |= VM_MIXEDMAP; > + vma->vm_flags |= VM_PFNMAP | VM_SHARED; > vma->vm_flags |= VM_IO | VM_DONTEXPAND | VM_DONTDUMP; > } > > -- > 2.25.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
