* [PATCH] drm/i915: Allow shrinking of userptr objects once again
@ 2016-11-01 13:28 Tvrtko Ursulin
2016-11-01 13:52 ` Chris Wilson
0 siblings, 1 reply; 6+ messages in thread
From: Tvrtko Ursulin @ 2016-11-01 13:28 UTC (permalink / raw)
To: Intel-gfx
Cc: Tvrtko Ursulin, Chris Wilson, Joonas Lahtinen, Mika Kuoppala,
stable
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Commit 1bec9b0bda3d ("drm/i915/shrinker: Only shmemfs objects
are backed by swap") stopped considering the userptr objects
in shrinker callbacks.
Restore that so idle userptr objects can be discarded in order
to free up memory.
One change further to what was introduced in 1bec9b0bda3d is
to start considering userptr objects in oom but that should
also be a correct thing to do.
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Fixes: 1bec9b0bda3d ("drm/i915/shrinker: Only shmemfs objects are backed by swap")
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: <stable@vger.kernel.org>
---
drivers/gpu/drm/i915/i915_gem_shrinker.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index 0993afc0e725..dfca1f6b3630 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -83,8 +83,8 @@ static bool can_release_pages(struct drm_i915_gem_object *obj)
if (!obj->mm.pages)
return false;
- /* Only shmemfs objects are backed by swap */
- if (!obj->base.filp)
+ /* shmemfs and userptr objects are backed by swap */
+ if (!obj->base.filp && !obj->userptr.mm)
return false;
/* Only report true if by unbinding the object and putting its pages
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Allow shrinking of userptr objects once again
2016-11-01 13:28 [PATCH] drm/i915: Allow shrinking of userptr objects once again Tvrtko Ursulin
@ 2016-11-01 13:52 ` Chris Wilson
2016-11-01 14:29 ` [Intel-gfx] " Tvrtko Ursulin
0 siblings, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2016-11-01 13:52 UTC (permalink / raw)
To: Tvrtko Ursulin
Cc: Intel-gfx, Tvrtko Ursulin, Joonas Lahtinen, Mika Kuoppala, stable
On Tue, Nov 01, 2016 at 01:28:06PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Commit 1bec9b0bda3d ("drm/i915/shrinker: Only shmemfs objects
> are backed by swap") stopped considering the userptr objects
> in shrinker callbacks.
>
> Restore that so idle userptr objects can be discarded in order
> to free up memory.
>
> One change further to what was introduced in 1bec9b0bda3d is
> to start considering userptr objects in oom but that should
> also be a correct thing to do.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Fixes: 1bec9b0bda3d ("drm/i915/shrinker: Only shmemfs objects are backed by swap")
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: <stable@vger.kernel.org>
> ---
> drivers/gpu/drm/i915/i915_gem_shrinker.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> index 0993afc0e725..dfca1f6b3630 100644
> --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> @@ -83,8 +83,8 @@ static bool can_release_pages(struct drm_i915_gem_object *obj)
> if (!obj->mm.pages)
> return false;
>
> - /* Only shmemfs objects are backed by swap */
> - if (!obj->base.filp)
> + /* shmemfs and userptr objects are backed by swap */
> + if (!obj->base.filp && !obj->userptr.mm)
Hmm. Another sticking point if we want to use the union again.
How about obj->ops->flags & I915_GEM_OBJECT_HAS_BACKING_STORE ?
Or I915_GEM_OBJECT_CAN_SWAP since we want this for i915_gem_internal.c
as well, and that techinically doesn't have a backing store but can be
reaped. Hmm.
if (!(obj->ops->flags & I915_GEM_OBJECT_HAS_BACKING_STORE ||
obj->mm.madv == I915_MADV_DONTNEED))
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Allow shrinking of userptr objects once again
2016-11-01 13:52 ` Chris Wilson
@ 2016-11-01 14:29 ` Tvrtko Ursulin
2016-11-01 14:34 ` Chris Wilson
0 siblings, 1 reply; 6+ messages in thread
From: Tvrtko Ursulin @ 2016-11-01 14:29 UTC (permalink / raw)
To: Chris Wilson, Tvrtko Ursulin, Intel-gfx, Tvrtko Ursulin,
Joonas Lahtinen, Mika Kuoppala, stable
On 01/11/2016 13:52, Chris Wilson wrote:
> On Tue, Nov 01, 2016 at 01:28:06PM +0000, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> Commit 1bec9b0bda3d ("drm/i915/shrinker: Only shmemfs objects
>> are backed by swap") stopped considering the userptr objects
>> in shrinker callbacks.
>>
>> Restore that so idle userptr objects can be discarded in order
>> to free up memory.
>>
>> One change further to what was introduced in 1bec9b0bda3d is
>> to start considering userptr objects in oom but that should
>> also be a correct thing to do.
>>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>> Fixes: 1bec9b0bda3d ("drm/i915/shrinker: Only shmemfs objects are backed by swap")
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
>> Cc: <stable@vger.kernel.org>
>> ---
>> drivers/gpu/drm/i915/i915_gem_shrinker.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
>> index 0993afc0e725..dfca1f6b3630 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
>> @@ -83,8 +83,8 @@ static bool can_release_pages(struct drm_i915_gem_object *obj)
>> if (!obj->mm.pages)
>> return false;
>>
>> - /* Only shmemfs objects are backed by swap */
>> - if (!obj->base.filp)
>> + /* shmemfs and userptr objects are backed by swap */
>> + if (!obj->base.filp && !obj->userptr.mm)
>
> Hmm. Another sticking point if we want to use the union again.
>
> How about obj->ops->flags & I915_GEM_OBJECT_HAS_BACKING_STORE ?
> Or I915_GEM_OBJECT_CAN_SWAP since we want this for i915_gem_internal.c
> as well, and that techinically doesn't have a backing store but can be
> reaped. Hmm.
>
> if (!(obj->ops->flags & I915_GEM_OBJECT_HAS_BACKING_STORE ||
> obj->mm.madv == I915_MADV_DONTNEED))
Hm I915_GEM_OBJECT_HAS_STRUCT_PAGE happens to be right - shm, userptr
and internal. But that would be bad. Neither do I like HAS_BACKING_STORE.
Maybe I915_GEM_OBJECT_IS_SHRINKABLE, fully dumb and explicit?
Regards,
Tvrtko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Intel-gfx] [PATCH] drm/i915: Allow shrinking of userptr objects once again
2016-11-01 14:29 ` [Intel-gfx] " Tvrtko Ursulin
@ 2016-11-01 14:34 ` Chris Wilson
2016-11-01 14:44 ` [PATCH v2] " Tvrtko Ursulin
0 siblings, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2016-11-01 14:34 UTC (permalink / raw)
To: Tvrtko Ursulin
Cc: Tvrtko Ursulin, Intel-gfx, Tvrtko Ursulin, Joonas Lahtinen,
Mika Kuoppala, stable
On Tue, Nov 01, 2016 at 02:29:39PM +0000, Tvrtko Ursulin wrote:
>
> On 01/11/2016 13:52, Chris Wilson wrote:
> >On Tue, Nov 01, 2016 at 01:28:06PM +0000, Tvrtko Ursulin wrote:
> >>From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>
> >>Commit 1bec9b0bda3d ("drm/i915/shrinker: Only shmemfs objects
> >>are backed by swap") stopped considering the userptr objects
> >>in shrinker callbacks.
> >>
> >>Restore that so idle userptr objects can be discarded in order
> >>to free up memory.
> >>
> >>One change further to what was introduced in 1bec9b0bda3d is
> >>to start considering userptr objects in oom but that should
> >>also be a correct thing to do.
> >>
> >>Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> >>Fixes: 1bec9b0bda3d ("drm/i915/shrinker: Only shmemfs objects are backed by swap")
> >>Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >>Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >>Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> >>Cc: <stable@vger.kernel.org>
> >>---
> >> drivers/gpu/drm/i915/i915_gem_shrinker.c | 4 ++--
> >> 1 file changed, 2 insertions(+), 2 deletions(-)
> >>
> >>diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> >>index 0993afc0e725..dfca1f6b3630 100644
> >>--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> >>+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> >>@@ -83,8 +83,8 @@ static bool can_release_pages(struct drm_i915_gem_object *obj)
> >> if (!obj->mm.pages)
> >> return false;
> >>
> >>- /* Only shmemfs objects are backed by swap */
> >>- if (!obj->base.filp)
> >>+ /* shmemfs and userptr objects are backed by swap */
> >>+ if (!obj->base.filp && !obj->userptr.mm)
> >
> >Hmm. Another sticking point if we want to use the union again.
> >
> >How about obj->ops->flags & I915_GEM_OBJECT_HAS_BACKING_STORE ?
> >Or I915_GEM_OBJECT_CAN_SWAP since we want this for i915_gem_internal.c
> >as well, and that techinically doesn't have a backing store but can be
> >reaped. Hmm.
> >
> >if (!(obj->ops->flags & I915_GEM_OBJECT_HAS_BACKING_STORE ||
> > obj->mm.madv == I915_MADV_DONTNEED))
>
> Hm I915_GEM_OBJECT_HAS_STRUCT_PAGE happens to be right - shm,
> userptr and internal. But that would be bad. Neither do I like
> HAS_BACKING_STORE.
Yeah, I was trying to keep the concept separate from has-struct-page
just in case it made it easier for future extension.
> Maybe I915_GEM_OBJECT_IS_SHRINKABLE, fully dumb and explicit?
Dumb describes me best, yup.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] drm/i915: Allow shrinking of userptr objects once again
2016-11-01 14:34 ` Chris Wilson
@ 2016-11-01 14:44 ` Tvrtko Ursulin
2016-11-01 15:05 ` Chris Wilson
0 siblings, 1 reply; 6+ messages in thread
From: Tvrtko Ursulin @ 2016-11-01 14:44 UTC (permalink / raw)
To: Intel-gfx
Cc: Tvrtko Ursulin, Chris Wilson, Joonas Lahtinen, Mika Kuoppala,
stable
From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Commit 1bec9b0bda3d ("drm/i915/shrinker: Only shmemfs objects
are backed by swap") stopped considering the userptr objects
in shrinker callbacks.
Restore that so idle userptr objects can be discarded in order
to free up memory.
One change further to what was introduced in 1bec9b0bda3d is
to start considering userptr objects in oom but that should
also be a correct thing to do.
v2: Introduce I915_GEM_OBJECT_IS_SHRINKABLE. (Chris Wilson)
Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Fixes: 1bec9b0bda3d ("drm/i915/shrinker: Only shmemfs objects are backed by swap")
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Cc: <stable@vger.kernel.org>
---
drivers/gpu/drm/i915/i915_drv.h | 7 +++++++
drivers/gpu/drm/i915/i915_gem.c | 3 ++-
drivers/gpu/drm/i915/i915_gem_internal.c | 3 ++-
drivers/gpu/drm/i915/i915_gem_shrinker.c | 4 ++--
drivers/gpu/drm/i915/i915_gem_userptr.c | 3 ++-
5 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 086396e16de7..0525e5b448b0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2181,6 +2181,7 @@ enum hdmi_force_audio {
struct drm_i915_gem_object_ops {
unsigned int flags;
#define I915_GEM_OBJECT_HAS_STRUCT_PAGE 0x1
+#define I915_GEM_OBJECT_IS_SHRINKABLE 0x2
/* Interface between the GEM object and its backing storage.
* get_pages() is called once prior to the use of the associated set
@@ -2430,6 +2431,12 @@ i915_gem_object_has_struct_page(const struct drm_i915_gem_object *obj)
}
static inline bool
+i915_gem_object_is_shrinkable(const struct drm_i915_gem_object *obj)
+{
+ return obj->ops->flags & I915_GEM_OBJECT_IS_SHRINKABLE;
+}
+
+static inline bool
i915_gem_object_is_active(const struct drm_i915_gem_object *obj)
{
return obj->active_count;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index cbbfaa7761b9..e1bf456c62c3 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4146,7 +4146,8 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
}
static const struct drm_i915_gem_object_ops i915_gem_object_ops = {
- .flags = I915_GEM_OBJECT_HAS_STRUCT_PAGE,
+ .flags = I915_GEM_OBJECT_HAS_STRUCT_PAGE |
+ I915_GEM_OBJECT_IS_SHRINKABLE,
.get_pages = i915_gem_object_get_pages_gtt,
.put_pages = i915_gem_object_put_pages_gtt,
};
diff --git a/drivers/gpu/drm/i915/i915_gem_internal.c b/drivers/gpu/drm/i915/i915_gem_internal.c
index 1b0607a44a9a..4b3ff3e5b911 100644
--- a/drivers/gpu/drm/i915/i915_gem_internal.c
+++ b/drivers/gpu/drm/i915/i915_gem_internal.c
@@ -132,7 +132,8 @@ static void i915_gem_object_put_pages_internal(struct drm_i915_gem_object *obj,
}
static const struct drm_i915_gem_object_ops i915_gem_object_internal_ops = {
- .flags = I915_GEM_OBJECT_HAS_STRUCT_PAGE,
+ .flags = I915_GEM_OBJECT_HAS_STRUCT_PAGE |
+ I915_GEM_OBJECT_IS_SHRINKABLE,
.get_pages = i915_gem_object_get_pages_internal,
.put_pages = i915_gem_object_put_pages_internal,
};
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index f988652f1e26..87dd27d5146c 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -83,8 +83,8 @@ static bool can_release_pages(struct drm_i915_gem_object *obj)
if (!obj->mm.pages)
return false;
- /* Only shmemfs objects are backed by swap */
- if (!obj->base.filp)
+ /* Consider only shrinkable ojects. */
+ if (!i915_gem_object_is_shrinkable(obj))
return false;
/* Only report true if by unbinding the object and putting its pages
diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
index 9bf44b5bca10..64261639f547 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -707,7 +707,8 @@ i915_gem_userptr_dmabuf_export(struct drm_i915_gem_object *obj)
}
static const struct drm_i915_gem_object_ops i915_gem_userptr_ops = {
- .flags = I915_GEM_OBJECT_HAS_STRUCT_PAGE,
+ .flags = I915_GEM_OBJECT_HAS_STRUCT_PAGE |
+ I915_GEM_OBJECT_IS_SHRINKABLE,
.get_pages = i915_gem_userptr_get_pages,
.put_pages = i915_gem_userptr_put_pages,
.dmabuf_export = i915_gem_userptr_dmabuf_export,
--
2.7.4
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] drm/i915: Allow shrinking of userptr objects once again
2016-11-01 14:44 ` [PATCH v2] " Tvrtko Ursulin
@ 2016-11-01 15:05 ` Chris Wilson
0 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2016-11-01 15:05 UTC (permalink / raw)
To: Tvrtko Ursulin
Cc: Intel-gfx, Tvrtko Ursulin, Joonas Lahtinen, Mika Kuoppala, stable
On Tue, Nov 01, 2016 at 02:44:10PM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> Commit 1bec9b0bda3d ("drm/i915/shrinker: Only shmemfs objects
> are backed by swap") stopped considering the userptr objects
> in shrinker callbacks.
>
> Restore that so idle userptr objects can be discarded in order
> to free up memory.
>
> One change further to what was introduced in 1bec9b0bda3d is
> to start considering userptr objects in oom but that should
> also be a correct thing to do.
>
> v2: Introduce I915_GEM_OBJECT_IS_SHRINKABLE. (Chris Wilson)
I think dmabuf would benefit. Happy to have a followup (or sign off on
that myself).
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Fixes: 1bec9b0bda3d ("drm/i915/shrinker: Only shmemfs objects are backed by swap")
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Cc: <stable@vger.kernel.org>
Reviewed-by Chris Wilson <chris@chris-wilson.co.uk>
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-11-01 15:05 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-01 13:28 [PATCH] drm/i915: Allow shrinking of userptr objects once again Tvrtko Ursulin
2016-11-01 13:52 ` Chris Wilson
2016-11-01 14:29 ` [Intel-gfx] " Tvrtko Ursulin
2016-11-01 14:34 ` Chris Wilson
2016-11-01 14:44 ` [PATCH v2] " Tvrtko Ursulin
2016-11-01 15:05 ` Chris Wilson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox