[PATCH v2 1/5] drm/i915: Encourage our shrinker more when our shmemfs allocations fails

[PATCH v2 1/5] drm/i915: Encourage our shrinker more when our shmemfs allocations fails

[Chris Wilson]

Commit 24f8e00a8a2e ("drm/i915: Prefer to report ENOMEM rather than
incur the oom for gfx allocations") made the bold decision to try and
avoid the oomkiller by reporting -ENOMEM to userspace if our allocation
failed after attempting to free enough buffer objects. In short, it
appears we were giving up too easily (even before we start wondering if
one pass of reclaim is as strong as we would like). Part of the problem
is that if we only shrink just enough pages for our expected allocation,
the likelihood of those pages becoming available to us is less than 100%
To counter-act that we ask for twice the number of pages to be made
available. Furthermore, we allow the shrinker to pull pages from the
active list in later passes.

v2: Be a little more cautious in paging out gfx buffers, and leave that
to a more balanced approach from shrink_slab(). Important when combined
with "drm/i915: Start writeback from the shrinker" as anything shrunk is
immediately swapped out and so should be more conservative.

Fixes: 24f8e00a8a2e ("drm/i915: Prefer to report ENOMEM rather than incur the oom for gfx allocations")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 50 ++++++++++++++++++++++++-----------------
 1 file changed, 29 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index aff449807399..ca61a0be1458 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2337,8 +2337,8 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
 	struct page *page;
 	unsigned long last_pfn = 0;	/* suppress gcc warning */
 	unsigned int max_segment;
+	gfp_t noreclaim;
 	int ret;
-	gfp_t gfp;
 
 	/* Assert that the object is not currently in any GPU domain. As it
 	 * wasn't in the GTT, there shouldn't be any way it could have been in
@@ -2367,22 +2367,31 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
 	 * Fail silently without starting the shrinker
 	 */
 	mapping = obj->base.filp->f_mapping;
-	gfp = mapping_gfp_constraint(mapping, ~(__GFP_IO | __GFP_RECLAIM));
-	gfp |= __GFP_NORETRY | __GFP_NOWARN;
+	noreclaim = mapping_gfp_constraint(mapping,
+					   ~(__GFP_IO | __GFP_RECLAIM));
+	noreclaim |= __GFP_NORETRY | __GFP_NOWARN;
+
 	sg = st->sgl;
 	st->nents = 0;
 	for (i = 0; i < page_count; i++) {
-		page = shmem_read_mapping_page_gfp(mapping, i, gfp);
-		if (unlikely(IS_ERR(page))) {
-			i915_gem_shrink(dev_priv,
-					page_count,
-					I915_SHRINK_BOUND |
-					I915_SHRINK_UNBOUND |
-					I915_SHRINK_PURGEABLE);
+		const unsigned int shrink[] = {
+			I915_SHRINK_BOUND | I915_SHRINK_UNBOUND | I915_SHRINK_PURGEABLE,
+			0,
+		}, *s = shrink;
+		gfp_t gfp = noreclaim;
+
+		do {
 			page = shmem_read_mapping_page_gfp(mapping, i, gfp);
-		}
-		if (unlikely(IS_ERR(page))) {
-			gfp_t reclaim;
+			if (likely(!IS_ERR(page)))
+				break;
+
+			if (!*s) {
+				ret = PTR_ERR(page);
+				goto err_sg;
+			}
+
+			i915_gem_shrink(dev_priv, 2 * page_count, *s++);
+			cond_resched();
 
 			/* We've tried hard to allocate the memory by reaping
 			 * our own buffer, now let the real VM do its job and
@@ -2392,15 +2401,13 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
 			 * defer the oom here by reporting the ENOMEM back
 			 * to userspace.
 			 */
-			reclaim = mapping_gfp_mask(mapping);
-			reclaim |= __GFP_NORETRY; /* reclaim, but no oom */
-
-			page = shmem_read_mapping_page_gfp(mapping, i, reclaim);
-			if (IS_ERR(page)) {
-				ret = PTR_ERR(page);
-				goto err_sg;
+			if (!*s) {
+				/* reclaim and warn, but no oom */
+				gfp = mapping_gfp_mask(mapping);
+				gfp |= __GFP_NORETRY;
 			}
-		}
+		} while (1);
+
 		if (!i ||
 		    sg->length >= max_segment ||
 		    page_to_pfn(page) != last_pfn + 1) {
@@ -4285,6 +4292,7 @@ i915_gem_object_create(struct drm_i915_private *dev_priv, u64 size)
 
 	mapping = obj->base.filp->f_mapping;
 	mapping_set_gfp_mask(mapping, mask);
+	GEM_BUG_ON(!(mapping_gfp_mask(mapping) & __GFP_RECLAIM));
 
 	i915_gem_object_init(obj, &i915_gem_object_ops);
 
-- 
2.11.0

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

[PATCH v2 2/5] drm/i915: Remove __GFP_NORETRY from our buffer allocator

[Chris Wilson]

I tried __GFP_NORETRY in the belief that __GFP_RECLAIM was effective. It
struggles with handling reclaim of our dirty buffers and relies on
reclaim via kswapd. As a result, a single pass of direct reclaim is
unreliable when i915 occupies the majority of available memory, and the
only means of effectively waiting on kswapd to amke progress is by not
setting the __GFP_NORETRY flag and lopping. That leaves us with the
dilemma of invoking the oomkiller instead of propagating the allocation
failure back to userspace where it can be handled more gracefully (one
hopes).  In the future we may have __GFP_MAYFAIL to allow repeats up until
we genuinely run out of memory and the oomkiller would have been invoked.
Until then, let the oomkiller wreck havoc.

v2: Stop playing with side-effects of gfp flags and await __GFP_MAYFAIL
v3: Update comments that direct reclaim only appears to be ignoring our
dirty buffers!

Fixes: 24f8e00a8a2e ("drm/i915: Prefer to report ENOMEM rather than incur the oom for gfx allocations")
Testcase: igt/gem_tiled_swapping
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Michal Hocko <mhocko@suse.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ca61a0be1458..b22145a876c5 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2404,7 +2404,20 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
 			if (!*s) {
 				/* reclaim and warn, but no oom */
 				gfp = mapping_gfp_mask(mapping);
-				gfp |= __GFP_NORETRY;
+
+				/* Our bo are always dirty and so we require
+				 * kswapd to reclaim our pages (direct reclaim
+				 * does not effectively begin pageout of our
+				 * buffers on its own). However, direct reclaim
+				 * only waits for kswapd when under allocation
+				 * congestion. So as a result __GFP_RECLAIM is
+				 * unreliable and fails to actually reclaim our
+				 * dirty pages -- unless you try over and over
+				 * again with !__GFP_NORETRY. However, we still
+				 * want to fail this allocation rather than
+				 * trigger the out-of-memory killer and for
+				 * this we want the future __GFP_MAYFAIL.
+				 */
 			}
 		} while (1);
 
-- 
2.11.0

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

[PATCH v2 3/5] drm/i915: Only restrict noreclaim in the early shrink passes

[Chris Wilson]

In our first pass, we do not want to use reclaim at all as we want to
solely reap the i915 buffer caches (its purgeable pages). But we don't
mind it initiates IO or pulls via the FS (but it shouldn't anyway as we
say no to reclaim!). Just drop the GFP_IO constraint for simplicity.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b22145a876c5..31cbe78171a9 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2367,8 +2367,7 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
 	 * Fail silently without starting the shrinker
 	 */
 	mapping = obj->base.filp->f_mapping;
-	noreclaim = mapping_gfp_constraint(mapping,
-					   ~(__GFP_IO | __GFP_RECLAIM));
+	noreclaim = mapping_gfp_constraint(mapping, ~__GFP_RECLAIM);
 	noreclaim |= __GFP_NORETRY | __GFP_NOWARN;
 
 	sg = st->sgl;
-- 
2.11.0

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

[PATCH v2 4/5] drm/i915: Spin for struct_mutex inside shrinker

[Chris Wilson]

Having resolved whether or not we would deadlock upon a call to
mutex_lock(&dev->struct_mutex), we can then spin for the contended
struct_mutex if we are not the owner. We cannot afford to simply block
and wait for the mutex, as the owner may itself be waiting for the
allocator -- i.e. a cyclic deadlock. This should significantly improve
the chance of running the shrinker for other processes whilst the GPU is
busy.

A more balanced approach would be to optimistically spin whilst the
mutex owner was on the cpu and there was an opportunity to acquire the
mutex for ourselves quickly. However, that requires support from
kernel/locking/ and a new mutex_spin_trylock() primitive.

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

diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index 58f27369183c..1032f98add11 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -38,16 +38,21 @@
 static bool shrinker_lock(struct drm_i915_private *dev_priv, bool *unlock)
 {
 	switch (mutex_trylock_recursive(&dev_priv->drm.struct_mutex)) {
-	case MUTEX_TRYLOCK_FAILED:
-		return false;
-
-	case MUTEX_TRYLOCK_SUCCESS:
-		*unlock = true;
-		return true;
-
 	case MUTEX_TRYLOCK_RECURSIVE:
 		*unlock = false;
 		return true;
+
+	case MUTEX_TRYLOCK_FAILED:
+		do {
+			cpu_relax();
+			if (mutex_trylock(&dev_priv->drm.struct_mutex)) {
+	case MUTEX_TRYLOCK_SUCCESS:
+				*unlock = true;
+				return true;
+			}
+		} while (!need_resched());
+
+		return false;
 	}
 
 	BUG();
-- 
2.11.0

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

[PATCH v2 5/5] drm/i915: Start writeback from the shrinker

[Chris Wilson]

When we are called to relieve mempressue via the shrinker, the only way
we can make progress is either by discarding unwanted pages (those
objects that userspace has marked MADV_DONTNEED) or by reclaiming the
dirty objects via swap. As we know that is the only way to make further
progress, we can initiate the writeback as we invalidate the objects.
This means the objects we put onto the inactive anon lru list are
already marked for reclaim+writeback and so will trigger a wait upon the
writeback inside direct reclaim, greatly improving the success rate of
direct reclaim on i915 objects.

The corollary is that we may start a slow swap on opportunistic
mempressure from the likes of the compaction + migration kthreads. This
is limited by those threads only being allowed to shrink idle pages, but
also that if we reactivate the page before it is swapped out by gpu
activity, we only page the cost of repinning the page. The cost is most
felt when an object is reused after mempressure, which hopefully
excludes the latency sensitive tasks (as we are just extending the
impact of swap thrashing to them).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Michal Hocko <mhocko@suse.com>
---
 drivers/gpu/drm/i915/i915_drv.h          |  2 +-
 drivers/gpu/drm/i915/i915_gem.c          | 27 ++--------------
 drivers/gpu/drm/i915/i915_gem_shrinker.c | 55 +++++++++++++++++++++++++++++++-
 3 files changed, 57 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 602fb3324484..3ffe6504b391 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3332,7 +3332,7 @@ enum i915_mm_subclass { /* lockdep subclass for obj->mm.lock */
 
 void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj,
 				 enum i915_mm_subclass subclass);
-void __i915_gem_object_invalidate(struct drm_i915_gem_object *obj);
+void __i915_gem_object_truncate(struct drm_i915_gem_object *obj);
 
 enum i915_map_type {
 	I915_MAP_WB = 0,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 31cbe78171a9..1de94a8399a2 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2176,8 +2176,7 @@ i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
 }
 
 /* Immediately discard the backing storage */
-static void
-i915_gem_object_truncate(struct drm_i915_gem_object *obj)
+void __i915_gem_object_truncate(struct drm_i915_gem_object *obj)
 {
 	i915_gem_object_free_mmap_offset(obj);
 
@@ -2194,28 +2193,6 @@ i915_gem_object_truncate(struct drm_i915_gem_object *obj)
 	obj->mm.pages = ERR_PTR(-EFAULT);
 }
 
-/* Try to discard unwanted pages */
-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);
-
-	switch (obj->mm.madv) {
-	case I915_MADV_DONTNEED:
-		i915_gem_object_truncate(obj);
-	case __I915_MADV_PURGED:
-		return;
-	}
-
-	if (obj->base.filp == NULL)
-		return;
-
-	mapping = obj->base.filp->f_mapping,
-	invalidate_mapping_pages(mapping, 0, (loff_t)-1);
-}
-
 static void
 i915_gem_object_put_pages_gtt(struct drm_i915_gem_object *obj,
 			      struct sg_table *pages)
@@ -4212,7 +4189,7 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
 
 	/* if the object is no longer attached, discard its backing storage */
 	if (obj->mm.madv == I915_MADV_DONTNEED && !obj->mm.pages)
-		i915_gem_object_truncate(obj);
+		__i915_gem_object_truncate(obj);
 
 	args->retained = obj->mm.madv != __I915_MADV_PURGED;
 	mutex_unlock(&obj->mm.lock);
diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
index 1032f98add11..9f68304d6862 100644
--- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
@@ -127,6 +127,59 @@ static bool unsafe_drop_pages(struct drm_i915_gem_object *obj)
 	return !READ_ONCE(obj->mm.pages);
 }
 
+static void __start_writeback(struct drm_i915_gem_object *obj)
+{
+	struct address_space *mapping;
+	struct writeback_control wbc = {
+		.sync_mode = WB_SYNC_NONE,
+		.nr_to_write = SWAP_CLUSTER_MAX,
+		.range_start = 0,
+		.range_end = LLONG_MAX,
+		.for_reclaim = 1,
+	};
+	unsigned long i;
+
+	lockdep_assert_held(&obj->mm.lock);
+	GEM_BUG_ON(obj->mm.pages);
+
+	switch (obj->mm.madv) {
+	case I915_MADV_DONTNEED:
+		__i915_gem_object_truncate(obj);
+	case __I915_MADV_PURGED:
+		return;
+	}
+
+	if (!obj->base.filp)
+		return;
+
+	/* Force any other users of this object to refault */
+	mapping = obj->base.filp->f_mapping;
+	unmap_mapping_range(mapping, 0, (loff_t)-1, 0);
+
+	/* Begin writeback on each dirty page */
+	for (i = 0; i < obj->base.size >> PAGE_SHIFT; i++) {
+		struct page *page;
+
+		page = find_lock_entry(mapping, i);
+		if (!page || radix_tree_exceptional_entry(page))
+			continue;
+
+		if (!page_mapped(page) && clear_page_dirty_for_io(page)) {
+			int ret;
+
+			SetPageReclaim(page);
+			ret = mapping->a_ops->writepage(page, &wbc);
+			if (!PageWriteback(page))
+				ClearPageReclaim(page);
+			if (!ret)
+				goto put;
+		}
+		unlock_page(page);
+put:
+		put_page(page);
+	}
+}
+
 /**
  * i915_gem_shrink - Shrink buffer object caches
  * @dev_priv: i915 device
@@ -239,7 +292,7 @@ i915_gem_shrink(struct drm_i915_private *dev_priv,
 				mutex_lock_nested(&obj->mm.lock,
 						  I915_MM_SHRINKER);
 				if (!obj->mm.pages) {
-					__i915_gem_object_invalidate(obj);
+					__start_writeback(obj);
 					list_del_init(&obj->global_link);
 					count += obj->base.size >> PAGE_SHIFT;
 				}
-- 
2.11.0

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

Re: [PATCH v2 2/5] drm/i915: Remove __GFP_NORETRY from our buffer allocator

[Michal Hocko]

On Fri 09-06-17 12:03:47, Chris Wilson wrote:
> I tried __GFP_NORETRY in the belief that __GFP_RECLAIM was effective. It
> struggles with handling reclaim of our dirty buffers and relies on
> reclaim via kswapd. As a result, a single pass of direct reclaim is
> unreliable when i915 occupies the majority of available memory, and the
> only means of effectively waiting on kswapd to amke progress is by not
> setting the __GFP_NORETRY flag and lopping. That leaves us with the
> dilemma of invoking the oomkiller instead of propagating the allocation
> failure back to userspace where it can be handled more gracefully (one
> hopes).  In the future we may have __GFP_MAYFAIL to allow repeats up until
> we genuinely run out of memory and the oomkiller would have been invoked.
> Until then, let the oomkiller wreck havoc.
> 
> v2: Stop playing with side-effects of gfp flags and await __GFP_MAYFAIL
> v3: Update comments that direct reclaim only appears to be ignoring our
> dirty buffers!
> 
> Fixes: 24f8e00a8a2e ("drm/i915: Prefer to report ENOMEM rather than incur the oom for gfx allocations")
> Testcase: igt/gem_tiled_swapping
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Michal Hocko <mhocko@suse.com>

OK, this looks good to me. I will follow with my __GFP_RETRY_MAYFAIL and
will convert this caller to use it sometimes next week.

Thanks!

> ---
>  drivers/gpu/drm/i915/i915_gem.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index ca61a0be1458..b22145a876c5 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2404,7 +2404,20 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object *obj)
>  			if (!*s) {
>  				/* reclaim and warn, but no oom */
>  				gfp = mapping_gfp_mask(mapping);
> -				gfp |= __GFP_NORETRY;
> +
> +				/* Our bo are always dirty and so we require
> +				 * kswapd to reclaim our pages (direct reclaim
> +				 * does not effectively begin pageout of our
> +				 * buffers on its own). However, direct reclaim
> +				 * only waits for kswapd when under allocation
> +				 * congestion. So as a result __GFP_RECLAIM is
> +				 * unreliable and fails to actually reclaim our
> +				 * dirty pages -- unless you try over and over
> +				 * again with !__GFP_NORETRY. However, we still
> +				 * want to fail this allocation rather than
> +				 * trigger the out-of-memory killer and for
> +				 * this we want the future __GFP_MAYFAIL.
> +				 */
>  			}
>  		} while (1);
>  
> -- 
> 2.11.0
> 

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

