[Intel-gfx] [PATCH v3] mm, drm/i915: mark pinned shmemfs pages as unevictable

2018-10-31 Thread Kuo-Hsin Yang
The i915 driver uses shmemfs to allocate backing storage for gem
objects. These shmemfs pages can be pinned (increased ref count) by
shmem_read_mapping_page_gfp(). When a lot of pages are pinned, vmscan
wastes a lot of time scanning these pinned pages. In some extreme case,
all pages in the inactive anon lru are pinned, and only the inactive
anon lru is scanned due to inactive_ratio, the system cannot swap and
invokes the oom-killer. Mark these pinned pages as unevictable to speed
up vmscan.

Add check_move_lru_page() to move page to appropriate lru list.

This patch was inspired by Chris Wilson's change [1].

[1]: https://patchwork.kernel.org/patch/9768741/

Cc: Chris Wilson 
Cc: Michal Hocko 
Cc: Joonas Lahtinen 
Cc: Peter Zijlstra 
Cc: Andrew Morton 
Cc: Dave Hansen 
Signed-off-by: Kuo-Hsin Yang 
---
The previous mapping_set_unevictable patch is worse on gem_syslatency
because it defers to vmscan to move these pages to the unevictable list
and the test measures latency to allocate 2MiB pages. This performance
impact can be solved by explicit moving pages to the unevictable list in
the i915 function.

Chris, can you help to run the "igt/benchmarks/gem_syslatency -t 120 -b -m"
test with this patch on your testing machine? I tried to run the test on
a Celeron N4000, 4GB Ram machine. The mean value with this patch is
similar to that with the mlock patch.

x tip-mean.txt # current stock i915
+ lock_vma-mean.txt # the old mlock patch
* mapping-mean.txt # this patch

   NMinMax MedianAvg Stddev
x 60548.898   2563.653   2149.573   1999.273480.837
+ 60479.049   2119.902   1964.399   1893.226314.736
* 60455.358   3212.368   1991.308   1903.686411.448

Changes for v3:
 Use check_move_lru_page instead of shmem_unlock_mapping to move pages
 to appropriate lru lists.

Changes for v2:
 Squashed the two patches.

 Documentation/vm/unevictable-lru.rst |  4 +++-
 drivers/gpu/drm/i915/i915_gem.c  | 20 +++-
 include/linux/swap.h |  1 +
 mm/vmscan.c  | 20 +---
 4 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/Documentation/vm/unevictable-lru.rst 
b/Documentation/vm/unevictable-lru.rst
index fdd84cb8d511..a812fb55136d 100644
--- a/Documentation/vm/unevictable-lru.rst
+++ b/Documentation/vm/unevictable-lru.rst
@@ -143,7 +143,7 @@ using a number of wrapper functions:
Query the address space, and return true if it is completely
unevictable.
 
-These are currently used in two places in the kernel:
+These are currently used in three places in the kernel:
 
  (1) By ramfs to mark the address spaces of its inodes when they are created,
  and this mark remains for the life of the inode.
@@ -154,6 +154,8 @@ These are currently used in two places in the kernel:
  swapped out; the application must touch the pages manually if it wants to
  ensure they're in memory.
 
