remap_pfn_range() has a nasty surprise if you try to handle two faults from the same vma concurrently: that is the second thread hits a BUG() to assert that the range is clear. As we hold our struct_mutex whilst manipulating the GTT, we have an opportunity to check ahead of time whether a second thread already processed the pagefault for us. We also have to take care of cleaning up the VMA should remap_pfn_range() encounter an error (likely ENOMEM) partway through processing the PTE.
Fixes potential BUG from commit c5158fabeaf53ed2c614c3333aaa4b3cce80f500 Author: Chris Wilson <[email protected]> Date: Tue Jun 10 12:14:41 2014 +0100 Testcase: igt/gem_mmap_gtt/wip Signed-off-by: Chris Wilson <[email protected]> Cc: Brad Volkin <[email protected]> --- drivers/gpu/drm/i915/i915_gem.c | 45 +++++++++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 242b595a0403..90791e9546b7 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1736,6 +1736,21 @@ i915_gem_mmap_ioctl(struct drm_device *dev, void *data, return 0; } +static bool pte_exists(struct vm_area_struct *vma, unsigned long addr) +{ + bool ret = false; + pte_t *pte; + spinlock_t *ptl; + + pte = get_locked_pte(vma->vm_mm, addr, &ptl); + if (pte) { + ret = !pte_none(*pte); + pte_unmap_unlock(pte, ptl); + } + + return ret; +} + /** * i915_gem_fault - fault a page into the GTT * vma: VMA in question @@ -1783,6 +1798,10 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf) if (ret) goto unlock; + /* Check if a second thread completed the prefaulting for us */ + if (obj->fault_mappable && pte_exists(vma, vma->vm_start)) + goto unlock; + /* Access to snoopable pages through the GTT is incoherent. */ if (obj->cache_level != I915_CACHE_NONE && !HAS_LLC(dev)) { ret = -EFAULT; @@ -1806,20 +1825,20 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf) pfn = dev_priv->gtt.mappable_base + i915_gem_obj_ggtt_offset(obj); pfn >>= PAGE_SHIFT; - if (!obj->fault_mappable) { - ret = remap_pfn_range(vma, - vma->vm_start, - pfn, - obj->base.size, - vma->vm_page_prot); - obj->fault_mappable = ret == 0; - } else { - ret = remap_pfn_range(vma, - (unsigned long)vmf->virtual_address, - pfn + page_offset, - PAGE_SIZE, - vma->vm_page_prot); + ret = remap_pfn_range(vma, vma->vm_start, + pfn, vma->vm_end - vma->vm_start, + vma->vm_page_prot); + if (ret) { + /* After passing the sanity checks on remap_pfn_range(), we may + * abort whilst updating the pagetables due to ENOMEM and leave + * the tables in an inconsistent state. Reset them now. + */ + zap_vma_ptes(vma, vma->vm_start, vma->vm_end - vma->vm_start); + goto unpin; } + + obj->fault_mappable = true; + unpin: i915_gem_object_ggtt_unpin(obj); unlock: -- 2.0.0 _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
