All of lore.kernel.org
 help / color / mirror / Atom feed
* vmap purge notifier for exhaustion of the vmalloc arena
@ 2016-04-04 13:46 Chris Wilson
  2016-04-04 13:46 ` [PATCH v2 1/3] drm/i915/shrinker: Account for unshrinkable unbound pages Chris Wilson
                   ` (4 more replies)
  0 siblings, 5 replies; 22+ messages in thread
From: Chris Wilson @ 2016-04-04 13:46 UTC (permalink / raw)
  To: intel-gfx

Andrew has already acked this for merge through the DRM tree, but it still
needs a review. vmalloc() is quite an extensively used interface so the key
question is have we broken anything by adding a blocking notifier into the
callchain (though we only block if gfp_t says we can).
-Chris

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

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

* [PATCH v2 1/3] drm/i915/shrinker: Account for unshrinkable unbound pages
  2016-04-04 13:46 vmap purge notifier for exhaustion of the vmalloc arena Chris Wilson
@ 2016-04-04 13:46 ` Chris Wilson
  2016-04-05  7:57   ` Joonas Lahtinen
  2016-04-04 13:46   ` Chris Wilson
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2016-04-04 13:46 UTC (permalink / raw)
  To: intel-gfx; +Cc: Akash Goel

Since we only attempt to purge an object if can_release_pages() report
true, we should also only add it to the count of potential recoverable
pages when can_release_pages() is true.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@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 d3c473ffb90a..e391ee3ec486 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -246,7 +246,7 @@ i915_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc)
 
 	count = 0;
 	list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_list)
-		if (obj->pages_pin_count == 0)
+		if (can_release_pages(obj))
 			count += obj->base.size >> PAGE_SHIFT;
 
 	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
-- 
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] 22+ messages in thread

* [PATCH v2 2/3] mm/vmap: Add a notifier for when we run out of vmap address space
  2016-04-04 13:46 vmap purge notifier for exhaustion of the vmalloc arena Chris Wilson
  2016-04-04 13:46 ` [PATCH v2 1/3] drm/i915/shrinker: Account for unshrinkable unbound pages Chris Wilson
@ 2016-04-04 13:46   ` Chris Wilson
  2016-04-04 13:46   ` Chris Wilson
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2016-04-04 13:46 UTC (permalink / raw)
  To: intel-gfx
  Cc: Chris Wilson, Andrew Morton, David Rientjes, Roman Peniaev,
	Mel Gorman, linux-mm, linux-kernel, Joonas Lahtinen,
	Tvrtko Ursulin

vmaps are temporary kernel mappings that may be of long duration.
Reusing a vmap on an object is preferrable for a driver as the cost of
setting up the vmap can otherwise dominate the operation on the object.
However, the vmap address space is rather limited on 32bit systems and
so we add a notification for vmap pressure in order for the driver to
release any cached vmappings.

The interface is styled after the oom-notifier where the callees are
passed a pointer to an unsigned long counter for them to indicate if they
have freed any space.

v2: Guard the blocking notifier call with gfpflags_allow_blocking()
v3: Correct typo in forward declaration and move to head of file

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Roman Peniaev <r.peniaev@gmail.com>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Acked-by: Andrew Morton <akpm@linux-foundation.org> # for inclusion via DRM
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 include/linux/vmalloc.h |  4 ++++
 mm/vmalloc.c            | 27 +++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
index d1f1d338af20..8b51df3ab334 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -8,6 +8,7 @@
 #include <linux/rbtree.h>
 
 struct vm_area_struct;		/* vma defining user mapping in mm_types.h */
+struct notifier_block;		/* in notifier.h */
 
 /* bits in flags of vmalloc's vm_struct below */
 #define VM_IOREMAP		0x00000001	/* ioremap() and friends */
@@ -187,4 +188,7 @@ pcpu_free_vm_areas(struct vm_struct **vms, int nr_vms)
 #define VMALLOC_TOTAL 0UL
 #endif
 
+int register_vmap_purge_notifier(struct notifier_block *nb);
+int unregister_vmap_purge_notifier(struct notifier_block *nb);
+
 #endif /* _LINUX_VMALLOC_H */
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index ae7d20b447ff..293889d7f482 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -21,6 +21,7 @@
 #include <linux/debugobjects.h>
 #include <linux/kallsyms.h>
 #include <linux/list.h>
+#include <linux/notifier.h>
 #include <linux/rbtree.h>
 #include <linux/radix-tree.h>
 #include <linux/rcupdate.h>
@@ -344,6 +345,8 @@ static void __insert_vmap_area(struct vmap_area *va)
 
 static void purge_vmap_area_lazy(void);
 
+static BLOCKING_NOTIFIER_HEAD(vmap_notify_list);
+
 /*
  * Allocate a region of KVA of the specified size and alignment, within the
  * vstart and vend.
@@ -363,6 +366,8 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
 	BUG_ON(offset_in_page(size));
 	BUG_ON(!is_power_of_2(align));
 
+	might_sleep_if(gfpflags_allow_blocking(gfp_mask));
+
 	va = kmalloc_node(sizeof(struct vmap_area),
 			gfp_mask & GFP_RECLAIM_MASK, node);
 	if (unlikely(!va))
@@ -468,6 +473,16 @@ overflow:
 		purged = 1;
 		goto retry;
 	}
+
+	if (gfpflags_allow_blocking(gfp_mask)) {
+		unsigned long freed = 0;
+		blocking_notifier_call_chain(&vmap_notify_list, 0, &freed);
+		if (freed > 0) {
+			purged = 0;
+			goto retry;
+		}
+	}
+
 	if (printk_ratelimit())
 		pr_warn("vmap allocation for size %lu failed: use vmalloc=<size> to increase size\n",
 			size);
@@ -475,6 +490,18 @@ overflow:
 	return ERR_PTR(-EBUSY);
 }
 
+int register_vmap_purge_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&vmap_notify_list, nb);
+}
+EXPORT_SYMBOL_GPL(register_vmap_purge_notifier);
+
+int unregister_vmap_purge_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&vmap_notify_list, nb);
+}
+EXPORT_SYMBOL_GPL(unregister_vmap_purge_notifier);
+
 static void __free_vmap_area(struct vmap_area *va)
 {
 	BUG_ON(RB_EMPTY_NODE(&va->rb_node));
-- 
2.8.0.rc3

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

* [PATCH v2 2/3] mm/vmap: Add a notifier for when we run out of vmap address space
@ 2016-04-04 13:46   ` Chris Wilson
  0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2016-04-04 13:46 UTC (permalink / raw)
  To: intel-gfx
  Cc: Chris Wilson, Andrew Morton, David Rientjes, Roman Peniaev,
	Mel Gorman, linux-mm, linux-kernel, Joonas Lahtinen,
	Tvrtko Ursulin

vmaps are temporary kernel mappings that may be of long duration.
Reusing a vmap on an object is preferrable for a driver as the cost of
setting up the vmap can otherwise dominate the operation on the object.
However, the vmap address space is rather limited on 32bit systems and
so we add a notification for vmap pressure in order for the driver to
release any cached vmappings.

The interface is styled after the oom-notifier where the callees are
passed a pointer to an unsigned long counter for them to indicate if they
have freed any space.

v2: Guard the blocking notifier call with gfpflags_allow_blocking()
v3: Correct typo in forward declaration and move to head of file

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Roman Peniaev <r.peniaev@gmail.com>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Acked-by: Andrew Morton <akpm@linux-foundation.org> # for inclusion via DRM
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 include/linux/vmalloc.h |  4 ++++
 mm/vmalloc.c            | 27 +++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
index d1f1d338af20..8b51df3ab334 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -8,6 +8,7 @@
 #include <linux/rbtree.h>
 
 struct vm_area_struct;		/* vma defining user mapping in mm_types.h */
+struct notifier_block;		/* in notifier.h */
 
 /* bits in flags of vmalloc's vm_struct below */
 #define VM_IOREMAP		0x00000001	/* ioremap() and friends */
@@ -187,4 +188,7 @@ pcpu_free_vm_areas(struct vm_struct **vms, int nr_vms)
 #define VMALLOC_TOTAL 0UL
 #endif
 
+int register_vmap_purge_notifier(struct notifier_block *nb);
+int unregister_vmap_purge_notifier(struct notifier_block *nb);
+
 #endif /* _LINUX_VMALLOC_H */
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index ae7d20b447ff..293889d7f482 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -21,6 +21,7 @@
 #include <linux/debugobjects.h>
 #include <linux/kallsyms.h>
 #include <linux/list.h>
+#include <linux/notifier.h>
 #include <linux/rbtree.h>
 #include <linux/radix-tree.h>
 #include <linux/rcupdate.h>
@@ -344,6 +345,8 @@ static void __insert_vmap_area(struct vmap_area *va)
 
 static void purge_vmap_area_lazy(void);
 