+ (3) By the i915 driver to mark pinned address space until it's unpinned.
+
 
 Detecting Unevictable Pages
 ---
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0c8aa57ce83b..6dc3ecef67e4 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2387,6 +2387,7 @@ i915_gem_object_put_pages_gtt(struct drm_i915_gem_object 
*obj,
 {
struct sgt_iter sgt_iter;
struct page *page;
+   struct address_space *mapping;
 
__i915_gem_object_release_shmem(obj, pages, true);
 
@@ -2395,6 +2396,9 @@ i915_gem_object_put_pages_gtt(struct drm_i915_gem_object 
*obj,
if (i915_gem_object_needs_bit17_swizzle(obj))
i915_gem_object_save_bit_17_swizzle(obj, pages);
 
+   mapping = file_inode(obj->base.filp)->i_mapping;
+   mapping_clear_unevictable(mapping);
+
for_each_sgt_page(page, sgt_iter, pages) {
if (obj->mm.dirty)
set_page_dirty(page);
@@ -2402,6 +2406,10 @@ i915_gem_object_put_pages_gtt(struct drm_i915_gem_object 
*obj,
if (obj->mm.madv == I915_MADV_WILLNEED)
mark_page_accessed(page);
 
+   lock_page(page);
+   check_move_lru_page(page);
+   unlock_page(page);
+
put_page(page);
}
obj->mm.dirty = false;
@@ -2559,6 +2567,7 @@ static int i915_gem_object_get_pages_gtt(struct 
drm_i915_gem_object *obj)
 * Fail silently without starting the shrinker
 */
mapping = obj->base.filp->f_mapping;
+   mapping_set_unevictable(mapping);
noreclaim = mapping_gfp_constraint(mapping, ~__GFP_RECLAIM);
noreclaim |= __GFP_NORETRY | __GFP_NOWARN;
 
@@ -2630,6 +2639,10 @@ static int i915_gem_object_get_pages_gtt(struct 
drm_i915_gem_object *obj)
}
last_pfn = page_to_pfn(page);
 
+   lock_page(p

[Intel-gfx] [PATCH v4] mm, drm/i915: mark pinned shmemfs pages as unevictable

2018-11-05 Thread Kuo-Hsin Yang
The i915 driver uses shmemfs to allocate backing storage for gem
objects. These shmemfs pages can be pinned (increased ref count) by
shmem_read_mapping_page_gfp(). When a lot of pages are pinned, vmscan
wastes a lot of time scanning these pinned pages. In some extreme case,
all pages in the inactive anon lru are pinned, and only the inactive
anon lru is scanned due to inactive_ratio, the system cannot swap and
invokes the oom-killer. Mark these pinned pages as unevictable to speed
up vmscan.

Export pagevec API check_move_unevictable_pages().

This patch was inspired by Chris Wilson's change [1].

[1]: https://patchwork.kernel.org/patch/9768741/

Cc: Chris Wilson 
Cc: Michal Hocko 
Cc: Joonas Lahtinen 
Cc: Peter Zijlstra 
Cc: Andrew Morton 
Cc: Dave Hansen 
Signed-off-by: Kuo-Hsin Yang 
---
Changes for v4:
 Export pagevec API check_move_unevictable_pages().

Changes for v3:
 Use check_move_lru_page instead of shmem_unlock_mapping to move pages
 to appropriate lru lists.

Changes for v2:
 Squashed the two patches.

 Documentation/vm/unevictable-lru.rst |  4 +++-
 drivers/gpu/drm/i915/i915_gem.c  | 25 +++--
 include/linux/swap.h |  4 +++-
 mm/shmem.c   |  2 +-
 mm/vmscan.c  | 18 +-
 5 files changed, 39 insertions(+), 14 deletions(-)

diff --git a/Documentation/vm/unevictable-lru.rst 
b/Documentation/vm/unevictable-lru.rst
index fdd84cb8d511..a812fb55136d 100644
--- a/Documentation/vm/unevictable-lru.rst
+++ b/Documentation/vm/unevictable-lru.rst
@@ -143,7 +143,7 @@ using a number of wrapper functions:
Query the address space, and return true if it is completely
unevictable.
 
-These are currently used in two places in the kernel:
+These are currently used in three places in the kernel:
 
  (1) By ramfs to mark the address spaces of its inodes when they are created,
  and this mark remains for the life of the inode.
@@ -154,6 +154,8 @@ These are currently used in two places in the kernel:
  swapped out; the application must touch the pages manually if it wants to
  ensure they're in memory.
 
+ (3) By the i915 driver to mark pinned address space until it's unpinned.
+
 
 Detecting Unevictable Pages
 ---
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0c8aa57ce83b..7972eeb2e921 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2381,12 +2381,22 @@ void __i915_gem_object_invalidate(struct 
drm_i915_gem_object *obj)
invalidate_mapping_pages(mapping, 0, (loff_t)-1);
 }
 
+/* Move pages to appropriate lru and release the pagevec */
+static inline void check_release_pagevec(struct pagevec *pvec)
+{
+   if (pagevec_count(pvec)) {
+   check_move_unevictable_pages(pvec);
+   __pagevec_release(pvec);
+   }
+}
+
 static void
 i915_gem_object_put_pages_gtt(struct drm_i915_gem_object *obj,
  struct sg_table *pages)
 {
struct sgt_iter sgt_iter;
struct page *page;
+   struct pagevec pvec;
 
__i915_gem_object_release_shmem(obj, pages, true);
 
@@ -2395,6 +2405,9 @@ i915_gem_object_put_pages_gtt(struct drm_i915_gem_object 
*obj,
if (i915_gem_object_needs_bit17_swizzle(obj))
i915_gem_object_save_bit_17_swizzle(obj, pages);
 
+   mapping_clear_unevictable(file_inode(obj->base.filp)->i_mapping);
+
+   pagevec_init(&pvec);
for_each_sgt_page(page, sgt_iter, pages) {
if (obj->mm.dirty)
set_page_dirty(page);
@@ -2402,8 +2415,10 @@ i915_gem_object_put_pages_gtt(struct drm_i915_gem_object 
*obj,
if (obj->mm.madv == I915_MADV_WILLNEED)
mark_page_accessed(page);
 
-   put_page(page);
+   if (!pagevec_add(&pvec, page))
+   check_release_pagevec(&pvec);
}
+   check_release_pagevec(&pvec);
obj->mm.dirty = false;
 
sg_free_table(pages);
@@ -2526,6 +2541,7 @@ static int i915_gem_object_get_pages_gtt(struct 
drm_i915_gem_object *obj)
unsigned int sg_page_sizes;
gfp_t noreclaim;
int ret;
+   struct pagevec pvec;
 
/*
 * Assert that the object is not currently in any GPU domain. As it
@@ -2559,6 +2575,7 @@ static int i915_gem_object_get_pages_gtt(struct 
drm_i915_gem_object *obj)
 * Fail silently without starting the shrinker
 */
mapping = obj->base.filp->f_mapping;
+   mapping_set_unevictable(mapping);
noreclaim = mapping_gfp_constraint(mapping, ~__GFP_RECLAIM);
noreclaim |= __GFP_NORETRY | __GFP_NOWARN;
 
@@ -2673,8 +2690,12 @@ static int i915_gem_object_get_pages_gtt(struct 
drm_i915_gem_object *obj)
 err_sg:
sg_mark_end(sg);
 err_pages:
+   mapping_clear_unevictable(mapping);
+  

Re: [Intel-gfx] [PATCH v3] mm, drm/i915: mark pinned shmemfs pages as unevictable

2018-11-05 Thread Kuo-Hsin Yang
On Fri, Nov 2, 2018 at 10:05 PM Dave Hansen  wrote:
> On 11/2/18 6:22 AM, Vovo Yang wrote:
> > Chris helped to answer this question:
> > Though it includes a few non-shmemfs objects, see
> > debugfs/dri/0/i915_gem_objects and the "bound objects".
> >
> > Example i915_gem_object output:
> >   591 objects, 95449088 bytes
> >   55 unbound objects, 1880064 bytes
> >   533 bound objects, 93040640 bytes
>
> Do those non-shmemfs objects show up on the unevictable list?  How far
> can the amount of memory on the unevictable list and the amount
> displayed in this "bound objects" value diverge?

Those non-shmemfs objects would not show up on the unevictable list.

On typical use case, The size of gtt bounded objects (in unevictable
list) is very close to the bound size in i915_gem_objects. E.g. on my
laptop: i915_gem_object shows 110075904 bytes bounded objects, and
there are 109760512 bytes gtt bounded objects, the difference is about
0.3%.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v4] mm, drm/i915: mark pinned shmemfs pages as unevictable

2018-11-05 Thread Kuo-Hsin Yang
On Mon, Nov 5, 2018 at 9:02 PM Michal Hocko  wrote:
>
> On Mon 05-11-18 19:13:48, Kuo-Hsin Yang wrote:
> > The i915 driver uses shmemfs to allocate backing storage for gem
> > objects. These shmemfs pages can be pinned (increased ref count) by
> > shmem_read_mapping_page_gfp(). When a lot of pages are pinned, vmscan
> > wastes a lot of time scanning these pinned pages. In some extreme case,
> > all pages in the inactive anon lru are pinned, and only the inactive
> > anon lru is scanned due to inactive_ratio, the system cannot swap and
> > invokes the oom-killer. Mark these pinned pages as unevictable to speed
> > up vmscan.
> >
> > Export pagevec API check_move_unevictable_pages().
>
> Thanks for reworking the patch. This looks much more to my taste. At
> least the mm part. I haven't really looked at the the drm part.
>
> Just a nit below
>
> > This patch was inspired by Chris Wilson's change [1].
> >
> > [1]: https://patchwork.kernel.org/patch/9768741/
>
> I would recommend using msg-id based url.

I didn't find a msg-id based url for the [1] patch. This patch is sent
to intel-gfx@lists.freedesktop.org and linux...@kvack.org, but not to
linux-ker...@vger.kernel.org .

>
> > Cc: Chris Wilson 
> > Cc: Michal Hocko 
> > Cc: Joonas Lahtinen 
> > Cc: Peter Zijlstra 
> > Cc: Andrew Morton 
> > Cc: Dave Hansen 
> > Signed-off-by: Kuo-Hsin Yang 
>
> other than that
> Acked-by: Michal Hocko 
>
> [...]
>
> > @@ -4184,15 +4185,13 @@ int page_evictable(struct page *page)
> >
> >  #ifdef CONFIG_SHMEM
> >  /**
> > - * check_move_unevictable_pages - check pages for evictability and move to 
> > appropriate zone lru list
> > - * @pages:   array of pages to check
> > - * @nr_pages:number of pages to check
> > + * check_move_unevictable_pages - move evictable pages to appropriate 
> > evictable
> > + * lru lists
>
> I am not sure this is an improvement. I would just keep the original
> wording. It is not great either but the explicit note about check for
> evictability sounds like a better fit to me.

OK, will keep the original wording.

>
> > + * @pvec: pagevec with pages to check
> >   *
> > - * Checks pages for evictability and moves them to the appropriate lru 
> > list.
> > - *
> > - * This function is only used for SysV IPC SHM_UNLOCK.
> > + * This function is only used to move shmem pages.
>
> I do not really see anything that would be shmem specific here. We can
> use this function for any LRU pages unless I am missing something
> obscure. I would just drop the last sentence.

OK, this function should not be specific to shmem pages.

Is it OK to remove the #ifdef SHMEM surrounding check_move_unevictable_pages?

>
> A note that this function should be only used for LRU pages would be
> nice.
>
> >   */
> > -void check_move_unevictable_pages(struct page **pages, int nr_pages)
> > +void check_move_unevictable_pages(struct pagevec *pvec)
> >  {
> >   struct lruvec *lruvec;
> >   struct pglist_data *pgdat = NULL;
> > @@ -4200,8 +4199,8 @@ void check_move_unevictable_pages(struct page 
> > **pages, int nr_pages)
> >   int pgrescued = 0;
> >   int i;
> >
> > - for (i = 0; i < nr_pages; i++) {
> > - struct page *page = pages[i];
> > + for (i = 0; i < pvec->nr; i++) {
> > + struct page *page = pvec->pages[i];
> >   struct pglist_data *pagepgdat = page_pgdat(page);
> >
> >   pgscanned++;
> > @@ -4233,4 +4232,5 @@ void check_move_unevictable_pages(struct page 
> > **pages, int nr_pages)
> >   spin_unlock_irq(&pgdat->lru_lock);
> >   }
> >  }
> > +EXPORT_SYMBOL(check_move_unevictable_pages);
> >  #endif /* CONFIG_SHMEM */
> > --
> > 2.19.1.930.g4563a0d9d0-goog
> >
>
> --
> Michal Hocko
> SUSE Labs
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v4] mm, drm/i915: mark pinned shmemfs pages as unevictable

2018-11-05 Thread Kuo-Hsin Yang
On Tue, Nov 6, 2018 at 12:41 AM Michal Hocko  wrote:
> On Mon 05-11-18 22:33:13, Kuo-Hsin Yang wrote:
> > OK, this function should not be specific to shmem pages.
> >
> > Is it OK to remove the #ifdef SHMEM surrounding 
> > check_move_unevictable_pages?
>
> Yes, I think so.

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


Re: [Intel-gfx] [PATCH v4] mm, drm/i915: mark pinned shmemfs pages as unevictable

2018-11-05 Thread Kuo-Hsin Yang
On Tue, Nov 6, 2018 at 2:52 AM Dave Hansen  wrote:
>
> On 11/5/18 3:13 AM, Kuo-Hsin Yang wrote:
> > -These are currently used in two places in the kernel:
> > +These are currently used in three places in the kernel:
> >
> >   (1) By ramfs to mark the address spaces of its inodes when they are 
> > created,
> >   and this mark remains for the life of the inode.
> > @@ -154,6 +154,8 @@ These are currently used in two places in the kernel:
> >   swapped out; the application must touch the pages manually if it 
> > wants to
> >   ensure they're in memory.
> >
> > + (3) By the i915 driver to mark pinned address space until it's unpinned.
>
> At a minimum, I think we owe some documentation here of how to tell
> approximately how much memory i915 is consuming with this mechanism.
> The debugfs stuff sounds like a halfway reasonable way to approximate
> it, although it's imperfect.

OK, I will add more comments here.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH v5] mm, drm/i915: mark pinned shmemfs pages as unevictable

2018-11-06 Thread Kuo-Hsin Yang
The i915 driver uses shmemfs to allocate backing storage for gem
objects. These shmemfs pages can be pinned (increased ref count) by
shmem_read_mapping_page_gfp(). When a lot of pages are pinned, vmscan
wastes a lot of time scanning these pinned pages. In some extreme case,
all pages in the inactive anon lru are pinned, and only the inactive
anon lru is scanned due to inactive_ratio, the system cannot swap and
invokes the oom-killer. Mark these pinned pages as unevictable to speed
up vmscan.

Export pagevec API check_move_unevictable_pages().

This patch was inspired by Chris Wilson's change [1].

[1]: https://patchwork.kernel.org/patch/9768741/

Cc: Chris Wilson 
Cc: Joonas Lahtinen 
Cc: Peter Zijlstra 
Cc: Andrew Morton 
Cc: Dave Hansen 
Signed-off-by: Kuo-Hsin Yang 
Acked-by: Michal Hocko 
---
Changes for v5:
 Modify doc and comments. Remove the ifdef surrounding
 check_move_unevictable_pages.

Changes for v4:
 Export pagevec API check_move_unevictable_pages().

Changes for v3:
 Use check_move_lru_page instead of shmem_unlock_mapping to move pages
 to appropriate lru lists.

Changes for v2:
 Squashed the two patches.

 Documentation/vm/unevictable-lru.rst |  6 +-
 drivers/gpu/drm/i915/i915_gem.c  | 28 ++--
 include/linux/swap.h |  4 +++-
 mm/shmem.c   |  2 +-
 mm/vmscan.c  | 22 +++---
 5 files changed, 46 insertions(+), 16 deletions(-)

diff --git a/Documentation/vm/unevictable-lru.rst 
b/Documentation/vm/unevictable-lru.rst
index fdd84cb8d511..b8e29f977f2d 100644
--- a/Documentation/vm/unevictable-lru.rst
+++ b/Documentation/vm/unevictable-lru.rst
@@ -143,7 +143,7 @@ using a number of wrapper functions:
Query the address space, and return true if it is completely
unevictable.
 
-These are currently used in two places in the kernel:
+These are currently used in three places in the kernel:
 
  (1) By ramfs to mark the address spaces of its inodes when they are created,
  and this mark remains for the life of the inode.
@@ -154,6 +154,10 @@ These are currently used in two places in the kernel:
  swapped out; the application must touch the pages manually if it wants to
  ensure they're in memory.
 
+ (3) By the i915 driver to mark pinned address space until it's unpinned. The
+ amount of unevictable memory marked by i915 driver is roughly the bounded
+ object size in debugfs/dri/0/i915_gem_objects.
+
 
 Detecting Unevictable Pages
 ---
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0c8aa57ce83b..c620891e0d02 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2381,12 +2381,25 @@ void __i915_gem_object_invalidate(struct 
drm_i915_gem_object *obj)
invalidate_mapping_pages(mapping, 0, (loff_t)-1);
 }
 
+/**
+ * Move pages to appropriate lru and release the pagevec. Decrement the ref
+ * count of these pages.
+ */
+static inline void check_release_pagevec(struct pagevec *pvec)
+{
+   if (pagevec_count(pvec)) {
+   check_move_unevictable_pages(pvec);
+   __pagevec_release(pvec);
+   }
+}
+
 static void
 i915_gem_object_put_pages_gtt(struct drm_i915_gem_object *obj,
  struct sg_table *pages)
 {
struct sgt_iter sgt_iter;
struct page *page;
+   struct pagevec pvec;
 
__i915_gem_object_release_shmem(obj, pages, true);
 
@@ -2395,6 +2408,9 @@ i915_gem_object_put_pages_gtt(struct drm_i915_gem_object 
*obj,
if (i915_gem_object_needs_bit17_swizzle(obj))
i915_gem_object_save_bit_17_swizzle(obj, pages);
 
+   mapping_clear_unevictable(file_inode(obj->base.filp)->i_mapping);
+
+   pagevec_init(&pvec);
for_each_sgt_page(page, sgt_iter, pages) {
if (obj->mm.dirty)
set_page_dirty(page);
@@ -2402,8 +2418,10 @@ i915_gem_object_put_pages_gtt(struct drm_i915_gem_object 
*obj,
if (obj->mm.madv == I915_MADV_WILLNEED)
mark_page_accessed(page);
 
-   put_page(page);
+   if (!pagevec_add(&pvec, page))
+   check_release_pagevec(&pvec);
}
+   check_release_pagevec(&pvec);
obj->mm.dirty = false;
 
sg_free_table(pages);
@@ -2526,6 +2544,7 @@ static int i915_gem_object_get_pages_gtt(struct 
drm_i915_gem_object *obj)
unsigned int sg_page_sizes;
gfp_t noreclaim;
int ret;
+   struct pagevec pvec;
 
/*
 * Assert that the object is not currently in any GPU domain. As it
@@ -2559,6 +2578,7 @@ static int i915_gem_object_get_pages_gtt(struct 
drm_i915_gem_object *obj)
 * Fail silently without starting the shrinker
 */
mapping = obj->base.filp->f_mapping;
+   mapping_set_unevictable(mapping)

Re: [Intel-gfx] [PATCH v5] mm, drm/i915: mark pinned shmemfs pages as unevictable

2018-11-06 Thread Kuo-Hsin Yang
On Tue, Nov 6, 2018 at 5:08 PM Michal Hocko  wrote:
>
> On Tue 06-11-18 17:03:51, Kuo-Hsin Yang wrote:
> > The i915 driver uses shmemfs to allocate backing storage for gem
> > objects. These shmemfs pages can be pinned (increased ref count) by
> > shmem_read_mapping_page_gfp(). When a lot of pages are pinned, vmscan
> > wastes a lot of time scanning these pinned pages. In some extreme case,
> > all pages in the inactive anon lru are pinned, and only the inactive
> > anon lru is scanned due to inactive_ratio, the system cannot swap and
> > invokes the oom-killer. Mark these pinned pages as unevictable to speed
> > up vmscan.
> >
> > Export pagevec API check_move_unevictable_pages().
> >
> > This patch was inspired by Chris Wilson's change [1].
> >
> > [1]: https://patchwork.kernel.org/patch/9768741/
> >
> > Cc: Chris Wilson 
> > Cc: Joonas Lahtinen 
> > Cc: Peter Zijlstra 
> > Cc: Andrew Morton 
> > Cc: Dave Hansen 
> > Signed-off-by: Kuo-Hsin Yang 
> > Acked-by: Michal Hocko 
>
> please make it explicit that the ack applies to mm part as i've
> mentioned when giving my ack to the previous version.
>
> E.g.
> Acked-by: Michal Hocko  # mm part
>
> because i am not familiar with the drm code to ack any changes there.

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


[Intel-gfx] [PATCH v6] mm, drm/i915: mark pinned shmemfs pages as unevictable

2018-11-06 Thread Kuo-Hsin Yang
The i915 driver uses shmemfs to allocate backing storage for gem
objects. These shmemfs pages can be pinned (increased ref count) by
shmem_read_mapping_page_gfp(). When a lot of pages are pinned, vmscan
wastes a lot of time scanning these pinned pages. In some extreme case,
all pages in the inactive anon lru are pinned, and only the inactive
anon lru is scanned due to inactive_ratio, the system cannot swap and
invokes the oom-killer. Mark these pinned pages as unevictable to speed
up vmscan.

Export pagevec API check_move_unevictable_pages().

This patch was inspired by Chris Wilson's change [1].

[1]: https://patchwork.kernel.org/patch/9768741/

Cc: Chris Wilson 
Cc: Joonas Lahtinen 
Cc: Peter Zijlstra 
Cc: Andrew Morton 
Cc: Dave Hansen 
Signed-off-by: Kuo-Hsin Yang 
Acked-by: Michal Hocko  # mm part
---
Changes for v6:
 Tweak the acked-by.

Changes for v5:
 Modify doc and comments. Remove the ifdef surrounding
 check_move_unevictable_pages.

Changes for v4:
 Export pagevec API check_move_unevictable_pages().

Changes for v3:
 Use check_move_lru_page instead of shmem_unlock_mapping to move pages
 to appropriate lru lists.

Changes for v2:
 Squashed the two patches.

 Documentation/vm/unevictable-lru.rst |  6 +-
 drivers/gpu/drm/i915/i915_gem.c  | 28 ++--
 include/linux/swap.h |  4 +++-
 mm/shmem.c   |  2 +-
 mm/vmscan.c  | 22 +++---
 5 files changed, 46 insertions(+), 16 deletions(-)

diff --git a/Documentation/vm/unevictable-lru.rst 
b/Documentation/vm/unevictable-lru.rst
index fdd84cb8d511..b8e29f977f2d 100644
--- a/Documentation/vm/unevictable-lru.rst
+++ b/Documentation/vm/unevictable-lru.rst
@@ -143,7 +143,7 @@ using a number of wrapper functions:
Query the address space, and return true if it is completely
unevictable.
 
-These are currently used in two places in the kernel:
+These are currently used in three places in the kernel:
 
  (1) By ramfs to mark the address spaces of its inodes when they are created,
  and this mark remains for the life of the inode.
@@ -154,6 +154,10 @@ These are currently used in two places in the kernel:
  swapped out; the application must touch the pages manually if it wants to
  ensure they're in memory.
 
+ (3) By the i915 driver to mark pinned address space until it's unpinned. The
+ amount of unevictable memory marked by i915 driver is roughly the bounded
+ object size in debugfs/dri/0/i915_gem_objects.
+
 
 Detecting Unevictable Pages
 ---
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 0c8aa57ce83b..c620891e0d02 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2381,12 +2381,25 @@ void __i915_gem_object_invalidate(struct 
drm_i915_gem_object *obj)
invalidate_mapping_pages(mapping, 0, (loff_t)-1);
 }
 
+/**
+ * Move pages to appropriate lru and release the pagevec. Decrement the ref
+ * count of these pages.
+ */
+static inline void check_release_pagevec(struct pagevec *pvec)
+{
+   if (pagevec_count(pvec)) {
+   check_move_unevictable_pages(pvec);
+   __pagevec_release(pvec);
+   }
+}
+
 static void
 i915_gem_object_put_pages_gtt(struct drm_i915_gem_object *obj,
  struct sg_table *pages)
 {
struct sgt_iter sgt_iter;
struct page *page;
+   struct pagevec pvec;
 
__i915_gem_object_release_shmem(obj, pages, true);
 
@@ -2395,6 +2408,9 @@ i915_gem_object_put_pages_gtt(struct drm_i915_gem_object 
*obj,
if (i915_gem_object_needs_bit17_swizzle(obj))
i915_gem_object_save_bit_17_swizzle(obj, pages);
 
+   mapping_clear_unevictable(file_inode(obj->base.filp)->i_mapping);
+
+   pagevec_init(&pvec);
for_each_sgt_page(page, sgt_iter, pages) {
if (obj->mm.dirty)
set_page_dirty(page);
@@ -2402,8 +2418,10 @@ i915_gem_object_put_pages_gtt(struct drm_i915_gem_object 
*obj,
if (obj->mm.madv == I915_MADV_WILLNEED)
mark_page_accessed(page);
 
-   put_page(page);
+   if (!pagevec_add(&pvec, page))
+   check_release_pagevec(&pvec);
}
+   check_release_pagevec(&pvec);
obj->mm.dirty = false;
 
sg_free_table(pages);
@@ -2526,6 +2544,7 @@ static int i915_gem_object_get_pages_gtt(struct 
drm_i915_gem_object *obj)
unsigned int sg_page_sizes;
gfp_t noreclaim;
int ret;
+   struct pagevec pvec;
 
/*
 * Assert that the object is not currently in any GPU domain. As it
@@ -2559,6 +2578,7 @@ static int i915_gem_object_get_pages_gtt(struct 
drm_i915_gem_object *obj)
 * Fail silently without starting the shrinker
 */
mapping = obj->base.filp-&g

Re: [Intel-gfx] [PATCH v6] mm, drm/i915: mark pinned shmemfs pages as unevictable

2018-11-06 Thread Kuo-Hsin Yang
On Tue, Nov 6, 2018 at 7:07 PM Chris Wilson  wrote:
> This gave disappointing syslatency results until I put a cond_resched()
> here and moved the one in put_pages_gtt to before the page alloc, see
> https://patchwork.freedesktop.org/patch/260332/
>
> The last really nasty wart for syslatency is the spin in
> i915_gem_shrinker, for which I'm investigating
> https://patchwork.freedesktop.org/patch/260365/
>
> All 3 patches together give very reasonable syslatency results! (So
> good that it's time to find a new worst case scenario!)
>
> The challenge for the patch as it stands, is who lands it? We can take
> it through drm-intel (for merging in 4.21) but need Andrew's ack on top
> of all to agree with that path. Or we split the patch and only land the
> i915 portion once we backmerge the mm tree. I think pushing the i915
> portion through the mm tree is going to cause the most conflicts, so
> would recommend against that.

Splitting the patch and landing the mm part first sounds reasonable to me.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v7] mm, drm/i915: mark pinned shmemfs pages as unevictable

2018-11-06 Thread Kuo-Hsin Yang
On Tue, Nov 6, 2018 at 9:23 PM Chris Wilson  wrote:
> Cc: Chris Wilson 
> Cc: Joonas Lahtinen 
> Cc: Peter Zijlstra 
> Cc: Andrew Morton 
> Cc: Dave Hansen 
> Signed-off-by: Kuo-Hsin Yang 
> Acked-by: Michal Hocko  # mm part
> Reviewed-by: Chris Wilson 

Thanks for your fixes and review.
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


Re: [Intel-gfx] [PATCH v6] mm, drm/i915: mark pinned shmemfs pages as unevictable

2018-11-06 Thread Kuo-Hsin Yang
On Tue, Nov 6, 2018 at 6:54 PM Daniel Vetter  wrote:
> There was ages ago some planes to have our own i915fs, so that we could
> overwrite the address_space hooks for page migration and eviction and that
> sort of thing, which would make all these pages evictable. Atm you have to
> ĥope our shrinker drops them on the floor, which I think is fairly
> confusing to core mm code (it's kinda like page eviction worked way back
> before rmaps).
>

Thanks for the explanation. Your blog posts helped a lot to get me
started on hacking drm/i915 driver.

> Just an side really.
> -Daniel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx


[Intel-gfx] [PATCH 0/2] shmem, drm/i915: Mark pinned shmemfs pages as unevictable

2018-10-16 Thread Kuo-Hsin Yang
When a graphics heavy application is running, i915 driver may pin a lot
of shmemfs pages and vmscan slows down significantly by scanning these
pinned pages. This patch is an alternative to the patch by Chris Wilson
[1]. As i915 driver pins all pages in an address space, marking an
address space as unevictable is sufficient to solve this issue.

[1]: https://patchwork.kernel.org/patch/9768741/

Kuo-Hsin Yang (2):
  shmem: export shmem_unlock_mapping
  drm/i915: Mark pinned shmemfs pages as unevictable

 Documentation/vm/unevictable-lru.rst | 4 +++-
 drivers/gpu/drm/i915/i915_gem.c  | 8 
 mm/shmem.c   | 2 ++
 3 files changed, 13 insertions(+), 1 deletion(-)

-- 
2.19.1.331.ge82ca0e54c-goog

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


[Intel-gfx] [PATCH 1/2] shmem: export shmem_unlock_mapping

2018-10-16 Thread Kuo-Hsin Yang
By exporting this function, drivers can mark/unmark a shmemfs address
space as unevictable in the following way: 1. mark an address space as
unevictable with mapping_set_unevictable(), pages in the address space
will be moved to unevictable list in vmscan. 2. mark an address space
evictable with mapping_clear_unevictable(), and move these pages back to
evictable list with shmem_unlock_mapping().

Signed-off-by: Kuo-Hsin Yang 
---
 Documentation/vm/unevictable-lru.rst | 4 +++-
 mm/shmem.c   | 2 ++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/Documentation/vm/unevictable-lru.rst 
b/Documentation/vm/unevictable-lru.rst
index fdd84cb8d511..a812fb55136d 100644
--- a/Documentation/vm/unevictable-lru.rst
+++ b/Documentation/vm/unevictable-lru.rst
@@ -143,7 +143,7 @@ using a number of wrapper functions:
Query the address space, and return true if it is completely
unevictable.
 
-These are currently used in two places in the kernel:
+These are currently used in three places in the kernel:
 
  (1) By ramfs to mark the address spaces of its inodes when they are created,
  and this mark remains for the life of the inode.
@@ -154,6 +154,8 @@ These are currently used in two places in the kernel:
  swapped out; the application must touch the pages manually if it wants to
  ensure they're in memory.
 
+ (3) By the i915 driver to mark pinned address space until it's unpinned.
+
 
 Detecting Unevictable Pages
 ---
diff --git a/mm/shmem.c b/mm/shmem.c
index 446942677cd4..d1ce34c09df6 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -786,6 +786,7 @@ void shmem_unlock_mapping(struct address_space *mapping)
cond_resched();
}
 }
+EXPORT_SYMBOL_GPL(shmem_unlock_mapping);
 
 /*
  * Remove range of pages and swap entries from radix tree, and free them.
@@ -3874,6 +3875,7 @@ int shmem_lock(struct file *file, int lock, struct 
user_struct *user)
 void shmem_unlock_mapping(struct address_space *mapping)
 {
 }
+EXPORT_SYMBOL_GPL(shmem_unlock_mapping);
 
 #ifdef CONFIG_MMU
 unsigned long shmem_get_unmapped_area(struct file *file,
-- 
2.19.1.331.ge82ca0e54c-goog

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


[Intel-gfx] [PATCH 2/2] drm/i915: Mark pinned shmemfs pages as unevictable

2018-10-16 Thread Kuo-Hsin Yang
The i915 driver use shmemfs to allocate backing storage for gem objects.
These shmemfs pages can be pinned (increased ref count) by
shmem_read_mapping_page_gfp(). When a lot of pages are pinned, vmscan
wastes a lot of time scanning these pinned pages. Mark these pinned
pages as unevictable to speed up vmscan.

Signed-off-by: Kuo-Hsin Yang 
---
 drivers/gpu/drm/i915/i915_gem.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index fcc73a6ab503..e0ff5b736128 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2390,6 +2390,7 @@ i915_gem_object_put_pages_gtt(struct drm_i915_gem_object 
*obj,
 {
struct sgt_iter sgt_iter;
struct page *page;
+   struct address_space *mapping;
 
__i915_gem_object_release_shmem(obj, pages, true);
 
@@ -2409,6 +2410,10 @@ i915_gem_object_put_pages_gtt(struct drm_i915_gem_object 
*obj,
}
obj->mm.dirty = false;
 
+   mapping = file_inode(obj->base.filp)->i_mapping;
+   mapping_clear_unevictable(mapping);
+   shmem_unlock_mapping(mapping);
+
sg_free_table(pages);
kfree(pages);
 }
@@ -2551,6 +2556,7 @@ static int i915_gem_object_get_pages_gtt(struct 
drm_i915_gem_object *obj)
 * Fail silently without starting the shrinker
 */
mapping = obj->base.filp->f_mapping;
+   mapping_set_unevictable(mapping);
noreclaim = mapping_gfp_constraint(mapping, ~__GFP_RECLAIM);
noreclaim |= __GFP_NORETRY | __GFP_NOWARN;
 
@@ -2664,6 +2670,8 @@ static int i915_gem_object_get_pages_gtt(struct 
drm_i915_gem_object *obj)
 err_pages:
for_each_sgt_page(page, sgt_iter, st)
put_page(page);
+   mapping_clear_unevictable(mapping);
+   shmem_unlock_mapping(mapping);
sg_free_table(st);
kfree(st);
 
-- 
2.19.1.331.ge82ca0e54c-goog

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


[Intel-gfx] [PATCH v2] shmem, drm/i915: mark pinned shmemfs pages as unevictable

2018-10-17 Thread Kuo-Hsin Yang
The i915 driver uses shmemfs to allocate backing storage for gem
objects. These shmemfs pages can be pinned (increased ref count) by
shmem_read_mapping_page_gfp(). When a lot of pages are pinned, vmscan
wastes a lot of time scanning these pinned pages. In some extreme case,
all pages in the inactive anon lru are pinned, and only the inactive
anon lru is scanned due to inactive_ratio, the system cannot swap and
invokes the oom-killer. Mark these pinned pages as unevictable to speed
up vmscan.

By exporting shmem_unlock_mapping, drivers can: 1. mark a shmemfs
address space as unevictable with mapping_set_unevictable(), pages in
the address space will be moved to unevictable list in vmscan. 2. mark
an address space as evictable with mapping_clear_unevictable(), and move
these pages back to evictable list with shmem_unlock_mapping().

This patch was inspired by Chris Wilson's change [1].

[1]: https://patchwork.kernel.org/patch/9768741/

Signed-off-by: Kuo-Hsin Yang 
---
Changes for v2:
 Squashed the two patches.

 Documentation/vm/unevictable-lru.rst | 4 +++-
 drivers/gpu/drm/i915/i915_gem.c  | 8 
 mm/shmem.c   | 2 ++
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/Documentation/vm/unevictable-lru.rst 
b/Documentation/vm/unevictable-lru.rst
index fdd84cb8d511..a812fb55136d 100644
--- a/Documentation/vm/unevictable-lru.rst
+++ b/Documentation/vm/unevictable-lru.rst
@@ -143,7 +143,7 @@ using a number of wrapper functions:
Query the address space, and return true if it is completely
unevictable.
 
-These are currently used in two places in the kernel:
+These are currently used in three places in the kernel:
 
  (1) By ramfs to mark the address spaces of its inodes when they are created,
  and this mark remains for the life of the inode.
@@ -154,6 +154,8 @@ These are currently used in two places in the kernel:
  swapped out; the application must touch the pages manually if it wants to
  ensure they're in memory.
 
+ (3) By the i915 driver to mark pinned address space until it's unpinned.
+
 
 Detecting Unevictable Pages
 ---
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index fcc73a6ab503..e0ff5b736128 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2390,6 +2390,7 @@ i915_gem_object_put_pages_gtt(struct drm_i915_gem_object 
*obj,
 {
struct sgt_iter sgt_iter;
struct page *page;
+   struct address_space *mapping;
 
__i915_gem_object_release_shmem(obj, pages, true);
 
@@ -2409,6 +2410,10 @@ i915_gem_object_put_pages_gtt(struct drm_i915_gem_object 
*obj,
}
obj->mm.dirty = false;
 
+   mapping = file_inode(obj->base.filp)->i_mapping;
+   mapping_clear_unevictable(mapping);
+   shmem_unlock_mapping(mapping);
+
sg_free_table(pages);
kfree(pages);
 }
@@ -2551,6 +2556,7 @@ static int i915_gem_object_get_pages_gtt(struct 
drm_i915_gem_object *obj)
 * Fail silently without starting the shrinker
 */
mapping = obj->base.filp->f_mapping;
+   mapping_set_unevictable(mapping);
noreclaim = mapping_gfp_constraint(mapping, ~__GFP_RECLAIM);
noreclaim |= __GFP_NORETRY | __GFP_NOWARN;
 
@@ -2664,6 +2670,8 @@ static int i915_gem_object_get_pages_gtt(struct 
drm_i915_gem_object *obj)
 err_pages:
for_each_sgt_page(page, sgt_iter, st)
put_page(page);
+   mapping_clear_unevictable(mapping);
+   shmem_unlock_mapping(mapping);
sg_free_table(st);
kfree(st);
 
diff --git a/mm/shmem.c b/mm/shmem.c
index 446942677cd4..d1ce34c09df6 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -786,6 +786,7 @@ void shmem_unlock_mapping(struct address_space *mapping)
cond_resched();
}
 }
+EXPORT_SYMBOL_GPL(shmem_unlock_mapping);
 
 /*
  * Remove range of pages and swap entries from radix tree, and free them.
@@ -3874,6 +3875,7 @@ int shmem_lock(struct file *file, int lock, struct 
user_struct *user)
 void shmem_unlock_mapping(struct address_space *mapping)
 {
 }
+EXPORT_SYMBOL_GPL(shmem_unlock_mapping);
 
 #ifdef CONFIG_MMU
 unsigned long shmem_get_unmapped_area(struct file *file,
-- 
2.19.1.331.ge82ca0e54c-goog

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