* [PATCH 1/9] drm/i915: Refactor testing obj->mm.pages
@ 2017-10-13 20:26 Chris Wilson
2017-10-13 20:26 ` [PATCH 2/9] drm/i915: Rename obj->pin_display to obj->pin_global Chris Wilson
` (12 more replies)
0 siblings, 13 replies; 24+ messages in thread
From: Chris Wilson @ 2017-10-13 20:26 UTC (permalink / raw)
To: intel-gfx
Since we occasionally stuff an error pointer into obj->mm.pages for a
semi-permanent or even permanent failure, we have to be more careful and
not just test against NULL when deciding if the object has a complete
set of its concurrent pages.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 10 ++++++++--
drivers/gpu/drm/i915/i915_gem.c | 19 ++++++++++---------
drivers/gpu/drm/i915/i915_gem_clflush.c | 1 +
drivers/gpu/drm/i915/i915_gem_render_state.c | 2 +-
drivers/gpu/drm/i915/i915_gem_shrinker.c | 10 +++++-----
drivers/gpu/drm/i915/i915_gem_tiling.c | 2 +-
drivers/gpu/drm/i915/i915_gem_userptr.c | 2 +-
7 files changed, 27 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c7b2ca6aff05..48be1d3c1d33 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3565,10 +3565,16 @@ i915_gem_object_pin_pages(struct drm_i915_gem_object *obj)
return __i915_gem_object_get_pages(obj);
}
+static inline bool
+i915_gem_object_has_pages(struct drm_i915_gem_object *obj)
+{
+ return !IS_ERR_OR_NULL(READ_ONCE(obj->mm.pages));
+}
+
static inline void
__i915_gem_object_pin_pages(struct drm_i915_gem_object *obj)
{
- GEM_BUG_ON(!obj->mm.pages);
+ GEM_BUG_ON(!i915_gem_object_has_pages(obj));
atomic_inc(&obj->mm.pages_pin_count);
}
@@ -3582,8 +3588,8 @@ i915_gem_object_has_pinned_pages(struct drm_i915_gem_object *obj)
static inline void
__i915_gem_object_unpin_pages(struct drm_i915_gem_object *obj)
{
+ GEM_BUG_ON(!i915_gem_object_has_pages(obj));
GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
- GEM_BUG_ON(!obj->mm.pages);
atomic_dec(&obj->mm.pages_pin_count);
}
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 20fcac37c85a..0c42ec876de0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2196,7 +2196,7 @@ void __i915_gem_object_invalidate(struct drm_i915_gem_object *obj)
struct address_space *mapping;
lockdep_assert_held(&obj->mm.lock);
- GEM_BUG_ON(obj->mm.pages);
+ GEM_BUG_ON(i915_gem_object_has_pages(obj));
switch (obj->mm.madv) {
case I915_MADV_DONTNEED:
@@ -2259,7 +2259,7 @@ void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj,
return;
GEM_BUG_ON(obj->bind_count);
- if (!READ_ONCE(obj->mm.pages))
+ if (!i915_gem_object_has_pages(obj))
return;
/* May be called by shrinker from within get_pages() (on another bo) */
@@ -2563,7 +2563,7 @@ int __i915_gem_object_get_pages(struct drm_i915_gem_object *obj)
if (err)
return err;
- if (unlikely(IS_ERR_OR_NULL(obj->mm.pages))) {
+ if (unlikely(!i915_gem_object_has_pages(obj))) {
GEM_BUG_ON(i915_gem_object_has_pinned_pages(obj));
err = ____i915_gem_object_get_pages(obj);
@@ -2648,7 +2648,7 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj,
type &= ~I915_MAP_OVERRIDE;
if (!atomic_inc_not_zero(&obj->mm.pages_pin_count)) {
- if (unlikely(IS_ERR_OR_NULL(obj->mm.pages))) {
+ if (unlikely(!i915_gem_object_has_pages(obj))) {
GEM_BUG_ON(i915_gem_object_has_pinned_pages(obj));
ret = ____i915_gem_object_get_pages(obj);
@@ -2660,7 +2660,7 @@ void *i915_gem_object_pin_map(struct drm_i915_gem_object *obj,
atomic_inc(&obj->mm.pages_pin_count);
pinned = false;
}
- GEM_BUG_ON(!obj->mm.pages);
+ GEM_BUG_ON(!i915_gem_object_has_pages(obj));
ptr = page_unpack_bits(obj->mm.mapping, &has_type);
if (ptr && has_type != type) {
@@ -2715,7 +2715,7 @@ i915_gem_object_pwrite_gtt(struct drm_i915_gem_object *obj,
* allows it to avoid the cost of retrieving a page (either swapin
* or clearing-before-use) before it is overwritten.
*/
- if (READ_ONCE(obj->mm.pages))
+ if (i915_gem_object_has_pages(obj))
return -ENODEV;
/* Before the pages are instantiated the object is treated as being
@@ -4280,7 +4280,7 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
if (err)
goto out;
- if (obj->mm.pages &&
+ if (i915_gem_object_has_pages(obj) &&
i915_gem_object_is_tiled(obj) &&
dev_priv->quirks & QUIRK_PIN_SWIZZLED_PAGES) {
if (obj->mm.madv == I915_MADV_WILLNEED) {
@@ -4299,7 +4299,8 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
obj->mm.madv = args->madv;
/* if the object is no longer attached, discard its backing storage */
- if (obj->mm.madv == I915_MADV_DONTNEED && !obj->mm.pages)
+ if (obj->mm.madv == I915_MADV_DONTNEED &&
+ !i915_gem_object_has_pages(obj))
i915_gem_object_truncate(obj);
args->retained = obj->mm.madv != __I915_MADV_PURGED;
@@ -4516,7 +4517,7 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
if (WARN_ON(i915_gem_object_has_pinned_pages(obj)))
atomic_set(&obj->mm.pages_pin_count, 0);
__i915_gem_object_put_pages(obj, I915_MM_NORMAL);
- GEM_BUG_ON(obj->mm.pages);
+ GEM_BUG_ON(i915_gem_object_has_pages(obj));
if (obj->base.import_attach)
drm_prime_gem_destroy(&obj->base, NULL);
diff --git a/drivers/gpu/drm/i915/i915_gem_clflush.c b/drivers/gpu/drm/i915/i915_gem_clflush.c
index 8a04d33055be..f663cd919795 100644
--- a/drivers/gpu/drm/i915/i915_gem_clflush.c
+++ b/drivers/gpu/drm/i915/i915_gem_clflush.c
@@ -70,6 +70,7 @@ static const struct dma_fence_ops i915_clflush_ops = {
static void __i915_do_clflush(struct drm_i915_gem_object *obj)
{
+ GEM_BUG_ON(!i915_gem_object_has_pages(obj));
drm_clflush_sg(obj->mm.pages);
intel_fb_obj_flush(obj, ORIGIN_CPU);
}
diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c
index 4dd4c2159a92..3703dc91eeda 100644
--- a/drivers/gpu/drm/i915/i915_gem_render_state.c
+++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
@@ -229,7 +229,7 @@ int i915_gem_render_state_emit(struct drm_i915_gem_request *req)
return 0;
/* Recreate the page after shrinking */
- if (!so->vma->obj->mm.pages)
+ if (!i915_gem_object_has_pages(so->vma->obj))
so->batch_offset = -1;
ret = i915_vma_pin(so->vma, 0, 0, PIN_GLOBAL | PIN_HIGH);
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index 74002b2d1b6f..b5c87d89777b 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -97,7 +97,7 @@ static bool swap_available(void)
static bool can_release_pages(struct drm_i915_gem_object *obj)
{
- if (!obj->mm.pages)
+ if (!i915_gem_object_has_pages(obj))
return false;
/* Consider only shrinkable ojects. */
@@ -129,7 +129,7 @@ static bool unsafe_drop_pages(struct drm_i915_gem_object *obj)
{
if (i915_gem_object_unbind(obj) == 0)
__i915_gem_object_put_pages(obj, I915_MM_SHRINKER);
- return !READ_ONCE(obj->mm.pages);
+ return !i915_gem_object_has_pages(obj);
}
/**
@@ -247,7 +247,7 @@ i915_gem_shrink(struct drm_i915_private *dev_priv,
/* May arrive from get_pages on another bo */
mutex_lock_nested(&obj->mm.lock,
I915_MM_SHRINKER);
- if (!obj->mm.pages) {
+ if (!i915_gem_object_has_pages(obj)) {
__i915_gem_object_invalidate(obj);
list_del_init(&obj->global_link);
count += obj->base.size >> PAGE_SHIFT;
@@ -413,7 +413,7 @@ 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_link) {
- if (!obj->mm.pages)
+ if (!i915_gem_object_has_pages(obj))
continue;
if (!can_release_pages(obj))
@@ -422,7 +422,7 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
unbound += obj->base.size >> PAGE_SHIFT;
}
list_for_each_entry(obj, &dev_priv->mm.bound_list, global_link) {
- if (!obj->mm.pages)
+ if (!i915_gem_object_has_pages(obj))
continue;
if (!can_release_pages(obj))
diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c
index fb5231f98c0d..1294cf695df0 100644
--- a/drivers/gpu/drm/i915/i915_gem_tiling.c
+++ b/drivers/gpu/drm/i915/i915_gem_tiling.c
@@ -269,7 +269,7 @@ i915_gem_object_set_tiling(struct drm_i915_gem_object *obj,
* due to the change in swizzling.
*/
mutex_lock(&obj->mm.lock);
- if (obj->mm.pages &&
+ if (i915_gem_object_has_pages(obj) &&
obj->mm.madv == I915_MADV_WILLNEED &&
i915->quirks & QUIRK_PIN_SWIZZLED_PAGES) {
if (tiling == I915_TILING_NONE) {
diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
index 4d712a4db63b..9afeb68e2d44 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -82,7 +82,7 @@ static void cancel_userptr(struct work_struct *work)
/* We are inside a kthread context and can't be interrupted */
if (i915_gem_object_unbind(obj) == 0)
__i915_gem_object_put_pages(obj, I915_MM_NORMAL);
- WARN_ONCE(obj->mm.pages,
+ WARN_ONCE(i915_gem_object_has_pages(obj),
"Failed to release pages: bind_count=%d, pages_pin_count=%d, pin_display=%d\n",
obj->bind_count,
atomic_read(&obj->mm.pages_pin_count),
--
2.15.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 2/9] drm/i915: Rename obj->pin_display to obj->pin_global
2017-10-13 20:26 [PATCH 1/9] drm/i915: Refactor testing obj->mm.pages Chris Wilson
@ 2017-10-13 20:26 ` Chris Wilson
2017-10-16 10:40 ` Joonas Lahtinen
2017-10-13 20:26 ` [PATCH 3/9] drm/i915: Drop debugfs/i915_gem_pin_display Chris Wilson
` (11 subsequent siblings)
12 siblings, 1 reply; 24+ messages in thread
From: Chris Wilson @ 2017-10-13 20:26 UTC (permalink / raw)
To: intel-gfx
In the next patch, we want to extend use of the global pin counter for
semi-permanent pinning of context/ring objects. Given that we plan to
extend the usage to encompass a disparate set of objects, we want a name
that reflects both and should entail less confusion.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_debugfs.c | 12 ++++++------
drivers/gpu/drm/i915/i915_gem.c | 20 ++++++++++----------
drivers/gpu/drm/i915/i915_gem_object.h | 3 ++-
drivers/gpu/drm/i915/i915_gem_userptr.c | 4 ++--
4 files changed, 20 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 0bb6e01121fc..fb9b34302811 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -83,7 +83,7 @@ static char get_active_flag(struct drm_i915_gem_object *obj)
static char get_pin_flag(struct drm_i915_gem_object *obj)
{
- return obj->pin_display ? 'p' : ' ';
+ return obj->pin_global ? 'p' : ' ';
}
static char get_tiling_flag(struct drm_i915_gem_object *obj)
@@ -180,8 +180,8 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
pin_count++;
}
seq_printf(m, " (pinned x %d)", pin_count);
- if (obj->pin_display)
- seq_printf(m, " (display)");
+ if (obj->pin_global)
+ seq_printf(m, " (global)");
list_for_each_entry(vma, &obj->vma_list, obj_link) {
if (!drm_mm_node_allocated(&vma->node))
continue;
@@ -481,7 +481,7 @@ static int i915_gem_object_info(struct seq_file *m, void *data)
size += obj->base.size;
++count;
- if (obj->pin_display) {
+ if (obj->pin_global) {
dpy_size += obj->base.size;
++dpy_count;
}
@@ -512,7 +512,7 @@ static int i915_gem_object_info(struct seq_file *m, void *data)
huge_count,
stringify_page_sizes(page_sizes, buf, sizeof(buf)),
huge_size);
- seq_printf(m, "%u display objects (pinned), %llu bytes\n",
+ seq_printf(m, "%u display objects (globally pinned), %llu bytes\n",
dpy_count, dpy_size);
seq_printf(m, "%llu [%llu] gtt total\n",
@@ -579,7 +579,7 @@ static int i915_gem_gtt_info(struct seq_file *m, void *data)
total_obj_size = total_gtt_size = count = 0;
list_for_each_entry(obj, &dev_priv->mm.bound_list, global_link) {
- if (show_pin_display_only && !obj->pin_display)
+ if (show_pin_display_only && !obj->pin_global)
continue;
seq_puts(m, " ");
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0c42ec876de0..ffb2e5973a08 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -56,7 +56,7 @@ static bool cpu_write_needs_clflush(struct drm_i915_gem_object *obj)
if (!(obj->cache_coherent & I915_BO_CACHE_COHERENT_FOR_WRITE))
return true;
- return obj->pin_display;
+ return obj->pin_global; /* currently in use by HW, keep flushed */
}
static int
@@ -3495,7 +3495,7 @@ static void __i915_gem_object_flush_for_display(struct drm_i915_gem_object *obj)
void i915_gem_object_flush_if_display(struct drm_i915_gem_object *obj)
{
- if (!READ_ONCE(obj->pin_display))
+ if (!READ_ONCE(obj->pin_global))
return;
mutex_lock(&obj->base.dev->struct_mutex);
@@ -3862,10 +3862,10 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
lockdep_assert_held(&obj->base.dev->struct_mutex);
- /* Mark the pin_display early so that we account for the
+ /* Mark the global pin early so that we account for the
* display coherency whilst setting up the cache domains.
*/
- obj->pin_display++;
+ obj->pin_global++;
/* The display engine is not coherent with the LLC cache on gen6. As
* a result, we make sure that the pinning that is about to occur is
@@ -3881,7 +3881,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
I915_CACHE_WT : I915_CACHE_NONE);
if (ret) {
vma = ERR_PTR(ret);
- goto err_unpin_display;
+ goto err_unpin_global;
}
/* As the user may map the buffer once pinned in the display plane
@@ -3912,7 +3912,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
vma = i915_gem_object_ggtt_pin(obj, view, 0, alignment, flags);
}
if (IS_ERR(vma))
- goto err_unpin_display;
+ goto err_unpin_global;
vma->display_alignment = max_t(u64, vma->display_alignment, alignment);
@@ -3927,8 +3927,8 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj,
return vma;
-err_unpin_display:
- obj->pin_display--;
+err_unpin_global:
+ obj->pin_global--;
return vma;
}
@@ -3937,10 +3937,10 @@ i915_gem_object_unpin_from_display_plane(struct i915_vma *vma)
{
lockdep_assert_held(&vma->vm->i915->drm.struct_mutex);
- if (WARN_ON(vma->obj->pin_display == 0))
+ if (WARN_ON(vma->obj->pin_global == 0))
return;
- if (--vma->obj->pin_display == 0)
+ if (--vma->obj->pin_global == 0)
vma->display_alignment = I915_GTT_MIN_ALIGNMENT;
/* Bump the LRU to try and avoid premature eviction whilst flipping */
diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h
index d67f1cbe842d..72aa02f3a0f8 100644
--- a/drivers/gpu/drm/i915/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/i915_gem_object.h
@@ -161,7 +161,8 @@ struct drm_i915_gem_object {
/** Count of VMA actually bound by this object */
unsigned int bind_count;
unsigned int active_count;
- unsigned int pin_display;
+ /** Count of how many global VMA are currently pinned for use by HW */
+ unsigned int pin_global;
struct {
struct mutex lock; /* protects the pages and their use */
diff --git a/drivers/gpu/drm/i915/i915_gem_userptr.c b/drivers/gpu/drm/i915/i915_gem_userptr.c
index 9afeb68e2d44..cdc9be799eee 100644
--- a/drivers/gpu/drm/i915/i915_gem_userptr.c
+++ b/drivers/gpu/drm/i915/i915_gem_userptr.c
@@ -83,10 +83,10 @@ static void cancel_userptr(struct work_struct *work)
if (i915_gem_object_unbind(obj) == 0)
__i915_gem_object_put_pages(obj, I915_MM_NORMAL);
WARN_ONCE(i915_gem_object_has_pages(obj),
- "Failed to release pages: bind_count=%d, pages_pin_count=%d, pin_display=%d\n",
+ "Failed to release pages: bind_count=%d, pages_pin_count=%d, pin_global=%d\n",
obj->bind_count,
atomic_read(&obj->mm.pages_pin_count),
- obj->pin_display);
+ obj->pin_global);
mutex_unlock(&obj->base.dev->struct_mutex);
--
2.15.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 3/9] drm/i915: Drop debugfs/i915_gem_pin_display
2017-10-13 20:26 [PATCH 1/9] drm/i915: Refactor testing obj->mm.pages Chris Wilson
2017-10-13 20:26 ` [PATCH 2/9] drm/i915: Rename obj->pin_display to obj->pin_global Chris Wilson
@ 2017-10-13 20:26 ` Chris Wilson
2017-10-16 10:41 ` Joonas Lahtinen
2017-10-13 20:26 ` [PATCH 4/9] drm/i915: Remove walk over obj->vma_list for the shrinker Chris Wilson
` (10 subsequent siblings)
12 siblings, 1 reply; 24+ messages in thread
From: Chris Wilson @ 2017-10-13 20:26 UTC (permalink / raw)
To: intel-gfx
It has now lost its meaning (it shows more than just pin_display), I do
not believe that we are using in preference to the complete listing from
i915_gem_gtt, or the listing from i915_gem_framebuffer, or the listing
of active display objects in i915_display_info.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_debugfs.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index fb9b34302811..7053d71b1ca9 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -568,7 +568,6 @@ static int i915_gem_gtt_info(struct seq_file *m, void *data)
struct drm_info_node *node = m->private;
struct drm_i915_private *dev_priv = node_to_i915(node);
struct drm_device *dev = &dev_priv->drm;
- bool show_pin_display_only = !!node->info_ent->data;
struct drm_i915_gem_object *obj;
u64 total_obj_size, total_gtt_size;
int count, ret;
@@ -579,9 +578,6 @@ static int i915_gem_gtt_info(struct seq_file *m, void *data)
total_obj_size = total_gtt_size = count = 0;
list_for_each_entry(obj, &dev_priv->mm.bound_list, global_link) {
- if (show_pin_display_only && !obj->pin_global)
- continue;
-
seq_puts(m, " ");
describe_obj(m, obj);
seq_putc(m, '\n');
@@ -4751,7 +4747,6 @@ static const struct drm_info_list i915_debugfs_list[] = {
{"i915_capabilities", i915_capabilities, 0},
{"i915_gem_objects", i915_gem_object_info, 0},
{"i915_gem_gtt", i915_gem_gtt_info, 0},
- {"i915_gem_pin_display", i915_gem_gtt_info, 0, (void *)1},
{"i915_gem_stolen", i915_gem_stolen_list_info },
{"i915_gem_request", i915_gem_request_info, 0},
{"i915_gem_seqno", i915_gem_seqno_info, 0},
--
2.15.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 4/9] drm/i915: Remove walk over obj->vma_list for the shrinker
2017-10-13 20:26 [PATCH 1/9] drm/i915: Refactor testing obj->mm.pages Chris Wilson
2017-10-13 20:26 ` [PATCH 2/9] drm/i915: Rename obj->pin_display to obj->pin_global Chris Wilson
2017-10-13 20:26 ` [PATCH 3/9] drm/i915: Drop debugfs/i915_gem_pin_display Chris Wilson
@ 2017-10-13 20:26 ` Chris Wilson
2017-10-13 20:26 ` [PATCH 5/9] drm/i915: Move dev_priv->mm.[un]bound_list to its own lock Chris Wilson
` (9 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Chris Wilson @ 2017-10-13 20:26 UTC (permalink / raw)
To: intel-gfx
In the next patch, we want to reduce the lock coverage within the
shrinker, and one of the dangerous walks we have is over obj->vma_list.
We are only walking the obj->vma_list in order to check whether it has
been permanently pinned by HW access, typically via use on the scanout.
But we have a couple of other long term pins, the context objects for
which we currently have to check the individual vma pin_count. If we
instead mark these using obj->pin_display, we can forgo the dangerous
and sometimes slow list iteration.
v2: Rearrange code to try and avoid confusion from false associations
due to arrangement of whitespace along with rebasing on obj->pin_global.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/i915_gem_shrinker.c | 27 +++++++--------------------
drivers/gpu/drm/i915/intel_lrc.c | 2 ++
drivers/gpu/drm/i915/intel_ringbuffer.c | 8 +++++++-
3 files changed, 16 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index b5c87d89777b..575a6b735f39 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -71,25 +71,6 @@ static void shrinker_unlock(struct drm_i915_private *dev_priv, bool unlock)
mutex_unlock(&dev_priv->drm.struct_mutex);
}
-static bool any_vma_pinned(struct drm_i915_gem_object *obj)
-{
- struct i915_vma *vma;
-
- list_for_each_entry(vma, &obj->vma_list, obj_link) {
- /* Only GGTT vma may be permanently pinned, and are always
- * at the start of the list. We can stop hunting as soon
- * as we see a ppGTT vma.
- */
- if (!i915_vma_is_ggtt(vma))
- break;
-
- if (i915_vma_is_pinned(vma))
- return true;
- }
-
- return false;
-}
-
static bool swap_available(void)
{
return get_nr_swap_pages() > 0;
@@ -115,7 +96,13 @@ static bool can_release_pages(struct drm_i915_gem_object *obj)
if (atomic_read(&obj->mm.pages_pin_count) > obj->bind_count)
return false;
- if (any_vma_pinned(obj))
+ /* If any vma are "permanently" pinned, it will prevent us from
+ * reclaiming the obj->mm.pages. We only allow scanout objects to claim
+ * a permanent pin, along with a few others like the context objects.
+ * To simplify the scan, and to avoid walking the list of vma under the
+ * object, we just check the count of its permanently pinned.
+ */
+ if (obj->pin_global)
return false;
/* We can only return physical pages to the system if we can either
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index fbfcf88d7fe3..57bef6963d2b 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1094,6 +1094,7 @@ execlists_context_pin(struct intel_engine_cs *engine,
i915_ggtt_offset(ce->ring->vma);
ce->state->obj->mm.dirty = true;
+ ce->state->obj->pin_global++;
i915_gem_context_get(ctx);
out:
@@ -1121,6 +1122,7 @@ static void execlists_context_unpin(struct intel_engine_cs *engine,
intel_ring_unpin(ce->ring);
+ ce->state->obj->pin_global--;
i915_gem_object_unpin_map(ce->state->obj);
i915_vma_unpin(ce->state);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 4285f09ff8b8..a0356e5bd4c8 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1246,6 +1246,8 @@ int intel_ring_pin(struct intel_ring *ring,
if (IS_ERR(addr))
goto err;
+ vma->obj->pin_global++;
+
ring->vaddr = addr;
return 0;
@@ -1277,6 +1279,7 @@ void intel_ring_unpin(struct intel_ring *ring)
i915_gem_object_unpin_map(ring->vma->obj);
ring->vaddr = NULL;
+ ring->vma->obj->pin_global--;
i915_vma_unpin(ring->vma);
}
@@ -1441,6 +1444,7 @@ intel_ring_context_pin(struct intel_engine_cs *engine,
goto err;
ce->state->obj->mm.dirty = true;
+ ce->state->obj->pin_global++;
}
/* The kernel context is only used as a placeholder for flushing the
@@ -1475,8 +1479,10 @@ static void intel_ring_context_unpin(struct intel_engine_cs *engine,
if (--ce->pin_count)
return;
- if (ce->state)
+ if (ce->state) {
+ ce->state->obj->pin_global--;
i915_vma_unpin(ce->state);
+ }
i915_gem_context_put(ctx);
}
--
2.15.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 5/9] drm/i915: Move dev_priv->mm.[un]bound_list to its own lock
2017-10-13 20:26 [PATCH 1/9] drm/i915: Refactor testing obj->mm.pages Chris Wilson
` (2 preceding siblings ...)
2017-10-13 20:26 ` [PATCH 4/9] drm/i915: Remove walk over obj->vma_list for the shrinker Chris Wilson
@ 2017-10-13 20:26 ` Chris Wilson
2017-10-13 22:36 ` [PATCH v2] " Chris Wilson
2017-10-16 11:40 ` Chris Wilson
2017-10-13 20:26 ` [PATCH 6/9] drm/i915: Wire up shrinkctl->nr_scanned Chris Wilson
` (8 subsequent siblings)
12 siblings, 2 replies; 24+ messages in thread
From: Chris Wilson @ 2017-10-13 20:26 UTC (permalink / raw)
To: intel-gfx
Remove the struct_mutex requirement around dev_priv->mm.bound_list and
dev_priv->mm.unbound_list by giving it its own spinlock. This reduces
one more requirement for struct_mutex and in the process gives us
slightly more accurate unbound_list tracking, which should improve the
shrinker - but the drawback is that we drop the retirement before
counting so i915_gem_object_is_active() may be stale and lead us to
underestimate the number of objects that may be shrunk (see commit
bed50aea61df ("drm/i915/shrinker: Flush active on objects before
counting")).
v2: Crosslink the spinlock to the lists it protects, and btw this
changes s/obj->global_link/obj->mm.link/
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 39 +++++++++++---
drivers/gpu/drm/i915/i915_drv.h | 3 ++
drivers/gpu/drm/i915/i915_gem.c | 41 ++++++++++++---
drivers/gpu/drm/i915/i915_gem_gtt.c | 3 +-
drivers/gpu/drm/i915/i915_gem_object.h | 7 ++-
drivers/gpu/drm/i915/i915_gem_shrinker.c | 64 +++++++++--------------
drivers/gpu/drm/i915/i915_gem_stolen.c | 5 +-
drivers/gpu/drm/i915/i915_vma.c | 16 ++++--
drivers/gpu/drm/i915/selftests/i915_gem_context.c | 2 +-
drivers/gpu/drm/i915/selftests/i915_gem_evict.c | 8 ++-
10 files changed, 121 insertions(+), 67 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 7053d71b1ca9..997feddb8436 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -271,7 +271,9 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
goto out;
total_obj_size = total_gtt_size = count = 0;
- list_for_each_entry(obj, &dev_priv->mm.bound_list, global_link) {
+
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_for_each_entry(obj, &dev_priv->mm.bound_list, mm.link) {
if (count == total)
break;
@@ -283,7 +285,7 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
total_gtt_size += i915_gem_obj_total_ggtt_size(obj);
}
- list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_link) {
+ list_for_each_entry(obj, &dev_priv->mm.unbound_list, mm.link) {
if (count == total)
break;
@@ -293,6 +295,7 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
objects[count++] = obj;
total_obj_size += obj->base.size;
}
+ spin_unlock(&dev_priv->mm.obj_lock);
sort(objects, count, sizeof(*objects), obj_rank_by_stolen, NULL);
@@ -454,7 +457,9 @@ static int i915_gem_object_info(struct seq_file *m, void *data)
mapped_size = mapped_count = 0;
purgeable_size = purgeable_count = 0;
huge_size = huge_count = 0;
- list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_link) {
+
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_for_each_entry(obj, &dev_priv->mm.unbound_list, mm.link) {
size += obj->base.size;
++count;
@@ -477,7 +482,7 @@ static int i915_gem_object_info(struct seq_file *m, void *data)
seq_printf(m, "%u unbound objects, %llu bytes\n", count, size);
size = count = dpy_size = dpy_count = 0;
- list_for_each_entry(obj, &dev_priv->mm.bound_list, global_link) {
+ list_for_each_entry(obj, &dev_priv->mm.bound_list, mm.link) {
size += obj->base.size;
++count;
@@ -502,6 +507,8 @@ static int i915_gem_object_info(struct seq_file *m, void *data)
page_sizes |= obj->mm.page_sizes.sg;
}
}
+ spin_unlock(&dev_priv->mm.obj_lock);
+
seq_printf(m, "%u bound objects, %llu bytes\n",
count, size);
seq_printf(m, "%u purgeable objects, %llu bytes\n",
@@ -568,28 +575,46 @@ static int i915_gem_gtt_info(struct seq_file *m, void *data)
struct drm_info_node *node = m->private;
struct drm_i915_private *dev_priv = node_to_i915(node);
struct drm_device *dev = &dev_priv->drm;
+ struct drm_i915_gem_object **objects;
struct drm_i915_gem_object *obj;
u64 total_obj_size, total_gtt_size;
+ unsigned long nobject, n;
int count, ret;
+ nobject = READ_ONCE(dev_priv->mm.object_count);
+ objects = kvmalloc_array(nobject, sizeof(*objects), GFP_KERNEL);
+ if (!objects)
+ return -ENOMEM;
+
ret = mutex_lock_interruptible(&dev->struct_mutex);
if (ret)
return ret;
- total_obj_size = total_gtt_size = count = 0;
- list_for_each_entry(obj, &dev_priv->mm.bound_list, global_link) {
+ count = 0;
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_for_each_entry(obj, &dev_priv->mm.bound_list, mm.link) {
+ objects[count++] = obj;
+ if (count == nobject)
+ break;
+ }
+ spin_unlock(&dev_priv->mm.obj_lock);
+
+ total_obj_size = total_gtt_size = 0;
+ for (n = 0; n < count; n++) {
+ obj = objects[n];
+
seq_puts(m, " ");
describe_obj(m, obj);
seq_putc(m, '\n');
total_obj_size += obj->base.size;
total_gtt_size += i915_gem_obj_total_ggtt_size(obj);
- count++;
}
mutex_unlock(&dev->struct_mutex);
seq_printf(m, "Total %d objects, %llu bytes, %llu GTT size\n",
count, total_obj_size, total_gtt_size);
+ kvfree(objects);
return 0;
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 48be1d3c1d33..c51c6020c7a9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1490,6 +1490,9 @@ struct i915_gem_mm {
* always the inner lock when overlapping with struct_mutex. */
struct mutex stolen_lock;
+ /* Protects bound_list/unbound_list and #drm_i915_gem_object.mm.link */
+ spinlock_t obj_lock;
+
/** List of all objects in gtt_space. Used to restore gtt
* mappings on resume */
struct list_head bound_list;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ffb2e5973a08..819866359d5d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1537,6 +1537,8 @@ static void i915_gem_object_bump_inactive_ggtt(struct drm_i915_gem_object *obj)
struct list_head *list;
struct i915_vma *vma;
+ GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
+
list_for_each_entry(vma, &obj->vma_list, obj_link) {
if (!i915_vma_is_ggtt(vma))
break;
@@ -1551,8 +1553,10 @@ static void i915_gem_object_bump_inactive_ggtt(struct drm_i915_gem_object *obj)
}
i915 = to_i915(obj->base.dev);
+ spin_lock(&i915->mm.obj_lock);
list = obj->bind_count ? &i915->mm.bound_list : &i915->mm.unbound_list;
- list_move_tail(&obj->global_link, list);
+ list_move_tail(&obj->mm.link, list);
+ spin_unlock(&i915->mm.obj_lock);
}
/**
@@ -2253,6 +2257,7 @@ static void __i915_gem_object_reset_page_iter(struct drm_i915_gem_object *obj)
void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj,
enum i915_mm_subclass subclass)
{
+ struct drm_i915_private *i915 = to_i915(obj->base.dev);
struct sg_table *pages;
if (i915_gem_object_has_pinned_pages(obj))
@@ -2273,6 +2278,10 @@ void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj,
pages = fetch_and_zero(&obj->mm.pages);
GEM_BUG_ON(!pages);
+ spin_lock(&i915->mm.obj_lock);
+ list_del(&obj->mm.link);
+ spin_unlock(&i915->mm.obj_lock);
+
if (obj->mm.mapping) {
void *ptr;
@@ -2507,7 +2516,7 @@ void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj,
obj->mm.pages = pages;
if (i915_gem_object_is_tiled(obj) &&
- to_i915(obj->base.dev)->quirks & QUIRK_PIN_SWIZZLED_PAGES) {
+ i915->quirks & QUIRK_PIN_SWIZZLED_PAGES) {
GEM_BUG_ON(obj->mm.quirked);
__i915_gem_object_pin_pages(obj);
obj->mm.quirked = true;
@@ -2529,8 +2538,11 @@ void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj,
if (obj->mm.page_sizes.phys & ~0u << i)
obj->mm.page_sizes.sg |= BIT(i);
}
-
GEM_BUG_ON(!HAS_PAGE_SIZES(i915, obj->mm.page_sizes.sg));
+
+ spin_lock(&i915->mm.obj_lock);
+ list_add(&obj->mm.link, &i915->mm.unbound_list);
+ spin_unlock(&i915->mm.obj_lock);
}
static int ____i915_gem_object_get_pages(struct drm_i915_gem_object *obj)
@@ -4326,7 +4338,6 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
{
mutex_init(&obj->mm.lock);
- INIT_LIST_HEAD(&obj->global_link);
INIT_LIST_HEAD(&obj->vma_list);
INIT_LIST_HEAD(&obj->lut_list);
INIT_LIST_HEAD(&obj->batch_pool_link);
@@ -4498,7 +4509,18 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
GEM_BUG_ON(!list_empty(&obj->vma_list));
GEM_BUG_ON(!RB_EMPTY_ROOT(&obj->vma_tree));
- list_del(&obj->global_link);
+ /* This serializes freeing with the shrinker. Since the free
+ * is delayed, first by RCU then by the workqueue, we want the
+ * shrinker to be able to free pages of unreferenced objects,
+ * or else we may oom whilst there are plenty of deferred
+ * freed objects.
+ */
+ if (i915_gem_object_has_pages(obj)) {
+ spin_lock(&i915->mm.obj_lock);
+ list_del_init(&obj->mm.link);
+ spin_unlock(&i915->mm.obj_lock);
+ }
+
}
intel_runtime_pm_put(i915);
mutex_unlock(&i915->drm.struct_mutex);
@@ -5037,11 +5059,14 @@ i915_gem_load_init(struct drm_i915_private *dev_priv)
goto err_priorities;
INIT_WORK(&dev_priv->mm.free_work, __i915_gem_free_work);
+
+ spin_lock_init(&dev_priv->mm.obj_lock);
init_llist_head(&dev_priv->mm.free_list);
INIT_LIST_HEAD(&dev_priv->mm.unbound_list);
INIT_LIST_HEAD(&dev_priv->mm.bound_list);
INIT_LIST_HEAD(&dev_priv->mm.fence_list);
INIT_LIST_HEAD(&dev_priv->mm.userfault_list);
+
INIT_DELAYED_WORK(&dev_priv->gt.retire_work,
i915_gem_retire_work_handler);
INIT_DELAYED_WORK(&dev_priv->gt.idle_work,
@@ -5135,12 +5160,12 @@ int i915_gem_freeze_late(struct drm_i915_private *dev_priv)
i915_gem_shrink(dev_priv, -1UL, NULL, I915_SHRINK_UNBOUND);
i915_gem_drain_freed_objects(dev_priv);
- mutex_lock(&dev_priv->drm.struct_mutex);
+ spin_lock(&dev_priv->mm.obj_lock);
for (p = phases; *p; p++) {
- list_for_each_entry(obj, *p, global_link)
+ list_for_each_entry(obj, *p, mm.link)
__start_cpu_write(obj);
}
- mutex_unlock(&dev_priv->drm.struct_mutex);
+ spin_unlock(&dev_priv->mm.obj_lock);
return 0;
}
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index daba55a4fce2..527a2d2d6281 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3594,8 +3594,7 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
ggtt->base.closed = true; /* skip rewriting PTE on VMA unbind */
/* clflush objects bound into the GGTT and rebind them. */
- list_for_each_entry_safe(obj, on,
- &dev_priv->mm.bound_list, global_link) {
+ list_for_each_entry_safe(obj, on, &dev_priv->mm.bound_list, mm.link) {
bool ggtt_bound = false;
struct i915_vma *vma;
diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h
index 72aa02f3a0f8..63ce38c1cce9 100644
--- a/drivers/gpu/drm/i915/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/i915_gem_object.h
@@ -114,7 +114,6 @@ struct drm_i915_gem_object {
/** Stolen memory for this object, instead of being backed by shmem. */
struct drm_mm_node *stolen;
- struct list_head global_link;
union {
struct rcu_head rcu;
struct llist_node freed;
@@ -208,6 +207,12 @@ struct drm_i915_gem_object {
struct mutex lock; /* protects this cache */
} get_page;
+ /**
+ * Element within i915->mm.unbound_list or i915->mm.bound_list,
+ * locked by i915->mm.obj_lock.
+ */
+ struct list_head link;
+
/**
* Advice: are the backing pages purgeable?
*/
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index 575a6b735f39..065d026b5092 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -78,9 +78,6 @@ static bool swap_available(void)
static bool can_release_pages(struct drm_i915_gem_object *obj)
{
- if (!i915_gem_object_has_pages(obj))
- return false;
-
/* Consider only shrinkable ojects. */
if (!i915_gem_object_is_shrinkable(obj))
return false;
@@ -102,7 +99,7 @@ static bool can_release_pages(struct drm_i915_gem_object *obj)
* To simplify the scan, and to avoid walking the list of vma under the
* object, we just check the count of its permanently pinned.
*/
- if (obj->pin_global)
+ if (READ_ONCE(obj->pin_global))
return false;
/* We can only return physical pages to the system if we can either
@@ -204,15 +201,20 @@ i915_gem_shrink(struct drm_i915_private *dev_priv,
continue;
INIT_LIST_HEAD(&still_in_list);
+
+ /*
+ * We serialize our access to unreferenced objects through
+ * the use of the struct_mutex. While the objects are not
+ * yet freed (due to RCU then a workqueue) we still want
+ * to be able to shrink their pages, so they remain on
+ * the unbound/bound list until actually freed.
+ */
+ spin_lock(&dev_priv->mm.obj_lock);
while (count < target &&
(obj = list_first_entry_or_null(phase->list,
typeof(*obj),
- global_link))) {
- list_move_tail(&obj->global_link, &still_in_list);
- if (!obj->mm.pages) {
- list_del_init(&obj->global_link);
- continue;
- }
+ mm.link))) {
+ list_move_tail(&obj->mm.link, &still_in_list);
if (flags & I915_SHRINK_PURGEABLE &&
obj->mm.madv != I915_MADV_DONTNEED)
@@ -230,20 +232,24 @@ i915_gem_shrink(struct drm_i915_private *dev_priv,
if (!can_release_pages(obj))
continue;
+ spin_unlock(&dev_priv->mm.obj_lock);
+
if (unsafe_drop_pages(obj)) {
/* May arrive from get_pages on another bo */
mutex_lock_nested(&obj->mm.lock,
I915_MM_SHRINKER);
if (!i915_gem_object_has_pages(obj)) {
__i915_gem_object_invalidate(obj);
- list_del_init(&obj->global_link);
count += obj->base.size >> PAGE_SHIFT;
}
mutex_unlock(&obj->mm.lock);
scanned += obj->base.size >> PAGE_SHIFT;
}
+
+ spin_lock(&dev_priv->mm.obj_lock);
}
list_splice_tail(&still_in_list, phase->list);
+ spin_unlock(&dev_priv->mm.obj_lock);
}
if (flags & I915_SHRINK_BOUND)
@@ -292,25 +298,17 @@ i915_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc)
struct drm_i915_private *dev_priv =
container_of(shrinker, struct drm_i915_private, mm.shrinker);
struct drm_i915_gem_object *obj;
- unsigned long count;
- bool unlock;
-
- if (!shrinker_lock(dev_priv, &unlock))
- return 0;
-
- i915_gem_retire_requests(dev_priv);
+ unsigned long count = 0;
- count = 0;
- list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_link)
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_for_each_entry(obj, &dev_priv->mm.unbound_list, mm.link)
if (can_release_pages(obj))
count += obj->base.size >> PAGE_SHIFT;
- list_for_each_entry(obj, &dev_priv->mm.bound_list, global_link) {
+ list_for_each_entry(obj, &dev_priv->mm.bound_list, mm.link)
if (!i915_gem_object_is_active(obj) && can_release_pages(obj))
count += obj->base.size >> PAGE_SHIFT;
- }
-
- shrinker_unlock(dev_priv, unlock);
+ spin_unlock(&dev_priv->mm.obj_lock);
return count;
}
@@ -387,10 +385,6 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
container_of(nb, struct drm_i915_private, mm.oom_notifier);
struct drm_i915_gem_object *obj;
unsigned long unevictable, bound, unbound, freed_pages;
- bool unlock;
-
- if (!shrinker_lock_uninterruptible(dev_priv, &unlock, 5000))
- return NOTIFY_DONE;
freed_pages = i915_gem_shrink_all(dev_priv);
@@ -399,26 +393,20 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
* being pointed to by hardware.
*/
unbound = bound = unevictable = 0;
- list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_link) {
- if (!i915_gem_object_has_pages(obj))
- continue;
-
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_for_each_entry(obj, &dev_priv->mm.unbound_list, mm.link) {
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_link) {
- if (!i915_gem_object_has_pages(obj))
- continue;
-
+ list_for_each_entry(obj, &dev_priv->mm.bound_list, mm.link) {
if (!can_release_pages(obj))
unevictable += obj->base.size >> PAGE_SHIFT;
else
bound += obj->base.size >> PAGE_SHIFT;
}
-
- shrinker_unlock(dev_priv, unlock);
+ spin_unlock(&dev_priv->mm.obj_lock);
if (freed_pages || unbound || bound)
pr_info("Purging GPU memory, %lu pages freed, "
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 54fd4cfa9d07..03e7abc7e043 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -724,8 +724,11 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_i915_private *dev_priv
vma->flags |= I915_VMA_GLOBAL_BIND;
__i915_vma_set_map_and_fenceable(vma);
list_move_tail(&vma->vm_link, &ggtt->base.inactive_list);
- list_move_tail(&obj->global_link, &dev_priv->mm.bound_list);
+
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_move_tail(&obj->mm.link, &dev_priv->mm.bound_list);
obj->bind_count++;
+ spin_unlock(&dev_priv->mm.obj_lock);
return obj;
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 4dce2e0197d9..fc77e8191eb5 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -58,8 +58,10 @@ i915_vma_retire(struct i915_gem_active *active,
* so that we don't steal from recently used but inactive objects
* (unless we are forced to ofc!)
*/
+ spin_lock(&rq->i915->mm.obj_lock);
if (obj->bind_count)
- list_move_tail(&obj->global_link, &rq->i915->mm.bound_list);
+ list_move_tail(&obj->mm.link, &rq->i915->mm.bound_list);
+ spin_unlock(&rq->i915->mm.obj_lock);
obj->mm.dirty = true; /* be paranoid */
@@ -563,9 +565,13 @@ i915_vma_insert(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
GEM_BUG_ON(!i915_gem_valid_gtt_space(vma, obj->cache_level));
- list_move_tail(&obj->global_link, &dev_priv->mm.bound_list);
list_move_tail(&vma->vm_link, &vma->vm->inactive_list);
+
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_move_tail(&obj->mm.link, &dev_priv->mm.bound_list);
obj->bind_count++;
+ spin_unlock(&dev_priv->mm.obj_lock);
+
GEM_BUG_ON(atomic_read(&obj->mm.pages_pin_count) < obj->bind_count);
return 0;
@@ -580,6 +586,7 @@ i915_vma_insert(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
static void
i915_vma_remove(struct i915_vma *vma)
{
+ struct drm_i915_private *i915 = vma->vm->i915;
struct drm_i915_gem_object *obj = vma->obj;
GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
@@ -593,9 +600,10 @@ i915_vma_remove(struct i915_vma *vma)
/* Since the unbound list is global, only move to that list if
* no more VMAs exist.
*/
+ spin_lock(&i915->mm.obj_lock);
if (--obj->bind_count == 0)
- list_move_tail(&obj->global_link,
- &to_i915(obj->base.dev)->mm.unbound_list);
+ list_move_tail(&obj->mm.link, &i915->mm.unbound_list);
+ spin_unlock(&i915->mm.obj_lock);
/* And finally now the object is completely decoupled from this vma,
* we can drop its hold on the backing storage and allow it to be
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
index fb0a58fc8348..def5052862ae 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
@@ -417,7 +417,7 @@ static int fake_aliasing_ppgtt_enable(struct drm_i915_private *i915)
if (err)
return err;
- list_for_each_entry(obj, &i915->mm.bound_list, global_link) {
+ list_for_each_entry(obj, &i915->mm.bound_list, mm.link) {
struct i915_vma *vma;
vma = i915_vma_instance(obj, &i915->ggtt.base, NULL);
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
index 473aa9631145..f463105ff48d 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
@@ -50,7 +50,7 @@ static int populate_ggtt(struct drm_i915_private *i915)
if (!list_empty(&i915->mm.unbound_list)) {
size = 0;
- list_for_each_entry(obj, &i915->mm.unbound_list, global_link)
+ list_for_each_entry(obj, &i915->mm.unbound_list, mm.link)
size++;
pr_err("Found %lld objects unbound!\n", size);
@@ -77,10 +77,10 @@ static void cleanup_objects(struct drm_i915_private *i915)
{
struct drm_i915_gem_object *obj, *on;
- list_for_each_entry_safe(obj, on, &i915->mm.unbound_list, global_link)
+ list_for_each_entry_safe(obj, on, &i915->mm.unbound_list, mm.link)
i915_gem_object_put(obj);
- list_for_each_entry_safe(obj, on, &i915->mm.bound_list, global_link)
+ list_for_each_entry_safe(obj, on, &i915->mm.bound_list, mm.link)
i915_gem_object_put(obj);
mutex_unlock(&i915->drm.struct_mutex);
@@ -152,8 +152,6 @@ static int igt_overcommit(void *arg)
goto cleanup;
}
- list_move(&obj->global_link, &i915->mm.unbound_list);
-
vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 0);
if (!IS_ERR(vma) || PTR_ERR(vma) != -ENOSPC) {
pr_err("Failed to evict+insert, i915_gem_object_ggtt_pin returned err=%d\n", (int)PTR_ERR(vma));
--
2.15.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 6/9] drm/i915: Wire up shrinkctl->nr_scanned
2017-10-13 20:26 [PATCH 1/9] drm/i915: Refactor testing obj->mm.pages Chris Wilson
` (3 preceding siblings ...)
2017-10-13 20:26 ` [PATCH 5/9] drm/i915: Move dev_priv->mm.[un]bound_list to its own lock Chris Wilson
@ 2017-10-13 20:26 ` Chris Wilson
2017-10-16 10:46 ` Joonas Lahtinen
2017-10-13 20:26 ` [PATCH 7/9] drm/i915: Set our shrinker->batch to 4096 (~16MiB) Chris Wilson
` (7 subsequent siblings)
12 siblings, 1 reply; 24+ messages in thread
From: Chris Wilson @ 2017-10-13 20:26 UTC (permalink / raw)
To: intel-gfx
shrink_slab() allows us to report back the number of objects we
successfully scanned (out of the target shrinkctl->nr_to_scan). As
report the number of pages owned by each GEM object as a separate item
to the shrinker, we cannot precisely control the number of shrinker
objects we scan on each pass; and indeed may free more than requested.
If we fail to tell the shrinker about the number of objects we process,
it will continue to hold a grudge against us as any objects left
unscanned are added to the next reclaim -- and so we will keep on
"unfairly" shrinking our own slab in comparison to other slabs.
v2: fixup the misplaced addition, we want to count everything we scan
(to match the number we reported earlier) not just the objects we
successfully validated and freed.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
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 065d026b5092..175765111f4e 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -243,8 +243,8 @@ i915_gem_shrink(struct drm_i915_private *dev_priv,
count += obj->base.size >> PAGE_SHIFT;
}
mutex_unlock(&obj->mm.lock);
- scanned += obj->base.size >> PAGE_SHIFT;
}
+ scanned += obj->base.size >> PAGE_SHIFT;
spin_lock(&dev_priv->mm.obj_lock);
}
--
2.15.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 7/9] drm/i915: Set our shrinker->batch to 4096 (~16MiB)
2017-10-13 20:26 [PATCH 1/9] drm/i915: Refactor testing obj->mm.pages Chris Wilson
` (4 preceding siblings ...)
2017-10-13 20:26 ` [PATCH 6/9] drm/i915: Wire up shrinkctl->nr_scanned Chris Wilson
@ 2017-10-13 20:26 ` Chris Wilson
2017-10-13 20:26 ` [PATCH 8/9] drm/i915: Only free the oldest stale object before a fresh allocation Chris Wilson
` (6 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Chris Wilson @ 2017-10-13 20:26 UTC (permalink / raw)
To: intel-gfx
Prefer to defer activating our GEM shrinker until we have a few
megabytes to free; or we have accumulated sufficient mempressure by
deferring the reclaim to force a shrink. The intent is that because our
objects may typically be large, we are too effective at shrinking and
are not rewarded for freeing more pages than the batch. It will also
defer the initial shrinking to hopefully put it at a lower priority than
say the buffer cache (although it will balance out over a number of
reclaims, with GEM being more bursty).
v2: Give it a feedback system to try and tune the batch size towards
an effective size for the available objects.
v3: Start keeping track of shrinker stats in debugfs
v4: Protect against finding no shrinkable objects (div-by-zero)
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 11 +++++++++++
drivers/gpu/drm/i915/i915_gem_shrinker.c | 34 +++++++++++++++++++++++++-------
2 files changed, 38 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 997feddb8436..6d643642e0ce 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3331,6 +3331,16 @@ static int i915_engine_info(struct seq_file *m, void *unused)
return 0;
}
+static int i915_shrinker_info(struct seq_file *m, void *unused)
+{
+ struct drm_i915_private *i915 = node_to_i915(m->private);
+
+ seq_printf(m, "seeks = %d\n", i915->mm.shrinker.seeks);
+ seq_printf(m, "batch = %lu\n", i915->mm.shrinker.batch);
+
+ return 0;
+}
+
static int i915_semaphore_status(struct seq_file *m, void *unused)
{
struct drm_i915_private *dev_priv = node_to_i915(m->private);
@@ -4811,6 +4821,7 @@ static const struct drm_info_list i915_debugfs_list[] = {
{"i915_dmc_info", i915_dmc_info, 0},
{"i915_display_info", i915_display_info, 0},
{"i915_engine_info", i915_engine_info, 0},
+ {"i915_shrinker_info", i915_shrinker_info, 0},
{"i915_semaphore_status", i915_semaphore_status, 0},
{"i915_shared_dplls_info", i915_shared_dplls_info, 0},
{"i915_dp_mst_info", i915_dp_mst_info, 0},
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index 175765111f4e..bb4079f9abfd 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -295,20 +295,39 @@ unsigned long i915_gem_shrink_all(struct drm_i915_private *dev_priv)
static unsigned long
i915_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc)
{
- struct drm_i915_private *dev_priv =
+ struct drm_i915_private *i915 =
container_of(shrinker, struct drm_i915_private, mm.shrinker);
struct drm_i915_gem_object *obj;
+ unsigned long num_objects = 0;
unsigned long count = 0;
- spin_lock(&dev_priv->mm.obj_lock);
- list_for_each_entry(obj, &dev_priv->mm.unbound_list, mm.link)
- if (can_release_pages(obj))
+ spin_lock(&i915->mm.obj_lock);
+ list_for_each_entry(obj, &i915->mm.unbound_list, mm.link)
+ if (can_release_pages(obj)) {
count += obj->base.size >> PAGE_SHIFT;
+ num_objects++;
+ }
- list_for_each_entry(obj, &dev_priv->mm.bound_list, mm.link)
- if (!i915_gem_object_is_active(obj) && can_release_pages(obj))
+ list_for_each_entry(obj, &i915->mm.bound_list, mm.link)
+ if (!i915_gem_object_is_active(obj) && can_release_pages(obj)) {
count += obj->base.size >> PAGE_SHIFT;
- spin_unlock(&dev_priv->mm.obj_lock);
+ num_objects++;
+ }
+ spin_unlock(&i915->mm.obj_lock);
+
+ /* Update our prefered vmscan batch size for the next pass.
+ * Our rough guess for an effective batch size is roughly 2
+ * available GEM objects worth of pages. That is we don't want
+ * the shrinker to fire, until it is worth the cost of freeing an
+ * entire GEM object.
+ */
+ if (num_objects) {
+ unsigned long avg = 2 * count / num_objects;
+
+ i915->mm.shrinker.batch =
+ max((i915->mm.shrinker.batch + avg) >> 1,
+ 128ul /* default SHRINK_BATCH */);
+ }
return count;
}
@@ -473,6 +492,7 @@ void i915_gem_shrinker_init(struct drm_i915_private *dev_priv)
dev_priv->mm.shrinker.scan_objects = i915_gem_shrinker_scan;
dev_priv->mm.shrinker.count_objects = i915_gem_shrinker_count;
dev_priv->mm.shrinker.seeks = DEFAULT_SEEKS;
+ dev_priv->mm.shrinker.batch = 4096;
WARN_ON(register_shrinker(&dev_priv->mm.shrinker));
dev_priv->mm.oom_notifier.notifier_call = i915_gem_shrinker_oom;
--
2.15.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 8/9] drm/i915: Only free the oldest stale object before a fresh allocation
2017-10-13 20:26 [PATCH 1/9] drm/i915: Refactor testing obj->mm.pages Chris Wilson
` (5 preceding siblings ...)
2017-10-13 20:26 ` [PATCH 7/9] drm/i915: Set our shrinker->batch to 4096 (~16MiB) Chris Wilson
@ 2017-10-13 20:26 ` Chris Wilson
2017-10-16 10:54 ` Joonas Lahtinen
2017-10-13 20:26 ` [PATCH 9/9] drm/i915: Trim struct_mutex hold duration for i915_gem_free_objects Chris Wilson
` (5 subsequent siblings)
12 siblings, 1 reply; 24+ messages in thread
From: Chris Wilson @ 2017-10-13 20:26 UTC (permalink / raw)
To: intel-gfx
Inspired by Tvrtko's critique of the reaping of the stale contexts
before allocating a new one, also limit the freed object reaping to the
oldest stale object before allocating a fresh object. Unlike contexts,
objects may have radically different sizes of backing storage, but
similar to contexts, whilst we want to prevent starvation due to
excessive freed lists, we also want do not want to delay fresh
allocations for too long. Only freeing the oldest on the freed object
list before each allocation is a reasonable compromise.
v2: Only a single consumer of llist_del_first() is allowed (although
multiple llist_add are still allowed in parallel). Unlike
i915_gem_context, i915_gem_flush_free_objects() is itself not serialized
and so we need to add our own spinlock. Otherwise KASAN eventually spots
a use-after-free for the race on *first->next.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> #v1
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_gem.c | 14 ++++++++++++--
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c51c6020c7a9..3ad17c69c53a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1513,6 +1513,7 @@ struct i915_gem_mm {
*/
struct llist_head free_list;
struct work_struct free_work;
+ spinlock_t free_lock;
/**
* Small stash of WC pages
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 819866359d5d..2c92048a00fc 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4557,9 +4557,18 @@ static void i915_gem_flush_free_objects(struct drm_i915_private *i915)
{
struct llist_node *freed;
- freed = llist_del_all(&i915->mm.free_list);
- if (unlikely(freed))
+ /* Free the oldest, most stale object to keep the free_list short */
+ freed = NULL;
+ if (!llist_empty(&i915->mm.free_list)) { /* quick test for hotpath */
+ /* Only one consumer of llist_del_first() allowed */
+ spin_lock(&i915->mm.free_lock);
+ freed = llist_del_first(&i915->mm.free_list);
+ spin_unlock(&i915->mm.free_lock);
+ }
+ if (unlikely(freed)) {
+ freed->next = NULL;
__i915_gem_free_objects(i915, freed);
+ }
}
static void __i915_gem_free_work(struct work_struct *work)
@@ -5061,6 +5070,7 @@ i915_gem_load_init(struct drm_i915_private *dev_priv)
INIT_WORK(&dev_priv->mm.free_work, __i915_gem_free_work);
spin_lock_init(&dev_priv->mm.obj_lock);
+ spin_lock_init(&dev_priv->mm.free_lock);
init_llist_head(&dev_priv->mm.free_list);
INIT_LIST_HEAD(&dev_priv->mm.unbound_list);
INIT_LIST_HEAD(&dev_priv->mm.bound_list);
--
2.15.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* [PATCH 9/9] drm/i915: Trim struct_mutex hold duration for i915_gem_free_objects
2017-10-13 20:26 [PATCH 1/9] drm/i915: Refactor testing obj->mm.pages Chris Wilson
` (6 preceding siblings ...)
2017-10-13 20:26 ` [PATCH 8/9] drm/i915: Only free the oldest stale object before a fresh allocation Chris Wilson
@ 2017-10-13 20:26 ` Chris Wilson
2017-10-13 21:46 ` ✗ Fi.CI.BAT: warning for series starting with [1/9] drm/i915: Refactor testing obj->mm.pages Patchwork
` (4 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Chris Wilson @ 2017-10-13 20:26 UTC (permalink / raw)
To: intel-gfx
We free objects in bulk after they wait for their RCU grace period.
Currently, we take struct_mutex and unbind all the objects. This can lead
to a long lock duration during which time those objects have their pages
unfreeable (i.e. the shrinker is prevented from reaping those pages). If
we only process a single object under the struct_mutex and then free the
pages, the number of objects locked away from the shrinker is minimal
and we allow regular clients better access to struct_mutex if they need
it.
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2c92048a00fc..86dbf3af5a04 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4492,13 +4492,14 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
{
struct drm_i915_gem_object *obj, *on;
- mutex_lock(&i915->drm.struct_mutex);
intel_runtime_pm_get(i915);
- llist_for_each_entry(obj, freed, freed) {
+ llist_for_each_entry_safe(obj, on, freed, freed) {
struct i915_vma *vma, *vn;
trace_i915_gem_object_destroy(obj);
+ mutex_lock(&i915->drm.struct_mutex);
+
GEM_BUG_ON(i915_gem_object_is_active(obj));
list_for_each_entry_safe(vma, vn,
&obj->vma_list, obj_link) {
@@ -4521,13 +4522,8 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
spin_unlock(&i915->mm.obj_lock);
}
- }
- intel_runtime_pm_put(i915);
- mutex_unlock(&i915->drm.struct_mutex);
-
- cond_resched();
+ mutex_unlock(&i915->drm.struct_mutex);
- llist_for_each_entry_safe(obj, on, freed, freed) {
GEM_BUG_ON(obj->bind_count);
GEM_BUG_ON(obj->userfault_count);
GEM_BUG_ON(atomic_read(&obj->frontbuffer_bits));
@@ -4550,7 +4546,11 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
kfree(obj->bit_17);
i915_gem_object_free(obj);
+
+ if (on)
+ cond_resched();
}
+ intel_runtime_pm_put(i915);
}
static void i915_gem_flush_free_objects(struct drm_i915_private *i915)
--
2.15.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* ✗ Fi.CI.BAT: warning for series starting with [1/9] drm/i915: Refactor testing obj->mm.pages
2017-10-13 20:26 [PATCH 1/9] drm/i915: Refactor testing obj->mm.pages Chris Wilson
` (7 preceding siblings ...)
2017-10-13 20:26 ` [PATCH 9/9] drm/i915: Trim struct_mutex hold duration for i915_gem_free_objects Chris Wilson
@ 2017-10-13 21:46 ` Patchwork
2017-10-13 22:32 ` Chris Wilson
2017-10-13 23:12 ` ✗ Fi.CI.BAT: failure for series starting with [1/9] drm/i915: Refactor testing obj->mm.pages (rev2) Patchwork
` (3 subsequent siblings)
12 siblings, 1 reply; 24+ messages in thread
From: Patchwork @ 2017-10-13 21:46 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/9] drm/i915: Refactor testing obj->mm.pages
URL : https://patchwork.freedesktop.org/series/31959/
State : warning
== Summary ==
Series 31959v1 series starting with [1/9] drm/i915: Refactor testing obj->mm.pages
https://patchwork.freedesktop.org/api/1.0/series/31959/revisions/1/mbox/
Test debugfs_test:
Subgroup read_all_entries:
dmesg-warn -> PASS (fi-kbl-7500u)
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-legacy:
pass -> DMESG-WARN (fi-gdg-551) fdo#102618
Subgroup basic-flip-after-cursor-legacy:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup basic-flip-after-cursor-varying-size:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup basic-flip-before-cursor-legacy:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup basic-flip-before-cursor-varying-size:
pass -> DMESG-WARN (fi-gdg-551)
Test kms_flip:
Subgroup basic-flip-vs-dpms:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup basic-flip-vs-modeset:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup basic-flip-vs-wf_vblank:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup basic-plain-flip:
pass -> DMESG-WARN (fi-gdg-551)
Test kms_pipe_crc_basic:
Subgroup bad-nb-words-1:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup bad-nb-words-3:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup bad-pipe:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup bad-source:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup hang-read-crc-pipe-a:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup hang-read-crc-pipe-b:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup nonblocking-crc-pipe-a:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup nonblocking-crc-pipe-a-frame-sequence:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup nonblocking-crc-pipe-b:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup nonblocking-crc-pipe-b-frame-sequence:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup read-crc-pipe-a:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup read-crc-pipe-a-frame-sequence:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup read-crc-pipe-b:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup read-crc-pipe-b-frame-sequence:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup suspend-read-crc-pipe-a:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup suspend-read-crc-pipe-b:
pass -> DMESG-WARN (fi-gdg-551)
Test kms_setmode:
Subgroup basic-clone-single-crtc:
pass -> DMESG-WARN (fi-gdg-551)
Test prime_self_import:
Subgroup basic-llseek-bad:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup basic-llseek-size:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup basic-with_fd_dup:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup basic-with_one_bo:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup basic-with_one_bo_two_files:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup basic-with_two_bos:
pass -> DMESG-WARN (fi-gdg-551)
Test prime_vgem:
Subgroup basic-fence-flip:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup basic-fence-mmap:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup basic-fence-read:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup basic-gtt:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup basic-read:
pass -> DMESG-WARN (fi-gdg-551)
Subgroup basic-write:
pass -> DMESG-WARN (fi-gdg-551)
Test vgem_basic:
Subgroup dmabuf-export:
pass -> DMESG-WARN (fi-gdg-551)
fdo#102618 https://bugs.freedesktop.org/show_bug.cgi?id=102618
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:452s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:470s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:386s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:574s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:285s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:523s
WARNING: Long output truncated
03d8d3c265cf402013c550833f32247082bfb551 drm-tip: 2017y-10m-13d-20h-44m-07s UTC integration manifest
9ebf96cf8043 drm/i915: Trim struct_mutex hold duration for i915_gem_free_objects
50620871ee40 drm/i915: Only free the oldest stale object before a fresh allocation
3f2311fe3735 drm/i915: Set our shrinker->batch to 4096 (~16MiB)
e6dedf206a6e drm/i915: Wire up shrinkctl->nr_scanned
83d1cae8f806 drm/i915: Move dev_priv->mm.[un]bound_list to its own lock
50f0dcff5d15 drm/i915: Remove walk over obj->vma_list for the shrinker
de96475e936a drm/i915: Drop debugfs/i915_gem_pin_display
1aa4c27c1c67 drm/i915: Rename obj->pin_display to obj->pin_global
3fb5212c3f64 drm/i915: Refactor testing obj->mm.pages
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6034/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: ✗ Fi.CI.BAT: warning for series starting with [1/9] drm/i915: Refactor testing obj->mm.pages
2017-10-13 21:46 ` ✗ Fi.CI.BAT: warning for series starting with [1/9] drm/i915: Refactor testing obj->mm.pages Patchwork
@ 2017-10-13 22:32 ` Chris Wilson
0 siblings, 0 replies; 24+ messages in thread
From: Chris Wilson @ 2017-10-13 22:32 UTC (permalink / raw)
To: Patchwork; +Cc: intel-gfx
Quoting Patchwork (2017-10-13 22:46:12)
> == Series Details ==
>
> Series: series starting with [1/9] drm/i915: Refactor testing obj->mm.pages
> URL : https://patchwork.freedesktop.org/series/31959/
> State : warning
>
> == Summary ==
>
> Series 31959v1 series starting with [1/9] drm/i915: Refactor testing obj->mm.pages
> https://patchwork.freedesktop.org/api/1.0/series/31959/revisions/1/mbox/
>
> Test debugfs_test:
> Subgroup read_all_entries:
> dmesg-warn -> PASS (fi-kbl-7500u)
> Test kms_cursor_legacy:
> Subgroup basic-busy-flip-before-cursor-legacy:
> pass -> DMESG-WARN (fi-gdg-551) fdo#102618
i915_gem_object_attach_phys() doing the double set-pages is upsetting
us. Looks easy enough, just need to add the missing decoupling there.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2] drm/i915: Move dev_priv->mm.[un]bound_list to its own lock
2017-10-13 20:26 ` [PATCH 5/9] drm/i915: Move dev_priv->mm.[un]bound_list to its own lock Chris Wilson
@ 2017-10-13 22:36 ` Chris Wilson
2017-10-13 23:23 ` [PATCH v3] " Chris Wilson
2017-10-16 11:40 ` Chris Wilson
1 sibling, 1 reply; 24+ messages in thread
From: Chris Wilson @ 2017-10-13 22:36 UTC (permalink / raw)
To: intel-gfx
Remove the struct_mutex requirement around dev_priv->mm.bound_list and
dev_priv->mm.unbound_list by giving it its own spinlock. This reduces
one more requirement for struct_mutex and in the process gives us
slightly more accurate unbound_list tracking, which should improve the
shrinker - but the drawback is that we drop the retirement before
counting so i915_gem_object_is_active() may be stale and lead us to
underestimate the number of objects that may be shrunk (see commit
bed50aea61df ("drm/i915/shrinker: Flush active on objects before
counting")).
v2: Crosslink the spinlock to the lists it protects, and btw this
changes s/obj->global_link/obj->mm.link/
v3: Fix decoupling of old links in i915_gem_object_attach_phys()
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> #v2
---
drivers/gpu/drm/i915/i915_debugfs.c | 39 +++++++++++---
drivers/gpu/drm/i915/i915_drv.h | 3 ++
drivers/gpu/drm/i915/i915_gem.c | 47 ++++++++++++++---
drivers/gpu/drm/i915/i915_gem_gtt.c | 3 +-
drivers/gpu/drm/i915/i915_gem_object.h | 7 ++-
drivers/gpu/drm/i915/i915_gem_shrinker.c | 64 +++++++++--------------
drivers/gpu/drm/i915/i915_gem_stolen.c | 5 +-
drivers/gpu/drm/i915/i915_vma.c | 16 ++++--
drivers/gpu/drm/i915/selftests/i915_gem_context.c | 2 +-
drivers/gpu/drm/i915/selftests/i915_gem_evict.c | 8 ++-
10 files changed, 127 insertions(+), 67 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 7053d71b1ca9..997feddb8436 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -271,7 +271,9 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
goto out;
total_obj_size = total_gtt_size = count = 0;
- list_for_each_entry(obj, &dev_priv->mm.bound_list, global_link) {
+
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_for_each_entry(obj, &dev_priv->mm.bound_list, mm.link) {
if (count == total)
break;
@@ -283,7 +285,7 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
total_gtt_size += i915_gem_obj_total_ggtt_size(obj);
}
- list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_link) {
+ list_for_each_entry(obj, &dev_priv->mm.unbound_list, mm.link) {
if (count == total)
break;
@@ -293,6 +295,7 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
objects[count++] = obj;
total_obj_size += obj->base.size;
}
+ spin_unlock(&dev_priv->mm.obj_lock);
sort(objects, count, sizeof(*objects), obj_rank_by_stolen, NULL);
@@ -454,7 +457,9 @@ static int i915_gem_object_info(struct seq_file *m, void *data)
mapped_size = mapped_count = 0;
purgeable_size = purgeable_count = 0;
huge_size = huge_count = 0;
- list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_link) {
+
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_for_each_entry(obj, &dev_priv->mm.unbound_list, mm.link) {
size += obj->base.size;
++count;
@@ -477,7 +482,7 @@ static int i915_gem_object_info(struct seq_file *m, void *data)
seq_printf(m, "%u unbound objects, %llu bytes\n", count, size);
size = count = dpy_size = dpy_count = 0;
- list_for_each_entry(obj, &dev_priv->mm.bound_list, global_link) {
+ list_for_each_entry(obj, &dev_priv->mm.bound_list, mm.link) {
size += obj->base.size;
++count;
@@ -502,6 +507,8 @@ static int i915_gem_object_info(struct seq_file *m, void *data)
page_sizes |= obj->mm.page_sizes.sg;
}
}
+ spin_unlock(&dev_priv->mm.obj_lock);
+
seq_printf(m, "%u bound objects, %llu bytes\n",
count, size);
seq_printf(m, "%u purgeable objects, %llu bytes\n",
@@ -568,28 +575,46 @@ static int i915_gem_gtt_info(struct seq_file *m, void *data)
struct drm_info_node *node = m->private;
struct drm_i915_private *dev_priv = node_to_i915(node);
struct drm_device *dev = &dev_priv->drm;
+ struct drm_i915_gem_object **objects;
struct drm_i915_gem_object *obj;
u64 total_obj_size, total_gtt_size;
+ unsigned long nobject, n;
int count, ret;
+ nobject = READ_ONCE(dev_priv->mm.object_count);
+ objects = kvmalloc_array(nobject, sizeof(*objects), GFP_KERNEL);
+ if (!objects)
+ return -ENOMEM;
+
ret = mutex_lock_interruptible(&dev->struct_mutex);
if (ret)
return ret;
- total_obj_size = total_gtt_size = count = 0;
- list_for_each_entry(obj, &dev_priv->mm.bound_list, global_link) {
+ count = 0;
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_for_each_entry(obj, &dev_priv->mm.bound_list, mm.link) {
+ objects[count++] = obj;
+ if (count == nobject)
+ break;
+ }
+ spin_unlock(&dev_priv->mm.obj_lock);
+
+ total_obj_size = total_gtt_size = 0;
+ for (n = 0; n < count; n++) {
+ obj = objects[n];
+
seq_puts(m, " ");
describe_obj(m, obj);
seq_putc(m, '\n');
total_obj_size += obj->base.size;
total_gtt_size += i915_gem_obj_total_ggtt_size(obj);
- count++;
}
mutex_unlock(&dev->struct_mutex);
seq_printf(m, "Total %d objects, %llu bytes, %llu GTT size\n",
count, total_obj_size, total_gtt_size);
+ kvfree(objects);
return 0;
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 48be1d3c1d33..c51c6020c7a9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1490,6 +1490,9 @@ struct i915_gem_mm {
* always the inner lock when overlapping with struct_mutex. */
struct mutex stolen_lock;
+ /* Protects bound_list/unbound_list and #drm_i915_gem_object.mm.link */
+ spinlock_t obj_lock;
+
/** List of all objects in gtt_space. Used to restore gtt
* mappings on resume */
struct list_head bound_list;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1f66a6168444..08a81d5fd185 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1537,6 +1537,8 @@ static void i915_gem_object_bump_inactive_ggtt(struct drm_i915_gem_object *obj)
struct list_head *list;
struct i915_vma *vma;
+ GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
+
list_for_each_entry(vma, &obj->vma_list, obj_link) {
if (!i915_vma_is_ggtt(vma))
break;
@@ -1551,8 +1553,10 @@ static void i915_gem_object_bump_inactive_ggtt(struct drm_i915_gem_object *obj)
}
i915 = to_i915(obj->base.dev);
+ spin_lock(&i915->mm.obj_lock);
list = obj->bind_count ? &i915->mm.bound_list : &i915->mm.unbound_list;
- list_move_tail(&obj->global_link, list);
+ list_move_tail(&obj->mm.link, list);
+ spin_unlock(&i915->mm.obj_lock);
}
/**
@@ -2253,6 +2257,7 @@ static void __i915_gem_object_reset_page_iter(struct drm_i915_gem_object *obj)
void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj,
enum i915_mm_subclass subclass)
{
+ struct drm_i915_private *i915 = to_i915(obj->base.dev);
struct sg_table *pages;
if (i915_gem_object_has_pinned_pages(obj))
@@ -2273,6 +2278,10 @@ void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj,
pages = fetch_and_zero(&obj->mm.pages);
GEM_BUG_ON(!pages);
+ spin_lock(&i915->mm.obj_lock);
+ list_del(&obj->mm.link);
+ spin_unlock(&i915->mm.obj_lock);
+
if (obj->mm.mapping) {
void *ptr;
@@ -2507,7 +2516,7 @@ void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj,
obj->mm.pages = pages;
if (i915_gem_object_is_tiled(obj) &&
- to_i915(obj->base.dev)->quirks & QUIRK_PIN_SWIZZLED_PAGES) {
+ i915->quirks & QUIRK_PIN_SWIZZLED_PAGES) {
GEM_BUG_ON(obj->mm.quirked);
__i915_gem_object_pin_pages(obj);
obj->mm.quirked = true;
@@ -2529,8 +2538,11 @@ void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj,
if (obj->mm.page_sizes.phys & ~0u << i)
obj->mm.page_sizes.sg |= BIT(i);
}
-
GEM_BUG_ON(!HAS_PAGE_SIZES(i915, obj->mm.page_sizes.sg));
+
+ spin_lock(&i915->mm.obj_lock);
+ list_add(&obj->mm.link, &i915->mm.unbound_list);
+ spin_unlock(&i915->mm.obj_lock);
}
static int ____i915_gem_object_get_pages(struct drm_i915_gem_object *obj)
@@ -4324,7 +4336,6 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
{
mutex_init(&obj->mm.lock);
- INIT_LIST_HEAD(&obj->global_link);
INIT_LIST_HEAD(&obj->vma_list);
INIT_LIST_HEAD(&obj->lut_list);
INIT_LIST_HEAD(&obj->batch_pool_link);
@@ -4496,7 +4507,18 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
GEM_BUG_ON(!list_empty(&obj->vma_list));
GEM_BUG_ON(!RB_EMPTY_ROOT(&obj->vma_tree));
- list_del(&obj->global_link);
+ /* This serializes freeing with the shrinker. Since the free
+ * is delayed, first by RCU then by the workqueue, we want the
+ * shrinker to be able to free pages of unreferenced objects,
+ * or else we may oom whilst there are plenty of deferred
+ * freed objects.
+ */
+ if (i915_gem_object_has_pages(obj)) {
+ spin_lock(&i915->mm.obj_lock);
+ list_del_init(&obj->mm.link);
+ spin_unlock(&i915->mm.obj_lock);
+ }
+
}
intel_runtime_pm_put(i915);
mutex_unlock(&i915->drm.struct_mutex);
@@ -5035,11 +5057,14 @@ i915_gem_load_init(struct drm_i915_private *dev_priv)
goto err_priorities;
INIT_WORK(&dev_priv->mm.free_work, __i915_gem_free_work);
+
+ spin_lock_init(&dev_priv->mm.obj_lock);
init_llist_head(&dev_priv->mm.free_list);
INIT_LIST_HEAD(&dev_priv->mm.unbound_list);
INIT_LIST_HEAD(&dev_priv->mm.bound_list);
INIT_LIST_HEAD(&dev_priv->mm.fence_list);
INIT_LIST_HEAD(&dev_priv->mm.userfault_list);
+
INIT_DELAYED_WORK(&dev_priv->gt.retire_work,
i915_gem_retire_work_handler);
INIT_DELAYED_WORK(&dev_priv->gt.idle_work,
@@ -5133,12 +5158,12 @@ int i915_gem_freeze_late(struct drm_i915_private *dev_priv)
i915_gem_shrink(dev_priv, -1UL, NULL, I915_SHRINK_UNBOUND);
i915_gem_drain_freed_objects(dev_priv);
- mutex_lock(&dev_priv->drm.struct_mutex);
+ spin_lock(&dev_priv->mm.obj_lock);
for (p = phases; *p; p++) {
- list_for_each_entry(obj, *p, global_link)
+ list_for_each_entry(obj, *p, mm.link)
__start_cpu_write(obj);
}
- mutex_unlock(&dev_priv->drm.struct_mutex);
+ spin_unlock(&dev_priv->mm.obj_lock);
return 0;
}
@@ -5460,6 +5485,12 @@ int i915_gem_object_attach_phys(struct drm_i915_gem_object *obj, int align)
pages = obj->mm.pages;
obj->ops = &i915_gem_phys_ops;
+ __i915_gem_object_reset_page_iter(obj);
+
+ spin_lock(&to_i915(obj->base.dev)->mm.obj_lock);
+ list_del(&obj->mm.link);
+ spin_unlock(&to_i915(obj->base.dev)->mm.obj_lock);
+
err = ____i915_gem_object_get_pages(obj);
if (err)
goto err_xfer;
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index daba55a4fce2..527a2d2d6281 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3594,8 +3594,7 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
ggtt->base.closed = true; /* skip rewriting PTE on VMA unbind */
/* clflush objects bound into the GGTT and rebind them. */
- list_for_each_entry_safe(obj, on,
- &dev_priv->mm.bound_list, global_link) {
+ list_for_each_entry_safe(obj, on, &dev_priv->mm.bound_list, mm.link) {
bool ggtt_bound = false;
struct i915_vma *vma;
diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h
index 72aa02f3a0f8..63ce38c1cce9 100644
--- a/drivers/gpu/drm/i915/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/i915_gem_object.h
@@ -114,7 +114,6 @@ struct drm_i915_gem_object {
/** Stolen memory for this object, instead of being backed by shmem. */
struct drm_mm_node *stolen;
- struct list_head global_link;
union {
struct rcu_head rcu;
struct llist_node freed;
@@ -208,6 +207,12 @@ struct drm_i915_gem_object {
struct mutex lock; /* protects this cache */
} get_page;
+ /**
+ * Element within i915->mm.unbound_list or i915->mm.bound_list,
+ * locked by i915->mm.obj_lock.
+ */
+ struct list_head link;
+
/**
* Advice: are the backing pages purgeable?
*/
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index 575a6b735f39..065d026b5092 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -78,9 +78,6 @@ static bool swap_available(void)
static bool can_release_pages(struct drm_i915_gem_object *obj)
{
- if (!i915_gem_object_has_pages(obj))
- return false;
-
/* Consider only shrinkable ojects. */
if (!i915_gem_object_is_shrinkable(obj))
return false;
@@ -102,7 +99,7 @@ static bool can_release_pages(struct drm_i915_gem_object *obj)
* To simplify the scan, and to avoid walking the list of vma under the
* object, we just check the count of its permanently pinned.
*/
- if (obj->pin_global)
+ if (READ_ONCE(obj->pin_global))
return false;
/* We can only return physical pages to the system if we can either
@@ -204,15 +201,20 @@ i915_gem_shrink(struct drm_i915_private *dev_priv,
continue;
INIT_LIST_HEAD(&still_in_list);
+
+ /*
+ * We serialize our access to unreferenced objects through
+ * the use of the struct_mutex. While the objects are not
+ * yet freed (due to RCU then a workqueue) we still want
+ * to be able to shrink their pages, so they remain on
+ * the unbound/bound list until actually freed.
+ */
+ spin_lock(&dev_priv->mm.obj_lock);
while (count < target &&
(obj = list_first_entry_or_null(phase->list,
typeof(*obj),
- global_link))) {
- list_move_tail(&obj->global_link, &still_in_list);
- if (!obj->mm.pages) {
- list_del_init(&obj->global_link);
- continue;
- }
+ mm.link))) {
+ list_move_tail(&obj->mm.link, &still_in_list);
if (flags & I915_SHRINK_PURGEABLE &&
obj->mm.madv != I915_MADV_DONTNEED)
@@ -230,20 +232,24 @@ i915_gem_shrink(struct drm_i915_private *dev_priv,
if (!can_release_pages(obj))
continue;
+ spin_unlock(&dev_priv->mm.obj_lock);
+
if (unsafe_drop_pages(obj)) {
/* May arrive from get_pages on another bo */
mutex_lock_nested(&obj->mm.lock,
I915_MM_SHRINKER);
if (!i915_gem_object_has_pages(obj)) {
__i915_gem_object_invalidate(obj);
- list_del_init(&obj->global_link);
count += obj->base.size >> PAGE_SHIFT;
}
mutex_unlock(&obj->mm.lock);
scanned += obj->base.size >> PAGE_SHIFT;
}
+
+ spin_lock(&dev_priv->mm.obj_lock);
}
list_splice_tail(&still_in_list, phase->list);
+ spin_unlock(&dev_priv->mm.obj_lock);
}
if (flags & I915_SHRINK_BOUND)
@@ -292,25 +298,17 @@ i915_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc)
struct drm_i915_private *dev_priv =
container_of(shrinker, struct drm_i915_private, mm.shrinker);
struct drm_i915_gem_object *obj;
- unsigned long count;
- bool unlock;
-
- if (!shrinker_lock(dev_priv, &unlock))
- return 0;
-
- i915_gem_retire_requests(dev_priv);
+ unsigned long count = 0;
- count = 0;
- list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_link)
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_for_each_entry(obj, &dev_priv->mm.unbound_list, mm.link)
if (can_release_pages(obj))
count += obj->base.size >> PAGE_SHIFT;
- list_for_each_entry(obj, &dev_priv->mm.bound_list, global_link) {
+ list_for_each_entry(obj, &dev_priv->mm.bound_list, mm.link)
if (!i915_gem_object_is_active(obj) && can_release_pages(obj))
count += obj->base.size >> PAGE_SHIFT;
- }
-
- shrinker_unlock(dev_priv, unlock);
+ spin_unlock(&dev_priv->mm.obj_lock);
return count;
}
@@ -387,10 +385,6 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
container_of(nb, struct drm_i915_private, mm.oom_notifier);
struct drm_i915_gem_object *obj;
unsigned long unevictable, bound, unbound, freed_pages;
- bool unlock;
-
- if (!shrinker_lock_uninterruptible(dev_priv, &unlock, 5000))
- return NOTIFY_DONE;
freed_pages = i915_gem_shrink_all(dev_priv);
@@ -399,26 +393,20 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
* being pointed to by hardware.
*/
unbound = bound = unevictable = 0;
- list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_link) {
- if (!i915_gem_object_has_pages(obj))
- continue;
-
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_for_each_entry(obj, &dev_priv->mm.unbound_list, mm.link) {
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_link) {
- if (!i915_gem_object_has_pages(obj))
- continue;
-
+ list_for_each_entry(obj, &dev_priv->mm.bound_list, mm.link) {
if (!can_release_pages(obj))
unevictable += obj->base.size >> PAGE_SHIFT;
else
bound += obj->base.size >> PAGE_SHIFT;
}
-
- shrinker_unlock(dev_priv, unlock);
+ spin_unlock(&dev_priv->mm.obj_lock);
if (freed_pages || unbound || bound)
pr_info("Purging GPU memory, %lu pages freed, "
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 54fd4cfa9d07..03e7abc7e043 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -724,8 +724,11 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_i915_private *dev_priv
vma->flags |= I915_VMA_GLOBAL_BIND;
__i915_vma_set_map_and_fenceable(vma);
list_move_tail(&vma->vm_link, &ggtt->base.inactive_list);
- list_move_tail(&obj->global_link, &dev_priv->mm.bound_list);
+
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_move_tail(&obj->mm.link, &dev_priv->mm.bound_list);
obj->bind_count++;
+ spin_unlock(&dev_priv->mm.obj_lock);
return obj;
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 4dce2e0197d9..fc77e8191eb5 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -58,8 +58,10 @@ i915_vma_retire(struct i915_gem_active *active,
* so that we don't steal from recently used but inactive objects
* (unless we are forced to ofc!)
*/
+ spin_lock(&rq->i915->mm.obj_lock);
if (obj->bind_count)
- list_move_tail(&obj->global_link, &rq->i915->mm.bound_list);
+ list_move_tail(&obj->mm.link, &rq->i915->mm.bound_list);
+ spin_unlock(&rq->i915->mm.obj_lock);
obj->mm.dirty = true; /* be paranoid */
@@ -563,9 +565,13 @@ i915_vma_insert(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
GEM_BUG_ON(!i915_gem_valid_gtt_space(vma, obj->cache_level));
- list_move_tail(&obj->global_link, &dev_priv->mm.bound_list);
list_move_tail(&vma->vm_link, &vma->vm->inactive_list);
+
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_move_tail(&obj->mm.link, &dev_priv->mm.bound_list);
obj->bind_count++;
+ spin_unlock(&dev_priv->mm.obj_lock);
+
GEM_BUG_ON(atomic_read(&obj->mm.pages_pin_count) < obj->bind_count);
return 0;
@@ -580,6 +586,7 @@ i915_vma_insert(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
static void
i915_vma_remove(struct i915_vma *vma)
{
+ struct drm_i915_private *i915 = vma->vm->i915;
struct drm_i915_gem_object *obj = vma->obj;
GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
@@ -593,9 +600,10 @@ i915_vma_remove(struct i915_vma *vma)
/* Since the unbound list is global, only move to that list if
* no more VMAs exist.
*/
+ spin_lock(&i915->mm.obj_lock);
if (--obj->bind_count == 0)
- list_move_tail(&obj->global_link,
- &to_i915(obj->base.dev)->mm.unbound_list);
+ list_move_tail(&obj->mm.link, &i915->mm.unbound_list);
+ spin_unlock(&i915->mm.obj_lock);
/* And finally now the object is completely decoupled from this vma,
* we can drop its hold on the backing storage and allow it to be
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
index fb0a58fc8348..def5052862ae 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
@@ -417,7 +417,7 @@ static int fake_aliasing_ppgtt_enable(struct drm_i915_private *i915)
if (err)
return err;
- list_for_each_entry(obj, &i915->mm.bound_list, global_link) {
+ list_for_each_entry(obj, &i915->mm.bound_list, mm.link) {
struct i915_vma *vma;
vma = i915_vma_instance(obj, &i915->ggtt.base, NULL);
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
index 473aa9631145..f463105ff48d 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
@@ -50,7 +50,7 @@ static int populate_ggtt(struct drm_i915_private *i915)
if (!list_empty(&i915->mm.unbound_list)) {
size = 0;
- list_for_each_entry(obj, &i915->mm.unbound_list, global_link)
+ list_for_each_entry(obj, &i915->mm.unbound_list, mm.link)
size++;
pr_err("Found %lld objects unbound!\n", size);
@@ -77,10 +77,10 @@ static void cleanup_objects(struct drm_i915_private *i915)
{
struct drm_i915_gem_object *obj, *on;
- list_for_each_entry_safe(obj, on, &i915->mm.unbound_list, global_link)
+ list_for_each_entry_safe(obj, on, &i915->mm.unbound_list, mm.link)
i915_gem_object_put(obj);
- list_for_each_entry_safe(obj, on, &i915->mm.bound_list, global_link)
+ list_for_each_entry_safe(obj, on, &i915->mm.bound_list, mm.link)
i915_gem_object_put(obj);
mutex_unlock(&i915->drm.struct_mutex);
@@ -152,8 +152,6 @@ static int igt_overcommit(void *arg)
goto cleanup;
}
- list_move(&obj->global_link, &i915->mm.unbound_list);
-
vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 0);
if (!IS_ERR(vma) || PTR_ERR(vma) != -ENOSPC) {
pr_err("Failed to evict+insert, i915_gem_object_ggtt_pin returned err=%d\n", (int)PTR_ERR(vma));
--
2.15.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [1/9] drm/i915: Refactor testing obj->mm.pages (rev2)
2017-10-13 20:26 [PATCH 1/9] drm/i915: Refactor testing obj->mm.pages Chris Wilson
` (8 preceding siblings ...)
2017-10-13 21:46 ` ✗ Fi.CI.BAT: warning for series starting with [1/9] drm/i915: Refactor testing obj->mm.pages Patchwork
@ 2017-10-13 23:12 ` Patchwork
2017-10-13 23:51 ` ✗ Fi.CI.BAT: failure for series starting with [1/9] drm/i915: Refactor testing obj->mm.pages (rev3) Patchwork
` (2 subsequent siblings)
12 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2017-10-13 23:12 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/9] drm/i915: Refactor testing obj->mm.pages (rev2)
URL : https://patchwork.freedesktop.org/series/31959/
State : failure
== Summary ==
Series 31959v2 series starting with [1/9] drm/i915: Refactor testing obj->mm.pages
https://patchwork.freedesktop.org/api/1.0/series/31959/revisions/2/mbox/
Test core_auth:
Subgroup basic-auth:
pass -> SKIP (fi-gdg-551)
Test core_prop_blob:
Subgroup basic:
pass -> SKIP (fi-gdg-551)
Test debugfs_test:
Subgroup read_all_entries:
pass -> SKIP (fi-gdg-551)
dmesg-warn -> PASS (fi-kbl-7500u)
Test drv_getparams_basic:
Subgroup basic-eu-total:
pass -> SKIP (fi-gdg-551)
Subgroup basic-subslice-total:
pass -> SKIP (fi-gdg-551)
Test drv_hangman:
Subgroup error-state-basic:
pass -> SKIP (fi-gdg-551)
Test gem_basic:
Subgroup bad-close:
pass -> SKIP (fi-gdg-551)
Subgroup create-close:
pass -> SKIP (fi-gdg-551)
Subgroup create-fd-close:
pass -> SKIP (fi-gdg-551)
Test gem_close_race:
Subgroup basic-process:
pass -> SKIP (fi-gdg-551)
Subgroup basic-threads:
pass -> SKIP (fi-gdg-551)
Test gem_cpu_reloc:
Subgroup basic:
pass -> SKIP (fi-gdg-551)
Test gem_cs_tlb:
Subgroup basic-default:
pass -> SKIP (fi-gdg-551)
Test gem_exec_basic:
Subgroup basic-default:
pass -> SKIP (fi-gdg-551)
Subgroup basic-render:
pass -> SKIP (fi-gdg-551)
Subgroup gtt-default:
pass -> SKIP (fi-gdg-551)
Subgroup gtt-render:
pass -> SKIP (fi-gdg-551)
Subgroup readonly-default:
pass -> SKIP (fi-gdg-551)
Subgroup readonly-render:
pass -> SKIP (fi-gdg-551)
Test gem_exec_create:
Subgroup basic:
pass -> SKIP (fi-gdg-551)
Test gem_exec_nop:
Subgroup basic-parallel:
pass -> SKIP (fi-gdg-551)
Subgroup basic-series:
pass -> SKIP (fi-gdg-551)
Test gem_exec_reloc:
Subgroup basic-cpu:
pass -> SKIP (fi-gdg-551)
Subgroup basic-gtt:
pass -> SKIP (fi-gdg-551)
Subgroup basic-cpu-gtt:
pass -> SKIP (fi-gdg-551)
Subgroup basic-gtt-cpu:
pass -> SKIP (fi-gdg-551)
Subgroup basic-cpu-read:
pass -> SKIP (fi-gdg-551)
Subgroup basic-gtt-read:
pass -> SKIP (fi-gdg-551)
Subgroup basic-write-cpu:
pass -> SKIP (fi-gdg-551)
Subgroup basic-write-gtt:
pass -> SKIP (fi-gdg-551)
Subgroup basic-write-read:
pass -> SKIP (fi-gdg-551)
Subgroup basic-cpu-noreloc:
pass -> SKIP (fi-gdg-551)
Subgroup basic-gtt-noreloc:
pass -> SKIP (fi-gdg-551)
Subgroup basic-cpu-gtt-noreloc:
pass -> SKIP (fi-gdg-551)
Subgroup basic-gtt-cpu-noreloc:
pass -> SKIP (fi-gdg-551)
Subgroup basic-cpu-read-noreloc:
pass -> SKIP (fi-gdg-551)
Subgroup basic-gtt-read-noreloc:
pass -> SKIP (fi-gdg-551)
Subgroup basic-write-cpu-noreloc:
pass -> SKIP (fi-gdg-551)
Subgroup basic-write-gtt-noreloc:
pass -> SKIP (fi-gdg-551)
Subgroup basic-write-read-noreloc:
pass -> SKIP (fi-gdg-551)
Subgroup basic-cpu-active:
pass -> SKIP (fi-gdg-551) fdo#102582 +6
Subgroup basic-write-cpu-active:
WARNING: Long output truncated
03d8d3c265cf402013c550833f32247082bfb551 drm-tip: 2017y-10m-13d-20h-44m-07s UTC integration manifest
8279c2f6a349 drm/i915: Trim struct_mutex hold duration for i915_gem_free_objects
b0d1d4ac48d7 drm/i915: Only free the oldest stale object before a fresh allocation
837699ebe03a drm/i915: Set our shrinker->batch to 4096 (~16MiB)
48b31fe834b5 drm/i915: Wire up shrinkctl->nr_scanned
fdb0d2050f0c drm/i915: Move dev_priv->mm.[un]bound_list to its own lock
1cc06aeb3962 drm/i915: Remove walk over obj->vma_list for the shrinker
0eb652d76a4b drm/i915: Drop debugfs/i915_gem_pin_display
209be1bb90d0 drm/i915: Rename obj->pin_display to obj->pin_global
ca7f67a1a89c drm/i915: Refactor testing obj->mm.pages
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6036/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v3] drm/i915: Move dev_priv->mm.[un]bound_list to its own lock
2017-10-13 22:36 ` [PATCH v2] " Chris Wilson
@ 2017-10-13 23:23 ` Chris Wilson
0 siblings, 0 replies; 24+ messages in thread
From: Chris Wilson @ 2017-10-13 23:23 UTC (permalink / raw)
To: intel-gfx
Remove the struct_mutex requirement around dev_priv->mm.bound_list and
dev_priv->mm.unbound_list by giving it its own spinlock. This reduces
one more requirement for struct_mutex and in the process gives us
slightly more accurate unbound_list tracking, which should improve the
shrinker - but the drawback is that we drop the retirement before
counting so i915_gem_object_is_active() may be stale and lead us to
underestimate the number of objects that may be shrunk (see commit
bed50aea61df ("drm/i915/shrinker: Flush active on objects before
counting")).
v2: Crosslink the spinlock to the lists it protects, and btw this
changes s/obj->global_link/obj->mm.link/
v3: Fix decoupling of old links in i915_gem_object_attach_phys()
v3.1: Fix the fix, only unlink if it was linked
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> #v2
---
drivers/gpu/drm/i915/i915_debugfs.c | 39 +++++++++++---
drivers/gpu/drm/i915/i915_drv.h | 3 ++
drivers/gpu/drm/i915/i915_gem.c | 51 ++++++++++++++----
drivers/gpu/drm/i915/i915_gem_gtt.c | 3 +-
drivers/gpu/drm/i915/i915_gem_object.h | 7 ++-
drivers/gpu/drm/i915/i915_gem_shrinker.c | 64 +++++++++--------------
drivers/gpu/drm/i915/i915_gem_stolen.c | 5 +-
drivers/gpu/drm/i915/i915_vma.c | 16 ++++--
drivers/gpu/drm/i915/selftests/i915_gem_context.c | 2 +-
drivers/gpu/drm/i915/selftests/i915_gem_evict.c | 8 ++-
10 files changed, 130 insertions(+), 68 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 7053d71b1ca9..997feddb8436 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -271,7 +271,9 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
goto out;
total_obj_size = total_gtt_size = count = 0;
- list_for_each_entry(obj, &dev_priv->mm.bound_list, global_link) {
+
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_for_each_entry(obj, &dev_priv->mm.bound_list, mm.link) {
if (count == total)
break;
@@ -283,7 +285,7 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
total_gtt_size += i915_gem_obj_total_ggtt_size(obj);
}
- list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_link) {
+ list_for_each_entry(obj, &dev_priv->mm.unbound_list, mm.link) {
if (count == total)
break;
@@ -293,6 +295,7 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
objects[count++] = obj;
total_obj_size += obj->base.size;
}
+ spin_unlock(&dev_priv->mm.obj_lock);
sort(objects, count, sizeof(*objects), obj_rank_by_stolen, NULL);
@@ -454,7 +457,9 @@ static int i915_gem_object_info(struct seq_file *m, void *data)
mapped_size = mapped_count = 0;
purgeable_size = purgeable_count = 0;
huge_size = huge_count = 0;
- list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_link) {
+
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_for_each_entry(obj, &dev_priv->mm.unbound_list, mm.link) {
size += obj->base.size;
++count;
@@ -477,7 +482,7 @@ static int i915_gem_object_info(struct seq_file *m, void *data)
seq_printf(m, "%u unbound objects, %llu bytes\n", count, size);
size = count = dpy_size = dpy_count = 0;
- list_for_each_entry(obj, &dev_priv->mm.bound_list, global_link) {
+ list_for_each_entry(obj, &dev_priv->mm.bound_list, mm.link) {
size += obj->base.size;
++count;
@@ -502,6 +507,8 @@ static int i915_gem_object_info(struct seq_file *m, void *data)
page_sizes |= obj->mm.page_sizes.sg;
}
}
+ spin_unlock(&dev_priv->mm.obj_lock);
+
seq_printf(m, "%u bound objects, %llu bytes\n",
count, size);
seq_printf(m, "%u purgeable objects, %llu bytes\n",
@@ -568,28 +575,46 @@ static int i915_gem_gtt_info(struct seq_file *m, void *data)
struct drm_info_node *node = m->private;
struct drm_i915_private *dev_priv = node_to_i915(node);
struct drm_device *dev = &dev_priv->drm;
+ struct drm_i915_gem_object **objects;
struct drm_i915_gem_object *obj;
u64 total_obj_size, total_gtt_size;
+ unsigned long nobject, n;
int count, ret;
+ nobject = READ_ONCE(dev_priv->mm.object_count);
+ objects = kvmalloc_array(nobject, sizeof(*objects), GFP_KERNEL);
+ if (!objects)
+ return -ENOMEM;
+
ret = mutex_lock_interruptible(&dev->struct_mutex);
if (ret)
return ret;
- total_obj_size = total_gtt_size = count = 0;
- list_for_each_entry(obj, &dev_priv->mm.bound_list, global_link) {
+ count = 0;
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_for_each_entry(obj, &dev_priv->mm.bound_list, mm.link) {
+ objects[count++] = obj;
+ if (count == nobject)
+ break;
+ }
+ spin_unlock(&dev_priv->mm.obj_lock);
+
+ total_obj_size = total_gtt_size = 0;
+ for (n = 0; n < count; n++) {
+ obj = objects[n];
+
seq_puts(m, " ");
describe_obj(m, obj);
seq_putc(m, '\n');
total_obj_size += obj->base.size;
total_gtt_size += i915_gem_obj_total_ggtt_size(obj);
- count++;
}
mutex_unlock(&dev->struct_mutex);
seq_printf(m, "Total %d objects, %llu bytes, %llu GTT size\n",
count, total_obj_size, total_gtt_size);
+ kvfree(objects);
return 0;
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 48be1d3c1d33..c51c6020c7a9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1490,6 +1490,9 @@ struct i915_gem_mm {
* always the inner lock when overlapping with struct_mutex. */
struct mutex stolen_lock;
+ /* Protects bound_list/unbound_list and #drm_i915_gem_object.mm.link */
+ spinlock_t obj_lock;
+
/** List of all objects in gtt_space. Used to restore gtt
* mappings on resume */
struct list_head bound_list;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 1f66a6168444..2ac20867a5a7 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1537,6 +1537,8 @@ static void i915_gem_object_bump_inactive_ggtt(struct drm_i915_gem_object *obj)
struct list_head *list;
struct i915_vma *vma;
+ GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
+
list_for_each_entry(vma, &obj->vma_list, obj_link) {
if (!i915_vma_is_ggtt(vma))
break;
@@ -1551,8 +1553,10 @@ static void i915_gem_object_bump_inactive_ggtt(struct drm_i915_gem_object *obj)
}
i915 = to_i915(obj->base.dev);
+ spin_lock(&i915->mm.obj_lock);
list = obj->bind_count ? &i915->mm.bound_list : &i915->mm.unbound_list;
- list_move_tail(&obj->global_link, list);
+ list_move_tail(&obj->mm.link, list);
+ spin_unlock(&i915->mm.obj_lock);
}
/**
@@ -2253,6 +2257,7 @@ static void __i915_gem_object_reset_page_iter(struct drm_i915_gem_object *obj)
void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj,
enum i915_mm_subclass subclass)
{
+ struct drm_i915_private *i915 = to_i915(obj->base.dev);
struct sg_table *pages;
if (i915_gem_object_has_pinned_pages(obj))
@@ -2273,6 +2278,10 @@ void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj,
pages = fetch_and_zero(&obj->mm.pages);
GEM_BUG_ON(!pages);
+ spin_lock(&i915->mm.obj_lock);
+ list_del(&obj->mm.link);
+ spin_unlock(&i915->mm.obj_lock);
+
if (obj->mm.mapping) {
void *ptr;
@@ -2507,7 +2516,7 @@ void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj,
obj->mm.pages = pages;
if (i915_gem_object_is_tiled(obj) &&
- to_i915(obj->base.dev)->quirks & QUIRK_PIN_SWIZZLED_PAGES) {
+ i915->quirks & QUIRK_PIN_SWIZZLED_PAGES) {
GEM_BUG_ON(obj->mm.quirked);
__i915_gem_object_pin_pages(obj);
obj->mm.quirked = true;
@@ -2529,8 +2538,11 @@ void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj,
if (obj->mm.page_sizes.phys & ~0u << i)
obj->mm.page_sizes.sg |= BIT(i);
}
-
GEM_BUG_ON(!HAS_PAGE_SIZES(i915, obj->mm.page_sizes.sg));
+
+ spin_lock(&i915->mm.obj_lock);
+ list_add(&obj->mm.link, &i915->mm.unbound_list);
+ spin_unlock(&i915->mm.obj_lock);
}
static int ____i915_gem_object_get_pages(struct drm_i915_gem_object *obj)
@@ -4324,7 +4336,6 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
{
mutex_init(&obj->mm.lock);
- INIT_LIST_HEAD(&obj->global_link);
INIT_LIST_HEAD(&obj->vma_list);
INIT_LIST_HEAD(&obj->lut_list);
INIT_LIST_HEAD(&obj->batch_pool_link);
@@ -4496,7 +4507,18 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
GEM_BUG_ON(!list_empty(&obj->vma_list));
GEM_BUG_ON(!RB_EMPTY_ROOT(&obj->vma_tree));
- list_del(&obj->global_link);
+ /* This serializes freeing with the shrinker. Since the free
+ * is delayed, first by RCU then by the workqueue, we want the
+ * shrinker to be able to free pages of unreferenced objects,
+ * or else we may oom whilst there are plenty of deferred
+ * freed objects.
+ */
+ if (i915_gem_object_has_pages(obj)) {
+ spin_lock(&i915->mm.obj_lock);
+ list_del_init(&obj->mm.link);
+ spin_unlock(&i915->mm.obj_lock);
+ }
+
}
intel_runtime_pm_put(i915);
mutex_unlock(&i915->drm.struct_mutex);
@@ -5035,11 +5057,14 @@ i915_gem_load_init(struct drm_i915_private *dev_priv)
goto err_priorities;
INIT_WORK(&dev_priv->mm.free_work, __i915_gem_free_work);
+
+ spin_lock_init(&dev_priv->mm.obj_lock);
init_llist_head(&dev_priv->mm.free_list);
INIT_LIST_HEAD(&dev_priv->mm.unbound_list);
INIT_LIST_HEAD(&dev_priv->mm.bound_list);
INIT_LIST_HEAD(&dev_priv->mm.fence_list);
INIT_LIST_HEAD(&dev_priv->mm.userfault_list);
+
INIT_DELAYED_WORK(&dev_priv->gt.retire_work,
i915_gem_retire_work_handler);
INIT_DELAYED_WORK(&dev_priv->gt.idle_work,
@@ -5133,12 +5158,12 @@ int i915_gem_freeze_late(struct drm_i915_private *dev_priv)
i915_gem_shrink(dev_priv, -1UL, NULL, I915_SHRINK_UNBOUND);
i915_gem_drain_freed_objects(dev_priv);
- mutex_lock(&dev_priv->drm.struct_mutex);
+ spin_lock(&dev_priv->mm.obj_lock);
for (p = phases; *p; p++) {
- list_for_each_entry(obj, *p, global_link)
+ list_for_each_entry(obj, *p, mm.link)
__start_cpu_write(obj);
}
- mutex_unlock(&dev_priv->drm.struct_mutex);
+ spin_unlock(&dev_priv->mm.obj_lock);
return 0;
}
@@ -5457,7 +5482,15 @@ int i915_gem_object_attach_phys(struct drm_i915_gem_object *obj, int align)
goto err_unlock;
}
- pages = obj->mm.pages;
+ pages = fetch_and_zero(&obj->mm.pages);
+ if (pages) {
+ __i915_gem_object_reset_page_iter(obj);
+
+ spin_lock(&to_i915(obj->base.dev)->mm.obj_lock);
+ list_del(&obj->mm.link);
+ spin_unlock(&to_i915(obj->base.dev)->mm.obj_lock);
+ }
+
obj->ops = &i915_gem_phys_ops;
err = ____i915_gem_object_get_pages(obj);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index daba55a4fce2..527a2d2d6281 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3594,8 +3594,7 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
ggtt->base.closed = true; /* skip rewriting PTE on VMA unbind */
/* clflush objects bound into the GGTT and rebind them. */
- list_for_each_entry_safe(obj, on,
- &dev_priv->mm.bound_list, global_link) {
+ list_for_each_entry_safe(obj, on, &dev_priv->mm.bound_list, mm.link) {
bool ggtt_bound = false;
struct i915_vma *vma;
diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h
index 72aa02f3a0f8..63ce38c1cce9 100644
--- a/drivers/gpu/drm/i915/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/i915_gem_object.h
@@ -114,7 +114,6 @@ struct drm_i915_gem_object {
/** Stolen memory for this object, instead of being backed by shmem. */
struct drm_mm_node *stolen;
- struct list_head global_link;
union {
struct rcu_head rcu;
struct llist_node freed;
@@ -208,6 +207,12 @@ struct drm_i915_gem_object {
struct mutex lock; /* protects this cache */
} get_page;
+ /**
+ * Element within i915->mm.unbound_list or i915->mm.bound_list,
+ * locked by i915->mm.obj_lock.
+ */
+ struct list_head link;
+
/**
* Advice: are the backing pages purgeable?
*/
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index 575a6b735f39..065d026b5092 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -78,9 +78,6 @@ static bool swap_available(void)
static bool can_release_pages(struct drm_i915_gem_object *obj)
{
- if (!i915_gem_object_has_pages(obj))
- return false;
-
/* Consider only shrinkable ojects. */
if (!i915_gem_object_is_shrinkable(obj))
return false;
@@ -102,7 +99,7 @@ static bool can_release_pages(struct drm_i915_gem_object *obj)
* To simplify the scan, and to avoid walking the list of vma under the
* object, we just check the count of its permanently pinned.
*/
- if (obj->pin_global)
+ if (READ_ONCE(obj->pin_global))
return false;
/* We can only return physical pages to the system if we can either
@@ -204,15 +201,20 @@ i915_gem_shrink(struct drm_i915_private *dev_priv,
continue;
INIT_LIST_HEAD(&still_in_list);
+
+ /*
+ * We serialize our access to unreferenced objects through
+ * the use of the struct_mutex. While the objects are not
+ * yet freed (due to RCU then a workqueue) we still want
+ * to be able to shrink their pages, so they remain on
+ * the unbound/bound list until actually freed.
+ */
+ spin_lock(&dev_priv->mm.obj_lock);
while (count < target &&
(obj = list_first_entry_or_null(phase->list,
typeof(*obj),
- global_link))) {
- list_move_tail(&obj->global_link, &still_in_list);
- if (!obj->mm.pages) {
- list_del_init(&obj->global_link);
- continue;
- }
+ mm.link))) {
+ list_move_tail(&obj->mm.link, &still_in_list);
if (flags & I915_SHRINK_PURGEABLE &&
obj->mm.madv != I915_MADV_DONTNEED)
@@ -230,20 +232,24 @@ i915_gem_shrink(struct drm_i915_private *dev_priv,
if (!can_release_pages(obj))
continue;
+ spin_unlock(&dev_priv->mm.obj_lock);
+
if (unsafe_drop_pages(obj)) {
/* May arrive from get_pages on another bo */
mutex_lock_nested(&obj->mm.lock,
I915_MM_SHRINKER);
if (!i915_gem_object_has_pages(obj)) {
__i915_gem_object_invalidate(obj);
- list_del_init(&obj->global_link);
count += obj->base.size >> PAGE_SHIFT;
}
mutex_unlock(&obj->mm.lock);
scanned += obj->base.size >> PAGE_SHIFT;
}
+
+ spin_lock(&dev_priv->mm.obj_lock);
}
list_splice_tail(&still_in_list, phase->list);
+ spin_unlock(&dev_priv->mm.obj_lock);
}
if (flags & I915_SHRINK_BOUND)
@@ -292,25 +298,17 @@ i915_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc)
struct drm_i915_private *dev_priv =
container_of(shrinker, struct drm_i915_private, mm.shrinker);
struct drm_i915_gem_object *obj;
- unsigned long count;
- bool unlock;
-
- if (!shrinker_lock(dev_priv, &unlock))
- return 0;
-
- i915_gem_retire_requests(dev_priv);
+ unsigned long count = 0;
- count = 0;
- list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_link)
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_for_each_entry(obj, &dev_priv->mm.unbound_list, mm.link)
if (can_release_pages(obj))
count += obj->base.size >> PAGE_SHIFT;
- list_for_each_entry(obj, &dev_priv->mm.bound_list, global_link) {
+ list_for_each_entry(obj, &dev_priv->mm.bound_list, mm.link)
if (!i915_gem_object_is_active(obj) && can_release_pages(obj))
count += obj->base.size >> PAGE_SHIFT;
- }
-
- shrinker_unlock(dev_priv, unlock);
+ spin_unlock(&dev_priv->mm.obj_lock);
return count;
}
@@ -387,10 +385,6 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
container_of(nb, struct drm_i915_private, mm.oom_notifier);
struct drm_i915_gem_object *obj;
unsigned long unevictable, bound, unbound, freed_pages;
- bool unlock;
-
- if (!shrinker_lock_uninterruptible(dev_priv, &unlock, 5000))
- return NOTIFY_DONE;
freed_pages = i915_gem_shrink_all(dev_priv);
@@ -399,26 +393,20 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
* being pointed to by hardware.
*/
unbound = bound = unevictable = 0;
- list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_link) {
- if (!i915_gem_object_has_pages(obj))
- continue;
-
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_for_each_entry(obj, &dev_priv->mm.unbound_list, mm.link) {
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_link) {
- if (!i915_gem_object_has_pages(obj))
- continue;
-
+ list_for_each_entry(obj, &dev_priv->mm.bound_list, mm.link) {
if (!can_release_pages(obj))
unevictable += obj->base.size >> PAGE_SHIFT;
else
bound += obj->base.size >> PAGE_SHIFT;
}
-
- shrinker_unlock(dev_priv, unlock);
+ spin_unlock(&dev_priv->mm.obj_lock);
if (freed_pages || unbound || bound)
pr_info("Purging GPU memory, %lu pages freed, "
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 54fd4cfa9d07..03e7abc7e043 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -724,8 +724,11 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_i915_private *dev_priv
vma->flags |= I915_VMA_GLOBAL_BIND;
__i915_vma_set_map_and_fenceable(vma);
list_move_tail(&vma->vm_link, &ggtt->base.inactive_list);
- list_move_tail(&obj->global_link, &dev_priv->mm.bound_list);
+
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_move_tail(&obj->mm.link, &dev_priv->mm.bound_list);
obj->bind_count++;
+ spin_unlock(&dev_priv->mm.obj_lock);
return obj;
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 4dce2e0197d9..fc77e8191eb5 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -58,8 +58,10 @@ i915_vma_retire(struct i915_gem_active *active,
* so that we don't steal from recently used but inactive objects
* (unless we are forced to ofc!)
*/
+ spin_lock(&rq->i915->mm.obj_lock);
if (obj->bind_count)
- list_move_tail(&obj->global_link, &rq->i915->mm.bound_list);
+ list_move_tail(&obj->mm.link, &rq->i915->mm.bound_list);
+ spin_unlock(&rq->i915->mm.obj_lock);
obj->mm.dirty = true; /* be paranoid */
@@ -563,9 +565,13 @@ i915_vma_insert(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
GEM_BUG_ON(!i915_gem_valid_gtt_space(vma, obj->cache_level));
- list_move_tail(&obj->global_link, &dev_priv->mm.bound_list);
list_move_tail(&vma->vm_link, &vma->vm->inactive_list);
+
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_move_tail(&obj->mm.link, &dev_priv->mm.bound_list);
obj->bind_count++;
+ spin_unlock(&dev_priv->mm.obj_lock);
+
GEM_BUG_ON(atomic_read(&obj->mm.pages_pin_count) < obj->bind_count);
return 0;
@@ -580,6 +586,7 @@ i915_vma_insert(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
static void
i915_vma_remove(struct i915_vma *vma)
{
+ struct drm_i915_private *i915 = vma->vm->i915;
struct drm_i915_gem_object *obj = vma->obj;
GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
@@ -593,9 +600,10 @@ i915_vma_remove(struct i915_vma *vma)
/* Since the unbound list is global, only move to that list if
* no more VMAs exist.
*/
+ spin_lock(&i915->mm.obj_lock);
if (--obj->bind_count == 0)
- list_move_tail(&obj->global_link,
- &to_i915(obj->base.dev)->mm.unbound_list);
+ list_move_tail(&obj->mm.link, &i915->mm.unbound_list);
+ spin_unlock(&i915->mm.obj_lock);
/* And finally now the object is completely decoupled from this vma,
* we can drop its hold on the backing storage and allow it to be
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
index fb0a58fc8348..def5052862ae 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
@@ -417,7 +417,7 @@ static int fake_aliasing_ppgtt_enable(struct drm_i915_private *i915)
if (err)
return err;
- list_for_each_entry(obj, &i915->mm.bound_list, global_link) {
+ list_for_each_entry(obj, &i915->mm.bound_list, mm.link) {
struct i915_vma *vma;
vma = i915_vma_instance(obj, &i915->ggtt.base, NULL);
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
index 473aa9631145..f463105ff48d 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
@@ -50,7 +50,7 @@ static int populate_ggtt(struct drm_i915_private *i915)
if (!list_empty(&i915->mm.unbound_list)) {
size = 0;
- list_for_each_entry(obj, &i915->mm.unbound_list, global_link)
+ list_for_each_entry(obj, &i915->mm.unbound_list, mm.link)
size++;
pr_err("Found %lld objects unbound!\n", size);
@@ -77,10 +77,10 @@ static void cleanup_objects(struct drm_i915_private *i915)
{
struct drm_i915_gem_object *obj, *on;
- list_for_each_entry_safe(obj, on, &i915->mm.unbound_list, global_link)
+ list_for_each_entry_safe(obj, on, &i915->mm.unbound_list, mm.link)
i915_gem_object_put(obj);
- list_for_each_entry_safe(obj, on, &i915->mm.bound_list, global_link)
+ list_for_each_entry_safe(obj, on, &i915->mm.bound_list, mm.link)
i915_gem_object_put(obj);
mutex_unlock(&i915->drm.struct_mutex);
@@ -152,8 +152,6 @@ static int igt_overcommit(void *arg)
goto cleanup;
}
- list_move(&obj->global_link, &i915->mm.unbound_list);
-
vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 0);
if (!IS_ERR(vma) || PTR_ERR(vma) != -ENOSPC) {
pr_err("Failed to evict+insert, i915_gem_object_ggtt_pin returned err=%d\n", (int)PTR_ERR(vma));
--
2.15.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* ✗ Fi.CI.BAT: failure for series starting with [1/9] drm/i915: Refactor testing obj->mm.pages (rev3)
2017-10-13 20:26 [PATCH 1/9] drm/i915: Refactor testing obj->mm.pages Chris Wilson
` (9 preceding siblings ...)
2017-10-13 23:12 ` ✗ Fi.CI.BAT: failure for series starting with [1/9] drm/i915: Refactor testing obj->mm.pages (rev2) Patchwork
@ 2017-10-13 23:51 ` Patchwork
2017-10-16 12:10 ` ✓ Fi.CI.BAT: success for series starting with [1/9] drm/i915: Refactor testing obj->mm.pages (rev4) Patchwork
2017-10-16 20:12 ` ✗ Fi.CI.IGT: failure " Patchwork
12 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2017-10-13 23:51 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/9] drm/i915: Refactor testing obj->mm.pages (rev3)
URL : https://patchwork.freedesktop.org/series/31959/
State : failure
== Summary ==
Series 31959v3 series starting with [1/9] drm/i915: Refactor testing obj->mm.pages
https://patchwork.freedesktop.org/api/1.0/series/31959/revisions/3/mbox/
Test debugfs_test:
Subgroup read_all_entries:
dmesg-warn -> PASS (fi-kbl-7500u)
Test kms_busy:
Subgroup basic-flip-b:
pass -> FAIL (fi-byt-n2820)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-b:
dmesg-warn -> PASS (fi-byt-n2820) fdo#101705
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:461s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:472s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:387s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:566s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:285s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:511s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:516s
fi-byt-j1900 total:289 pass:253 dwarn:1 dfail:0 fail:0 skip:35 time:533s
fi-byt-n2820 total:289 pass:249 dwarn:0 dfail:0 fail:1 skip:39 time:516s
fi-cfl-s total:289 pass:253 dwarn:4 dfail:0 fail:0 skip:32 time:569s
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:436s
fi-gdg-551 total:289 pass:178 dwarn:1 dfail:0 fail:1 skip:109 time:274s
fi-glk-1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:597s
fi-hsw-4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:438s
fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:460s
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:504s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:472s
fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:505s
fi-kbl-7567u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:484s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:588s
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:653s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:466s
fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:659s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:534s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:515s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:472s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:580s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:430s
03d8d3c265cf402013c550833f32247082bfb551 drm-tip: 2017y-10m-13d-20h-44m-07s UTC integration manifest
06187df8a421 drm/i915: Trim struct_mutex hold duration for i915_gem_free_objects
61a574cb207e drm/i915: Only free the oldest stale object before a fresh allocation
ccc5e2cdbb65 drm/i915: Set our shrinker->batch to 4096 (~16MiB)
c8021764d1f6 drm/i915: Wire up shrinkctl->nr_scanned
6f6d47d8b86c drm/i915: Move dev_priv->mm.[un]bound_list to its own lock
23618c87cb60 drm/i915: Remove walk over obj->vma_list for the shrinker
6ea9209a9299 drm/i915: Drop debugfs/i915_gem_pin_display
ded40d8e8470 drm/i915: Rename obj->pin_display to obj->pin_global
0989fff9a8e6 drm/i915: Refactor testing obj->mm.pages
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6037/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/9] drm/i915: Rename obj->pin_display to obj->pin_global
2017-10-13 20:26 ` [PATCH 2/9] drm/i915: Rename obj->pin_display to obj->pin_global Chris Wilson
@ 2017-10-16 10:40 ` Joonas Lahtinen
0 siblings, 0 replies; 24+ messages in thread
From: Joonas Lahtinen @ 2017-10-16 10:40 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On Fri, 2017-10-13 at 21:26 +0100, Chris Wilson wrote:
> In the next patch, we want to extend use of the global pin counter for
> semi-permanent pinning of context/ring objects. Given that we plan to
> extend the usage to encompass a disparate set of objects, we want a name
> that reflects both and should entail less confusion.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Regards, Joonas
--
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] 24+ messages in thread
* Re: [PATCH 3/9] drm/i915: Drop debugfs/i915_gem_pin_display
2017-10-13 20:26 ` [PATCH 3/9] drm/i915: Drop debugfs/i915_gem_pin_display Chris Wilson
@ 2017-10-16 10:41 ` Joonas Lahtinen
0 siblings, 0 replies; 24+ messages in thread
From: Joonas Lahtinen @ 2017-10-16 10:41 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On Fri, 2017-10-13 at 21:26 +0100, Chris Wilson wrote:
> It has now lost its meaning (it shows more than just pin_display), I do
> not believe that we are using in preference to the complete listing from
> i915_gem_gtt, or the listing from i915_gem_framebuffer, or the listing
> of active display objects in i915_display_info.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Regards, Joonas
--
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] 24+ messages in thread
* Re: [PATCH 6/9] drm/i915: Wire up shrinkctl->nr_scanned
2017-10-13 20:26 ` [PATCH 6/9] drm/i915: Wire up shrinkctl->nr_scanned Chris Wilson
@ 2017-10-16 10:46 ` Joonas Lahtinen
2017-10-16 10:51 ` Chris Wilson
0 siblings, 1 reply; 24+ messages in thread
From: Joonas Lahtinen @ 2017-10-16 10:46 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On Fri, 2017-10-13 at 21:26 +0100, Chris Wilson wrote:
> shrink_slab() allows us to report back the number of objects we
> successfully scanned (out of the target shrinkctl->nr_to_scan). As
> report the number of pages owned by each GEM object as a separate item
> to the shrinker, we cannot precisely control the number of shrinker
> objects we scan on each pass; and indeed may free more than requested.
> If we fail to tell the shrinker about the number of objects we process,
> it will continue to hold a grudge against us as any objects left
> unscanned are added to the next reclaim -- and so we will keep on
> "unfairly" shrinking our own slab in comparison to other slabs.
>
> v2: fixup the misplaced addition, we want to count everything we scan
> (to match the number we reported earlier) not just the objects we
> successfully validated and freed.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Umm, full explanation and "v2" is bit misleading. Should not this be
one Fixes: if significant enough, or just a clear reference to
912d572d63b8 ("drm/i915: wire up shrinkctl->nr_scanned")?
Change itself is;
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Regards, Joonas
--
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] 24+ messages in thread
* Re: [PATCH 6/9] drm/i915: Wire up shrinkctl->nr_scanned
2017-10-16 10:46 ` Joonas Lahtinen
@ 2017-10-16 10:51 ` Chris Wilson
2017-10-16 12:57 ` Joonas Lahtinen
0 siblings, 1 reply; 24+ messages in thread
From: Chris Wilson @ 2017-10-16 10:51 UTC (permalink / raw)
To: Joonas Lahtinen, intel-gfx
Quoting Joonas Lahtinen (2017-10-16 11:46:09)
> On Fri, 2017-10-13 at 21:26 +0100, Chris Wilson wrote:
> > shrink_slab() allows us to report back the number of objects we
> > successfully scanned (out of the target shrinkctl->nr_to_scan). As
> > report the number of pages owned by each GEM object as a separate item
> > to the shrinker, we cannot precisely control the number of shrinker
> > objects we scan on each pass; and indeed may free more than requested.
> > If we fail to tell the shrinker about the number of objects we process,
> > it will continue to hold a grudge against us as any objects left
> > unscanned are added to the next reclaim -- and so we will keep on
> > "unfairly" shrinking our own slab in comparison to other slabs.
> >
> > v2: fixup the misplaced addition, we want to count everything we scan
> > (to match the number we reported earlier) not just the objects we
> > successfully validated and freed.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> Umm, full explanation and "v2" is bit misleading. Should not this be
> one Fixes: if significant enough, or just a clear reference to
> 912d572d63b8 ("drm/i915: wire up shrinkctl->nr_scanned")?
It's the patch I had sent, and thought I acked for mm; I didn't check
carefully enough. The current code is more or less a no-op, the counter
is identical to free, so we don't change the reported value to
shrinkctl. This patch is required to make the code do as the commitlog
describes.
So the code isn't broken and we don't need a Fixes tag, maybe just a
References?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 8/9] drm/i915: Only free the oldest stale object before a fresh allocation
2017-10-13 20:26 ` [PATCH 8/9] drm/i915: Only free the oldest stale object before a fresh allocation Chris Wilson
@ 2017-10-16 10:54 ` Joonas Lahtinen
0 siblings, 0 replies; 24+ messages in thread
From: Joonas Lahtinen @ 2017-10-16 10:54 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On Fri, 2017-10-13 at 21:26 +0100, Chris Wilson wrote:
> Inspired by Tvrtko's critique of the reaping of the stale contexts
> before allocating a new one, also limit the freed object reaping to the
> oldest stale object before allocating a fresh object. Unlike contexts,
> objects may have radically different sizes of backing storage, but
> similar to contexts, whilst we want to prevent starvation due to
> excessive freed lists, we also want do not want to delay fresh
"we also want do not want to", maybe just "we also do not want to"
> allocations for too long. Only freeing the oldest on the freed object
> list before each allocation is a reasonable compromise.
>
> v2: Only a single consumer of llist_del_first() is allowed (although
> multiple llist_add are still allowed in parallel). Unlike
> i915_gem_context, i915_gem_flush_free_objects() is itself not serialized
> and so we need to add our own spinlock. Otherwise KASAN eventually spots
> a use-after-free for the race on *first->next.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> #v1
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Regards, Joonas
--
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] 24+ messages in thread
* [PATCH v3] drm/i915: Move dev_priv->mm.[un]bound_list to its own lock
2017-10-13 20:26 ` [PATCH 5/9] drm/i915: Move dev_priv->mm.[un]bound_list to its own lock Chris Wilson
2017-10-13 22:36 ` [PATCH v2] " Chris Wilson
@ 2017-10-16 11:40 ` Chris Wilson
1 sibling, 0 replies; 24+ messages in thread
From: Chris Wilson @ 2017-10-16 11:40 UTC (permalink / raw)
To: intel-gfx
Remove the struct_mutex requirement around dev_priv->mm.bound_list and
dev_priv->mm.unbound_list by giving it its own spinlock. This reduces
one more requirement for struct_mutex and in the process gives us
slightly more accurate unbound_list tracking, which should improve the
shrinker - but the drawback is that we drop the retirement before
counting so i915_gem_object_is_active() may be stale and lead us to
underestimate the number of objects that may be shrunk (see commit
bed50aea61df ("drm/i915/shrinker: Flush active on objects before
counting")).
v2: Crosslink the spinlock to the lists it protects, and btw this
changes s/obj->global_link/obj->mm.link/
v3: Fix decoupling of old links in i915_gem_object_attach_phys()
v3.1: Fix the fix, only unlink if it was linked
v3.2: Use a local for to_i915(obj->base.dev)->mm.obj_lock
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 39 +++++++++++---
drivers/gpu/drm/i915/i915_drv.h | 3 ++
drivers/gpu/drm/i915/i915_gem.c | 53 +++++++++++++++----
drivers/gpu/drm/i915/i915_gem_gtt.c | 3 +-
drivers/gpu/drm/i915/i915_gem_object.h | 7 ++-
drivers/gpu/drm/i915/i915_gem_shrinker.c | 64 +++++++++--------------
drivers/gpu/drm/i915/i915_gem_stolen.c | 5 +-
drivers/gpu/drm/i915/i915_vma.c | 16 ++++--
drivers/gpu/drm/i915/selftests/i915_gem_context.c | 2 +-
drivers/gpu/drm/i915/selftests/i915_gem_evict.c | 8 ++-
10 files changed, 132 insertions(+), 68 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 7962fe6a5150..d82631afb3bf 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -271,7 +271,9 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
goto out;
total_obj_size = total_gtt_size = count = 0;
- list_for_each_entry(obj, &dev_priv->mm.bound_list, global_link) {
+
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_for_each_entry(obj, &dev_priv->mm.bound_list, mm.link) {
if (count == total)
break;
@@ -283,7 +285,7 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
total_gtt_size += i915_gem_obj_total_ggtt_size(obj);
}
- list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_link) {
+ list_for_each_entry(obj, &dev_priv->mm.unbound_list, mm.link) {
if (count == total)
break;
@@ -293,6 +295,7 @@ static int i915_gem_stolen_list_info(struct seq_file *m, void *data)
objects[count++] = obj;
total_obj_size += obj->base.size;
}
+ spin_unlock(&dev_priv->mm.obj_lock);
sort(objects, count, sizeof(*objects), obj_rank_by_stolen, NULL);
@@ -454,7 +457,9 @@ static int i915_gem_object_info(struct seq_file *m, void *data)
mapped_size = mapped_count = 0;
purgeable_size = purgeable_count = 0;
huge_size = huge_count = 0;
- list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_link) {
+
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_for_each_entry(obj, &dev_priv->mm.unbound_list, mm.link) {
size += obj->base.size;
++count;
@@ -477,7 +482,7 @@ static int i915_gem_object_info(struct seq_file *m, void *data)
seq_printf(m, "%u unbound objects, %llu bytes\n", count, size);
size = count = dpy_size = dpy_count = 0;
- list_for_each_entry(obj, &dev_priv->mm.bound_list, global_link) {
+ list_for_each_entry(obj, &dev_priv->mm.bound_list, mm.link) {
size += obj->base.size;
++count;
@@ -502,6 +507,8 @@ static int i915_gem_object_info(struct seq_file *m, void *data)
page_sizes |= obj->mm.page_sizes.sg;
}
}
+ spin_unlock(&dev_priv->mm.obj_lock);
+
seq_printf(m, "%u bound objects, %llu bytes\n",
count, size);
seq_printf(m, "%u purgeable objects, %llu bytes\n",
@@ -568,28 +575,46 @@ static int i915_gem_gtt_info(struct seq_file *m, void *data)
struct drm_info_node *node = m->private;
struct drm_i915_private *dev_priv = node_to_i915(node);
struct drm_device *dev = &dev_priv->drm;
+ struct drm_i915_gem_object **objects;
struct drm_i915_gem_object *obj;
u64 total_obj_size, total_gtt_size;
+ unsigned long nobject, n;
int count, ret;
+ nobject = READ_ONCE(dev_priv->mm.object_count);
+ objects = kvmalloc_array(nobject, sizeof(*objects), GFP_KERNEL);
+ if (!objects)
+ return -ENOMEM;
+
ret = mutex_lock_interruptible(&dev->struct_mutex);
if (ret)
return ret;
- total_obj_size = total_gtt_size = count = 0;
- list_for_each_entry(obj, &dev_priv->mm.bound_list, global_link) {
+ count = 0;
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_for_each_entry(obj, &dev_priv->mm.bound_list, mm.link) {
+ objects[count++] = obj;
+ if (count == nobject)
+ break;
+ }
+ spin_unlock(&dev_priv->mm.obj_lock);
+
+ total_obj_size = total_gtt_size = 0;
+ for (n = 0; n < count; n++) {
+ obj = objects[n];
+
seq_puts(m, " ");
describe_obj(m, obj);
seq_putc(m, '\n');
total_obj_size += obj->base.size;
total_gtt_size += i915_gem_obj_total_ggtt_size(obj);
- count++;
}
mutex_unlock(&dev->struct_mutex);
seq_printf(m, "Total %d objects, %llu bytes, %llu GTT size\n",
count, total_obj_size, total_gtt_size);
+ kvfree(objects);
return 0;
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 48be1d3c1d33..c51c6020c7a9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1490,6 +1490,9 @@ struct i915_gem_mm {
* always the inner lock when overlapping with struct_mutex. */
struct mutex stolen_lock;
+ /* Protects bound_list/unbound_list and #drm_i915_gem_object.mm.link */
+ spinlock_t obj_lock;
+
/** List of all objects in gtt_space. Used to restore gtt
* mappings on resume */
struct list_head bound_list;
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index a7d180d4ad0c..84b021f23679 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1537,6 +1537,8 @@ static void i915_gem_object_bump_inactive_ggtt(struct drm_i915_gem_object *obj)
struct list_head *list;
struct i915_vma *vma;
+ GEM_BUG_ON(!i915_gem_object_has_pinned_pages(obj));
+
list_for_each_entry(vma, &obj->vma_list, obj_link) {
if (!i915_vma_is_ggtt(vma))
break;
@@ -1551,8 +1553,10 @@ static void i915_gem_object_bump_inactive_ggtt(struct drm_i915_gem_object *obj)
}
i915 = to_i915(obj->base.dev);
+ spin_lock(&i915->mm.obj_lock);
list = obj->bind_count ? &i915->mm.bound_list : &i915->mm.unbound_list;
- list_move_tail(&obj->global_link, list);
+ list_move_tail(&obj->mm.link, list);
+ spin_unlock(&i915->mm.obj_lock);
}
/**
@@ -2253,6 +2257,7 @@ static void __i915_gem_object_reset_page_iter(struct drm_i915_gem_object *obj)
void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj,
enum i915_mm_subclass subclass)
{
+ struct drm_i915_private *i915 = to_i915(obj->base.dev);
struct sg_table *pages;
if (i915_gem_object_has_pinned_pages(obj))
@@ -2273,6 +2278,10 @@ void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj,
pages = fetch_and_zero(&obj->mm.pages);
GEM_BUG_ON(!pages);
+ spin_lock(&i915->mm.obj_lock);
+ list_del(&obj->mm.link);
+ spin_unlock(&i915->mm.obj_lock);
+
if (obj->mm.mapping) {
void *ptr;
@@ -2507,7 +2516,7 @@ void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj,
obj->mm.pages = pages;
if (i915_gem_object_is_tiled(obj) &&
- to_i915(obj->base.dev)->quirks & QUIRK_PIN_SWIZZLED_PAGES) {
+ i915->quirks & QUIRK_PIN_SWIZZLED_PAGES) {
GEM_BUG_ON(obj->mm.quirked);
__i915_gem_object_pin_pages(obj);
obj->mm.quirked = true;
@@ -2529,8 +2538,11 @@ void __i915_gem_object_set_pages(struct drm_i915_gem_object *obj,
if (obj->mm.page_sizes.phys & ~0u << i)
obj->mm.page_sizes.sg |= BIT(i);
}
-
GEM_BUG_ON(!HAS_PAGE_SIZES(i915, obj->mm.page_sizes.sg));
+
+ spin_lock(&i915->mm.obj_lock);
+ list_add(&obj->mm.link, &i915->mm.unbound_list);
+ spin_unlock(&i915->mm.obj_lock);
}
static int ____i915_gem_object_get_pages(struct drm_i915_gem_object *obj)
@@ -4324,7 +4336,6 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj,
{
mutex_init(&obj->mm.lock);
- INIT_LIST_HEAD(&obj->global_link);
INIT_LIST_HEAD(&obj->vma_list);
INIT_LIST_HEAD(&obj->lut_list);
INIT_LIST_HEAD(&obj->batch_pool_link);
@@ -4496,7 +4507,18 @@ static void __i915_gem_free_objects(struct drm_i915_private *i915,
GEM_BUG_ON(!list_empty(&obj->vma_list));
GEM_BUG_ON(!RB_EMPTY_ROOT(&obj->vma_tree));
- list_del(&obj->global_link);
+ /* This serializes freeing with the shrinker. Since the free
+ * is delayed, first by RCU then by the workqueue, we want the
+ * shrinker to be able to free pages of unreferenced objects,
+ * or else we may oom whilst there are plenty of deferred
+ * freed objects.
+ */
+ if (i915_gem_object_has_pages(obj)) {
+ spin_lock(&i915->mm.obj_lock);
+ list_del_init(&obj->mm.link);
+ spin_unlock(&i915->mm.obj_lock);
+ }
+
}
intel_runtime_pm_put(i915);
mutex_unlock(&i915->drm.struct_mutex);
@@ -5041,11 +5063,14 @@ i915_gem_load_init(struct drm_i915_private *dev_priv)
goto err_priorities;
INIT_WORK(&dev_priv->mm.free_work, __i915_gem_free_work);
+
+ spin_lock_init(&dev_priv->mm.obj_lock);
init_llist_head(&dev_priv->mm.free_list);
INIT_LIST_HEAD(&dev_priv->mm.unbound_list);
INIT_LIST_HEAD(&dev_priv->mm.bound_list);
INIT_LIST_HEAD(&dev_priv->mm.fence_list);
INIT_LIST_HEAD(&dev_priv->mm.userfault_list);
+
INIT_DELAYED_WORK(&dev_priv->gt.retire_work,
i915_gem_retire_work_handler);
INIT_DELAYED_WORK(&dev_priv->gt.idle_work,
@@ -5139,12 +5164,12 @@ int i915_gem_freeze_late(struct drm_i915_private *dev_priv)
i915_gem_shrink(dev_priv, -1UL, NULL, I915_SHRINK_UNBOUND);
i915_gem_drain_freed_objects(dev_priv);
- mutex_lock(&dev_priv->drm.struct_mutex);
+ spin_lock(&dev_priv->mm.obj_lock);
for (p = phases; *p; p++) {
- list_for_each_entry(obj, *p, global_link)
+ list_for_each_entry(obj, *p, mm.link)
__start_cpu_write(obj);
}
- mutex_unlock(&dev_priv->drm.struct_mutex);
+ spin_unlock(&dev_priv->mm.obj_lock);
return 0;
}
@@ -5463,7 +5488,17 @@ int i915_gem_object_attach_phys(struct drm_i915_gem_object *obj, int align)
goto err_unlock;
}
- pages = obj->mm.pages;
+ pages = fetch_and_zero(&obj->mm.pages);
+ if (pages) {
+ struct drm_i915_private *i915 = to_i915(obj->base.dev);
+
+ __i915_gem_object_reset_page_iter(obj);
+
+ spin_lock(&i915->mm.obj_lock);
+ list_del(&obj->mm.link);
+ spin_unlock(&i915->mm.obj_lock);
+ }
+
obj->ops = &i915_gem_phys_ops;
err = ____i915_gem_object_get_pages(obj);
diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index daba55a4fce2..527a2d2d6281 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -3594,8 +3594,7 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
ggtt->base.closed = true; /* skip rewriting PTE on VMA unbind */
/* clflush objects bound into the GGTT and rebind them. */
- list_for_each_entry_safe(obj, on,
- &dev_priv->mm.bound_list, global_link) {
+ list_for_each_entry_safe(obj, on, &dev_priv->mm.bound_list, mm.link) {
bool ggtt_bound = false;
struct i915_vma *vma;
diff --git a/drivers/gpu/drm/i915/i915_gem_object.h b/drivers/gpu/drm/i915/i915_gem_object.h
index 72aa02f3a0f8..63ce38c1cce9 100644
--- a/drivers/gpu/drm/i915/i915_gem_object.h
+++ b/drivers/gpu/drm/i915/i915_gem_object.h
@@ -114,7 +114,6 @@ struct drm_i915_gem_object {
/** Stolen memory for this object, instead of being backed by shmem. */
struct drm_mm_node *stolen;
- struct list_head global_link;
union {
struct rcu_head rcu;
struct llist_node freed;
@@ -208,6 +207,12 @@ struct drm_i915_gem_object {
struct mutex lock; /* protects this cache */
} get_page;
+ /**
+ * Element within i915->mm.unbound_list or i915->mm.bound_list,
+ * locked by i915->mm.obj_lock.
+ */
+ struct list_head link;
+
/**
* Advice: are the backing pages purgeable?
*/
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index 575a6b735f39..065d026b5092 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -78,9 +78,6 @@ static bool swap_available(void)
static bool can_release_pages(struct drm_i915_gem_object *obj)
{
- if (!i915_gem_object_has_pages(obj))
- return false;
-
/* Consider only shrinkable ojects. */
if (!i915_gem_object_is_shrinkable(obj))
return false;
@@ -102,7 +99,7 @@ static bool can_release_pages(struct drm_i915_gem_object *obj)
* To simplify the scan, and to avoid walking the list of vma under the
* object, we just check the count of its permanently pinned.
*/
- if (obj->pin_global)
+ if (READ_ONCE(obj->pin_global))
return false;
/* We can only return physical pages to the system if we can either
@@ -204,15 +201,20 @@ i915_gem_shrink(struct drm_i915_private *dev_priv,
continue;
INIT_LIST_HEAD(&still_in_list);
+
+ /*
+ * We serialize our access to unreferenced objects through
+ * the use of the struct_mutex. While the objects are not
+ * yet freed (due to RCU then a workqueue) we still want
+ * to be able to shrink their pages, so they remain on
+ * the unbound/bound list until actually freed.
+ */
+ spin_lock(&dev_priv->mm.obj_lock);
while (count < target &&
(obj = list_first_entry_or_null(phase->list,
typeof(*obj),
- global_link))) {
- list_move_tail(&obj->global_link, &still_in_list);
- if (!obj->mm.pages) {
- list_del_init(&obj->global_link);
- continue;
- }
+ mm.link))) {
+ list_move_tail(&obj->mm.link, &still_in_list);
if (flags & I915_SHRINK_PURGEABLE &&
obj->mm.madv != I915_MADV_DONTNEED)
@@ -230,20 +232,24 @@ i915_gem_shrink(struct drm_i915_private *dev_priv,
if (!can_release_pages(obj))
continue;
+ spin_unlock(&dev_priv->mm.obj_lock);
+
if (unsafe_drop_pages(obj)) {
/* May arrive from get_pages on another bo */
mutex_lock_nested(&obj->mm.lock,
I915_MM_SHRINKER);
if (!i915_gem_object_has_pages(obj)) {
__i915_gem_object_invalidate(obj);
- list_del_init(&obj->global_link);
count += obj->base.size >> PAGE_SHIFT;
}
mutex_unlock(&obj->mm.lock);
scanned += obj->base.size >> PAGE_SHIFT;
}
+
+ spin_lock(&dev_priv->mm.obj_lock);
}
list_splice_tail(&still_in_list, phase->list);
+ spin_unlock(&dev_priv->mm.obj_lock);
}
if (flags & I915_SHRINK_BOUND)
@@ -292,25 +298,17 @@ i915_gem_shrinker_count(struct shrinker *shrinker, struct shrink_control *sc)
struct drm_i915_private *dev_priv =
container_of(shrinker, struct drm_i915_private, mm.shrinker);
struct drm_i915_gem_object *obj;
- unsigned long count;
- bool unlock;
-
- if (!shrinker_lock(dev_priv, &unlock))
- return 0;
-
- i915_gem_retire_requests(dev_priv);
+ unsigned long count = 0;
- count = 0;
- list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_link)
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_for_each_entry(obj, &dev_priv->mm.unbound_list, mm.link)
if (can_release_pages(obj))
count += obj->base.size >> PAGE_SHIFT;
- list_for_each_entry(obj, &dev_priv->mm.bound_list, global_link) {
+ list_for_each_entry(obj, &dev_priv->mm.bound_list, mm.link)
if (!i915_gem_object_is_active(obj) && can_release_pages(obj))
count += obj->base.size >> PAGE_SHIFT;
- }
-
- shrinker_unlock(dev_priv, unlock);
+ spin_unlock(&dev_priv->mm.obj_lock);
return count;
}
@@ -387,10 +385,6 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
container_of(nb, struct drm_i915_private, mm.oom_notifier);
struct drm_i915_gem_object *obj;
unsigned long unevictable, bound, unbound, freed_pages;
- bool unlock;
-
- if (!shrinker_lock_uninterruptible(dev_priv, &unlock, 5000))
- return NOTIFY_DONE;
freed_pages = i915_gem_shrink_all(dev_priv);
@@ -399,26 +393,20 @@ i915_gem_shrinker_oom(struct notifier_block *nb, unsigned long event, void *ptr)
* being pointed to by hardware.
*/
unbound = bound = unevictable = 0;
- list_for_each_entry(obj, &dev_priv->mm.unbound_list, global_link) {
- if (!i915_gem_object_has_pages(obj))
- continue;
-
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_for_each_entry(obj, &dev_priv->mm.unbound_list, mm.link) {
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_link) {
- if (!i915_gem_object_has_pages(obj))
- continue;
-
+ list_for_each_entry(obj, &dev_priv->mm.bound_list, mm.link) {
if (!can_release_pages(obj))
unevictable += obj->base.size >> PAGE_SHIFT;
else
bound += obj->base.size >> PAGE_SHIFT;
}
-
- shrinker_unlock(dev_priv, unlock);
+ spin_unlock(&dev_priv->mm.obj_lock);
if (freed_pages || unbound || bound)
pr_info("Purging GPU memory, %lu pages freed, "
diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c b/drivers/gpu/drm/i915/i915_gem_stolen.c
index 54fd4cfa9d07..03e7abc7e043 100644
--- a/drivers/gpu/drm/i915/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/i915_gem_stolen.c
@@ -724,8 +724,11 @@ i915_gem_object_create_stolen_for_preallocated(struct drm_i915_private *dev_priv
vma->flags |= I915_VMA_GLOBAL_BIND;
__i915_vma_set_map_and_fenceable(vma);
list_move_tail(&vma->vm_link, &ggtt->base.inactive_list);
- list_move_tail(&obj->global_link, &dev_priv->mm.bound_list);
+
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_move_tail(&obj->mm.link, &dev_priv->mm.bound_list);
obj->bind_count++;
+ spin_unlock(&dev_priv->mm.obj_lock);
return obj;
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 4dce2e0197d9..fc77e8191eb5 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -58,8 +58,10 @@ i915_vma_retire(struct i915_gem_active *active,
* so that we don't steal from recently used but inactive objects
* (unless we are forced to ofc!)
*/
+ spin_lock(&rq->i915->mm.obj_lock);
if (obj->bind_count)
- list_move_tail(&obj->global_link, &rq->i915->mm.bound_list);
+ list_move_tail(&obj->mm.link, &rq->i915->mm.bound_list);
+ spin_unlock(&rq->i915->mm.obj_lock);
obj->mm.dirty = true; /* be paranoid */
@@ -563,9 +565,13 @@ i915_vma_insert(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
GEM_BUG_ON(!i915_gem_valid_gtt_space(vma, obj->cache_level));
- list_move_tail(&obj->global_link, &dev_priv->mm.bound_list);
list_move_tail(&vma->vm_link, &vma->vm->inactive_list);
+
+ spin_lock(&dev_priv->mm.obj_lock);
+ list_move_tail(&obj->mm.link, &dev_priv->mm.bound_list);
obj->bind_count++;
+ spin_unlock(&dev_priv->mm.obj_lock);
+
GEM_BUG_ON(atomic_read(&obj->mm.pages_pin_count) < obj->bind_count);
return 0;
@@ -580,6 +586,7 @@ i915_vma_insert(struct i915_vma *vma, u64 size, u64 alignment, u64 flags)
static void
i915_vma_remove(struct i915_vma *vma)
{
+ struct drm_i915_private *i915 = vma->vm->i915;
struct drm_i915_gem_object *obj = vma->obj;
GEM_BUG_ON(!drm_mm_node_allocated(&vma->node));
@@ -593,9 +600,10 @@ i915_vma_remove(struct i915_vma *vma)
/* Since the unbound list is global, only move to that list if
* no more VMAs exist.
*/
+ spin_lock(&i915->mm.obj_lock);
if (--obj->bind_count == 0)
- list_move_tail(&obj->global_link,
- &to_i915(obj->base.dev)->mm.unbound_list);
+ list_move_tail(&obj->mm.link, &i915->mm.unbound_list);
+ spin_unlock(&i915->mm.obj_lock);
/* And finally now the object is completely decoupled from this vma,
* we can drop its hold on the backing storage and allow it to be
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
index fb0a58fc8348..def5052862ae 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
@@ -417,7 +417,7 @@ static int fake_aliasing_ppgtt_enable(struct drm_i915_private *i915)
if (err)
return err;
- list_for_each_entry(obj, &i915->mm.bound_list, global_link) {
+ list_for_each_entry(obj, &i915->mm.bound_list, mm.link) {
struct i915_vma *vma;
vma = i915_vma_instance(obj, &i915->ggtt.base, NULL);
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
index 473aa9631145..f463105ff48d 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
@@ -50,7 +50,7 @@ static int populate_ggtt(struct drm_i915_private *i915)
if (!list_empty(&i915->mm.unbound_list)) {
size = 0;
- list_for_each_entry(obj, &i915->mm.unbound_list, global_link)
+ list_for_each_entry(obj, &i915->mm.unbound_list, mm.link)
size++;
pr_err("Found %lld objects unbound!\n", size);
@@ -77,10 +77,10 @@ static void cleanup_objects(struct drm_i915_private *i915)
{
struct drm_i915_gem_object *obj, *on;
- list_for_each_entry_safe(obj, on, &i915->mm.unbound_list, global_link)
+ list_for_each_entry_safe(obj, on, &i915->mm.unbound_list, mm.link)
i915_gem_object_put(obj);
- list_for_each_entry_safe(obj, on, &i915->mm.bound_list, global_link)
+ list_for_each_entry_safe(obj, on, &i915->mm.bound_list, mm.link)
i915_gem_object_put(obj);
mutex_unlock(&i915->drm.struct_mutex);
@@ -152,8 +152,6 @@ static int igt_overcommit(void *arg)
goto cleanup;
}
- list_move(&obj->global_link, &i915->mm.unbound_list);
-
vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 0);
if (!IS_ERR(vma) || PTR_ERR(vma) != -ENOSPC) {
pr_err("Failed to evict+insert, i915_gem_object_ggtt_pin returned err=%d\n", (int)PTR_ERR(vma));
--
2.15.0.rc0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 24+ messages in thread
* ✓ Fi.CI.BAT: success for series starting with [1/9] drm/i915: Refactor testing obj->mm.pages (rev4)
2017-10-13 20:26 [PATCH 1/9] drm/i915: Refactor testing obj->mm.pages Chris Wilson
` (10 preceding siblings ...)
2017-10-13 23:51 ` ✗ Fi.CI.BAT: failure for series starting with [1/9] drm/i915: Refactor testing obj->mm.pages (rev3) Patchwork
@ 2017-10-16 12:10 ` Patchwork
2017-10-16 20:12 ` ✗ Fi.CI.IGT: failure " Patchwork
12 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2017-10-16 12:10 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/9] drm/i915: Refactor testing obj->mm.pages (rev4)
URL : https://patchwork.freedesktop.org/series/31959/
State : success
== Summary ==
Series 31959v4 series starting with [1/9] drm/i915: Refactor testing obj->mm.pages
https://patchwork.freedesktop.org/api/1.0/series/31959/revisions/4/mbox/
Test gem_exec_suspend:
Subgroup basic-s3:
pass -> DMESG-WARN (fi-cfl-s) fdo#103186
Test kms_cursor_legacy:
Subgroup basic-busy-flip-before-cursor-legacy:
fail -> PASS (fi-gdg-551) fdo#102618
fdo#103186 https://bugs.freedesktop.org/show_bug.cgi?id=103186
fdo#102618 https://bugs.freedesktop.org/show_bug.cgi?id=102618
fi-bdw-5557u total:289 pass:268 dwarn:0 dfail:0 fail:0 skip:21 time:452s
fi-bdw-gvtdvm total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:471s
fi-blb-e6850 total:289 pass:223 dwarn:1 dfail:0 fail:0 skip:65 time:387s
fi-bsw-n3050 total:289 pass:243 dwarn:0 dfail:0 fail:0 skip:46 time:561s
fi-bwr-2160 total:289 pass:183 dwarn:0 dfail:0 fail:0 skip:106 time:284s
fi-bxt-dsi total:289 pass:259 dwarn:0 dfail:0 fail:0 skip:30 time:521s
fi-bxt-j4205 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:518s
fi-byt-j1900 total:289 pass:253 dwarn:1 dfail:0 fail:0 skip:35 time:538s
fi-byt-n2820 total:289 pass:249 dwarn:1 dfail:0 fail:0 skip:39 time:524s
fi-cfl-s total:289 pass:253 dwarn:4 dfail:0 fail:0 skip:32 time:570s
fi-elk-e7500 total:289 pass:229 dwarn:0 dfail:0 fail:0 skip:60 time:439s
fi-gdg-551 total:289 pass:178 dwarn:1 dfail:0 fail:1 skip:109 time:272s
fi-glk-1 total:289 pass:261 dwarn:0 dfail:0 fail:0 skip:28 time:603s
fi-hsw-4770r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:438s
fi-ilk-650 total:289 pass:228 dwarn:0 dfail:0 fail:0 skip:61 time:458s
fi-ivb-3520m total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:503s
fi-ivb-3770 total:289 pass:260 dwarn:0 dfail:0 fail:0 skip:29 time:476s
fi-kbl-7500u total:289 pass:264 dwarn:1 dfail:0 fail:0 skip:24 time:505s
fi-kbl-7567u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:487s
fi-kbl-r total:289 pass:262 dwarn:0 dfail:0 fail:0 skip:27 time:592s
fi-pnv-d510 total:289 pass:222 dwarn:1 dfail:0 fail:0 skip:66 time:656s
fi-skl-6260u total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:467s
fi-skl-6700hq total:289 pass:263 dwarn:0 dfail:0 fail:0 skip:26 time:660s
fi-skl-6700k total:289 pass:265 dwarn:0 dfail:0 fail:0 skip:24 time:534s
fi-skl-6770hq total:289 pass:269 dwarn:0 dfail:0 fail:0 skip:20 time:511s
fi-skl-gvtdvm total:289 pass:266 dwarn:0 dfail:0 fail:0 skip:23 time:473s
fi-snb-2520m total:289 pass:250 dwarn:0 dfail:0 fail:0 skip:39 time:585s
fi-snb-2600 total:289 pass:249 dwarn:0 dfail:0 fail:0 skip:40 time:429s
6972ebb8e378659b83aa5afaca67dd1220cafd72 drm-tip: 2017y-10m-16d-11h-00m-11s UTC integration manifest
2f364bd52333 drm/i915: Trim struct_mutex hold duration for i915_gem_free_objects
d6c24668318e drm/i915: Only free the oldest stale object before a fresh allocation
590f0affe536 drm/i915: Set our shrinker->batch to 4096 (~16MiB)
adb2ebc6a860 drm/i915: Wire up shrinkctl->nr_scanned
e2120cf9c95b drm/i915: Move dev_priv->mm.[un]bound_list to its own lock
95934ab19a67 drm/i915: Remove walk over obj->vma_list for the shrinker
5bf28485bf09 drm/i915: Drop debugfs/i915_gem_pin_display
16ebd1932909 drm/i915: Rename obj->pin_display to obj->pin_global
613983baf4dd drm/i915: Refactor testing obj->mm.pages
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6049/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 6/9] drm/i915: Wire up shrinkctl->nr_scanned
2017-10-16 10:51 ` Chris Wilson
@ 2017-10-16 12:57 ` Joonas Lahtinen
0 siblings, 0 replies; 24+ messages in thread
From: Joonas Lahtinen @ 2017-10-16 12:57 UTC (permalink / raw)
To: Chris Wilson, intel-gfx
On Mon, 2017-10-16 at 11:51 +0100, Chris Wilson wrote:
> Quoting Joonas Lahtinen (2017-10-16 11:46:09)
> > On Fri, 2017-10-13 at 21:26 +0100, Chris Wilson wrote:
> > > shrink_slab() allows us to report back the number of objects we
> > > successfully scanned (out of the target shrinkctl->nr_to_scan). As
> > > report the number of pages owned by each GEM object as a separate item
> > > to the shrinker, we cannot precisely control the number of shrinker
> > > objects we scan on each pass; and indeed may free more than requested.
> > > If we fail to tell the shrinker about the number of objects we process,
> > > it will continue to hold a grudge against us as any objects left
> > > unscanned are added to the next reclaim -- and so we will keep on
> > > "unfairly" shrinking our own slab in comparison to other slabs.
> > >
> > > v2: fixup the misplaced addition, we want to count everything we scan
> > > (to match the number we reported earlier) not just the objects we
> > > successfully validated and freed.
> > >
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >
> > Umm, full explanation and "v2" is bit misleading. Should not this be
> > one Fixes: if significant enough, or just a clear reference to
> > 912d572d63b8 ("drm/i915: wire up shrinkctl->nr_scanned")?
>
> It's the patch I had sent, and thought I acked for mm; I didn't check
> carefully enough. The current code is more or less a no-op, the counter
> is identical to free, so we don't change the reported value to
> shrinkctl. This patch is required to make the code do as the commitlog
> describes.
>
> So the code isn't broken and we don't need a Fixes tag, maybe just a
> References?
"References:" should be good.
Regards, Joonas
--
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] 24+ messages in thread
* ✗ Fi.CI.IGT: failure for series starting with [1/9] drm/i915: Refactor testing obj->mm.pages (rev4)
2017-10-13 20:26 [PATCH 1/9] drm/i915: Refactor testing obj->mm.pages Chris Wilson
` (11 preceding siblings ...)
2017-10-16 12:10 ` ✓ Fi.CI.BAT: success for series starting with [1/9] drm/i915: Refactor testing obj->mm.pages (rev4) Patchwork
@ 2017-10-16 20:12 ` Patchwork
12 siblings, 0 replies; 24+ messages in thread
From: Patchwork @ 2017-10-16 20:12 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: series starting with [1/9] drm/i915: Refactor testing obj->mm.pages (rev4)
URL : https://patchwork.freedesktop.org/series/31959/
State : failure
== Summary ==
Test kms_flip:
Subgroup modeset-vs-vblank-race:
fail -> PASS (shard-hsw) fdo#102919
Subgroup modeset-vs-vblank-race-interruptible:
pass -> DMESG-FAIL (shard-hsw)
Test gem_userptr_blits:
Subgroup sync-unmap-cycles:
dmesg-warn -> PASS (shard-hsw) fdo#103251
Test kms_frontbuffer_tracking:
Subgroup fbc-1p-primscrn-spr-indfb-move:
skip -> PASS (shard-hsw)
Test gem_madvise:
Subgroup dontneed-before-pwrite:
pass -> FAIL (shard-hsw)
Test prime_busy:
Subgroup wait-after-bsd1:
skip -> INCOMPLETE (shard-hsw)
Test gem_flink_race:
Subgroup flink_close:
fail -> PASS (shard-hsw) fdo#102655
fdo#102919 https://bugs.freedesktop.org/show_bug.cgi?id=102919
fdo#103251 https://bugs.freedesktop.org/show_bug.cgi?id=103251
fdo#102655 https://bugs.freedesktop.org/show_bug.cgi?id=102655
shard-hsw total:2553 pass:1409 dwarn:0 dfail:1 fail:8 skip:1084 time:9450s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6049/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2017-10-16 20:12 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-13 20:26 [PATCH 1/9] drm/i915: Refactor testing obj->mm.pages Chris Wilson
2017-10-13 20:26 ` [PATCH 2/9] drm/i915: Rename obj->pin_display to obj->pin_global Chris Wilson
2017-10-16 10:40 ` Joonas Lahtinen
2017-10-13 20:26 ` [PATCH 3/9] drm/i915: Drop debugfs/i915_gem_pin_display Chris Wilson
2017-10-16 10:41 ` Joonas Lahtinen
2017-10-13 20:26 ` [PATCH 4/9] drm/i915: Remove walk over obj->vma_list for the shrinker Chris Wilson
2017-10-13 20:26 ` [PATCH 5/9] drm/i915: Move dev_priv->mm.[un]bound_list to its own lock Chris Wilson
2017-10-13 22:36 ` [PATCH v2] " Chris Wilson
2017-10-13 23:23 ` [PATCH v3] " Chris Wilson
2017-10-16 11:40 ` Chris Wilson
2017-10-13 20:26 ` [PATCH 6/9] drm/i915: Wire up shrinkctl->nr_scanned Chris Wilson
2017-10-16 10:46 ` Joonas Lahtinen
2017-10-16 10:51 ` Chris Wilson
2017-10-16 12:57 ` Joonas Lahtinen
2017-10-13 20:26 ` [PATCH 7/9] drm/i915: Set our shrinker->batch to 4096 (~16MiB) Chris Wilson
2017-10-13 20:26 ` [PATCH 8/9] drm/i915: Only free the oldest stale object before a fresh allocation Chris Wilson
2017-10-16 10:54 ` Joonas Lahtinen
2017-10-13 20:26 ` [PATCH 9/9] drm/i915: Trim struct_mutex hold duration for i915_gem_free_objects Chris Wilson
2017-10-13 21:46 ` ✗ Fi.CI.BAT: warning for series starting with [1/9] drm/i915: Refactor testing obj->mm.pages Patchwork
2017-10-13 22:32 ` Chris Wilson
2017-10-13 23:12 ` ✗ Fi.CI.BAT: failure for series starting with [1/9] drm/i915: Refactor testing obj->mm.pages (rev2) Patchwork
2017-10-13 23:51 ` ✗ Fi.CI.BAT: failure for series starting with [1/9] drm/i915: Refactor testing obj->mm.pages (rev3) Patchwork
2017-10-16 12:10 ` ✓ Fi.CI.BAT: success for series starting with [1/9] drm/i915: Refactor testing obj->mm.pages (rev4) Patchwork
2017-10-16 20:12 ` ✗ Fi.CI.IGT: failure " 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.