+static BLOCKING_NOTIFIER_HEAD(vmap_notify_list);
+
 /*
  * Allocate a region of KVA of the specified size and alignment, within the
  * vstart and vend.
@@ -363,6 +366,8 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
 	BUG_ON(offset_in_page(size));
 	BUG_ON(!is_power_of_2(align));
 
+	might_sleep_if(gfpflags_allow_blocking(gfp_mask));
+
 	va = kmalloc_node(sizeof(struct vmap_area),
 			gfp_mask & GFP_RECLAIM_MASK, node);
 	if (unlikely(!va))
@@ -468,6 +473,16 @@ overflow:
 		purged = 1;
 		goto retry;
 	}
+
+	if (gfpflags_allow_blocking(gfp_mask)) {
+		unsigned long freed = 0;
+		blocking_notifier_call_chain(&vmap_notify_list, 0, &freed);
+		if (freed > 0) {
+			purged = 0;
+			goto retry;
+		}
+	}
+
 	if (printk_ratelimit())
 		pr_warn("vmap allocation for size %lu failed: use vmalloc=<size> to increase size\n",
 			size);
@@ -475,6 +490,18 @@ overflow:
 	return ERR_PTR(-EBUSY);
 }
 
+int register_vmap_purge_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&vmap_notify_list, nb);
+}
+EXPORT_SYMBOL_GPL(register_vmap_purge_notifier);
+
+int unregister_vmap_purge_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&vmap_notify_list, nb);
+}
+EXPORT_SYMBOL_GPL(unregister_vmap_purge_notifier);
+
 static void __free_vmap_area(struct vmap_area *va)
 {
 	BUG_ON(RB_EMPTY_NODE(&va->rb_node));
-- 
2.8.0.rc3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* [PATCH v2 2/3] mm/vmap: Add a notifier for when we run out of vmap address space
@ 2016-04-04 13:46   ` Chris Wilson
  0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2016-04-04 13:46 UTC (permalink / raw)
  To: intel-gfx
  Cc: linux-kernel, Roman Peniaev, linux-mm, David Rientjes,
	Andrew Morton, Mel Gorman

vmaps are temporary kernel mappings that may be of long duration.
Reusing a vmap on an object is preferrable for a driver as the cost of
setting up the vmap can otherwise dominate the operation on the object.
However, the vmap address space is rather limited on 32bit systems and
so we add a notification for vmap pressure in order for the driver to
release any cached vmappings.

The interface is styled after the oom-notifier where the callees are
passed a pointer to an unsigned long counter for them to indicate if they
have freed any space.

v2: Guard the blocking notifier call with gfpflags_allow_blocking()
v3: Correct typo in forward declaration and move to head of file

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Roman Peniaev <r.peniaev@gmail.com>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Acked-by: Andrew Morton <akpm@linux-foundation.org> # for inclusion via DRM
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 include/linux/vmalloc.h |  4 ++++
 mm/vmalloc.c            | 27 +++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
index d1f1d338af20..8b51df3ab334 100644
--- a/include/linux/vmalloc.h
+++ b/include/linux/vmalloc.h
@@ -8,6 +8,7 @@
 #include <linux/rbtree.h>
 
 struct vm_area_struct;		/* vma defining user mapping in mm_types.h */
+struct notifier_block;		/* in notifier.h */
 
 /* bits in flags of vmalloc's vm_struct below */
 #define VM_IOREMAP		0x00000001	/* ioremap() and friends */
@@ -187,4 +188,7 @@ pcpu_free_vm_areas(struct vm_struct **vms, int nr_vms)
 #define VMALLOC_TOTAL 0UL
 #endif
 
+int register_vmap_purge_notifier(struct notifier_block *nb);
+int unregister_vmap_purge_notifier(struct notifier_block *nb);
+
 #endif /* _LINUX_VMALLOC_H */
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index ae7d20b447ff..293889d7f482 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -21,6 +21,7 @@
 #include <linux/debugobjects.h>
 #include <linux/kallsyms.h>
 #include <linux/list.h>
+#include <linux/notifier.h>
 #include <linux/rbtree.h>
 #include <linux/radix-tree.h>
 #include <linux/rcupdate.h>
@@ -344,6 +345,8 @@ static void __insert_vmap_area(struct vmap_area *va)
 
 static void purge_vmap_area_lazy(void);
 
+static BLOCKING_NOTIFIER_HEAD(vmap_notify_list);
+
 /*
  * Allocate a region of KVA of the specified size and alignment, within the
  * vstart and vend.
@@ -363,6 +366,8 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
 	BUG_ON(offset_in_page(size));
 	BUG_ON(!is_power_of_2(align));
 
+	might_sleep_if(gfpflags_allow_blocking(gfp_mask));
+
 	va = kmalloc_node(sizeof(struct vmap_area),
 			gfp_mask & GFP_RECLAIM_MASK, node);
 	if (unlikely(!va))
@@ -468,6 +473,16 @@ overflow:
 		purged = 1;
 		goto retry;
 	}
+
+	if (gfpflags_allow_blocking(gfp_mask)) {
+		unsigned long freed = 0;
+		blocking_notifier_call_chain(&vmap_notify_list, 0, &freed);
+		if (freed > 0) {
+			purged = 0;
+			goto retry;
+		}
+	}
+
 	if (printk_ratelimit())
 		pr_warn("vmap allocation for size %lu failed: use vmalloc=<size> to increase size\n",
 			size);
@@ -475,6 +490,18 @@ overflow:
 	return ERR_PTR(-EBUSY);
 }
 
+int register_vmap_purge_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&vmap_notify_list, nb);
+}
+EXPORT_SYMBOL_GPL(register_vmap_purge_notifier);
+
+int unregister_vmap_purge_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&vmap_notify_list, nb);
+}
+EXPORT_SYMBOL_GPL(unregister_vmap_purge_notifier);
+
 static void __free_vmap_area(struct vmap_area *va)
 {
 	BUG_ON(RB_EMPTY_NODE(&va->rb_node));
-- 
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] 22+ messages in thread

* [PATCH v2 3/3] drm/i915/shrinker: Hook up vmap allocation failure notifier
  2016-04-04 13:46 vmap purge notifier for exhaustion of the vmalloc arena Chris Wilson
@ 2016-04-04 13:46   ` Chris Wilson
  2016-04-04 13:46   ` Chris Wilson
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2016-04-04 13:46 UTC (permalink / raw)
  To: intel-gfx
  Cc: Chris Wilson, Andrew Morton, David Rientjes, Roman Pen,
	Mel Gorman, linux-mm, linux-kernel, Joonas Lahtinen,
	Tvrtko Ursulin, Mika Kahola

If the core runs out of vmap address space, it will call a notifier in
case any driver can reap some of its vmaps. As i915.ko is possibily
holding onto vmap address space that could be recovered, hook into the
notifier chain and try and reap objects holding onto vmaps.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Roman Pen <r.peniaev@gmail.com>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kahola <mika.kahola@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h          |  1 +
 drivers/gpu/drm/i915/i915_gem_shrinker.c | 39 ++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index dd187727c813..6443745d4182 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1257,6 +1257,7 @@ struct i915_gem_mm {
 	struct i915_hw_ppgtt *aliasing_ppgtt;
 
 	struct notifier_block oom_notifier;
+	struct notifier_block vmap_notifier;
 	struct shrinker shrinker;
 	bool shrinker_no_lock_stealing;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index e391ee3ec486..be7501afb59e 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -28,6 +28,7 @@
 #include <linux/swap.h>
 #include <linux/pci.h>
 #include <linux/dma-buf.h>
+#include <linux/vmalloc.h>
 #include <drm/drmP.h>
 #include <drm/i915_drm.h>
 
@@ -356,6 +357,40 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
 	return NOTIFY_DONE;
 }
 
+static int
+i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr)
+{
+	struct drm_i915_private *dev_priv =
+		container_of(nb, struct drm_i915_private, mm.vmap_notifier);
+	struct drm_device *dev = dev_priv->dev;
+	unsigned long timeout = msecs_to_jiffies(5000) + 1;
+	unsigned long freed_pages;
+	bool was_interruptible;
+	bool unlock;
+
+	while (!i915_gem_shrinker_lock(dev, &unlock) && --timeout) {
+		schedule_timeout_killable(1);
+		if (fatal_signal_pending(current))
+			return NOTIFY_DONE;
+	}
+	if (timeout == 0) {
+		pr_err("Unable to purge GPU vmaps due to lock contention.\n");
+		return NOTIFY_DONE;
+	}
+
+	was_interruptible = dev_priv->mm.interruptible;
+	dev_priv->mm.interruptible = false;
+
+	freed_pages = i915_gem_shrink_all(dev_priv);
+
+	dev_priv->mm.interruptible = was_interruptible;
+	if (unlock)
+		mutex_unlock(&dev->struct_mutex);
+
+	*(unsigned long *)ptr += freed_pages;
+	return NOTIFY_DONE;
+}
+
 /**
  * i915_gem_shrinker_init - Initialize i915 shrinker
  * @dev_priv: i915 device
@@ -371,6 +406,9 @@ void i915_gem_shrinker_init(struct drm_i915_private *dev_priv)
 
 	dev_priv->mm.oom_notifier.notifier_call = i915_gem_shrinker_oom;
 	WARN_ON(register_oom_notifier(&dev_priv->mm.oom_notifier));
+
+	dev_priv->mm.vmap_notifier.notifier_call = i915_gem_shrinker_vmap;
+	WARN_ON(register_vmap_purge_notifier(&dev_priv->mm.vmap_notifier));
 }
 
 /**
@@ -381,6 +419,7 @@ void i915_gem_shrinker_init(struct drm_i915_private *dev_priv)
  */
 void i915_gem_shrinker_cleanup(struct drm_i915_private *dev_priv)
 {
+	WARN_ON(unregister_vmap_purge_notifier(&dev_priv->mm.vmap_notifier));
 	WARN_ON(unregister_oom_notifier(&dev_priv->mm.oom_notifier));
 	unregister_shrinker(&dev_priv->mm.shrinker);
 }
-- 
2.8.0.rc3

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

* [PATCH v2 3/3] drm/i915/shrinker: Hook up vmap allocation failure notifier
@ 2016-04-04 13:46   ` Chris Wilson
  0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2016-04-04 13:46 UTC (permalink / raw)
  To: intel-gfx
  Cc: Chris Wilson, Andrew Morton, David Rientjes, Roman Pen,
	Mel Gorman, linux-mm, linux-kernel, Joonas Lahtinen,
	Tvrtko Ursulin, Mika Kahola

If the core runs out of vmap address space, it will call a notifier in
case any driver can reap some of its vmaps. As i915.ko is possibily
holding onto vmap address space that could be recovered, hook into the
notifier chain and try and reap objects holding onto vmaps.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: David Rientjes <rientjes@google.com>
Cc: Roman Pen <r.peniaev@gmail.com>
Cc: Mel Gorman <mgorman@techsingularity.net>
Cc: linux-mm@kvack.org
Cc: linux-kernel@vger.kernel.org
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kahola <mika.kahola@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h          |  1 +
 drivers/gpu/drm/i915/i915_gem_shrinker.c | 39 ++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index dd187727c813..6443745d4182 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1257,6 +1257,7 @@ struct i915_gem_mm {
 	struct i915_hw_ppgtt *aliasing_ppgtt;
 
 	struct notifier_block oom_notifier;
+	struct notifier_block vmap_notifier;
 	struct shrinker shrinker;
 	bool shrinker_no_lock_stealing;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index e391ee3ec486..be7501afb59e 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -28,6 +28,7 @@
 #include <linux/swap.h>
 #include <linux/pci.h>
 #include <linux/dma-buf.h>
+#include <linux/vmalloc.h>
 #include <drm/drmP.h>
 #include <drm/i915_drm.h>
 
@@ -356,6 +357,40 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
 	return NOTIFY_DONE;
 }
 
+static int
+i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr)
+{
+	struct drm_i915_private *dev_priv =
+		container_of(nb, struct drm_i915_private, mm.vmap_notifier);
+	struct drm_device *dev = dev_priv->dev;
+	unsigned long timeout = msecs_to_jiffies(5000) + 1;
+	unsigned long freed_pages;
+	bool was_interruptible;
+	bool unlock;
+
+	while (!i915_gem_shrinker_lock(dev, &unlock) && --timeout) {
+		schedule_timeout_killable(1);
+		if (fatal_signal_pending(current))
+			return NOTIFY_DONE;
+	}
+	if (timeout == 0) {
+		pr_err("Unable to purge GPU vmaps due to lock contention.\n");
+		return NOTIFY_DONE;
+	}
+
+	was_interruptible = dev_priv->mm.interruptible;
+	dev_priv->mm.interruptible = false;
+
+	freed_pages = i915_gem_shrink_all(dev_priv);
+
+	dev_priv->mm.interruptible = was_interruptible;
+	if (unlock)
+		mutex_unlock(&dev->struct_mutex);
+
+	*(unsigned long *)ptr += freed_pages;
+	return NOTIFY_DONE;
+}
+
 /**
  * i915_gem_shrinker_init - Initialize i915 shrinker
  * @dev_priv: i915 device
@@ -371,6 +406,9 @@ void i915_gem_shrinker_init(struct drm_i915_private *dev_priv)
 
 	dev_priv->mm.oom_notifier.notifier_call = i915_gem_shrinker_oom;
 	WARN_ON(register_oom_notifier(&dev_priv->mm.oom_notifier));
+
+	dev_priv->mm.vmap_notifier.notifier_call = i915_gem_shrinker_vmap;
+	WARN_ON(register_vmap_purge_notifier(&dev_priv->mm.vmap_notifier));
 }
 
 /**
@@ -381,6 +419,7 @@ void i915_gem_shrinker_init(struct drm_i915_private *dev_priv)
  */
 void i915_gem_shrinker_cleanup(struct drm_i915_private *dev_priv)
 {
+	WARN_ON(unregister_vmap_purge_notifier(&dev_priv->mm.vmap_notifier));
 	WARN_ON(unregister_oom_notifier(&dev_priv->mm.oom_notifier));
 	unregister_shrinker(&dev_priv->mm.shrinker);
 }
-- 
2.8.0.rc3

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* ✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915/shrinker: Account for unshrinkable unbound pages
  2016-04-04 13:46 vmap purge notifier for exhaustion of the vmalloc arena Chris Wilson
                   ` (2 preceding siblings ...)
  2016-04-04 13:46   ` Chris Wilson
@ 2016-04-04 15:26 ` Patchwork
  2016-04-05 10:08 ` ✗ Fi.CI.BAT: failure for series starting with [v2,1/3] drm/i915/shrinker: Account for unshrinkable unbound pages (rev2) Patchwork
  4 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2016-04-04 15:26 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/3] drm/i915/shrinker: Account for unshrinkable unbound pages
URL   : https://patchwork.freedesktop.org/series/5276/
State : success

== Summary ==

Series 5276v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/5276/revisions/1/mbox/

Test kms_force_connector_basic:
        Subgroup prune-stale-modes:
                skip       -> PASS       (ilk-hp8440p)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                incomplete -> PASS       (hsw-gt2)

bdw-ultra        total:196  pass:175  dwarn:0   dfail:0   fail:0   skip:21 
bsw-nuc-2        total:196  pass:159  dwarn:0   dfail:0   fail:0   skip:37 
byt-nuc          total:196  pass:161  dwarn:0   dfail:0   fail:0   skip:35 
hsw-brixbox      total:196  pass:174  dwarn:0   dfail:0   fail:0   skip:22 
hsw-gt2          total:196  pass:178  dwarn:0   dfail:0   fail:0   skip:18 
ilk-hp8440p      total:196  pass:132  dwarn:0   dfail:0   fail:0   skip:64 
ivb-t430s        total:196  pass:171  dwarn:0   dfail:0   fail:0   skip:25 
skl-i7k-2        total:196  pass:173  dwarn:0   dfail:0   fail:0   skip:23 
skl-nuci5        total:196  pass:185  dwarn:0   dfail:0   fail:0   skip:11 
snb-dellxps      total:196  pass:162  dwarn:0   dfail:0   fail:0   skip:34 
snb-x220t        total:196  pass:162  dwarn:0   dfail:0   fail:1   skip:33 

Results at /archive/results/CI_IGT_test/Patchwork_1791/

aedfaaef290af9c8df7d9f4adf22cbe21704d091 drm-intel-nightly: 2016y-04m-04d-13h-09m-54s UTC integration manifest
59131f4a7f435c15b4797e95193837956c3e2783 drm/i915/shrinker: Hook up vmap allocation failure notifier
f5f0a32a712bb00a9f81d1dda84a327fc92b8ee2 mm/vmap: Add a notifier for when we run out of vmap address space
28d83aa606ac3fcd8d36da20939c964b185dd78e drm/i915/shrinker: Account for unshrinkable unbound pages

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

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

* Re: [PATCH v2 1/3] drm/i915/shrinker: Account for unshrinkable unbound pages
  2016-04-04 13:46 ` [PATCH v2 1/3] drm/i915/shrinker: Account for unshrinkable unbound pages Chris Wilson
@ 2016-04-05  7:57   ` Joonas Lahtinen
  0 siblings, 0 replies; 22+ messages in thread
From: Joonas Lahtinen @ 2016-04-05  7:57 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: Akash Goel

On ma, 2016-04-04 at 14:46 +0100, Chris Wilson wrote:
> Since we only attempt to purge an object if can_release_pages() report
> true, we should also only add it to the count of potential recoverable
> pages when can_release_pages() is true.
> 
> 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>

> Cc: Tvrtko Ursulin <tvrtko.ursulin@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 d3c473ffb90a..e391ee3ec486 100644
> --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> @@ -246,7 +246,7 @@ i915_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc)
>  
>  	count = 0;
>  	list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_list)
> -		if (obj->pages_pin_count == 0)
> +		if (can_release_pages(obj))
>  			count += obj->base.size >> PAGE_SHIFT;
>  
>  	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
-- 
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] 22+ messages in thread

* Re: [PATCH v2 2/3] mm/vmap: Add a notifier for when we run out of vmap address space
  2016-04-04 13:46   ` Chris Wilson
  (?)
@ 2016-04-05  8:01     ` Joonas Lahtinen
  -1 siblings, 0 replies; 22+ messages in thread
From: Joonas Lahtinen @ 2016-04-05  8:01 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx
  Cc: Andrew Morton, David Rientjes, Roman Peniaev, Mel Gorman,
	linux-mm, linux-kernel, Tvrtko Ursulin

On ma, 2016-04-04 at 14:46 +0100, Chris Wilson wrote:
> vmaps are temporary kernel mappings that may be of long duration.
> Reusing a vmap on an object is preferrable for a driver as the cost of
> setting up the vmap can otherwise dominate the operation on the object.
> However, the vmap address space is rather limited on 32bit systems and
> so we add a notification for vmap pressure in order for the driver to
> release any cached vmappings.
> 
> The interface is styled after the oom-notifier where the callees are
> passed a pointer to an unsigned long counter for them to indicate if they
> have freed any space.
> 
> v2: Guard the blocking notifier call with gfpflags_allow_blocking()
> v3: Correct typo in forward declaration and move to head of file
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Rientjes <rientjes@google.com>
> Cc: Roman Peniaev <r.peniaev@gmail.com>
> Cc: Mel Gorman <mgorman@techsingularity.net>
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Acked-by: Andrew Morton <akpm@linux-foundation.org> # for inclusion via DRM
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

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

> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  include/linux/vmalloc.h |  4 ++++
>  mm/vmalloc.c            | 27 +++++++++++++++++++++++++++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> index d1f1d338af20..8b51df3ab334 100644
> --- a/include/linux/vmalloc.h
> +++ b/include/linux/vmalloc.h
> @@ -8,6 +8,7 @@
>  #include 
>  
>  struct vm_area_struct;		/* vma defining user mapping in mm_types.h */
> +struct notifier_block;		/* in notifier.h */
>  
>  /* bits in flags of vmalloc's vm_struct below */
>  #define VM_IOREMAP		0x00000001	/* ioremap() and friends */
> @@ -187,4 +188,7 @@ pcpu_free_vm_areas(struct vm_struct **vms, int nr_vms)
>  #define VMALLOC_TOTAL 0UL
>  #endif
>  
> +int register_vmap_purge_notifier(struct notifier_block *nb);
> +int unregister_vmap_purge_notifier(struct notifier_block *nb);
> +
>  #endif /* _LINUX_VMALLOC_H */
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index ae7d20b447ff..293889d7f482 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -21,6 +21,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -344,6 +345,8 @@ static void __insert_vmap_area(struct vmap_area *va)
>  
>  static void purge_vmap_area_lazy(void);
>  
> +static BLOCKING_NOTIFIER_HEAD(vmap_notify_list);
> +
>  /*
>   * Allocate a region of KVA of the specified size and alignment, within the
>   * vstart and vend.
> @@ -363,6 +366,8 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
>  	BUG_ON(offset_in_page(size));
>  	BUG_ON(!is_power_of_2(align));
>  
> +	might_sleep_if(gfpflags_allow_blocking(gfp_mask));
> +
>  	va = kmalloc_node(sizeof(struct vmap_area),
>  			gfp_mask & GFP_RECLAIM_MASK, node);
>  	if (unlikely(!va))
> @@ -468,6 +473,16 @@ overflow:
>  		purged = 1;
>  		goto retry;
>  	}
> +
> +	if (gfpflags_allow_blocking(gfp_mask)) {
> +		unsigned long freed = 0;
> +		blocking_notifier_call_chain(&vmap_notify_list, 0, &freed);
> +		if (freed > 0) {
> +			purged = 0;
> +			goto retry;
> +		}
> +	}
> +
>  	if (printk_ratelimit())
>  		pr_warn("vmap allocation for size %lu failed: use vmalloc= to increase size\n",
>  			size);
> @@ -475,6 +490,18 @@ overflow:
>  	return ERR_PTR(-EBUSY);
>  }
>  
> +int register_vmap_purge_notifier(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_register(&vmap_notify_list, nb);
> +}
> +EXPORT_SYMBOL_GPL(register_vmap_purge_notifier);
> +
> +int unregister_vmap_purge_notifier(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_unregister(&vmap_notify_list, nb);
> +}
> +EXPORT_SYMBOL_GPL(unregister_vmap_purge_notifier);
> +
>  static void __free_vmap_area(struct vmap_area *va)
>  {
>  	BUG_ON(RB_EMPTY_NODE(&va->rb_node));
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation

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

* Re: [PATCH v2 2/3] mm/vmap: Add a notifier for when we run out of vmap address space
@ 2016-04-05  8:01     ` Joonas Lahtinen
  0 siblings, 0 replies; 22+ messages in thread
From: Joonas Lahtinen @ 2016-04-05  8:01 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx
  Cc: Andrew Morton, David Rientjes, Roman Peniaev, Mel Gorman,
	linux-mm, linux-kernel, Tvrtko Ursulin

On ma, 2016-04-04 at 14:46 +0100, Chris Wilson wrote:
> vmaps are temporary kernel mappings that may be of long duration.
> Reusing a vmap on an object is preferrable for a driver as the cost of
> setting up the vmap can otherwise dominate the operation on the object.
> However, the vmap address space is rather limited on 32bit systems and
> so we add a notification for vmap pressure in order for the driver to
> release any cached vmappings.
> 
> The interface is styled after the oom-notifier where the callees are
> passed a pointer to an unsigned long counter for them to indicate if they
> have freed any space.
> 
> v2: Guard the blocking notifier call with gfpflags_allow_blocking()
> v3: Correct typo in forward declaration and move to head of file
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Rientjes <rientjes@google.com>
> Cc: Roman Peniaev <r.peniaev@gmail.com>
> Cc: Mel Gorman <mgorman@techsingularity.net>
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Acked-by: Andrew Morton <akpm@linux-foundation.org> # for inclusion via DRM
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

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

> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
> A include/linux/vmalloc.h |A A 4 ++++
> A mm/vmalloc.cA A A A A A A A A A A A | 27 +++++++++++++++++++++++++++
> A 2 files changed, 31 insertions(+)
> 
> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> index d1f1d338af20..8b51df3ab334 100644
> --- a/include/linux/vmalloc.h
> +++ b/include/linux/vmalloc.h
> @@ -8,6 +8,7 @@
> A #include 
> A 
> A struct vm_area_struct;		/* vma defining user mapping in mm_types.h */
> +struct notifier_block;		/* in notifier.h */
> A 
> A /* bits in flags of vmalloc's vm_struct below */
> A #define VM_IOREMAP		0x00000001	/* ioremap() and friends */
> @@ -187,4 +188,7 @@ pcpu_free_vm_areas(struct vm_struct **vms, int nr_vms)
> A #define VMALLOC_TOTAL 0UL
> A #endif
> A 
> +int register_vmap_purge_notifier(struct notifier_block *nb);
> +int unregister_vmap_purge_notifier(struct notifier_block *nb);
> +
> A #endif /* _LINUX_VMALLOC_H */
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index ae7d20b447ff..293889d7f482 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -21,6 +21,7 @@
> A #include 
> A #include 
> A #include 
> +#include 
> A #include 
> A #include 
> A #include 
> @@ -344,6 +345,8 @@ static void __insert_vmap_area(struct vmap_area *va)
> A 
> A static void purge_vmap_area_lazy(void);
> A 
> +static BLOCKING_NOTIFIER_HEAD(vmap_notify_list);
> +
> A /*
> A  * Allocate a region of KVA of the specified size and alignment, within the
> A  * vstart and vend.
> @@ -363,6 +366,8 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
> A 	BUG_ON(offset_in_page(size));
> A 	BUG_ON(!is_power_of_2(align));
> A 
> +	might_sleep_if(gfpflags_allow_blocking(gfp_mask));
> +
> A 	va = kmalloc_node(sizeof(struct vmap_area),
> A 			gfp_mask & GFP_RECLAIM_MASK, node);
> A 	if (unlikely(!va))
> @@ -468,6 +473,16 @@ overflow:
> A 		purged = 1;
> A 		goto retry;
> A 	}
> +
> +	if (gfpflags_allow_blocking(gfp_mask)) {
> +		unsigned long freed = 0;
> +		blocking_notifier_call_chain(&vmap_notify_list, 0, &freed);
> +		if (freed > 0) {
> +			purged = 0;
> +			goto retry;
> +		}
> +	}
> +
> A 	if (printk_ratelimit())
> A 		pr_warn("vmap allocation for size %lu failed: use vmalloc= to increase size\n",
> A 			size);
> @@ -475,6 +490,18 @@ overflow:
> A 	return ERR_PTR(-EBUSY);
> A }
> A 
> +int register_vmap_purge_notifier(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_register(&vmap_notify_list, nb);
> +}
> +EXPORT_SYMBOL_GPL(register_vmap_purge_notifier);
> +
> +int unregister_vmap_purge_notifier(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_unregister(&vmap_notify_list, nb);
> +}
> +EXPORT_SYMBOL_GPL(unregister_vmap_purge_notifier);
> +
> A static void __free_vmap_area(struct vmap_area *va)
> A {
> A 	BUG_ON(RB_EMPTY_NODE(&va->rb_node));
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [PATCH v2 2/3] mm/vmap: Add a notifier for when we run out of vmap address space
@ 2016-04-05  8:01     ` Joonas Lahtinen
  0 siblings, 0 replies; 22+ messages in thread
From: Joonas Lahtinen @ 2016-04-05  8:01 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx
  Cc: linux-kernel, Roman Peniaev, linux-mm, David Rientjes,
	Andrew Morton, Mel Gorman

On ma, 2016-04-04 at 14:46 +0100, Chris Wilson wrote:
> vmaps are temporary kernel mappings that may be of long duration.
> Reusing a vmap on an object is preferrable for a driver as the cost of
> setting up the vmap can otherwise dominate the operation on the object.
> However, the vmap address space is rather limited on 32bit systems and
> so we add a notification for vmap pressure in order for the driver to
> release any cached vmappings.
> 
> The interface is styled after the oom-notifier where the callees are
> passed a pointer to an unsigned long counter for them to indicate if they
> have freed any space.
> 
> v2: Guard the blocking notifier call with gfpflags_allow_blocking()
> v3: Correct typo in forward declaration and move to head of file
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Rientjes <rientjes@google.com>
> Cc: Roman Peniaev <r.peniaev@gmail.com>
> Cc: Mel Gorman <mgorman@techsingularity.net>
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Acked-by: Andrew Morton <akpm@linux-foundation.org> # for inclusion via DRM
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

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

> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  include/linux/vmalloc.h |  4 ++++
>  mm/vmalloc.c            | 27 +++++++++++++++++++++++++++
>  2 files changed, 31 insertions(+)
> 
> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> index d1f1d338af20..8b51df3ab334 100644
> --- a/include/linux/vmalloc.h
> +++ b/include/linux/vmalloc.h
> @@ -8,6 +8,7 @@
>  #include 
>  
>  struct vm_area_struct;		/* vma defining user mapping in mm_types.h */
> +struct notifier_block;		/* in notifier.h */
>  
>  /* bits in flags of vmalloc's vm_struct below */
>  #define VM_IOREMAP		0x00000001	/* ioremap() and friends */
> @@ -187,4 +188,7 @@ pcpu_free_vm_areas(struct vm_struct **vms, int nr_vms)
>  #define VMALLOC_TOTAL 0UL
>  #endif
>  
> +int register_vmap_purge_notifier(struct notifier_block *nb);
> +int unregister_vmap_purge_notifier(struct notifier_block *nb);
> +
>  #endif /* _LINUX_VMALLOC_H */
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index ae7d20b447ff..293889d7f482 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -21,6 +21,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  #include 
> @@ -344,6 +345,8 @@ static void __insert_vmap_area(struct vmap_area *va)
>  
>  static void purge_vmap_area_lazy(void);
>  
> +static BLOCKING_NOTIFIER_HEAD(vmap_notify_list);
> +
>  /*
>   * Allocate a region of KVA of the specified size and alignment, within the
>   * vstart and vend.
> @@ -363,6 +366,8 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
>  	BUG_ON(offset_in_page(size));
>  	BUG_ON(!is_power_of_2(align));
>  
> +	might_sleep_if(gfpflags_allow_blocking(gfp_mask));
> +
>  	va = kmalloc_node(sizeof(struct vmap_area),
>  			gfp_mask & GFP_RECLAIM_MASK, node);
>  	if (unlikely(!va))
> @@ -468,6 +473,16 @@ overflow:
>  		purged = 1;
>  		goto retry;
>  	}
> +
> +	if (gfpflags_allow_blocking(gfp_mask)) {
> +		unsigned long freed = 0;
> +		blocking_notifier_call_chain(&vmap_notify_list, 0, &freed);
> +		if (freed > 0) {
> +			purged = 0;
> +			goto retry;
> +		}
> +	}
> +
>  	if (printk_ratelimit())
>  		pr_warn("vmap allocation for size %lu failed: use vmalloc= to increase size\n",
>  			size);
> @@ -475,6 +490,18 @@ overflow:
>  	return ERR_PTR(-EBUSY);
>  }
>  
> +int register_vmap_purge_notifier(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_register(&vmap_notify_list, nb);
> +}
> +EXPORT_SYMBOL_GPL(register_vmap_purge_notifier);
> +
> +int unregister_vmap_purge_notifier(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_unregister(&vmap_notify_list, nb);
> +}
> +EXPORT_SYMBOL_GPL(unregister_vmap_purge_notifier);
> +
>  static void __free_vmap_area(struct vmap_area *va)
>  {
>  	BUG_ON(RB_EMPTY_NODE(&va->rb_node));
-- 
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] 22+ messages in thread

* Re: [PATCH v2 3/3] drm/i915/shrinker: Hook up vmap allocation failure notifier
  2016-04-04 13:46   ` Chris Wilson
  (?)
@ 2016-04-05  8:19     ` Joonas Lahtinen
  -1 siblings, 0 replies; 22+ messages in thread
From: Joonas Lahtinen @ 2016-04-05  8:19 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx
  Cc: Andrew Morton, David Rientjes, Roman Pen, Mel Gorman, linux-mm,
	linux-kernel, Tvrtko Ursulin, Mika Kahola

On ma, 2016-04-04 at 14:46 +0100, Chris Wilson wrote:
> If the core runs out of vmap address space, it will call a notifier in
> case any driver can reap some of its vmaps. As i915.ko is possibily
> holding onto vmap address space that could be recovered, hook into the
> notifier chain and try and reap objects holding onto vmaps.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Rientjes <rientjes@google.com>
> Cc: Roman Pen <r.peniaev@gmail.com>
> Cc: Mel Gorman <mgorman@techsingularity.net>
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

A comment below. But regardless;

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

> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Mika Kahola <mika.kahola@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h          |  1 +
>  drivers/gpu/drm/i915/i915_gem_shrinker.c | 39 ++++++++++++++++++++++++++++++++
>  2 files changed, 40 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index dd187727c813..6443745d4182 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1257,6 +1257,7 @@ struct i915_gem_mm {
>  	struct i915_hw_ppgtt *aliasing_ppgtt;
>  
>  	struct notifier_block oom_notifier;
> +	struct notifier_block vmap_notifier;
>  	struct shrinker shrinker;
>  	bool shrinker_no_lock_stealing;
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> index e391ee3ec486..be7501afb59e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> @@ -28,6 +28,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -356,6 +357,40 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
>  	return NOTIFY_DONE;
>  }
>  
> +static int
> +i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr)
> +{
> +	struct drm_i915_private *dev_priv =
> +		container_of(nb, struct drm_i915_private, mm.vmap_notifier);
> +	struct drm_device *dev = dev_priv->dev;
> +	unsigned long timeout = msecs_to_jiffies(5000) + 1;
> +	unsigned long freed_pages;
> +	bool was_interruptible;
> +	bool unlock;
> +
> +	while (!i915_gem_shrinker_lock(dev, &unlock) && --timeout) {
> +		schedule_timeout_killable(1);
> +		if (fatal_signal_pending(current))
> +			return NOTIFY_DONE;
> +	}
> +	if (timeout == 0) {
> +		pr_err("Unable to purge GPU vmaps due to lock contention.\n");
> +		return NOTIFY_DONE;
> +	}
> +
> +	was_interruptible = dev_priv->mm.interruptible;
> +	dev_priv->mm.interruptible = false;
> +
> +	freed_pages = i915_gem_shrink_all(dev_priv);
> +
> +	dev_priv->mm.interruptible = was_interruptible;

Up until here this is same function as the oom shrinker, so I would
combine these two and here do "if (vmap) goto out;" sort of thing.

Would just need a way to distinct between two calling sites. I did not
come up with a quick solution as both are passing 0 as event.

Regards, Joonas

> +	if (unlock)
> +		mutex_unlock(&dev->struct_mutex);
> +
> +	*(unsigned long *)ptr += freed_pages;
> +	return NOTIFY_DONE;
> +}
> +
>  /**
>   * i915_gem_shrinker_init - Initialize i915 shrinker
>   * @dev_priv: i915 device
> @@ -371,6 +406,9 @@ void i915_gem_shrinker_init(struct drm_i915_private *dev_priv)
>  
>  	dev_priv->mm.oom_notifier.notifier_call = i915_gem_shrinker_oom;
>  	WARN_ON(register_oom_notifier(&dev_priv->mm.oom_notifier));
> +
> +	dev_priv->mm.vmap_notifier.notifier_call = i915_gem_shrinker_vmap;
> +	WARN_ON(register_vmap_purge_notifier(&dev_priv->mm.vmap_notifier));
>  }
>  
>  /**
> @@ -381,6 +419,7 @@ void i915_gem_shrinker_init(struct drm_i915_private *dev_priv)
>   */
>  void i915_gem_shrinker_cleanup(struct drm_i915_private *dev_priv)
>  {
> +	WARN_ON(unregister_vmap_purge_notifier(&dev_priv->mm.vmap_notifier));
>  	WARN_ON(unregister_oom_notifier(&dev_priv->mm.oom_notifier));
>  	unregister_shrinker(&dev_priv->mm.shrinker);
>  }
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation

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

* Re: [PATCH v2 3/3] drm/i915/shrinker: Hook up vmap allocation failure notifier
@ 2016-04-05  8:19     ` Joonas Lahtinen
  0 siblings, 0 replies; 22+ messages in thread
From: Joonas Lahtinen @ 2016-04-05  8:19 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx
  Cc: Andrew Morton, David Rientjes, Roman Pen, Mel Gorman, linux-mm,
	linux-kernel, Tvrtko Ursulin, Mika Kahola

On ma, 2016-04-04 at 14:46 +0100, Chris Wilson wrote:
> If the core runs out of vmap address space, it will call a notifier in
> case any driver can reap some of its vmaps. As i915.ko is possibily
> holding onto vmap address space that could be recovered, hook into the
> notifier chain and try and reap objects holding onto vmaps.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Rientjes <rientjes@google.com>
> Cc: Roman Pen <r.peniaev@gmail.com>
> Cc: Mel Gorman <mgorman@techsingularity.net>
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

A comment below. But regardless;

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

> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Mika Kahola <mika.kahola@intel.com>
> ---
> A drivers/gpu/drm/i915/i915_drv.hA A A A A A A A A A |A A 1 +
> A drivers/gpu/drm/i915/i915_gem_shrinker.c | 39 ++++++++++++++++++++++++++++++++
> A 2 files changed, 40 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index dd187727c813..6443745d4182 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1257,6 +1257,7 @@ struct i915_gem_mm {
> A 	struct i915_hw_ppgtt *aliasing_ppgtt;
> A 
> A 	struct notifier_block oom_notifier;
> +	struct notifier_block vmap_notifier;
> A 	struct shrinker shrinker;
> A 	bool shrinker_no_lock_stealing;
> A 
> diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> index e391ee3ec486..be7501afb59e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> @@ -28,6 +28,7 @@
> A #include 
> A #include 
> A #include 
> +#include 
> A #include 
> A #include 
> A 
> @@ -356,6 +357,40 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
> A 	return NOTIFY_DONE;
> A }
> A 
> +static int
> +i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr)
> +{
> +	struct drm_i915_private *dev_priv =
> +		container_of(nb, struct drm_i915_private, mm.vmap_notifier);
> +	struct drm_device *dev = dev_priv->dev;
> +	unsigned long timeout = msecs_to_jiffies(5000) + 1;
> +	unsigned long freed_pages;
> +	bool was_interruptible;
> +	bool unlock;
> +
> +	while (!i915_gem_shrinker_lock(dev, &unlock) && --timeout) {
> +		schedule_timeout_killable(1);
> +		if (fatal_signal_pending(current))
> +			return NOTIFY_DONE;
> +	}
> +	if (timeout == 0) {
> +		pr_err("Unable to purge GPU vmaps due to lock contention.\n");
> +		return NOTIFY_DONE;
> +	}
> +
> +	was_interruptible = dev_priv->mm.interruptible;
> +	dev_priv->mm.interruptible = false;
> +
> +	freed_pages = i915_gem_shrink_all(dev_priv);
> +
> +	dev_priv->mm.interruptible = was_interruptible;

Up until here this is same function as the oom shrinker, so I would
combine these two and here do "if (vmap) goto out;" sort of thing.

Would just need a way to distinct between two calling sites. I did not
come up with a quick solution as both are passing 0 as event.

Regards, Joonas

> +	if (unlock)
> +		mutex_unlock(&dev->struct_mutex);
> +
> +	*(unsigned long *)ptr += freed_pages;
> +	return NOTIFY_DONE;
> +}
> +
> A /**
> A  * i915_gem_shrinker_init - Initialize i915 shrinker
> A  * @dev_priv: i915 device
> @@ -371,6 +406,9 @@ void i915_gem_shrinker_init(struct drm_i915_private *dev_priv)
> A 
> A 	dev_priv->mm.oom_notifier.notifier_call = i915_gem_shrinker_oom;
> A 	WARN_ON(register_oom_notifier(&dev_priv->mm.oom_notifier));
> +
> +	dev_priv->mm.vmap_notifier.notifier_call = i915_gem_shrinker_vmap;
> +	WARN_ON(register_vmap_purge_notifier(&dev_priv->mm.vmap_notifier));
> A }
> A 
> A /**
> @@ -381,6 +419,7 @@ void i915_gem_shrinker_init(struct drm_i915_private *dev_priv)
> A  */
> A void i915_gem_shrinker_cleanup(struct drm_i915_private *dev_priv)
> A {
> +	WARN_ON(unregister_vmap_purge_notifier(&dev_priv->mm.vmap_notifier));
> A 	WARN_ON(unregister_oom_notifier(&dev_priv->mm.oom_notifier));
> A 	unregister_shrinker(&dev_priv->mm.shrinker);
> A }
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [PATCH v2 3/3] drm/i915/shrinker: Hook up vmap allocation failure notifier
@ 2016-04-05  8:19     ` Joonas Lahtinen
  0 siblings, 0 replies; 22+ messages in thread
From: Joonas Lahtinen @ 2016-04-05  8:19 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx
  Cc: linux-kernel, Roman Pen, linux-mm, David Rientjes, Andrew Morton,
	Mel Gorman

On ma, 2016-04-04 at 14:46 +0100, Chris Wilson wrote:
> If the core runs out of vmap address space, it will call a notifier in
> case any driver can reap some of its vmaps. As i915.ko is possibily
> holding onto vmap address space that could be recovered, hook into the
> notifier chain and try and reap objects holding onto vmaps.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: David Rientjes <rientjes@google.com>
> Cc: Roman Pen <r.peniaev@gmail.com>
> Cc: Mel Gorman <mgorman@techsingularity.net>
> Cc: linux-mm@kvack.org
> Cc: linux-kernel@vger.kernel.org
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

A comment below. But regardless;

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

> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Mika Kahola <mika.kahola@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h          |  1 +
>  drivers/gpu/drm/i915/i915_gem_shrinker.c | 39 ++++++++++++++++++++++++++++++++
>  2 files changed, 40 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index dd187727c813..6443745d4182 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1257,6 +1257,7 @@ struct i915_gem_mm {
>  	struct i915_hw_ppgtt *aliasing_ppgtt;
>  
>  	struct notifier_block oom_notifier;
> +	struct notifier_block vmap_notifier;
>  	struct shrinker shrinker;
>  	bool shrinker_no_lock_stealing;
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> index e391ee3ec486..be7501afb59e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> @@ -28,6 +28,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>  
> @@ -356,6 +357,40 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
>  	return NOTIFY_DONE;
>  }
>  
> +static int
> +i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr)
> +{
> +	struct drm_i915_private *dev_priv =
> +		container_of(nb, struct drm_i915_private, mm.vmap_notifier);
> +	struct drm_device *dev = dev_priv->dev;
> +	unsigned long timeout = msecs_to_jiffies(5000) + 1;
> +	unsigned long freed_pages;
> +	bool was_interruptible;
> +	bool unlock;
> +
> +	while (!i915_gem_shrinker_lock(dev, &unlock) && --timeout) {
> +		schedule_timeout_killable(1);
> +		if (fatal_signal_pending(current))
> +			return NOTIFY_DONE;
> +	}
> +	if (timeout == 0) {
> +		pr_err("Unable to purge GPU vmaps due to lock contention.\n");
> +		return NOTIFY_DONE;
> +	}
> +
> +	was_interruptible = dev_priv->mm.interruptible;
> +	dev_priv->mm.interruptible = false;
> +
> +	freed_pages = i915_gem_shrink_all(dev_priv);
> +
> +	dev_priv->mm.interruptible = was_interruptible;

Up until here this is same function as the oom shrinker, so I would
combine these two and here do "if (vmap) goto out;" sort of thing.

Would just need a way to distinct between two calling sites. I did not
come up with a quick solution as both are passing 0 as event.

Regards, Joonas

> +	if (unlock)
> +		mutex_unlock(&dev->struct_mutex);
> +
> +	*(unsigned long *)ptr += freed_pages;
> +	return NOTIFY_DONE;
> +}
> +
>  /**
>   * i915_gem_shrinker_init - Initialize i915 shrinker
>   * @dev_priv: i915 device
> @@ -371,6 +406,9 @@ void i915_gem_shrinker_init(struct drm_i915_private *dev_priv)
>  
>  	dev_priv->mm.oom_notifier.notifier_call = i915_gem_shrinker_oom;
>  	WARN_ON(register_oom_notifier(&dev_priv->mm.oom_notifier));
> +
> +	dev_priv->mm.vmap_notifier.notifier_call = i915_gem_shrinker_vmap;
> +	WARN_ON(register_vmap_purge_notifier(&dev_priv->mm.vmap_notifier));
>  }
>  
>  /**
> @@ -381,6 +419,7 @@ void i915_gem_shrinker_init(struct drm_i915_private *dev_priv)
>   */
>  void i915_gem_shrinker_cleanup(struct drm_i915_private *dev_priv)
>  {
> +	WARN_ON(unregister_vmap_purge_notifier(&dev_priv->mm.vmap_notifier));
>  	WARN_ON(unregister_oom_notifier(&dev_priv->mm.oom_notifier));
>  	unregister_shrinker(&dev_priv->mm.shrinker);
>  }
-- 
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] 22+ messages in thread

* Re: [PATCH v2 3/3] drm/i915/shrinker: Hook up vmap allocation failure notifier
  2016-04-05  8:19     ` Joonas Lahtinen
  (?)
@ 2016-04-05  9:03       ` Chris Wilson
  -1 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2016-04-05  9:03 UTC (permalink / raw)
  To: Joonas Lahtinen
  Cc: intel-gfx, Andrew Morton, David Rientjes, Roman Pen, Mel Gorman,
	linux-mm, linux-kernel, Tvrtko Ursulin, Mika Kahola

On Tue, Apr 05, 2016 at 11:19:38AM +0300, Joonas Lahtinen wrote:
> On ma, 2016-04-04 at 14:46 +0100, Chris Wilson wrote:
> > If the core runs out of vmap address space, it will call a notifier in
> > case any driver can reap some of its vmaps. As i915.ko is possibily
> > holding onto vmap address space that could be recovered, hook into the
> > notifier chain and try and reap objects holding onto vmaps.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: David Rientjes <rientjes@google.com>
> > Cc: Roman Pen <r.peniaev@gmail.com>
> > Cc: Mel Gorman <mgorman@techsingularity.net>
> > Cc: linux-mm@kvack.org
> > Cc: linux-kernel@vger.kernel.org
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> A comment below. But regardless;
> 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Mika Kahola <mika.kahola@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h          |  1 +
> >  drivers/gpu/drm/i915/i915_gem_shrinker.c | 39 ++++++++++++++++++++++++++++++++
> >  2 files changed, 40 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index dd187727c813..6443745d4182 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1257,6 +1257,7 @@ struct i915_gem_mm {
> >  	struct i915_hw_ppgtt *aliasing_ppgtt;
> >  
> >  	struct notifier_block oom_notifier;
> > +	struct notifier_block vmap_notifier;
> >  	struct shrinker shrinker;
> >  	bool shrinker_no_lock_stealing;
> >  
> > diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> > index e391ee3ec486..be7501afb59e 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> > @@ -28,6 +28,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  
> > @@ -356,6 +357,40 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
> >  	return NOTIFY_DONE;
> >  }
> >  
> > +static int
> > +i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr)
> > +{
> > +	struct drm_i915_private *dev_priv =
> > +		container_of(nb, struct drm_i915_private, mm.vmap_notifier);
> > +	struct drm_device *dev = dev_priv->dev;
> > +	unsigned long timeout = msecs_to_jiffies(5000) + 1;
> > +	unsigned long freed_pages;
> > +	bool was_interruptible;
> > +	bool unlock;
> > +
> > +	while (!i915_gem_shrinker_lock(dev, &unlock) && --timeout) {
> > +		schedule_timeout_killable(1);
> > +		if (fatal_signal_pending(current))
> > +			return NOTIFY_DONE;
> > +	}
> > +	if (timeout == 0) {
> > +		pr_err("Unable to purge GPU vmaps due to lock contention.\n");
> > +		return NOTIFY_DONE;
> > +	}
> > +
> > +	was_interruptible = dev_priv->mm.interruptible;
> > +	dev_priv->mm.interruptible = false;
> > +
> > +	freed_pages = i915_gem_shrink_all(dev_priv);
> > +
> > +	dev_priv->mm.interruptible = was_interruptible;
> 
> Up until here this is same function as the oom shrinker, so I would
> combine these two and here do "if (vmap) goto out;" sort of thing.
> 
> Would just need a way to distinct between two calling sites. I did not
> come up with a quick solution as both are passing 0 as event.

Less thrilled about merging the two notifier callbacks, but we could
wrap i915_gem_shrinker_lock_killable().
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

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

* Re: [PATCH v2 3/3] drm/i915/shrinker: Hook up vmap allocation failure notifier
@ 2016-04-05  9:03       ` Chris Wilson
  0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2016-04-05  9:03 UTC (permalink / raw)
  To: Joonas Lahtinen
  Cc: intel-gfx, Andrew Morton, David Rientjes, Roman Pen, Mel Gorman,
	linux-mm, linux-kernel, Tvrtko Ursulin, Mika Kahola

On Tue, Apr 05, 2016 at 11:19:38AM +0300, Joonas Lahtinen wrote:
> On ma, 2016-04-04 at 14:46 +0100, Chris Wilson wrote:
> > If the core runs out of vmap address space, it will call a notifier in
> > case any driver can reap some of its vmaps. As i915.ko is possibily
> > holding onto vmap address space that could be recovered, hook into the
> > notifier chain and try and reap objects holding onto vmaps.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: David Rientjes <rientjes@google.com>
> > Cc: Roman Pen <r.peniaev@gmail.com>
> > Cc: Mel Gorman <mgorman@techsingularity.net>
> > Cc: linux-mm@kvack.org
> > Cc: linux-kernel@vger.kernel.org
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> A comment below. But regardless;
> 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Mika Kahola <mika.kahola@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h          |  1 +
> >  drivers/gpu/drm/i915/i915_gem_shrinker.c | 39 ++++++++++++++++++++++++++++++++
> >  2 files changed, 40 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index dd187727c813..6443745d4182 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1257,6 +1257,7 @@ struct i915_gem_mm {
> >  	struct i915_hw_ppgtt *aliasing_ppgtt;
> >  
> >  	struct notifier_block oom_notifier;
> > +	struct notifier_block vmap_notifier;
> >  	struct shrinker shrinker;
> >  	bool shrinker_no_lock_stealing;
> >  
> > diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> > index e391ee3ec486..be7501afb59e 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> > @@ -28,6 +28,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  
> > @@ -356,6 +357,40 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
> >  	return NOTIFY_DONE;
> >  }
> >  
> > +static int
> > +i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr)
> > +{
> > +	struct drm_i915_private *dev_priv =
> > +		container_of(nb, struct drm_i915_private, mm.vmap_notifier);
> > +	struct drm_device *dev = dev_priv->dev;
> > +	unsigned long timeout = msecs_to_jiffies(5000) + 1;
> > +	unsigned long freed_pages;
> > +	bool was_interruptible;
> > +	bool unlock;
> > +
> > +	while (!i915_gem_shrinker_lock(dev, &unlock) && --timeout) {
> > +		schedule_timeout_killable(1);
> > +		if (fatal_signal_pending(current))
> > +			return NOTIFY_DONE;
> > +	}
> > +	if (timeout == 0) {
> > +		pr_err("Unable to purge GPU vmaps due to lock contention.\n");
> > +		return NOTIFY_DONE;
> > +	}
> > +
> > +	was_interruptible = dev_priv->mm.interruptible;
> > +	dev_priv->mm.interruptible = false;
> > +
> > +	freed_pages = i915_gem_shrink_all(dev_priv);
> > +
> > +	dev_priv->mm.interruptible = was_interruptible;
> 
> Up until here this is same function as the oom shrinker, so I would
> combine these two and here do "if (vmap) goto out;" sort of thing.
> 
> Would just need a way to distinct between two calling sites. I did not
> come up with a quick solution as both are passing 0 as event.

Less thrilled about merging the two notifier callbacks, but we could
wrap i915_gem_shrinker_lock_killable().
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* Re: [PATCH v2 3/3] drm/i915/shrinker: Hook up vmap allocation failure notifier
@ 2016-04-05  9:03       ` Chris Wilson
  0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2016-04-05  9:03 UTC (permalink / raw)
  To: Joonas Lahtinen
  Cc: intel-gfx, Andrew Morton, David Rientjes, Roman Pen, Mel Gorman,
	linux-mm, linux-kernel, Tvrtko Ursulin, Mika Kahola

On Tue, Apr 05, 2016 at 11:19:38AM +0300, Joonas Lahtinen wrote:
> On ma, 2016-04-04 at 14:46 +0100, Chris Wilson wrote:
> > If the core runs out of vmap address space, it will call a notifier in
> > case any driver can reap some of its vmaps. As i915.ko is possibily
> > holding onto vmap address space that could be recovered, hook into the
> > notifier chain and try and reap objects holding onto vmaps.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: David Rientjes <rientjes@google.com>
> > Cc: Roman Pen <r.peniaev@gmail.com>
> > Cc: Mel Gorman <mgorman@techsingularity.net>
> > Cc: linux-mm@kvack.org
> > Cc: linux-kernel@vger.kernel.org
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> A comment below. But regardless;
> 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Mika Kahola <mika.kahola@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h          |  1 +
> >  drivers/gpu/drm/i915/i915_gem_shrinker.c | 39 ++++++++++++++++++++++++++++++++
> >  2 files changed, 40 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index dd187727c813..6443745d4182 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1257,6 +1257,7 @@ struct i915_gem_mm {
> >  	struct i915_hw_ppgtt *aliasing_ppgtt;
> >  
> >  	struct notifier_block oom_notifier;
> > +	struct notifier_block vmap_notifier;
> >  	struct shrinker shrinker;
> >  	bool shrinker_no_lock_stealing;
> >  
> > diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> > index e391ee3ec486..be7501afb59e 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> > @@ -28,6 +28,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  #include 
> >  #include 
> >  
> > @@ -356,6 +357,40 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
> >  	return NOTIFY_DONE;
> >  }
> >  
> > +static int
> > +i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr)
> > +{
> > +	struct drm_i915_private *dev_priv =
> > +		container_of(nb, struct drm_i915_private, mm.vmap_notifier);
> > +	struct drm_device *dev = dev_priv->dev;
> > +	unsigned long timeout = msecs_to_jiffies(5000) + 1;
> > +	unsigned long freed_pages;
> > +	bool was_interruptible;
> > +	bool unlock;
> > +
> > +	while (!i915_gem_shrinker_lock(dev, &unlock) && --timeout) {
> > +		schedule_timeout_killable(1);
> > +		if (fatal_signal_pending(current))
> > +			return NOTIFY_DONE;
> > +	}
> > +	if (timeout == 0) {
> > +		pr_err("Unable to purge GPU vmaps due to lock contention.\n");
> > +		return NOTIFY_DONE;
> > +	}
> > +
> > +	was_interruptible = dev_priv->mm.interruptible;
> > +	dev_priv->mm.interruptible = false;
> > +
> > +	freed_pages = i915_gem_shrink_all(dev_priv);
> > +
> > +	dev_priv->mm.interruptible = was_interruptible;
> 
> Up until here this is same function as the oom shrinker, so I would
> combine these two and here do "if (vmap) goto out;" sort of thing.
> 
> Would just need a way to distinct between two calling sites. I did not
> come up with a quick solution as both are passing 0 as event.

Less thrilled about merging the two notifier callbacks, but we could
wrap i915_gem_shrinker_lock_killable().
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

* [PATCH] drm/i915/shrinker: Refactor common uninterruptible locking
  2016-04-05  8:19     ` Joonas Lahtinen
                       ` (2 preceding siblings ...)
  (?)
@ 2016-04-05  9:22     ` Chris Wilson
  2016-04-05 10:02       ` Joonas Lahtinen
  -1 siblings, 1 reply; 22+ messages in thread
From: Chris Wilson @ 2016-04-05  9:22 UTC (permalink / raw)
  To: intel-gfx

Both the oom and vmap notifier callbacks have a loop to acquire the
struct_mutex and set the device as uninterruptible, within a certain
time. Refactor the common code into a pair of functions.

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

diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index be7501afb59e..39943793edcc 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -289,35 +289,56 @@ i915_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc)
 	return freed;
 }
 
+struct shrinker_lock_uninterruptible {
+	bool was_interruptible;
+	bool unlock;
+};
+
+static bool
+i915_gem_shrinker_lock_uninterruptible(struct drm_i915_private *dev_priv,
+				       struct shrinker_lock_uninterruptible *slu,
+				       int timeout_ms)
+{
+	unsigned long timeout = msecs_to_jiffies(timeout_ms) + 1;
+
+	while (!i915_gem_shrinker_lock(dev_priv->dev, &slu->unlock)) {
+		schedule_timeout_killable(1);
+		if (fatal_signal_pending(current))
+			return false;
+		if (--timeout == 0) {
+			pr_err("Unable to lock GPU to purge memory.\n");
+			return false;
+		}
+	}
+
+	slu->was_interruptible = dev_priv->mm.interruptible;
+	dev_priv->mm.interruptible = false;
+	return true;
+}
+
+static void
+i915_gem_shrinker_unlock_uninterruptible(struct drm_i915_private *dev_priv,
+					 struct shrinker_lock_uninterruptible *slu)
+{
+	dev_priv->mm.interruptible = slu->was_interruptible;
+	if (slu->unlock)
+		mutex_unlock(&dev_priv->dev->struct_mutex);
+}
+
 static int
 i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
 {
 	struct drm_i915_private *dev_priv =
 		container_of(nb, struct drm_i915_private, mm.oom_notifier);
-	struct drm_device *dev = dev_priv->dev;
+	struct shrinker_lock_uninterruptible slu;
 	struct drm_i915_gem_object *obj;
-	unsigned long timeout = msecs_to_jiffies(5000) + 1;
 	unsigned long pinned, bound, unbound, freed_pages;
-	bool was_interruptible;
-	bool unlock;
 
-	while (!i915_gem_shrinker_lock(dev, &unlock) && --timeout) {
-		schedule_timeout_killable(1);
-		if (fatal_signal_pending(current))
-			return NOTIFY_DONE;
-	}
-	if (timeout == 0) {
-		pr_err("Unable to purge GPU memory due lock contention.\n");
+	if (!i915_gem_shrinker_lock_uninterruptible(dev_priv, &slu, 5000))
 		return NOTIFY_DONE;
-	}
-
-	was_interruptible = dev_priv->mm.interruptible;
-	dev_priv->mm.interruptible = false;
 
 	freed_pages = i915_gem_shrink_all(dev_priv);
 
-	dev_priv->mm.interruptible = was_interruptible;
-
 	/* Because we may be allocating inside our own driver, we cannot
 	 * assert that there are no objects with pinned pages that are not
 	 * being pointed to by hardware.
@@ -342,8 +363,7 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
 			bound += obj->base.size;
 	}
 
-	if (unlock)
-		mutex_unlock(&dev->struct_mutex);
+	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",
@@ -362,30 +382,15 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr
 {
 	struct drm_i915_private *dev_priv =
 		container_of(nb, struct drm_i915_private, mm.vmap_notifier);
-	struct drm_device *dev = dev_priv->dev;
-	unsigned long timeout = msecs_to_jiffies(5000) + 1;
+	struct shrinker_lock_uninterruptible slu;
 	unsigned long freed_pages;
-	bool was_interruptible;
-	bool unlock;
 
-	while (!i915_gem_shrinker_lock(dev, &unlock) && --timeout) {
-		schedule_timeout_killable(1);
-		if (fatal_signal_pending(current))
-			return NOTIFY_DONE;
-	}
-	if (timeout == 0) {
-		pr_err("Unable to purge GPU vmaps due to lock contention.\n");
+	if (!i915_gem_shrinker_lock_uninterruptible(dev_priv, &slu, 5000))
 		return NOTIFY_DONE;
-	}
-
-	was_interruptible = dev_priv->mm.interruptible;
-	dev_priv->mm.interruptible = false;
 
 	freed_pages = i915_gem_shrink_all(dev_priv);
 
-	dev_priv->mm.interruptible = was_interruptible;
-	if (unlock)
-		mutex_unlock(&dev->struct_mutex);
+	i915_gem_shrinker_unlock_uninterruptible(dev_priv, &slu);
 
 	*(unsigned long *)ptr += freed_pages;
 	return NOTIFY_DONE;
-- 
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] 22+ messages in thread

* Re: [PATCH] drm/i915/shrinker: Refactor common uninterruptible locking
  2016-04-05  9:22     ` [PATCH] drm/i915/shrinker: Refactor common uninterruptible locking Chris Wilson
@ 2016-04-05 10:02       ` Joonas Lahtinen
  2016-04-05 10:59         ` Chris Wilson
  0 siblings, 1 reply; 22+ messages in thread
From: Joonas Lahtinen @ 2016-04-05 10:02 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On ti, 2016-04-05 at 10:22 +0100, Chris Wilson wrote:
> Both the oom and vmap notifier callbacks have a loop to acquire the
> struct_mutex and set the device as uninterruptible, within a certain
> time. Refactor the common code into a pair of functions.
> 
> Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Looks good.

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

> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_shrinker.c | 79 +++++++++++++++++---------------
>  1 file changed, 42 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> index be7501afb59e..39943793edcc 100644
> --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> @@ -289,35 +289,56 @@ i915_gem_shrinker_scan(struct shrinker *shrinker, struct shrink_control *sc)
>  	return freed;
>  }
>  
> +struct shrinker_lock_uninterruptible {
> +	bool was_interruptible;
> +	bool unlock;
> +};
> +
> +static bool
> +i915_gem_shrinker_lock_uninterruptible(struct drm_i915_private *dev_priv,
> +				       struct shrinker_lock_uninterruptible *slu,
> +				       int timeout_ms)
> +{
> +	unsigned long timeout = msecs_to_jiffies(timeout_ms) + 1;
> +
> +	while (!i915_gem_shrinker_lock(dev_priv->dev, &slu->unlock)) {
> +		schedule_timeout_killable(1);
> +		if (fatal_signal_pending(current))
> +			return false;
> +		if (--timeout == 0) {
> +			pr_err("Unable to lock GPU to purge memory.\n");
> +			return false;
> +		}
> +	}
> +
> +	slu->was_interruptible = dev_priv->mm.interruptible;
> +	dev_priv->mm.interruptible = false;
> +	return true;
> +}
> +
> +static void
> +i915_gem_shrinker_unlock_uninterruptible(struct drm_i915_private *dev_priv,
> +					 struct shrinker_lock_uninterruptible *slu)
> +{
> +	dev_priv->mm.interruptible = slu->was_interruptible;
> +	if (slu->unlock)
> +		mutex_unlock(&dev_priv->dev->struct_mutex);
> +}
> +
>  static int
>  i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
>  {
>  	struct drm_i915_private *dev_priv =
>  		container_of(nb, struct drm_i915_private, mm.oom_notifier);
> -	struct drm_device *dev = dev_priv->dev;
> +	struct shrinker_lock_uninterruptible slu;
>  	struct drm_i915_gem_object *obj;
> -	unsigned long timeout = msecs_to_jiffies(5000) + 1;
>  	unsigned long pinned, bound, unbound, freed_pages;
> -	bool was_interruptible;
> -	bool unlock;
>  
> -	while (!i915_gem_shrinker_lock(dev, &unlock) && --timeout) {
> -		schedule_timeout_killable(1);
> -		if (fatal_signal_pending(current))
> -			return NOTIFY_DONE;
> -	}
> -	if (timeout == 0) {
> -		pr_err("Unable to purge GPU memory due lock contention.\n");
> +	if (!i915_gem_shrinker_lock_uninterruptible(dev_priv, &slu, 5000))
>  		return NOTIFY_DONE;
> -	}
> -
> -	was_interruptible = dev_priv->mm.interruptible;
> -	dev_priv->mm.interruptible = false;
>  
>  	freed_pages = i915_gem_shrink_all(dev_priv);
>  
> -	dev_priv->mm.interruptible = was_interruptible;
> -
>  	/* Because we may be allocating inside our own driver, we cannot
>  	 * assert that there are no objects with pinned pages that are not
>  	 * being pointed to by hardware.
> @@ -342,8 +363,7 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
>  			bound += obj->base.size;
>  	}
>  
> -	if (unlock)
> -		mutex_unlock(&dev->struct_mutex);
> +	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",
> @@ -362,30 +382,15 @@ i915_gem_shrinker_vmap(struct notifier_block *nb, unsigned long event, void *ptr
>  {
>  	struct drm_i915_private *dev_priv =
>  		container_of(nb, struct drm_i915_private, mm.vmap_notifier);
> -	struct drm_device *dev = dev_priv->dev;
> -	unsigned long timeout = msecs_to_jiffies(5000) + 1;
> +	struct shrinker_lock_uninterruptible slu;
>  	unsigned long freed_pages;
> -	bool was_interruptible;
> -	bool unlock;
>  
> -	while (!i915_gem_shrinker_lock(dev, &unlock) && --timeout) {
> -		schedule_timeout_killable(1);
> -		if (fatal_signal_pending(current))
> -			return NOTIFY_DONE;
> -	}
> -	if (timeout == 0) {
> -		pr_err("Unable to purge GPU vmaps due to lock contention.\n");
> +	if (!i915_gem_shrinker_lock_uninterruptible(dev_priv, &slu, 5000))
>  		return NOTIFY_DONE;
> -	}
> -
> -	was_interruptible = dev_priv->mm.interruptible;
> -	dev_priv->mm.interruptible = false;
>  
>  	freed_pages = i915_gem_shrink_all(dev_priv);
>  
> -	dev_priv->mm.interruptible = was_interruptible;
> -	if (unlock)
> -		mutex_unlock(&dev->struct_mutex);
> +	i915_gem_shrinker_unlock_uninterruptible(dev_priv, &slu);
>  
>  	*(unsigned long *)ptr += freed_pages;
>  	return NOTIFY_DONE;
-- 
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] 22+ messages in thread

* ✗ Fi.CI.BAT: failure for series starting with [v2,1/3] drm/i915/shrinker: Account for unshrinkable unbound pages (rev2)
  2016-04-04 13:46 vmap purge notifier for exhaustion of the vmalloc arena Chris Wilson
                   ` (3 preceding siblings ...)
  2016-04-04 15:26 ` ✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915/shrinker: Account for unshrinkable unbound pages Patchwork
@ 2016-04-05 10:08 ` Patchwork
  4 siblings, 0 replies; 22+ messages in thread
From: Patchwork @ 2016-04-05 10:08 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [v2,1/3] drm/i915/shrinker: Account for unshrinkable unbound pages (rev2)
URL   : https://patchwork.freedesktop.org/series/5276/
State : failure

== Summary ==

  CC [M]  drivers/net/ethernet/intel/igbvf/mbx.o
  LD      drivers/net/ethernet/synopsys/built-in.o
  CC [M]  drivers/net/ethernet/intel/igbvf/ethtool.o
  CC [M]  drivers/net/ethernet/intel/igbvf/netdev.o
  CC      drivers/video/fbdev/vesafb.o
  LD [M]  drivers/net/ethernet/intel/e1000/e1000.o
  CC [M]  drivers/net/ethernet/intel/igb/e1000_82575.o
  CC [M]  drivers/net/ethernet/intel/igb/e1000_mac.o
  CC      drivers/video/fbdev/efifb.o
  CC [M]  drivers/net/ethernet/intel/igb/e1000_nvm.o
  CC [M]  drivers/net/ethernet/intel/igb/e1000_mbx.o
  CC [M]  drivers/net/ethernet/intel/igb/e1000_phy.o
  LD      drivers/video/fbdev/core/built-in.o
  CC [M]  drivers/net/ethernet/intel/igb/e1000_i210.o
  LD      drivers/usb/host/xhci-hcd.o
  LD      drivers/usb/host/built-in.o
  CC [M]  drivers/net/ethernet/intel/igb/igb_ptp.o
  CC [M]  drivers/net/ethernet/intel/igb/igb_hwmon.o
  LD      drivers/usb/storage/usb-storage.o
  LD      drivers/usb/storage/built-in.o
  LD      drivers/usb/built-in.o
  LD      drivers/video/fbdev/built-in.o
  LD      drivers/video/built-in.o
  LD [M]  drivers/net/ethernet/intel/igb/igb.o
  LD [M]  drivers/net/ethernet/intel/igbvf/igbvf.o
  LD [M]  drivers/net/ethernet/intel/e1000e/e1000e.o
  LD      drivers/net/ethernet/built-in.o
  LD      drivers/net/built-in.o
Makefile:962: recipe for target 'drivers' failed
make: *** [drivers] Error 2

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

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

* Re: [PATCH] drm/i915/shrinker: Refactor common uninterruptible locking
  2016-04-05 10:02       ` Joonas Lahtinen
@ 2016-04-05 10:59         ` Chris Wilson
  0 siblings, 0 replies; 22+ messages in thread
From: Chris Wilson @ 2016-04-05 10:59 UTC (permalink / raw)
  To: Joonas Lahtinen; +Cc: intel-gfx

On Tue, Apr 05, 2016 at 01:02:14PM +0300, Joonas Lahtinen wrote:
> On ti, 2016-04-05 at 10:22 +0100, Chris Wilson wrote:
> > Both the oom and vmap notifier callbacks have a loop to acquire the
> > struct_mutex and set the device as uninterruptible, within a certain
> > time. Refactor the common code into a pair of functions.
> > 
> > Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> Looks good.
> 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Thanks for all the review, pushed. Now on to stage 2.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2016-04-05 10:59 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-04 13:46 vmap purge notifier for exhaustion of the vmalloc arena Chris Wilson
2016-04-04 13:46 ` [PATCH v2 1/3] drm/i915/shrinker: Account for unshrinkable unbound pages Chris Wilson
2016-04-05  7:57   ` Joonas Lahtinen
2016-04-04 13:46 ` [PATCH v2 2/3] mm/vmap: Add a notifier for when we run out of vmap address space Chris Wilson
2016-04-04 13:46   ` Chris Wilson
2016-04-04 13:46   ` Chris Wilson
2016-04-05  8:01   ` Joonas Lahtinen
2016-04-05  8:01     ` Joonas Lahtinen
2016-04-05  8:01     ` Joonas Lahtinen
2016-04-04 13:46 ` [PATCH v2 3/3] drm/i915/shrinker: Hook up vmap allocation failure notifier Chris Wilson
2016-04-04 13:46   ` Chris Wilson
2016-04-05  8:19   ` Joonas Lahtinen
2016-04-05  8:19     ` Joonas Lahtinen
2016-04-05  8:19     ` Joonas Lahtinen
2016-04-05  9:03     ` Chris Wilson
2016-04-05  9:03       ` Chris Wilson
2016-04-05  9:03       ` Chris Wilson
2016-04-05  9:22     ` [PATCH] drm/i915/shrinker: Refactor common uninterruptible locking Chris Wilson
2016-04-05 10:02       ` Joonas Lahtinen
2016-04-05 10:59         ` Chris Wilson
2016-04-04 15:26 ` ✓ Fi.CI.BAT: success for series starting with [v2,1/3] drm/i915/shrinker: Account for unshrinkable unbound pages Patchwork
2016-04-05 10:08 ` ✗ Fi.CI.BAT: failure for series starting with [v2,1/3] drm/i915/shrinker: Account for unshrinkable unbound pages (rev2) 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.