[Intel-gfx] [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
Have i915 replace the core drm_gem_mmap implementation to overcome its
limitation in having only a single mmap offset node per gem object.
This change allows us to have multiple mmap offsets per object and
enables a mmapping instance to use unique fault-handlers per user vma.
This allows i915 to store extra data within vma->vm_private_data and
assign the pagefault ops for each mmap instance allowing objects to use
multiple fault handlers depending on its backing storage.
v2:
- Fix race condition exposed by gem_mmap_gtt@close-race. Simplify
lifetime management of the mmap offset objects be ensuring it is
owned by the parent gem object instead of refcounting.
- Track mmo used by fencing to Avoid locking when revoking mmaps
during GPU reset.
- Rebase.
v3:
- Simplify mmo tracking
v4:
- use vma->mmo in __i915_gem_object_release_mmap_gtt
Signed-off-by: Abdiel Janulgue
Cc: Matthew Auld
Cc: Joonas Lahtinen
Cc: Chris Wilson
---
drivers/gpu/drm/i915/gem/i915_gem_domain.c| 2 +-
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 229 --
drivers/gpu/drm/i915/gem/i915_gem_object.c| 13 +
drivers/gpu/drm/i915/gem/i915_gem_object.h| 9 +-
.../gpu/drm/i915/gem/i915_gem_object_types.h | 19 ++
.../drm/i915/gem/selftests/i915_gem_mman.c| 12 +-
drivers/gpu/drm/i915/gt/intel_reset.c | 7 +-
drivers/gpu/drm/i915/i915_drv.c | 10 +-
drivers/gpu/drm/i915/i915_drv.h | 3 +-
drivers/gpu/drm/i915/i915_gem.c | 2 +-
drivers/gpu/drm/i915/i915_vma.c | 15 +-
drivers/gpu/drm/i915/i915_vma.h | 3 +
12 files changed, 274 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index 9937b4c341f1..40792d2017a7 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -254,7 +254,7 @@ int i915_gem_object_set_cache_level(struct
drm_i915_gem_object *obj,
}
if (obj->userfault_count)
- __i915_gem_object_release_mmap(obj);
+ __i915_gem_object_release_mmap_gtt(obj);
/*
* As we no longer need a fence for GTT access,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index fd4122d8c0a9..3491bb06606b 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -219,7 +219,8 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
{
#define MIN_CHUNK_PAGES (SZ_1M >> PAGE_SHIFT)
struct vm_area_struct *area = vmf->vma;
- struct drm_i915_gem_object *obj = to_intel_bo(area->vm_private_data);
+ struct i915_mmap_offset *priv = area->vm_private_data;
+ struct drm_i915_gem_object *obj = priv->obj;
struct drm_device *dev = obj->base.dev;
struct drm_i915_private *i915 = to_i915(dev);
struct intel_runtime_pm *rpm = &i915->runtime_pm;
@@ -312,6 +313,9 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
list_add(&obj->userfault_link, &i915->ggtt.userfault_list);
mutex_unlock(&i915->ggtt.vm.mutex);
+ /* Track the mmo associated with the fenced vma */
+ vma->mmo = priv;
+
if (CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND)
intel_wakeref_auto(&i915->ggtt.userfault_wakeref,
msecs_to_jiffies_timeout(CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND));
@@ -358,7 +362,7 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
}
}
-void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
+void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj)
{
struct i915_vma *vma;
@@ -366,20 +370,16 @@ void __i915_gem_object_release_mmap(struct
drm_i915_gem_object *obj)
obj->userfault_count = 0;
list_del(&obj->userfault_link);
- drm_vma_node_unmap(&obj->base.vma_node,
- obj->base.dev->anon_inode->i_mapping);
- for_each_ggtt_vma(vma, obj)
+ for_each_ggtt_vma(vma, obj) {
+ if (vma->mmo)
+ drm_vma_node_unmap(&vma->mmo->vma_node,
+
obj->base.dev->anon_inode->i_mapping);
i915_vma_unset_userfault(vma);
+ }
}
/**
- * i915_gem_object_release_mmap - remove physical page mappings
- * @obj: obj in question
- *
- * Preserve the reservation of the mmapping with the DRM core code, but
- * relinquish ownership of the pages back to the system.
- *
* It is vital that we remove the page mapping if we have mapped a tiled
* object through the GTT and then lose the fence register due to
* resource pressure. Similarly if the object has been moved out of the
@@ -387,7 +387,7 @@ void __i915_gem_object_release_mmap(struct
drm_i915_gem_object *obj)
* mappin
[Intel-gfx] [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
Have i915 replace the core drm_gem_mmap implementation to overcome its
limitation in having only a single mmap offset node per gem object.
This change allows us to have multiple mmap offsets per object and
enables a mmapping instance to use unique fault-handlers per user vma.
This allows i915 to store extra data within vma->vm_private_data and
assign the pagefault ops for each mmap instance allowing objects to use
multiple fault handlers depending on its backing storage.
v2:
- Fix race condition exposed by gem_mmap_gtt@close-race. Simplify
lifetime management of the mmap offset objects be ensuring it is
owned by the parent gem object instead of refcounting.
- Track mmo used by fencing to avoid locking when revoking mmaps
during GPU reset.
- Rebase.
v3:
- Simplify mmo tracking
v4:
- use vma->mmo in __i915_gem_object_release_mmap_gtt
Signed-off-by: Abdiel Janulgue
Cc: Matthew Auld
Cc: Joonas Lahtinen
Cc: Chris Wilson
---
drivers/gpu/drm/i915/gem/i915_gem_domain.c| 2 +-
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 229 --
drivers/gpu/drm/i915/gem/i915_gem_object.c| 13 +
drivers/gpu/drm/i915/gem/i915_gem_object.h| 9 +-
.../gpu/drm/i915/gem/i915_gem_object_types.h | 19 ++
.../drm/i915/gem/selftests/i915_gem_mman.c| 12 +-
drivers/gpu/drm/i915/gt/intel_reset.c | 7 +-
drivers/gpu/drm/i915/i915_drv.c | 10 +-
drivers/gpu/drm/i915/i915_drv.h | 3 +-
drivers/gpu/drm/i915/i915_gem.c | 2 +-
drivers/gpu/drm/i915/i915_vma.c | 15 +-
drivers/gpu/drm/i915/i915_vma.h | 3 +
12 files changed, 274 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index 9937b4c341f1..40792d2017a7 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -254,7 +254,7 @@ int i915_gem_object_set_cache_level(struct
drm_i915_gem_object *obj,
}
if (obj->userfault_count)
- __i915_gem_object_release_mmap(obj);
+ __i915_gem_object_release_mmap_gtt(obj);
/*
* As we no longer need a fence for GTT access,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index fd4122d8c0a9..3491bb06606b 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -219,7 +219,8 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
{
#define MIN_CHUNK_PAGES (SZ_1M >> PAGE_SHIFT)
struct vm_area_struct *area = vmf->vma;
- struct drm_i915_gem_object *obj = to_intel_bo(area->vm_private_data);
+ struct i915_mmap_offset *priv = area->vm_private_data;
+ struct drm_i915_gem_object *obj = priv->obj;
struct drm_device *dev = obj->base.dev;
struct drm_i915_private *i915 = to_i915(dev);
struct intel_runtime_pm *rpm = &i915->runtime_pm;
@@ -312,6 +313,9 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
list_add(&obj->userfault_link, &i915->ggtt.userfault_list);
mutex_unlock(&i915->ggtt.vm.mutex);
+ /* Track the mmo associated with the fenced vma */
+ vma->mmo = priv;
+
if (CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND)
intel_wakeref_auto(&i915->ggtt.userfault_wakeref,
msecs_to_jiffies_timeout(CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND));
@@ -358,7 +362,7 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
}
}
-void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
+void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj)
{
struct i915_vma *vma;
@@ -366,20 +370,16 @@ void __i915_gem_object_release_mmap(struct
drm_i915_gem_object *obj)
obj->userfault_count = 0;
list_del(&obj->userfault_link);
- drm_vma_node_unmap(&obj->base.vma_node,
- obj->base.dev->anon_inode->i_mapping);
- for_each_ggtt_vma(vma, obj)
+ for_each_ggtt_vma(vma, obj) {
+ if (vma->mmo)
+ drm_vma_node_unmap(&vma->mmo->vma_node,
+
obj->base.dev->anon_inode->i_mapping);
i915_vma_unset_userfault(vma);
+ }
}
/**
- * i915_gem_object_release_mmap - remove physical page mappings
- * @obj: obj in question
- *
- * Preserve the reservation of the mmapping with the DRM core code, but
- * relinquish ownership of the pages back to the system.
- *
* It is vital that we remove the page mapping if we have mapped a tiled
* object through the GTT and then lose the fence register due to
* resource pressure. Similarly if the object has been moved out of the
@@ -387,7 +387,7 @@ void __i915_gem_object_release_mmap(struct
drm_i915_gem_object *obj)
* mappin
[Intel-gfx] [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
Have i915 replace the core drm_gem_mmap implementation to overcome its
limitation in having only a single mmap offset node per gem object.
This change allows us to have multiple mmap offsets per object and
enables a mmapping instance to use unique fault-handlers per user vma.
This allows i915 to store extra data within vma->vm_private_data and
assign the pagefault ops for each mmap instance allowing objects to use
multiple fault handlers depending on its backing storage.
v2:
- Fix race condition exposed by gem_mmap_gtt@close-race. Simplify
lifetime management of the mmap offset objects be ensuring it is
owned by the parent gem object instead of refcounting.
- Track mmo used by fencing to Avoid locking when revoking mmaps
during GPU reset.
- Rebase.
v3:
- Simplify mmo tracking
v4:
- use vma->mmo in __i915_gem_object_release_mmap_gtt
Signed-off-by: Abdiel Janulgue
Cc: Matthew Auld
Cc: Joonas Lahtinen
Cc: Chris Wilson
---
drivers/gpu/drm/i915/gem/i915_gem_domain.c| 2 +-
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 229 --
drivers/gpu/drm/i915/gem/i915_gem_object.c| 13 +
drivers/gpu/drm/i915/gem/i915_gem_object.h| 9 +-
.../gpu/drm/i915/gem/i915_gem_object_types.h | 19 ++
.../drm/i915/gem/selftests/i915_gem_mman.c| 12 +-
drivers/gpu/drm/i915/gt/intel_reset.c | 7 +-
drivers/gpu/drm/i915/i915_drv.c | 10 +-
drivers/gpu/drm/i915/i915_drv.h | 3 +-
drivers/gpu/drm/i915/i915_gem.c | 2 +-
drivers/gpu/drm/i915/i915_vma.c | 15 +-
drivers/gpu/drm/i915/i915_vma.h | 4 +
12 files changed, 275 insertions(+), 50 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
index 9937b4c341f1..40792d2017a7 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_domain.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_domain.c
@@ -254,7 +254,7 @@ int i915_gem_object_set_cache_level(struct
drm_i915_gem_object *obj,
}
if (obj->userfault_count)
- __i915_gem_object_release_mmap(obj);
+ __i915_gem_object_release_mmap_gtt(obj);
/*
* As we no longer need a fence for GTT access,
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index fd4122d8c0a9..3491bb06606b 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -219,7 +219,8 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
{
#define MIN_CHUNK_PAGES (SZ_1M >> PAGE_SHIFT)
struct vm_area_struct *area = vmf->vma;
- struct drm_i915_gem_object *obj = to_intel_bo(area->vm_private_data);
+ struct i915_mmap_offset *priv = area->vm_private_data;
+ struct drm_i915_gem_object *obj = priv->obj;
struct drm_device *dev = obj->base.dev;
struct drm_i915_private *i915 = to_i915(dev);
struct intel_runtime_pm *rpm = &i915->runtime_pm;
@@ -312,6 +313,9 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
list_add(&obj->userfault_link, &i915->ggtt.userfault_list);
mutex_unlock(&i915->ggtt.vm.mutex);
+ /* Track the mmo associated with the fenced vma */
+ vma->mmo = priv;
+
if (CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND)
intel_wakeref_auto(&i915->ggtt.userfault_wakeref,
msecs_to_jiffies_timeout(CONFIG_DRM_I915_USERFAULT_AUTOSUSPEND));
@@ -358,7 +362,7 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
}
}
-void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
+void __i915_gem_object_release_mmap_gtt(struct drm_i915_gem_object *obj)
{
struct i915_vma *vma;
@@ -366,20 +370,16 @@ void __i915_gem_object_release_mmap(struct
drm_i915_gem_object *obj)
obj->userfault_count = 0;
list_del(&obj->userfault_link);
- drm_vma_node_unmap(&obj->base.vma_node,
- obj->base.dev->anon_inode->i_mapping);
- for_each_ggtt_vma(vma, obj)
+ for_each_ggtt_vma(vma, obj) {
+ if (vma->mmo)
+ drm_vma_node_unmap(&vma->mmo->vma_node,
+
obj->base.dev->anon_inode->i_mapping);
i915_vma_unset_userfault(vma);
+ }
}
/**
- * i915_gem_object_release_mmap - remove physical page mappings
- * @obj: obj in question
- *
- * Preserve the reservation of the mmapping with the DRM core code, but
- * relinquish ownership of the pages back to the system.
- *
* It is vital that we remove the page mapping if we have mapped a tiled
* object through the GTT and then lose the fence register due to
* resource pressure. Similarly if the object has been moved out of the
@@ -387,7 +387,7 @@ void __i915_gem_object_release_mmap(struct
drm_i915_gem_object *obj)
* mappin
Re: [Intel-gfx] [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
On Mon, Aug 26, 2019 at 2:21 PM Abdiel Janulgue
wrote:
>
> Have i915 replace the core drm_gem_mmap implementation to overcome its
> limitation in having only a single mmap offset node per gem object.
> The change allows us to have multiple mmap offsets per object. This
> enables a mmapping instance to use unique fault-handlers per user vma.
>
> This enables us to store extra data within vma->vm_private_data and assign
> the pagefault ops for each mmap instance allowing objects to use multiple
> fault handlers depending on its backing storage.
>
> Signed-off-by: Abdiel Janulgue
> Cc: Joonas Lahtinen
> Cc: Chris Wilson
> ---
> drivers/gpu/drm/i915/gem/i915_gem_mman.c | 193 --
> drivers/gpu/drm/i915/gem/i915_gem_object.c| 19 ++
> drivers/gpu/drm/i915/gem/i915_gem_object.h| 7 +-
> .../gpu/drm/i915/gem/i915_gem_object_types.h | 18 ++
> .../drm/i915/gem/selftests/i915_gem_mman.c| 12 +-
> drivers/gpu/drm/i915/gt/intel_reset.c | 11 +-
> drivers/gpu/drm/i915/i915_drv.c | 9 +-
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/i915_vma.c | 20 +-
> 9 files changed, 254 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> index 595539a09e38..fb7e39f115d7 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
> @@ -218,7 +218,8 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
> {
> #define MIN_CHUNK_PAGES (SZ_1M >> PAGE_SHIFT)
> struct vm_area_struct *area = vmf->vma;
> - struct drm_i915_gem_object *obj = to_intel_bo(area->vm_private_data);
> + struct i915_mmap_offset *priv = area->vm_private_data;
> + struct drm_i915_gem_object *obj = priv->obj;
> struct drm_device *dev = obj->base.dev;
> struct drm_i915_private *i915 = to_i915(dev);
> struct intel_runtime_pm *rpm = &i915->runtime_pm;
> @@ -372,13 +373,20 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
> void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
> {
> struct i915_vma *vma;
> + struct i915_mmap_offset *mmo;
>
> GEM_BUG_ON(!obj->userfault_count);
>
> obj->userfault_count = 0;
> list_del(&obj->userfault_link);
> - drm_vma_node_unmap(&obj->base.vma_node,
> - obj->base.dev->anon_inode->i_mapping);
> +
> + mutex_lock(&obj->mmo_lock);
> + list_for_each_entry(mmo, &obj->mmap_offsets, offset) {
> + if (mmo->mmap_type == I915_MMAP_TYPE_GTT)
> + drm_vma_node_unmap(&mmo->vma_node,
> +
> obj->base.dev->anon_inode->i_mapping);
> + }
> + mutex_unlock(&obj->mmo_lock);
>
> for_each_ggtt_vma(vma, obj)
> i915_vma_unset_userfault(vma);
> @@ -433,14 +441,33 @@ void i915_gem_object_release_mmap(struct
> drm_i915_gem_object *obj)
> intel_runtime_pm_put(&i915->runtime_pm, wakeref);
> }
>
> -static int create_mmap_offset(struct drm_i915_gem_object *obj)
> +static void init_mmap_offset(struct drm_i915_gem_object *obj,
> +struct i915_mmap_offset *mmo)
> +{
> + mutex_lock(&obj->mmo_lock);
> + kref_init(&mmo->ref);
> + list_add(&mmo->offset, &obj->mmap_offsets);
> + mutex_unlock(&obj->mmo_lock);
> +
> + mmo->obj = obj;
> +}
> +
> +static int create_mmap_offset(struct drm_i915_gem_object *obj,
> + struct i915_mmap_offset *mmo)
> {
> struct drm_i915_private *i915 = to_i915(obj->base.dev);
> + struct drm_device *dev = obj->base.dev;
> int err;
>
> - err = drm_gem_create_mmap_offset(&obj->base);
> - if (likely(!err))
> + drm_vma_node_reset(&mmo->vma_node);
> + if (mmo->file)
> + drm_vma_node_allow(&mmo->vma_node, mmo->file);
> + err = drm_vma_offset_add(dev->vma_offset_manager, &mmo->vma_node,
> +obj->base.size / PAGE_SIZE);
> + if (likely(!err)) {
> + init_mmap_offset(obj, mmo);
> return 0;
> + }
>
> /* Attempt to reap some mmap space from dead objects */
> do {
> @@ -451,32 +478,48 @@ static int create_mmap_offset(struct
> drm_i915_gem_object *obj)
> break;
>
> i915_gem_drain_freed_objects(i915);
> - err = drm_gem_create_mmap_offset(&obj->base);
> - if (!err)
> + err = drm_vma_offset_add(dev->vma_offset_manager,
> &mmo->vma_node,
> +obj->base.size / PAGE_SIZE);
> + if (!err) {
> + init_mmap_offset(obj, mmo);
> break;
> + }
>
> } while (flush_delayed_work(&i915->gem.retire_work));
>
> return err;
> }
>
> -int
> -i9
Re: [Intel-gfx] [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
Quoting Abdiel Janulgue (2019-08-26 13:20:58)
> @@ -641,10 +642,18 @@ static void revoke_mmaps(struct intel_gt *gt)
> GEM_BUG_ON(vma->fence != >->ggtt->fence_regs[i]);
> node = &vma->obj->base.vma_node;
> vma_offset = vma->ggtt_view.partial.offset << PAGE_SHIFT;
> - unmap_mapping_range(gt->i915->drm.anon_inode->i_mapping,
> +
> + list_for_each_entry(mmo, &vma->obj->mmap_offsets, offset) {
> + node = &mmo->vma_node;
> + if (!drm_mm_node_allocated(&node->vm_node) ||
> + mmo->mmap_type != I915_MMAP_TYPE_GTT)
> + continue;
That list needs locking as is not protected by the reset srcu (and you
are not allowed your own locking in here, unless you have a jolly good
reason and can prove it will never block a reset).
One thing to observe is is that you only ever need the mmo associated
with a fence for revocation upon reset, and you could use a local list
for tracking the active mmo.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
Quoting Abdiel Janulgue (2019-08-26 13:20:58)
> -static int create_mmap_offset(struct drm_i915_gem_object *obj)
> +static void init_mmap_offset(struct drm_i915_gem_object *obj,
> +struct i915_mmap_offset *mmo)
> +{
> + mutex_lock(&obj->mmo_lock);
This lock should only be guarding obj->mmap_offsets. You don't need to
take it around every kref, just be careful to remove the link on close.
> + kref_init(&mmo->ref);
> + list_add(&mmo->offset, &obj->mmap_offsets);
> + mutex_unlock(&obj->mmo_lock);
> +
> + mmo->obj = obj;
> +}
> +/* This overcomes the limitation in drm_gem_mmap's assignment of a
> + * drm_gem_object as the vma->vm_private_data. Since we need to
> + * be able to resolve multiple mmap offsets which could be tied
> + * to a single gem object.
> + */
> +int i915_gem_mmap(struct file *filp, struct vm_area_struct *vma)
> +{
> + struct drm_vma_offset_node *node;
> + struct drm_file *priv = filp->private_data;
> + struct drm_device *dev = priv->minor->dev;
> + struct i915_mmap_offset *mmo = NULL;
> + struct drm_gem_object *obj = NULL;
> +
> + if (drm_dev_is_unplugged(dev))
> + return -ENODEV;
> +
> + drm_vma_offset_lock_lookup(dev->vma_offset_manager);
> + node = drm_vma_offset_exact_lookup_locked(dev->vma_offset_manager,
> + vma->vm_pgoff,
> + vma_pages(vma));
> + if (likely(node)) {
> + mmo = container_of(node, struct i915_mmap_offset,
> + vma_node);
> + /*
> +* Skip 0-refcnted objects as it is in the process of being
> +* destroyed and will be invalid when the vma manager lock
> +* is released.
> +*/
> + obj = &mmo->obj->base;
> + if (!kref_get_unless_zero(&obj->refcount))
> + obj = NULL;
Hmm, references are still weird. This doesn't seem like it protects
against
Thread AThread B
mmap(fd, offset_of_A); gem_close(fd, A);
Time for a gem_mmap_gtt/close-race.
-Chris
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/5] drm/i915: Allow i915 to manage the vma offset nodes instead of drm core
Have i915 replace the core drm_gem_mmap implementation to overcome its
limitation in having only a single mmap offset node per gem object.
The change allows us to have multiple mmap offsets per object. This
enables a mmapping instance to use unique fault-handlers per user vma.
This enables us to store extra data within vma->vm_private_data and assign
the pagefault ops for each mmap instance allowing objects to use multiple
fault handlers depending on its backing storage.
Signed-off-by: Abdiel Janulgue
Cc: Joonas Lahtinen
Cc: Chris Wilson
---
drivers/gpu/drm/i915/gem/i915_gem_mman.c | 193 --
drivers/gpu/drm/i915/gem/i915_gem_object.c| 19 ++
drivers/gpu/drm/i915/gem/i915_gem_object.h| 7 +-
.../gpu/drm/i915/gem/i915_gem_object_types.h | 18 ++
.../drm/i915/gem/selftests/i915_gem_mman.c| 12 +-
drivers/gpu/drm/i915/gt/intel_reset.c | 11 +-
drivers/gpu/drm/i915/i915_drv.c | 9 +-
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_vma.c | 20 +-
9 files changed, 254 insertions(+), 36 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
index 595539a09e38..fb7e39f115d7 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_mman.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_mman.c
@@ -218,7 +218,8 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
{
#define MIN_CHUNK_PAGES (SZ_1M >> PAGE_SHIFT)
struct vm_area_struct *area = vmf->vma;
- struct drm_i915_gem_object *obj = to_intel_bo(area->vm_private_data);
+ struct i915_mmap_offset *priv = area->vm_private_data;
+ struct drm_i915_gem_object *obj = priv->obj;
struct drm_device *dev = obj->base.dev;
struct drm_i915_private *i915 = to_i915(dev);
struct intel_runtime_pm *rpm = &i915->runtime_pm;
@@ -372,13 +373,20 @@ vm_fault_t i915_gem_fault(struct vm_fault *vmf)
void __i915_gem_object_release_mmap(struct drm_i915_gem_object *obj)
{
struct i915_vma *vma;
+ struct i915_mmap_offset *mmo;
GEM_BUG_ON(!obj->userfault_count);
obj->userfault_count = 0;
list_del(&obj->userfault_link);
- drm_vma_node_unmap(&obj->base.vma_node,
- obj->base.dev->anon_inode->i_mapping);
+
+ mutex_lock(&obj->mmo_lock);
+ list_for_each_entry(mmo, &obj->mmap_offsets, offset) {
+ if (mmo->mmap_type == I915_MMAP_TYPE_GTT)
+ drm_vma_node_unmap(&mmo->vma_node,
+
obj->base.dev->anon_inode->i_mapping);
+ }
+ mutex_unlock(&obj->mmo_lock);
for_each_ggtt_vma(vma, obj)
i915_vma_unset_userfault(vma);
@@ -433,14 +441,33 @@ void i915_gem_object_release_mmap(struct
drm_i915_gem_object *obj)
intel_runtime_pm_put(&i915->runtime_pm, wakeref);
}
-static int create_mmap_offset(struct drm_i915_gem_object *obj)
+static void init_mmap_offset(struct drm_i915_gem_object *obj,
+struct i915_mmap_offset *mmo)
+{
+ mutex_lock(&obj->mmo_lock);
+ kref_init(&mmo->ref);
+ list_add(&mmo->offset, &obj->mmap_offsets);
+ mutex_unlock(&obj->mmo_lock);
+
+ mmo->obj = obj;
+}
+
+static int create_mmap_offset(struct drm_i915_gem_object *obj,
+ struct i915_mmap_offset *mmo)
{
struct drm_i915_private *i915 = to_i915(obj->base.dev);
+ struct drm_device *dev = obj->base.dev;
int err;
- err = drm_gem_create_mmap_offset(&obj->base);
- if (likely(!err))
+ drm_vma_node_reset(&mmo->vma_node);
+ if (mmo->file)
+ drm_vma_node_allow(&mmo->vma_node, mmo->file);
+ err = drm_vma_offset_add(dev->vma_offset_manager, &mmo->vma_node,
+obj->base.size / PAGE_SIZE);
+ if (likely(!err)) {
+ init_mmap_offset(obj, mmo);
return 0;
+ }
/* Attempt to reap some mmap space from dead objects */
do {
@@ -451,32 +478,48 @@ static int create_mmap_offset(struct drm_i915_gem_object
*obj)
break;
i915_gem_drain_freed_objects(i915);
- err = drm_gem_create_mmap_offset(&obj->base);
- if (!err)
+ err = drm_vma_offset_add(dev->vma_offset_manager,
&mmo->vma_node,
+obj->base.size / PAGE_SIZE);
+ if (!err) {
+ init_mmap_offset(obj, mmo);
break;
+ }
} while (flush_delayed_work(&i915->gem.retire_work));
return err;
}
-int
-i915_gem_mmap_gtt(struct drm_file *file,
- struct drm_device *dev,
- u32 handle,
- u64 *offset)
+static int
+__assign_gem_object_mmap_data(struct drm_file *file,
+ u32 handle,
+ e
