* [PATCH] drm/i915: Fix type mismatch and accounting in i915_gem_shrink
@ 2013-09-24 9:52 Chris Wilson
2013-09-24 10:29 ` Daniel Vetter
0 siblings, 1 reply; 4+ messages in thread
From: Chris Wilson @ 2013-09-24 9:52 UTC (permalink / raw)
To: intel-gfx
The interface uses an unsigned long, and we can use the unsigned counter
throughout our code, so do so. In the process, we notice one instance
where the shrink count is based on a heuristic rather than the result,
and another where we ask for too many pages to be purged.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_gem.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1bfb792..af6ff15 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -64,8 +64,8 @@ static unsigned long i915_gem_inactive_count(struct shrinker *shrinker,
struct shrink_control *sc);
static unsigned long i915_gem_inactive_scan(struct shrinker *shrinker,
struct shrink_control *sc);
-static long i915_gem_purge(struct drm_i915_private *dev_priv, long target);
-static long i915_gem_shrink_all(struct drm_i915_private *dev_priv);
+static unsigned long i915_gem_purge(struct drm_i915_private *dev_priv, long target);
+static unsigned long i915_gem_shrink_all(struct drm_i915_private *dev_priv);
static void i915_gem_object_truncate(struct drm_i915_gem_object *obj);
static bool cpu_cache_is_coherent(struct drm_device *dev,
@@ -1825,13 +1825,13 @@ i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
return 0;
}
-static long
+static unsigned long
__i915_gem_shrink(struct drm_i915_private *dev_priv, long target,
bool purgeable_only)
{
struct list_head still_bound_list;
struct drm_i915_gem_object *obj, *next;
- long count = 0;
+ unsigned long count = 0;
list_for_each_entry_safe(obj, next,
&dev_priv->mm.unbound_list,
@@ -1897,13 +1897,13 @@ __i915_gem_shrink(struct drm_i915_private *dev_priv, long target,
return count;
}
-static long
+static unsigned long
i915_gem_purge(struct drm_i915_private *dev_priv, long target)
{
return __i915_gem_shrink(dev_priv, target, true);
}
-static long
+static unsigned long
i915_gem_shrink_all(struct drm_i915_private *dev_priv)
{
struct drm_i915_gem_object *obj, *next;
@@ -1913,9 +1913,8 @@ i915_gem_shrink_all(struct drm_i915_private *dev_priv)
list_for_each_entry_safe(obj, next, &dev_priv->mm.unbound_list,
global_list) {
- if (obj->pages_pin_count == 0)
+ if (i915_gem_object_put_pages(obj) == 0)
freed += obj->base.size >> PAGE_SHIFT;
- i915_gem_object_put_pages(obj);
}
return freed;
}
@@ -5091,6 +5090,7 @@ i915_gem_inactive_count(struct shrinker *shrinker, struct shrink_control *sc)
if (unlock)
mutex_unlock(&dev->struct_mutex);
+
return count;
}
@@ -5178,12 +5178,14 @@ i915_gem_inactive_scan(struct shrinker *shrinker, struct shrink_control *sc)
freed = i915_gem_purge(dev_priv, nr_to_scan);
if (freed < nr_to_scan)
- freed += __i915_gem_shrink(dev_priv, nr_to_scan,
- false);
+ freed += __i915_gem_shrink(dev_priv,
+ nr_to_scan - freed,
+ false);
if (freed < nr_to_scan)
freed += i915_gem_shrink_all(dev_priv);
if (unlock)
mutex_unlock(&dev->struct_mutex);
+
return freed;
}
--
1.8.4.rc3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Fix type mismatch and accounting in i915_gem_shrink
2013-09-24 9:52 [PATCH] drm/i915: Fix type mismatch and accounting in i915_gem_shrink Chris Wilson
@ 2013-09-24 10:29 ` Daniel Vetter
2013-10-04 9:33 ` Chris Wilson
0 siblings, 1 reply; 4+ messages in thread
From: Daniel Vetter @ 2013-09-24 10:29 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On Tue, Sep 24, 2013 at 10:52:18AM +0100, Chris Wilson wrote:
> The interface uses an unsigned long, and we can use the unsigned counter
> throughout our code, so do so. In the process, we notice one instance
> where the shrink count is based on a heuristic rather than the result,
> and another where we ask for too many pages to be purged.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/i915_gem.c | 22 ++++++++++++----------
> 1 file changed, 12 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 1bfb792..af6ff15 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -64,8 +64,8 @@ static unsigned long i915_gem_inactive_count(struct shrinker *shrinker,
> struct shrink_control *sc);
> static unsigned long i915_gem_inactive_scan(struct shrinker *shrinker,
> struct shrink_control *sc);
> -static long i915_gem_purge(struct drm_i915_private *dev_priv, long target);
> -static long i915_gem_shrink_all(struct drm_i915_private *dev_priv);
> +static unsigned long i915_gem_purge(struct drm_i915_private *dev_priv, long target);
> +static unsigned long i915_gem_shrink_all(struct drm_i915_private *dev_priv);
> static void i915_gem_object_truncate(struct drm_i915_gem_object *obj);
>
> static bool cpu_cache_is_coherent(struct drm_device *dev,
> @@ -1825,13 +1825,13 @@ i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
> return 0;
> }
>
> -static long
> +static unsigned long
> __i915_gem_shrink(struct drm_i915_private *dev_priv, long target,
> bool purgeable_only)
> {
> struct list_head still_bound_list;
> struct drm_i915_gem_object *obj, *next;
> - long count = 0;
> + unsigned long count = 0;
>
> list_for_each_entry_safe(obj, next,
> &dev_priv->mm.unbound_list,
> @@ -1897,13 +1897,13 @@ __i915_gem_shrink(struct drm_i915_private *dev_priv, long target,
> return count;
> }
>
> -static long
> +static unsigned long
> i915_gem_purge(struct drm_i915_private *dev_priv, long target)
> {
> return __i915_gem_shrink(dev_priv, target, true);
> }
>
> -static long
> +static unsigned long
> i915_gem_shrink_all(struct drm_i915_private *dev_priv)
> {
> struct drm_i915_gem_object *obj, *next;
> @@ -1913,9 +1913,8 @@ i915_gem_shrink_all(struct drm_i915_private *dev_priv)
>
> list_for_each_entry_safe(obj, next, &dev_priv->mm.unbound_list,
> global_list) {
> - if (obj->pages_pin_count == 0)
> + if (i915_gem_object_put_pages(obj) == 0)
> freed += obj->base.size >> PAGE_SHIFT;
> - i915_gem_object_put_pages(obj);
> }
> return freed;
> }
> @@ -5091,6 +5090,7 @@ i915_gem_inactive_count(struct shrinker *shrinker, struct shrink_control *sc)
>
> if (unlock)
> mutex_unlock(&dev->struct_mutex);
> +
> return count;
> }
>
> @@ -5178,12 +5178,14 @@ i915_gem_inactive_scan(struct shrinker *shrinker, struct shrink_control *sc)
nr_to_scan here still seems to be an int. Otherwise looks good.
-Daniel
>
> freed = i915_gem_purge(dev_priv, nr_to_scan);
> if (freed < nr_to_scan)
> - freed += __i915_gem_shrink(dev_priv, nr_to_scan,
> - false);
> + freed += __i915_gem_shrink(dev_priv,
> + nr_to_scan - freed,
> + false);
> if (freed < nr_to_scan)
> freed += i915_gem_shrink_all(dev_priv);
>
> if (unlock)
> mutex_unlock(&dev->struct_mutex);
> +
> return freed;
> }
> --
> 1.8.4.rc3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] drm/i915: Fix type mismatch and accounting in i915_gem_shrink
2013-09-24 10:29 ` Daniel Vetter
@ 2013-10-04 9:33 ` Chris Wilson
2013-10-04 12:38 ` Daniel Vetter
0 siblings, 1 reply; 4+ messages in thread
From: Chris Wilson @ 2013-10-04 9:33 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter
The interface uses an unsigned long, and we can use the unsigned counter
throughout our code, so do so. In the process, we notice one instance
where the shrink count is based on a heuristic rather than the result,
and another where we ask for too many pages to be purged.
v2: nr_to_scan needs to be promoted to a long as well, so just use
sc->nr_to_scan directly.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/i915_gem.c | 29 +++++++++++++++--------------
1 file changed, 15 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3a6c71b029b3..f0c4c86d03b6 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -64,8 +64,8 @@ static unsigned long i915_gem_inactive_count(struct shrinker *shrinker,
struct shrink_control *sc);
static unsigned long i915_gem_inactive_scan(struct shrinker *shrinker,
struct shrink_control *sc);
-static long i915_gem_purge(struct drm_i915_private *dev_priv, long target);
-static long i915_gem_shrink_all(struct drm_i915_private *dev_priv);
+static unsigned long i915_gem_purge(struct drm_i915_private *dev_priv, long target);
+static unsigned long i915_gem_shrink_all(struct drm_i915_private *dev_priv);
static void i915_gem_object_truncate(struct drm_i915_gem_object *obj);
static bool cpu_cache_is_coherent(struct drm_device *dev,
@@ -1825,13 +1825,13 @@ i915_gem_object_put_pages(struct drm_i915_gem_object *obj)
return 0;
}
-static long
+static unsigned long
__i915_gem_shrink(struct drm_i915_private *dev_priv, long target,
bool purgeable_only)
{
struct list_head still_bound_list;
struct drm_i915_gem_object *obj, *next;
- long count = 0;
+ unsigned long count = 0;
list_for_each_entry_safe(obj, next,
&dev_priv->mm.unbound_list,
@@ -1897,13 +1897,13 @@ __i915_gem_shrink(struct drm_i915_private *dev_priv, long target,
return count;
}
-static long
+static unsigned long
i915_gem_purge(struct drm_i915_private *dev_priv, long target)
{
return __i915_gem_shrink(dev_priv, target, true);
}
-static long
+static unsigned long
i915_gem_shrink_all(struct drm_i915_private *dev_priv)
{
struct drm_i915_gem_object *obj, *next;
@@ -1913,9 +1913,8 @@ i915_gem_shrink_all(struct drm_i915_private *dev_priv)
list_for_each_entry_safe(obj, next, &dev_priv->mm.unbound_list,
global_list) {
- if (obj->pages_pin_count == 0)
+ if (i915_gem_object_put_pages(obj) == 0)
freed += obj->base.size >> PAGE_SHIFT;
- i915_gem_object_put_pages(obj);
}
return freed;
}
@@ -5092,6 +5091,7 @@ i915_gem_inactive_count(struct shrinker *shrinker, struct shrink_control *sc)
if (unlock)
mutex_unlock(&dev->struct_mutex);
+
return count;
}
@@ -5163,7 +5163,6 @@ i915_gem_inactive_scan(struct shrinker *shrinker, struct shrink_control *sc)
struct drm_i915_private,
mm.inactive_shrinker);
struct drm_device *dev = dev_priv->dev;
- int nr_to_scan = sc->nr_to_scan;
unsigned long freed;
bool unlock = true;
@@ -5177,15 +5176,17 @@ i915_gem_inactive_scan(struct shrinker *shrinker, struct shrink_control *sc)
unlock = false;
}
- freed = i915_gem_purge(dev_priv, nr_to_scan);
- if (freed < nr_to_scan)
- freed += __i915_gem_shrink(dev_priv, nr_to_scan,
- false);
- if (freed < nr_to_scan)
+ freed = i915_gem_purge(dev_priv, sc->nr_to_scan);
+ if (freed < sc->nr_to_scan)
+ freed += __i915_gem_shrink(dev_priv,
+ sc->nr_to_scan - freed,
+ false);
+ if (freed < sc->nr_to_scan)
freed += i915_gem_shrink_all(dev_priv);
if (unlock)
mutex_unlock(&dev->struct_mutex);
+
return freed;
}
--
1.8.4.rc3
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Fix type mismatch and accounting in i915_gem_shrink
2013-10-04 9:33 ` Chris Wilson
@ 2013-10-04 12:38 ` Daniel Vetter
0 siblings, 0 replies; 4+ messages in thread
From: Daniel Vetter @ 2013-10-04 12:38 UTC (permalink / raw)
To: Chris Wilson; +Cc: Daniel Vetter, intel-gfx
On Fri, Oct 04, 2013 at 10:33:00AM +0100, Chris Wilson wrote:
> The interface uses an unsigned long, and we can use the unsigned counter
> throughout our code, so do so. In the process, we notice one instance
> where the shrink count is based on a heuristic rather than the result,
> and another where we ask for too many pages to be purged.
>
> v2: nr_to_scan needs to be promoted to a long as well, so just use
> sc->nr_to_scan directly.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Queued for -next, thanks for the patch.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-10-04 12:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-24 9:52 [PATCH] drm/i915: Fix type mismatch and accounting in i915_gem_shrink Chris Wilson
2013-09-24 10:29 ` Daniel Vetter
2013-10-04 9:33 ` Chris Wilson
2013-10-04 12:38 ` Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox