We use the global device inode, shared amongst all files, and not the
user's device filp to provide the backing storage for the mmap. The
vma->vm_file provides a redundant reference that breaks existing
expected behaviour that closing the user's device fd will release the
resources bound to it, if a mmap persists. (Even without the
vma->vm_file, the mmap will persist past the user's fd as the storage is
bound to the device, i.e. our reference is on the object not file.)

Fixes: cc662126b413 ("drm/i915: Introduce DRM_I915_GEM_MMAP_OFFSET")
Closes: https://gitlab.freedesktop.org/drm/intel/issues/919
Signed-off-by: Chris Wilson <[email protected]>
---
 drivers/gpu/drm/i915/gem/i915_gem_mman.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c 
b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index 905527ce2999..20f065e02fe2 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -699,6 +699,7 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct 
*vma)
        struct drm_device *dev = priv->minor->dev;
        struct i915_mmap_offset *mmo = NULL;
        struct drm_gem_object *obj = NULL;
+       struct file *anon;
 
        if (drm_dev_is_unplugged(dev))
                return -ENODEV;
@@ -747,9 +748,26 @@ int i915_gem_mmap(struct file *filp, struct vm_area_struct 
*vma)
                vma->vm_flags &= ~VM_MAYWRITE;
        }
 
+       anon = drm_minor_create_anonfile(dev->primary, vma->vm_file->f_flags);
+       if (IS_ERR(anon)) {
+               drm_gem_object_put_unlocked(obj);
+               return PTR_ERR(anon);
+       }
+
        vma->vm_flags |= VM_PFNMAP | VM_DONTEXPAND | VM_DONTDUMP;
        vma->vm_private_data = mmo;
 
+       /*
+        * We keep the ref on mmo->obj, not vm_file, but we require
+        * vma->vm_file->f_mapping, see vma_link(), for later revocation.
+        * Our userspace is accustomed to having per-file resource cleanup
+        * (i.e. contexts, objects and requests) on their close(fd), which
+        * requires avoiding extraneous references to their filp, hence why
+        * we prefer to use an anonymous file for their mmaps.
+        */
+       fput(vma->vm_file);
+       vma->vm_file = anon;
+
        switch (mmo->mmap_type) {
        case I915_MMAP_TYPE_WC:
                vma->vm_page_prot =
-- 
2.25.0.rc0

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to