Re: [PATCH v2 5/5] drm/i915: Start writeback from the shrinker

[Michal Hocko]

[Add Hugh]

On Fri 09-06-17 12:03:50, Chris Wilson wrote:
> When we are called to relieve mempressue via the shrinker, the only way
> we can make progress is either by discarding unwanted pages (those
> objects that userspace has marked MADV_DONTNEED) or by reclaiming the
> dirty objects via swap. As we know that is the only way to make further
> progress, we can initiate the writeback as we invalidate the objects.
> This means the objects we put onto the inactive anon lru list are
> already marked for reclaim+writeback and so will trigger a wait upon the
> writeback inside direct reclaim, greatly improving the success rate of
> direct reclaim on i915 objects.
> 
> The corollary is that we may start a slow swap on opportunistic
> mempressure from the likes of the compaction + migration kthreads. This
> is limited by those threads only being allowed to shrink idle pages, but
> also that if we reactivate the page before it is swapped out by gpu
> activity, we only page the cost of repinning the page. The cost is most
> felt when an object is reused after mempressure, which hopefully
> excludes the latency sensitive tasks (as we are just extending the
> impact of swap thrashing to them).

I am not sure you can start writeback on shmem while it is not in the
swapcache. Hugh?

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Michal Hocko <mhocko@suse.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h          |  2 +-
>  drivers/gpu/drm/i915/i915_gem.c          | 27 ++--------------
>  drivers/gpu/drm/i915/i915_gem_shrinker.c | 55 +++++++++++++++++++++++++++++++-
>  3 files changed, 57 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 602fb3324484..3ffe6504b391 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3332,7 +3332,7 @@ enum i915_mm_subclass { /* lockdep subclass for obj->mm.lock */
>  
>  void __i915_gem_object_put_pages(struct drm_i915_gem_object *obj,
>  				 enum i915_mm_subclass subclass);
> -void __i915_gem_object_invalidate(struct drm_i915_gem_object *obj);
> +void __i915_gem_object_truncate(struct drm_i915_gem_object *obj);
>  
>  enum i915_map_type {
>  	I915_MAP_WB = 0,
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 31cbe78171a9..1de94a8399a2 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2176,8 +2176,7 @@ i915_gem_mmap_gtt_ioctl(struct drm_device *dev, void *data,
>  }
>  
>  /* Immediately discard the backing storage */
> -static void
> -i915_gem_object_truncate(struct drm_i915_gem_object *obj)
> +void __i915_gem_object_truncate(struct drm_i915_gem_object *obj)
>  {
>  	i915_gem_object_free_mmap_offset(obj);
>  
> @@ -2194,28 +2193,6 @@ i915_gem_object_truncate(struct drm_i915_gem_object *obj)
>  	obj->mm.pages = ERR_PTR(-EFAULT);
>  }
>  
> -/* Try to discard unwanted pages */
> -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);
> -
> -	switch (obj->mm.madv) {
> -	case I915_MADV_DONTNEED:
> -		i915_gem_object_truncate(obj);
> -	case __I915_MADV_PURGED:
> -		return;
> -	}
> -
> -	if (obj->base.filp == NULL)
> -		return;
> -
> -	mapping = obj->base.filp->f_mapping,
> -	invalidate_mapping_pages(mapping, 0, (loff_t)-1);
> -}
> -
>  static void
>  i915_gem_object_put_pages_gtt(struct drm_i915_gem_object *obj,
>  			      struct sg_table *pages)
> @@ -4212,7 +4189,7 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
>  
>  	/* if the object is no longer attached, discard its backing storage */
>  	if (obj->mm.madv == I915_MADV_DONTNEED && !obj->mm.pages)
> -		i915_gem_object_truncate(obj);
> +		__i915_gem_object_truncate(obj);
>  
>  	args->retained = obj->mm.madv != __I915_MADV_PURGED;
>  	mutex_unlock(&obj->mm.lock);
> diff --git a/drivers/gpu/drm/i915/i915_gem_shrinker.c b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> index 1032f98add11..9f68304d6862 100644
> --- a/drivers/gpu/drm/i915/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/i915_gem_shrinker.c
> @@ -127,6 +127,59 @@ static bool unsafe_drop_pages(struct drm_i915_gem_object *obj)
>  	return !READ_ONCE(obj->mm.pages);
>  }
>  
> +static void __start_writeback(struct drm_i915_gem_object *obj)
> +{
> +	struct address_space *mapping;
> +	struct writeback_control wbc = {
> +		.sync_mode = WB_SYNC_NONE,
> +		.nr_to_write = SWAP_CLUSTER_MAX,
> +		.range_start = 0,
> +		.range_end = LLONG_MAX,
> +		.for_reclaim = 1,
> +	};
> +	unsigned long i;
> +
> +	lockdep_assert_held(&obj->mm.lock);
> +	GEM_BUG_ON(obj->mm.pages);
> +
> +	switch (obj->mm.madv) {
> +	case I915_MADV_DONTNEED:
> +		__i915_gem_object_truncate(obj);
> +	case __I915_MADV_PURGED:
> +		return;
> +	}
> +
> +	if (!obj->base.filp)
> +		return;
> +
> +	/* Force any other users of this object to refault */
> +	mapping = obj->base.filp->f_mapping;
> +	unmap_mapping_range(mapping, 0, (loff_t)-1, 0);
> +
> +	/* Begin writeback on each dirty page */
> +	for (i = 0; i < obj->base.size >> PAGE_SHIFT; i++) {
> +		struct page *page;
> +
> +		page = find_lock_entry(mapping, i);
> +		if (!page || radix_tree_exceptional_entry(page))
> +			continue;
> +
> +		if (!page_mapped(page) && clear_page_dirty_for_io(page)) {
> +			int ret;
> +
> +			SetPageReclaim(page);
> +			ret = mapping->a_ops->writepage(page, &wbc);
> +			if (!PageWriteback(page))
> +				ClearPageReclaim(page);
> +			if (!ret)
> +				goto put;
> +		}
> +		unlock_page(page);
> +put:
> +		put_page(page);
> +	}
> +}
> +
>  /**
>   * i915_gem_shrink - Shrink buffer object caches
>   * @dev_priv: i915 device
> @@ -239,7 +292,7 @@ i915_gem_shrink(struct drm_i915_private *dev_priv,
>  				mutex_lock_nested(&obj->mm.lock,
>  						  I915_MM_SHRINKER);
>  				if (!obj->mm.pages) {
> -					__i915_gem_object_invalidate(obj);
> +					__start_writeback(obj);
>  					list_del_init(&obj->global_link);
>  					count += obj->base.size >> PAGE_SHIFT;
>  				}
> -- 
> 2.11.0
> 

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

✓ Fi.CI.BAT: success for series starting with [v2,1/5] drm/i915: Encourage our shrinker more when our shmemfs allocations fails

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/5] drm/i915: Encourage our shrinker more when our shmemfs allocations fails
URL   : https://patchwork.freedesktop.org/series/25553/
State : success

== Summary ==

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

Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                pass       -> INCOMPLETE (fi-kbl-7500u) fdo#100904
                pass       -> INCOMPLETE (fi-kbl-7560u) fdo#100125 +1

fdo#100904 https://bugs.freedesktop.org/show_bug.cgi?id=100904
fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:441s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:432s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:588s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:519s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:486s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:480s
fi-glk-2a        total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:594s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:432s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:413s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:414s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:479s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:461s
fi-kbl-7500u     total:109  pass:97   dwarn:0   dfail:0   fail:0   skip:11 
fi-kbl-7560u     total:109  pass:105  dwarn:0   dfail:0   fail:0   skip:3  
fi-kbl-r         total:109  pass:97   dwarn:0   dfail:0   fail:0   skip:11 
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:465s
fi-skl-6700hq    total:278  pass:228  dwarn:1   dfail:0   fail:27  skip:22  time:398s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:463s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:477s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:439s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:529s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:401s

5b363842b49a66879d4c67d652923fb04e44b271 drm-tip: 2017y-06m-08d-20h-34m-37s UTC integration manifest
3263d9b drm/i915: Start writeback from the shrinker
5ba7db3 drm/i915: Spin for struct_mutex inside shrinker
7b42145 drm/i915: Only restrict noreclaim in the early shrink passes
75befba drm/i915: Remove __GFP_NORETRY from our buffer allocator
ad67f83 drm/i915: Encourage our shrinker more when our shmemfs allocations fails

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4926/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 5/5] drm/i915: Start writeback from the shrinker

[Chris Wilson]

Quoting Michal Hocko (2017-06-09 12:17:26)
> [Add Hugh]
> 
> On Fri 09-06-17 12:03:50, Chris Wilson wrote:
> > When we are called to relieve mempressue via the shrinker, the only way
> > we can make progress is either by discarding unwanted pages (those
> > objects that userspace has marked MADV_DONTNEED) or by reclaiming the
> > dirty objects via swap. As we know that is the only way to make further
> > progress, we can initiate the writeback as we invalidate the objects.
> > This means the objects we put onto the inactive anon lru list are
> > already marked for reclaim+writeback and so will trigger a wait upon the
> > writeback inside direct reclaim, greatly improving the success rate of
> > direct reclaim on i915 objects.
> > 
> > The corollary is that we may start a slow swap on opportunistic
> > mempressure from the likes of the compaction + migration kthreads. This
> > is limited by those threads only being allowed to shrink idle pages, but
> > also that if we reactivate the page before it is swapped out by gpu
> > activity, we only page the cost of repinning the page. The cost is most
> > felt when an object is reused after mempressure, which hopefully
> > excludes the latency sensitive tasks (as we are just extending the
> > impact of swap thrashing to them).
> 
> I am not sure you can start writeback on shmem while it is not in the
> swapcache. Hugh?

Note this is just mm/vmscan.c::pageout(), and we call into shmem which
is indeed responsible for adding it to the swapcache. My intention was
to simply start pageout() earlier to ensure the pages we tried to shrink
were indeed being reclaimed.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 5/5] drm/i915: Start writeback from the shrinker

[Michal Hocko]

On Fri 09-06-17 12:51:57, Chris Wilson wrote:
> Quoting Michal Hocko (2017-06-09 12:17:26)
> > [Add Hugh]
> > 
> > On Fri 09-06-17 12:03:50, Chris Wilson wrote:
> > > When we are called to relieve mempressue via the shrinker, the only way
> > > we can make progress is either by discarding unwanted pages (those
> > > objects that userspace has marked MADV_DONTNEED) or by reclaiming the
> > > dirty objects via swap. As we know that is the only way to make further
> > > progress, we can initiate the writeback as we invalidate the objects.
> > > This means the objects we put onto the inactive anon lru list are
> > > already marked for reclaim+writeback and so will trigger a wait upon the
> > > writeback inside direct reclaim, greatly improving the success rate of
> > > direct reclaim on i915 objects.
> > > 
> > > The corollary is that we may start a slow swap on opportunistic
> > > mempressure from the likes of the compaction + migration kthreads. This
> > > is limited by those threads only being allowed to shrink idle pages, but
> > > also that if we reactivate the page before it is swapped out by gpu
> > > activity, we only page the cost of repinning the page. The cost is most
> > > felt when an object is reused after mempressure, which hopefully
> > > excludes the latency sensitive tasks (as we are just extending the
> > > impact of swap thrashing to them).
> > 
> > I am not sure you can start writeback on shmem while it is not in the
> > swapcache. Hugh?
> 
> Note this is just mm/vmscan.c::pageout(), and we call into shmem which
> is indeed responsible for adding it to the swapcache.

You are right. I would still like to hear from Hugh this is OK. It is
quite some time since I've looked into shmem.c.

> My intention was
> to simply start pageout() earlier to ensure the pages we tried to shrink
> were indeed being reclaimed.

Yes the intention is clear...

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

Re: [PATCH v2 4/5] drm/i915: Spin for struct_mutex inside shrinker

[Joonas Lahtinen]

On pe, 2017-06-09 at 12:03 +0100, Chris Wilson wrote:
> Having resolved whether or not we would deadlock upon a call to
> mutex_lock(&dev->struct_mutex), we can then spin for the contended
> struct_mutex if we are not the owner. We cannot afford to simply block
> and wait for the mutex, as the owner may itself be waiting for the
> allocator -- i.e. a cyclic deadlock. This should significantly improve
> the chance of running the shrinker for other processes whilst the GPU is
> busy.
> 
> A more balanced approach would be to optimistically spin whilst the
> mutex owner was on the cpu and there was an opportunity to acquire the
> mutex for ourselves quickly. However, that requires support from
> kernel/locking/ and a new mutex_spin_trylock() primitive.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

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

Re: [PATCH v2 2/5] drm/i915: Remove __GFP_NORETRY from our buffer allocator

[Joonas Lahtinen]

On pe, 2017-06-09 at 12:03 +0100, Chris Wilson wrote:
> I tried __GFP_NORETRY in the belief that __GFP_RECLAIM was effective. It
> struggles with handling reclaim of our dirty buffers and relies on
> reclaim via kswapd. As a result, a single pass of direct reclaim is
> unreliable when i915 occupies the majority of available memory, and the
> only means of effectively waiting on kswapd to amke progress is by not
> setting the __GFP_NORETRY flag and lopping. That leaves us with the
> dilemma of invoking the oomkiller instead of propagating the allocation
> failure back to userspace where it can be handled more gracefully (one
> hopes).  In the future we may have __GFP_MAYFAIL to allow repeats up until
> we genuinely run out of memory and the oomkiller would have been invoked.
> Until then, let the oomkiller wreck havoc.
> 
> v2: Stop playing with side-effects of gfp flags and await __GFP_MAYFAIL
> v3: Update comments that direct reclaim only appears to be ignoring our
> dirty buffers!
> 
> Fixes: 24f8e00a8a2e ("drm/i915: Prefer to report ENOMEM rather than incur the oom for gfx allocations")
> Testcase: igt/gem_tiled_swapping
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Michal Hocko <mhocko@suse.com>

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

Re: [PATCH v2 3/5] drm/i915: Only restrict noreclaim in the early shrink passes

[Joonas Lahtinen]

On pe, 2017-06-09 at 12:03 +0100, Chris Wilson wrote:
> In our first pass, we do not want to use reclaim at all as we want to
> solely reap the i915 buffer caches (its purgeable pages). But we don't
> mind it initiates IO or pulls via the FS (but it shouldn't anyway as we
> say no to reclaim!). Just drop the GFP_IO constraint for simplicity.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>

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

Re: [PATCH v2 5/5] drm/i915: Start writeback from the shrinker

[Joonas Lahtinen]

On pe, 2017-06-09 at 12:03 +0100, Chris Wilson wrote:
> When we are called to relieve mempressue via the shrinker, the only way
> we can make progress is either by discarding unwanted pages (those
> objects that userspace has marked MADV_DONTNEED) or by reclaiming the
> dirty objects via swap. As we know that is the only way to make further
> progress, we can initiate the writeback as we invalidate the objects.
> This means the objects we put onto the inactive anon lru list are
> already marked for reclaim+writeback and so will trigger a wait upon the
> writeback inside direct reclaim, greatly improving the success rate of
> direct reclaim on i915 objects.
> 
> The corollary is that we may start a slow swap on opportunistic
> mempressure from the likes of the compaction + migration kthreads. This
> is limited by those threads only being allowed to shrink idle pages, but
> also that if we reactivate the page before it is swapped out by gpu
> activity, we only page the cost of repinning the page. The cost is most
> felt when an object is reused after mempressure, which hopefully
> excludes the latency sensitive tasks (as we are just extending the
> impact of swap thrashing to them).
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Michal Hocko <mhocko@suse.com>

<SNIP>

> +static void __start_writeback(struct drm_i915_gem_object *obj)
> +{

<SNIP>

> +	/* Force any other users of this object to refault */
> +	mapping = obj->base.filp->f_mapping;
> +	unmap_mapping_range(mapping, 0, (loff_t)-1, 0);
> +
> +	/* Begin writeback on each dirty page */
> +	for (i = 0; i < obj->base.size >> PAGE_SHIFT; i++) {
> +		struct page *page;
> +
> +		page = find_lock_entry(mapping, i);
> +		if (!page || radix_tree_exceptional_entry(page))
> +			continue;
> +
> +		if (!page_mapped(page) && clear_page_dirty_for_io(page)) {
> +			int ret;
> +
> +			SetPageReclaim(page);
> +			ret = mapping->a_ops->writepage(page, &wbc);
> +			if (!PageWriteback(page))
> +				ClearPageReclaim(page);
> +			if (!ret)
> +				goto put;
> +		}
> +		unlock_page(page);
> +put:
> +		put_page(page);
> +	}

Apart from this part (which should probably be a helper function
outside of i915), the code 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

Re: [PATCH v2 5/5] drm/i915: Start writeback from the shrinker

[Chris Wilson]

Quoting Joonas Lahtinen (2017-06-13 15:07:04)
> On pe, 2017-06-09 at 12:03 +0100, Chris Wilson wrote:
> > When we are called to relieve mempressue via the shrinker, the only way
> > we can make progress is either by discarding unwanted pages (those
> > objects that userspace has marked MADV_DONTNEED) or by reclaiming the
> > dirty objects via swap. As we know that is the only way to make further
> > progress, we can initiate the writeback as we invalidate the objects.
> > This means the objects we put onto the inactive anon lru list are
> > already marked for reclaim+writeback and so will trigger a wait upon the
> > writeback inside direct reclaim, greatly improving the success rate of
> > direct reclaim on i915 objects.
> > 
> > The corollary is that we may start a slow swap on opportunistic
> > mempressure from the likes of the compaction + migration kthreads. This
> > is limited by those threads only being allowed to shrink idle pages, but
> > also that if we reactivate the page before it is swapped out by gpu
> > activity, we only page the cost of repinning the page. The cost is most
> > felt when an object is reused after mempressure, which hopefully
> > excludes the latency sensitive tasks (as we are just extending the
> > impact of swap thrashing to them).
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> > Cc: Matthew Auld <matthew.auld@intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Michal Hocko <mhocko@suse.com>
> 
> <SNIP>
> 
> > +static void __start_writeback(struct drm_i915_gem_object *obj)
> > +{
> 
> <SNIP>
> 
> > +     /* Force any other users of this object to refault */
> > +     mapping = obj->base.filp->f_mapping;
> > +     unmap_mapping_range(mapping, 0, (loff_t)-1, 0);
> > +
> > +     /* Begin writeback on each dirty page */
> > +     for (i = 0; i < obj->base.size >> PAGE_SHIFT; i++) {
> > +             struct page *page;
> > +
> > +             page = find_lock_entry(mapping, i);
> > +             if (!page || radix_tree_exceptional_entry(page))
> > +                     continue;
> > +
> > +             if (!page_mapped(page) && clear_page_dirty_for_io(page)) {
> > +                     int ret;
> > +
> > +                     SetPageReclaim(page);
> > +                     ret = mapping->a_ops->writepage(page, &wbc);
> > +                     if (!PageWriteback(page))
> > +                             ClearPageReclaim(page);
> > +                     if (!ret)
> > +                             goto put;
> > +             }
> > +             unlock_page(page);
> > +put:
> > +             put_page(page);
> > +     }
> 
> Apart from this part (which should probably be a helper function
> outside of i915), the code is:
> 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Thanks for the review, I've pushed the fix plus simple patches, leaving
this one for more feedback.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx