Re: [Intel-gfx] [PATCH v4 2/2] drm/i915: Make GPU pages movable

[Joonas Lahtinen]

On ti, 2016-10-18 at 14:39 +0100, Chris Wilson wrote:
> It's in my tree (on top of nightly) already with Joonas' r-b.

Patch 1/2 seems to have my comments already, could be addressed and
respined too.

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: [Intel-gfx] [PATCH v4 2/2] drm/i915: Make GPU pages movable

[Chris Wilson]

On Tue, Oct 18, 2016 at 06:55:12PM +0530, Goel, Akash wrote:
> 
> 
> On 10/18/2016 5:35 PM, Joonas Lahtinen wrote:
> >On ma, 2016-04-04 at 14:18 +0100, Chris Wilson wrote:
> >>From: Akash Goel 
> >>
> >>On a long run of more than 2-3 days, physical memory tends to get
> >>fragmented severely, which considerably slows down the system. In such a
> >>scenario, the shrinker is also unable to help as lack of memory is not
> >>the actual problem, since it has been observed that there are enough free
> >>pages of 0 order. This also manifests itself when an indiviual zone in
> >>the mm runs out of pages and if we cannot migrate pages between zones,
> >>the kernel hits an out-of-memory even though there are free pages (and
> >>often all of swap) available.
> >>
> >>To address the issue of external fragementation, kernel does a compaction
> >>(which involves migration of pages) but it's efficacy depends upon how
> >>many pages are marked as MOVABLE, as only those pages can be migrated.
> >>
> >>Currently the backing pages for GFX buffers are allocated from shmemfs
> >>with GFP_RECLAIMABLE flag, in units of 4KB pages.  In the case of limited
> >>swap space, it may not be possible always to reclaim or swap-out pages of
> >>all the inactive objects, to make way for free space allowing formation
> >>of higher order groups of physically-contiguous pages on compaction.
> >>
> >>Just marking the GPU pages as MOVABLE will not suffice, as i915.ko has to
> >>pin the pages if they are in use by GPU, which will prevent their
> >>migration. So the migratepage callback in shmem is also hooked up to get
> >>a notification when kernel initiates the page migration. On the
> >>notification, i915.ko appropriately unpin the pages.  With this we can
> >>effectively mark the GPU pages as MOVABLE and hence mitigate the
> >>fragmentation problem.
> >>
> >>v2:
> >> - Rename the migration routine to gem_shrink_migratepage, move it to the
> >>   shrinker file, and use the existing constructs (Chris)
> >> - To cleanup, add a new helper function to encapsulate all page migration
> >>   skip conditions (Chris)
> >> - Add a new local helper function in shrinker file, for dropping the
> >>   backing pages, and call the same from gem_shrink() also (Chris)
> >>
> >>v3:
> >> - Fix/invert the check on the return value of unsafe_drop_pages (Chris)
> >>
> >>v4:
> >> - Minor tidy
> >>
> >>Testcase: igt/gem_shrink
> >>Bugzilla: (e.g.) https://bugs.freedesktop.org/show_bug.cgi?id=90254
> >>Cc: Hugh Dickins 
> >>Cc: linux...@kvack.org
> >>Signed-off-by: Sourab Gupta 
> >>Signed-off-by: Akash Goel 
> >>Reviewed-by: Chris Wilson 
> >
> >Could this patch be re-spinned on top of current nightly?
> >
> Sure will rebase it on top of nightly.

It's in my tree (on top of nightly) already with Joonas' r-b.
-Chris

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

Re: [Intel-gfx] [PATCH v4 2/2] drm/i915: Make GPU pages movable

[Goel, Akash]

On 10/18/2016 5:35 PM, Joonas Lahtinen wrote:

On ma, 2016-04-04 at 14:18 +0100, Chris Wilson wrote:

From: Akash Goel 

On a long run of more than 2-3 days, physical memory tends to get
fragmented severely, which considerably slows down the system. In such a
scenario, the shrinker is also unable to help as lack of memory is not
the actual problem, since it has been observed that there are enough free
pages of 0 order. This also manifests itself when an indiviual zone in
the mm runs out of pages and if we cannot migrate pages between zones,
the kernel hits an out-of-memory even though there are free pages (and
often all of swap) available.

To address the issue of external fragementation, kernel does a compaction
(which involves migration of pages) but it's efficacy depends upon how
many pages are marked as MOVABLE, as only those pages can be migrated.

Currently the backing pages for GFX buffers are allocated from shmemfs
with GFP_RECLAIMABLE flag, in units of 4KB pages.  In the case of limited
swap space, it may not be possible always to reclaim or swap-out pages of
all the inactive objects, to make way for free space allowing formation
of higher order groups of physically-contiguous pages on compaction.

Just marking the GPU pages as MOVABLE will not suffice, as i915.ko has to
pin the pages if they are in use by GPU, which will prevent their
migration. So the migratepage callback in shmem is also hooked up to get
a notification when kernel initiates the page migration. On the
notification, i915.ko appropriately unpin the pages.  With this we can
effectively mark the GPU pages as MOVABLE and hence mitigate the
fragmentation problem.

v2:
 - Rename the migration routine to gem_shrink_migratepage, move it to the
   shrinker file, and use the existing constructs (Chris)
 - To cleanup, add a new helper function to encapsulate all page migration
   skip conditions (Chris)
 - Add a new local helper function in shrinker file, for dropping the
   backing pages, and call the same from gem_shrink() also (Chris)

v3:
 - Fix/invert the check on the return value of unsafe_drop_pages (Chris)

v4:
 - Minor tidy

Testcase: igt/gem_shrink
Bugzilla: (e.g.) https://bugs.freedesktop.org/show_bug.cgi?id=90254
Cc: Hugh Dickins 
Cc: linux...@kvack.org
Signed-off-by: Sourab Gupta 
Signed-off-by: Akash Goel 
Reviewed-by: Chris Wilson 


Could this patch be re-spinned on top of current nightly?


Sure will rebase it on top of nightly.


After removing;


WARN(page_count(newpage) != 1, "Unexpected ref count for newpage\n")


and


if (ret)
DRM_DEBUG_DRIVER("page=%p migration returned %d\n", page, ret);


This is;

Reviewed-by: Joonas Lahtinen 

Thanks much for the review.
But there is a precursor patch also, there has been no traction on that.
[1/2] shmem: Support for registration of Driver/file owner specific ops
https://patchwork.freedesktop.org/patch/77935/

Best regards
Akash



Regards, Joonas


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

Re: [Intel-gfx] [PATCH v4 2/2] drm/i915: Make GPU pages movable

[Joonas Lahtinen]

On ma, 2016-04-04 at 14:18 +0100, Chris Wilson wrote:
> From: Akash Goel 
> 
> On a long run of more than 2-3 days, physical memory tends to get
> fragmented severely, which considerably slows down the system. In such a
> scenario, the shrinker is also unable to help as lack of memory is not
> the actual problem, since it has been observed that there are enough free
> pages of 0 order. This also manifests itself when an indiviual zone in
> the mm runs out of pages and if we cannot migrate pages between zones,
> the kernel hits an out-of-memory even though there are free pages (and
> often all of swap) available.
> 
> To address the issue of external fragementation, kernel does a compaction
> (which involves migration of pages) but it's efficacy depends upon how
> many pages are marked as MOVABLE, as only those pages can be migrated.
> 
> Currently the backing pages for GFX buffers are allocated from shmemfs
> with GFP_RECLAIMABLE flag, in units of 4KB pages.  In the case of limited
> swap space, it may not be possible always to reclaim or swap-out pages of
> all the inactive objects, to make way for free space allowing formation
> of higher order groups of physically-contiguous pages on compaction.
> 
> Just marking the GPU pages as MOVABLE will not suffice, as i915.ko has to
> pin the pages if they are in use by GPU, which will prevent their
> migration. So the migratepage callback in shmem is also hooked up to get
> a notification when kernel initiates the page migration. On the
> notification, i915.ko appropriately unpin the pages.  With this we can
> effectively mark the GPU pages as MOVABLE and hence mitigate the
> fragmentation problem.
> 
> v2:
>  - Rename the migration routine to gem_shrink_migratepage, move it to the
>    shrinker file, and use the existing constructs (Chris)
>  - To cleanup, add a new helper function to encapsulate all page migration
>    skip conditions (Chris)
>  - Add a new local helper function in shrinker file, for dropping the
>    backing pages, and call the same from gem_shrink() also (Chris)
> 
> v3:
>  - Fix/invert the check on the return value of unsafe_drop_pages (Chris)
> 
> v4:
>  - Minor tidy
> 
> Testcase: igt/gem_shrink
> Bugzilla: (e.g.) https://bugs.freedesktop.org/show_bug.cgi?id=90254
> Cc: Hugh Dickins 
> Cc: linux...@kvack.org
> Signed-off-by: Sourab Gupta 
> Signed-off-by: Akash Goel 
> Reviewed-by: Chris Wilson 

Could this patch be re-spinned on top of current nightly?

After removing;

> WARN(page_count(newpage) != 1, "Unexpected ref count for newpage\n")

and

>   if (ret)
>   DRM_DEBUG_DRIVER("page=%p migration returned %d\n", page, ret);

This is;

Reviewed-by: Joonas Lahtinen 

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

[Intel-gfx] [PATCH v4 2/2] drm/i915: Make GPU pages movable

[Chris Wilson]

From: Akash Goel 

On a long run of more than 2-3 days, physical memory tends to get
fragmented severely, which considerably slows down the system. In such a
scenario, the shrinker is also unable to help as lack of memory is not
the actual problem, since it has been observed that there are enough free
pages of 0 order. This also manifests itself when an indiviual zone in
the mm runs out of pages and if we cannot migrate pages between zones,
the kernel hits an out-of-memory even though there are free pages (and
often all of swap) available.

To address the issue of external fragementation, kernel does a compaction
(which involves migration of pages) but it's efficacy depends upon how
many pages are marked as MOVABLE, as only those pages can be migrated.

Currently the backing pages for GFX buffers are allocated from shmemfs
with GFP_RECLAIMABLE flag, in units of 4KB pages.  In the case of limited
swap space, it may not be possible always to reclaim or swap-out pages of
all the inactive objects, to make way for free space allowing formation
of higher order groups of physically-contiguous pages on compaction.

Just marking the GPU pages as MOVABLE will not suffice, as i915.ko has to
pin the pages if they are in use by GPU, which will prevent their
migration. So the migratepage callback in shmem is also hooked up to get
a notification when kernel initiates the page migration. On the
notification, i915.ko appropriately unpin the pages.  With this we can
effectively mark the GPU pages as MOVABLE and hence mitigate the
fragmentation problem.

v2:
 - Rename the migration routine to gem_shrink_migratepage, move it to the
   shrinker file, and use the existing constructs (Chris)
 - To cleanup, add a new helper function to encapsulate all page migration
   skip conditions (Chris)
 - Add a new local helper function in shrinker file, for dropping the
   backing pages, and call the same from gem_shrink() also (Chris)

v3:
 - Fix/invert the check on the return value of unsafe_drop_pages (Chris)

v4:
 - Minor tidy

Testcase: igt/gem_shrink
Bugzilla: (e.g.) https://bugs.freedesktop.org/show_bug.cgi?id=90254
Cc: Hugh Dickins 
Cc: linux...@kvack.org
Signed-off-by: Sourab Gupta 
Signed-off-by: Akash Goel 
Reviewed-by: Chris Wilson 
---
 drivers/gpu/drm/i915/i915_drv.h  |   3 +
 drivers/gpu/drm/i915/i915_gem.c  |  13 ++-
 drivers/gpu/drm/i915/i915_gem_shrinker.c | 174 ---
 3 files changed, 175 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index dd187727c813..2c1d5c3af780 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -52,6 +52,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "intel_guc.h"
 #include "intel_dpll_mgr.h"
 
@@ -1234,6 +1235,8 @@ struct intel_l3_parity {
 };
 
 struct i915_gem_mm {
+   struct shmem_dev_info shmem_info;
+
/** Memory allocator for GTT stolen memory */
struct drm_mm stolen;
/** Protects the usage of the GTT stolen memory allocator. This is
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ca96fc12cdf4..7cef03efb539 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2206,6 +2206,7 @@ i915_gem_object_put_pages_gtt(struct drm_i915_gem_object 
*obj)
if (obj->madv == I915_MADV_WILLNEED)
mark_page_accessed(page);
 
+   set_page_private(page, 0);
page_cache_release(page);
}
obj->dirty = 0;
@@ -2320,6 +2321,7 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object 
*obj)
sg->length += PAGE_SIZE;
}
last_pfn = page_to_pfn(page);
+   set_page_private(page, (unsigned long)obj);
 
/* Check that the i965g/gm workaround works. */
WARN_ON((gfp & __GFP_DMA32) && (last_pfn >= 0x0010UL));
@@ -2345,8 +2347,11 @@ i915_gem_object_get_pages_gtt(struct drm_i915_gem_object 
*obj)
 
 err_pages:
sg_mark_end(sg);
-   for_each_sg_page(st->sgl, _iter, st->nents, 0)
-   page_cache_release(sg_page_iter_page(_iter));
+   for_each_sg_page(st->sgl, _iter, st->nents, 0) {
+   page = sg_page_iter_page(_iter);
+   set_page_private(page, 0);
+   page_cache_release(page);
+   }
sg_free_table(st);
kfree(st);
 
@@ -4468,6 +4473,7 @@ static const struct drm_i915_gem_object_ops 
i915_gem_object_ops = {
 struct drm_i915_gem_object *i915_gem_alloc_object(struct drm_device *dev,
  size_t size)
 {
+   struct drm_i915_private *dev_priv = to_i915(dev);
struct drm_i915_gem_object *obj;
struct address_space *mapping;
gfp_t mask;
@@ -4481,7