All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/shrinker: Only report objects with extra pinned pages as pinned
@ 2016-04-20  7:43 Chris Wilson
  2016-04-20  8:24 ` ✗ Fi.CI.BAT: failure for " Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Chris Wilson @ 2016-04-20  7:43 UTC (permalink / raw)
  To: intel-gfx; +Cc: Akash Goel

When iterating over the bound list, we expect all objects there to have
their pages pinned (by the bound VMA). So only add those objects with
additional pin count on their pages as "pinned". These should be those
objects used for display and hardware access.

Reported-by: Akash Goel <akash.goel@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Akash Goel <akash.goel@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_shrinker.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index d46388f25e04..4e4fcfa76b4c 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -361,7 +361,7 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
 		if (!obj->base.filp)
 			continue;
 
-		if (obj->pages_pin_count)
+		if (obj->pages_pin_count > num_vma_bound(obj))
 			pinned += obj->base.size;
 		else
 			bound += obj->base.size;
-- 
2.8.0.rc3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* ✗ Fi.CI.BAT: failure for drm/i915/shrinker: Only report objects with extra pinned pages as pinned
  2016-04-20  7:43 [PATCH] drm/i915/shrinker: Only report objects with extra pinned pages as pinned Chris Wilson
@ 2016-04-20  8:24 ` Patchwork
  2016-04-20  8:56 ` [PATCH] " Joonas Lahtinen
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2016-04-20  8:24 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/shrinker: Only report objects with extra pinned pages as pinned
URL   : https://patchwork.freedesktop.org/series/5968/
State : failure

== Summary ==

Series 5968v1 drm/i915/shrinker: Only report objects with extra pinned pages as pinned
http://patchwork.freedesktop.org/api/1.0/series/5968/revisions/1/mbox/

Test gem_busy:
        Subgroup basic-blt:
                skip       -> PASS       (bsw-nuc-2)
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                fail       -> PASS       (byt-nuc)

bdw-nuci7        total:192  pass:180  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:192  pass:169  dwarn:0   dfail:0   fail:0   skip:23 
bsw-nuc-2        total:191  pass:152  dwarn:0   dfail:0   fail:0   skip:39 
byt-nuc          total:191  pass:153  dwarn:0   dfail:0   fail:0   skip:38 
hsw-brixbox      total:192  pass:168  dwarn:0   dfail:0   fail:0   skip:24 
hsw-gt2          total:192  pass:173  dwarn:0   dfail:0   fail:0   skip:19 
ivb-t430s        total:192  pass:164  dwarn:0   dfail:0   fail:0   skip:28 
skl-i7k-2        total:192  pass:167  dwarn:0   dfail:0   fail:0   skip:25 
skl-nuci5        total:192  pass:181  dwarn:0   dfail:0   fail:0   skip:11 
snb-dellxps      total:192  pass:153  dwarn:1   dfail:0   fail:0   skip:38 
snb-x220t        total:192  pass:154  dwarn:0   dfail:0   fail:1   skip:37 
BOOT FAILED for ilk-hp8440p

Results at /archive/results/CI_IGT_test/Patchwork_1951/

30468e0d25064b544b8da9d14fd7727fdd650064 drm-intel-nightly: 2016y-04m-19d-16h-24m-23s UTC integration manifest
18a28e7 drm/i915/shrinker: Only report objects with extra pinned pages as pinned

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915/shrinker: Only report objects with extra pinned pages as pinned
  2016-04-20  7:43 [PATCH] drm/i915/shrinker: Only report objects with extra pinned pages as pinned Chris Wilson
  2016-04-20  8:24 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2016-04-20  8:56 ` Joonas Lahtinen
  2016-04-20 11:04   ` [PATCH v2 1/3] " Chris Wilson
  2016-04-20  9:03 ` ✓ Fi.CI.BAT: success for drm/i915/shrinker: Only report objects with extra pinned pages as pinned Patchwork
  2016-04-20 10:23 ` Patchwork
  3 siblings, 1 reply; 10+ messages in thread
From: Joonas Lahtinen @ 2016-04-20  8:56 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: Akash Goel

On ke, 2016-04-20 at 08:43 +0100, Chris Wilson wrote:
> When iterating over the bound list, we expect all objects there to have
> their pages pinned (by the bound VMA). So only add those objects with
> additional pin count on their pages as "pinned". These should be those
> objects used for display and hardware access.
> 
> Reported-by: Akash Goel <akash.goel@intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> Cc: Akash Goel <akash.goel@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_shrinker.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> index d46388f25e04..4e4fcfa76b4c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> @@ -361,7 +361,7 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
>  		if (!obj->base.filp)
>  			continue;
>  
> -		if (obj->pages_pin_count)
> +		if (obj->pages_pin_count > num_vma_bound(obj))
>  			pinned += obj->base.size;
>  		else
>  			bound += obj->base.size;
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915/shrinker: Only report objects with extra pinned pages as pinned
  2016-04-20  7:43 [PATCH] drm/i915/shrinker: Only report objects with extra pinned pages as pinned Chris Wilson
  2016-04-20  8:24 ` ✗ Fi.CI.BAT: failure for " Patchwork
  2016-04-20  8:56 ` [PATCH] " Joonas Lahtinen
@ 2016-04-20  9:03 ` Patchwork
  2016-04-20 10:23 ` Patchwork
  3 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2016-04-20  9:03 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/shrinker: Only report objects with extra pinned pages as pinned
URL   : https://patchwork.freedesktop.org/series/5968/
State : success

== Summary ==

Series 5968v1 drm/i915/shrinker: Only report objects with extra pinned pages as pinned
http://patchwork.freedesktop.org/api/1.0/series/5968/revisions/1/mbox/

Test gem_busy:
        Subgroup basic-blt:
                skip       -> PASS       (bsw-nuc-2)
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                fail       -> PASS       (byt-nuc)

bdw-nuci7        total:192  pass:180  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:192  pass:169  dwarn:0   dfail:0   fail:0   skip:23 
bsw-nuc-2        total:191  pass:152  dwarn:0   dfail:0   fail:0   skip:39 
byt-nuc          total:191  pass:153  dwarn:0   dfail:0   fail:0   skip:38 
hsw-brixbox      total:192  pass:168  dwarn:0   dfail:0   fail:0   skip:24 
hsw-gt2          total:192  pass:173  dwarn:0   dfail:0   fail:0   skip:19 
ivb-t430s        total:192  pass:164  dwarn:0   dfail:0   fail:0   skip:28 
skl-i7k-2        total:192  pass:167  dwarn:0   dfail:0   fail:0   skip:25 
skl-nuci5        total:192  pass:181  dwarn:0   dfail:0   fail:0   skip:11 
snb-dellxps      total:192  pass:153  dwarn:1   dfail:0   fail:0   skip:38 
snb-x220t        total:192  pass:154  dwarn:0   dfail:0   fail:1   skip:37 
BOOT FAILED for ilk-hp8440p (might be CI problem)

Results at /archive/results/CI_IGT_test/Patchwork_1951/

30468e0d25064b544b8da9d14fd7727fdd650064 drm-intel-nightly: 2016y-04m-19d-16h-24m-23s UTC integration manifest
18a28e7 drm/i915/shrinker: Only report objects with extra pinned pages as pinned

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915/shrinker: Only report objects with extra pinned pages as pinned
  2016-04-20  7:43 [PATCH] drm/i915/shrinker: Only report objects with extra pinned pages as pinned Chris Wilson
                   ` (2 preceding siblings ...)
  2016-04-20  9:03 ` ✓ Fi.CI.BAT: success for drm/i915/shrinker: Only report objects with extra pinned pages as pinned Patchwork
@ 2016-04-20 10:23 ` Patchwork
  3 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2016-04-20 10:23 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/shrinker: Only report objects with extra pinned pages as pinned
URL   : https://patchwork.freedesktop.org/series/5968/
State : success

== Summary ==

Series 5968v1 drm/i915/shrinker: Only report objects with extra pinned pages as pinned
http://patchwork.freedesktop.org/api/1.0/series/5968/revisions/1/mbox/


bdw-nuci7        total:192  pass:180  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:192  pass:169  dwarn:0   dfail:0   fail:0   skip:23 
byt-nuc          total:191  pass:153  dwarn:0   dfail:0   fail:0   skip:38 
hsw-brixbox      total:192  pass:168  dwarn:0   dfail:0   fail:0   skip:24 
ilk-hp8440p      total:192  pass:135  dwarn:0   dfail:0   fail:0   skip:57 
ivb-t430s        total:192  pass:164  dwarn:0   dfail:0   fail:0   skip:28 
skl-i7k-2        total:192  pass:167  dwarn:0   dfail:0   fail:0   skip:25 
skl-nuci5        total:192  pass:181  dwarn:0   dfail:0   fail:0   skip:11 
snb-x220t        total:192  pass:154  dwarn:0   dfail:0   fail:1   skip:37 
BOOT FAILED for bsw-nuc-2 (might be CI problem)
BOOT FAILED for hsw-gt2 (might be CI problem)
BOOT FAILED for snb-dellxps (might be CI problem)

Results at /archive/results/CI_IGT_test/Patchwork_1952/

9ab57e8f35c57ac8d8ac50f24b540039741cfbcc drm-intel-nightly: 2016y-04m-20d-09h-27m-27s UTC integration manifest
53a8c98 drm/i915/shrinker: Only report objects with extra pinned pages as pinned

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2 1/3] drm/i915/shrinker: Only report objects with extra pinned pages as pinned
  2016-04-20  8:56 ` [PATCH] " Joonas Lahtinen
@ 2016-04-20 11:04   ` Chris Wilson
  2016-04-20 11:04     ` [PATCH v2 2/3] drm/i915/shrinker: Report "unevictable" pages Chris Wilson
  2016-04-20 11:04     ` [PATCH v2 3/3] drm/i915/shrinker: Only shmemfs objects are backed by swap Chris Wilson
  0 siblings, 2 replies; 10+ messages in thread
From: Chris Wilson @ 2016-04-20 11:04 UTC (permalink / raw)
  To: intel-gfx; +Cc: Akash Goel

When iterating over the bound list, we expect all objects there to have
their pages pinned (by the bound VMA). So only report those objects with
additional pin count on their pages as "pinned". These should be those
objects used for display and hardware access.

Reported-by: Akash Goel <akash.goel@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Akash Goel <akash.goel@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_shrinker.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index d46388f25e04..4e4fcfa76b4c 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -361,7 +361,7 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
 		if (!obj->base.filp)
 			continue;
 
-		if (obj->pages_pin_count)
+		if (obj->pages_pin_count > num_vma_bound(obj))
 			pinned += obj->base.size;
 		else
 			bound += obj->base.size;
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 2/3] drm/i915/shrinker: Report "unevictable" pages
  2016-04-20 11:04   ` [PATCH v2 1/3] " Chris Wilson
@ 2016-04-20 11:04     ` Chris Wilson
  2016-04-20 12:02       ` Joonas Lahtinen
  2016-04-20 11:04     ` [PATCH v2 3/3] drm/i915/shrinker: Only shmemfs objects are backed by swap Chris Wilson
  1 sibling, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2016-04-20 11:04 UTC (permalink / raw)
  To: intel-gfx

Inside the shrinker we call can_release_pages() to indicate whether or
not we can make forward progress in freeing up memory by unbinding that
object. When adding our report to oom, we should be using the same
logic.

Whilst here, change the reporting from bytes to pages so that it looks
smaller to the user!, is consistent with the neighbouring oom report
itself which displays counts in pages, and makes the unsigned long
overflow less likely.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_shrinker.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index 4e4fcfa76b4c..cb225e039d48 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -336,7 +336,7 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
 		container_of(nb, struct drm_i915_private, mm.oom_notifier);
 	struct shrinker_lock_uninterruptible slu;
 	struct drm_i915_gem_object *obj;
-	unsigned long pinned, bound, unbound, freed_pages;
+	unsigned long unevictable, bound, unbound, freed_pages;
 
 	if (!i915_gem_shrinker_lock_uninterruptible(dev_priv, &slu, 5000))
 		return NOTIFY_DONE;
@@ -347,33 +347,33 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
 	 * assert that there are no objects with pinned pages that are not
 	 * being pointed to by hardware.
 	 */
-	unbound = bound = pinned = 0;
+	unbound = bound = unevictable = 0;
 	list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_list) {
 		if (!obj->base.filp) /* not backed by a freeable object */
 			continue;
 
-		if (obj->pages_pin_count)
-			pinned += obj->base.size;
+		if (!can_release_pages(obj))
+			unevictable += obj->base.size >> PAGE_SHIFT;
 		else
-			unbound += obj->base.size;
+			unbound += obj->base.size >> PAGE_SHIFT;
 	}
 	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
 		if (!obj->base.filp)
 			continue;
 
-		if (obj->pages_pin_count > num_vma_bound(obj))
-			pinned += obj->base.size;
+		if (!can_release_pages(obj))
+			unevictable += obj->base.size >> PAGE_SHIFT;
 		else
-			bound += obj->base.size;
+			bound += obj->base.size >> PAGE_SHIFT;
 	}
 
 	i915_gem_shrinker_unlock_uninterruptible(dev_priv, &slu);
 
 	if (freed_pages || unbound || bound)
-		pr_info("Purging GPU memory, %lu bytes freed, %lu bytes still pinned.\n",
-			freed_pages << PAGE_SHIFT, pinned);
+		pr_info("Purging GPU memory, %lu pages freed, %lu pages still pinned.\n",
+			freed_pages, unevictable);
 	if (unbound || bound)
-		pr_err("%lu and %lu bytes still available in the "
+		pr_err("%lu and %lu pages still available in the "
 		       "bound and unbound GPU page lists.\n",
 		       bound, unbound);
 
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v2 3/3] drm/i915/shrinker: Only shmemfs objects are backed by swap
  2016-04-20 11:04   ` [PATCH v2 1/3] " Chris Wilson
  2016-04-20 11:04     ` [PATCH v2 2/3] drm/i915/shrinker: Report "unevictable" pages Chris Wilson
@ 2016-04-20 11:04     ` Chris Wilson
  2016-04-20 12:07       ` Joonas Lahtinen
  1 sibling, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2016-04-20 11:04 UTC (permalink / raw)
  To: intel-gfx

Since we can only swap out shmemfs objects, those are the only ones that
influence the ability of the shrinker to can free pages. Currently, all
non-shmemfs objects have a raised pages_pin_count to protect them from
the shrinker, so this just makes the logic for can_release_pages()
clearer (and safer in future so that we don't over estimate our ability
to free up pages from future non-swappable objects).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem_shrinker.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index cb225e039d48..e44c6358bd5a 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -70,6 +70,10 @@ static bool swap_available(void)
 
 static bool can_release_pages(struct drm_i915_gem_object *obj)
 {
+	/* Only shmemfs objects are backed by swapped */
+	if (!obj->base.filp)
+		return false;
+
 	/* Only report true if by unbinding the object and putting its pages
 	 * we can actually make forward progress towards freeing physical
 	 * pages.
@@ -349,18 +353,12 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
 	 */
 	unbound = bound = unevictable = 0;
 	list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_list) {
-		if (!obj->base.filp) /* not backed by a freeable object */
-			continue;
-
 		if (!can_release_pages(obj))
 			unevictable += obj->base.size >> PAGE_SHIFT;
 		else
 			unbound += obj->base.size >> PAGE_SHIFT;
 	}
 	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
-		if (!obj->base.filp)
-			continue;
-
 		if (!can_release_pages(obj))
 			unevictable += obj->base.size >> PAGE_SHIFT;
 		else
-- 
2.8.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 2/3] drm/i915/shrinker: Report "unevictable" pages
  2016-04-20 11:04     ` [PATCH v2 2/3] drm/i915/shrinker: Report "unevictable" pages Chris Wilson
@ 2016-04-20 12:02       ` Joonas Lahtinen
  0 siblings, 0 replies; 10+ messages in thread
From: Joonas Lahtinen @ 2016-04-20 12:02 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On ke, 2016-04-20 at 12:04 +0100, Chris Wilson wrote:
> Inside the shrinker we call can_release_pages() to indicate whether or
> not we can make forward progress in freeing up memory by unbinding that
> object. When adding our report to oom, we should be using the same
> logic.
> 
> Whilst here, change the reporting from bytes to pages so that it looks
> smaller to the user!, is consistent with the neighbouring oom report
> itself which displays counts in pages, and makes the unsigned long
> overflow less likely.

Makes sense as that is the smallest pinning unit, too.

> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_shrinker.c | 22 +++++++++++-----------
>  1 file changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> index 4e4fcfa76b4c..cb225e039d48 100644
> --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> @@ -336,7 +336,7 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
>  		container_of(nb, struct drm_i915_private, mm.oom_notifier);
>  	struct shrinker_lock_uninterruptible slu;
>  	struct drm_i915_gem_object *obj;
> -	unsigned long pinned, bound, unbound, freed_pages;
> +	unsigned long unevictable, bound, unbound, freed_pages;
>  
>  	if (!i915_gem_shrinker_lock_uninterruptible(dev_priv, &slu, 5000))
>  		return NOTIFY_DONE;
> @@ -347,33 +347,33 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
>  	 * assert that there are no objects with pinned pages that are not
>  	 * being pointed to by hardware.
>  	 */
> -	unbound = bound = pinned = 0;
> +	unbound = bound = unevictable = 0;
>  	list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_list) {
>  		if (!obj->base.filp) /* not backed by a freeable object */
>  			continue;
>  
> -		if (obj->pages_pin_count)
> -			pinned += obj->base.size;
> +		if (!can_release_pages(obj))
> +			unevictable += obj->base.size >> PAGE_SHIFT;
>  		else
> -			unbound += obj->base.size;
> +			unbound += obj->base.size >> PAGE_SHIFT;
>  	}
>  	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
>  		if (!obj->base.filp)
>  			continue;
>  
> -		if (obj->pages_pin_count > num_vma_bound(obj))
> -			pinned += obj->base.size;
> +		if (!can_release_pages(obj))
> +			unevictable += obj->base.size >> PAGE_SHIFT;
>  		else
> -			bound += obj->base.size;
> +			bound += obj->base.size >> PAGE_SHIFT;
>  	}
>  
>  	i915_gem_shrinker_unlock_uninterruptible(dev_priv, &slu);
>  
>  	if (freed_pages || unbound || bound)
> -		pr_info("Purging GPU memory, %lu bytes freed, %lu bytes still pinned.\n",
> -			freed_pages << PAGE_SHIFT, pinned);
> +		pr_info("Purging GPU memory, %lu pages freed, %lu pages still pinned.\n",
> +			freed_pages, unevictable);

My code checker senses are tingling, this looks very much like
oversized line, mind cutting it down?

With that addressed,

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

>  	if (unbound || bound)
> -		pr_err("%lu and %lu bytes still available in the "
> +		pr_err("%lu and %lu pages still available in the "
>  		       "bound and unbound GPU page lists.\n",
>  		       bound, unbound);
>  
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2 3/3] drm/i915/shrinker: Only shmemfs objects are backed by swap
  2016-04-20 11:04     ` [PATCH v2 3/3] drm/i915/shrinker: Only shmemfs objects are backed by swap Chris Wilson
@ 2016-04-20 12:07       ` Joonas Lahtinen
  0 siblings, 0 replies; 10+ messages in thread
From: Joonas Lahtinen @ 2016-04-20 12:07 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On ke, 2016-04-20 at 12:04 +0100, Chris Wilson wrote:
> Since we can only swap out shmemfs objects, those are the only ones that
> influence the ability of the shrinker to can free pages. Currently, all
> non-shmemfs objects have a raised pages_pin_count to protect them from
> the shrinker, so this just makes the logic for can_release_pages()
> clearer (and safer in future so that we don't over estimate our ability
> to free up pages from future non-swappable objects).
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_gem_shrinker.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> index cb225e039d48..e44c6358bd5a 100644
> --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> @@ -70,6 +70,10 @@ static bool swap_available(void)
>  
>  static bool can_release_pages(struct drm_i915_gem_object *obj)
>  {
> +	/* Only shmemfs objects are backed by swapped */
> +	if (!obj->base.filp)
> +		return false;
> +
>  	/* Only report true if by unbinding the object and putting its pages
>  	 * we can actually make forward progress towards freeing physical
>  	 * pages.
> @@ -349,18 +353,12 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
>  	 */
>  	unbound = bound = unevictable = 0;
>  	list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_list) {
> -		if (!obj->base.filp) /* not backed by a freeable object */
> -			continue;
> -
>  		if (!can_release_pages(obj))
>  			unevictable += obj->base.size >> PAGE_SHIFT;
>  		else
>  			unbound += obj->base.size >> PAGE_SHIFT;
>  	}
>  	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
> -		if (!obj->base.filp)
> -			continue;
> -
>  		if (!can_release_pages(obj))
>  			unevictable += obj->base.size >> PAGE_SHIFT;
>  		else
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2016-04-20 12:06 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-20  7:43 [PATCH] drm/i915/shrinker: Only report objects with extra pinned pages as pinned Chris Wilson
2016-04-20  8:24 ` ✗ Fi.CI.BAT: failure for " Patchwork
2016-04-20  8:56 ` [PATCH] " Joonas Lahtinen
2016-04-20 11:04   ` [PATCH v2 1/3] " Chris Wilson
2016-04-20 11:04     ` [PATCH v2 2/3] drm/i915/shrinker: Report "unevictable" pages Chris Wilson
2016-04-20 12:02       ` Joonas Lahtinen
2016-04-20 11:04     ` [PATCH v2 3/3] drm/i915/shrinker: Only shmemfs objects are backed by swap Chris Wilson
2016-04-20 12:07       ` Joonas Lahtinen
2016-04-20  9:03 ` ✓ Fi.CI.BAT: success for drm/i915/shrinker: Only report objects with extra pinned pages as pinned Patchwork
2016-04-20 10:23 ` Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.