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

Joonas Lahtinen
2016-10-18T12:05:09Z

On ma, 2016-04-04 at 14:18 +0100, Chris Wilson wrote:
> From: Akash Goel <akash.goel@intel.com>
>
> 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.A A 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.A A With this we can
> effectively mark the GPU pages as MOVABLE and hence mitigate the
> fragmentation problem.
>
> v2:
> A - Rename the migration routine to gem_shrink_migratepage, move it to the
> A A A shrinker file, and use the existing constructs (Chris)
> A - To cleanup, add a new helper function to encapsulate all page migration
> A A A skip conditions (Chris)
> A - Add a new local helper function in shrinker file, for dropping the
> A A A backing pages, and call the same from gem_shrink() also (Chris)
>
> v3:
> A - Fix/invert the check on the return value of unsafe_drop_pages (Chris)
>
> v4:
> A - Minor tidy
>
> Testcase: igt/gem_shrink
> Bugzilla: (e.g.) https://bugs.freedesktop.org/show_bug.cgi?id=90254
> Cc: Hugh Dickins <hughd@google.com>
> Cc: linux-mm@kvack.org
> Signed-off-by: Sourab Gupta <sourab.gupta@intel.com>
> Signed-off-by: Akash Goel <akash.goel@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

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 <joonas.lahtinen@linux.intel.com>

Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation