Re: [RFC 0/3] zsmalloc: make its pages movable

Re: [RFC 0/3] zsmalloc: make its pages movable

[Sergey Senozhatsky]

On (10/08/15 14:35), Hui Zhu wrote:
> 
> As the discussion in the list, the zsmalloc introduce some problems
> around pages because its pages are unmovable.
> 
> These patches introduced page move function to zsmalloc.  And they also
> add interface to struct page.
> 

Hi,

have you seen
 http://lkml.iu.edu/hypermail/linux/kernel/1507.0/03233.html
 http://lkml.iu.edu/hypermail/linux/kernel/1508.1/00696.html

?


	-ss

> Hui Zhu (3):
> page: add new flags "PG_movable" and add interfaces to control these pages
> zsmalloc: mark its page "PG_movable"
> zram: make create "__GFP_MOVABLE" pool
>  drivers/block/zram/zram_drv.c |    4 
>  include/linux/mm_types.h      |   11 +
>  include/linux/page-flags.h    |    3 
>  mm/compaction.c               |    6 
>  mm/debug.c                    |    1 
>  mm/migrate.c                  |   17 +
>  mm/vmscan.c                   |    2 
>  mm/zsmalloc.c                 |  409 ++++++++++++++++++++++++++++++++++++++++--
>  8 files changed, 428 insertions(+), 25 deletions(-)
> 

Re: [PATCH 1/3] page: add new flags "PG_movable" and add interfaces to control these pages

[Vlastimil Babka]

On 10/08/2015 08:35 AM, Hui Zhu wrote:
> This patch add PG_movable to mark a page as movable.
> And when system call migrate function, it will call the interfaces isolate,
> put and migrate to control it.
>
> There is a patch for page migrate interface in LKML.  But for zsmalloc,
> it is too deep inside the file system.  So I add another one.
>
> Signed-off-by: Hui Zhu <zhuhui@xiaomi.com>
> ---
>   include/linux/mm_types.h   |  6 ++++++
>   include/linux/page-flags.h |  3 +++
>   mm/compaction.c            |  6 ++++++
>   mm/debug.c                 |  1 +
>   mm/migrate.c               | 17 +++++++++++++----
>   mm/vmscan.c                |  2 +-
>   6 files changed, 30 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 3d6baa7..132afb0 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -12,6 +12,7 @@
>   #include <linux/cpumask.h>
>   #include <linux/uprobes.h>
>   #include <linux/page-flags-layout.h>
> +#include <linux/migrate_mode.h>
>   #include <asm/page.h>
>   #include <asm/mmu.h>
>
> @@ -196,6 +197,11 @@ struct page {
>   #ifdef LAST_CPUPID_NOT_IN_PAGE_FLAGS
>   	int _last_cpupid;
>   #endif
> +
> +	int (*isolate)(struct page *page);
> +	void (*put)(struct page *page);
> +	int (*migrate)(struct page *page, struct page *newpage, int force,
> +		       enum migrate_mode mode);

Three new function pointers in a struct page? No way! Nowadays it has 
around 64 bytes IIRC and we do quite some crazy stuff to keep it packed.
We can't just like this add extra 24 bytes of overhead per 4096 bytes of 
useful data.

>   }
>   /*
>    * The struct page can be forced to be double word aligned so that atomic ops
> diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
> index 416509e..d91e98a 100644
> --- a/include/linux/page-flags.h
> +++ b/include/linux/page-flags.h
> @@ -113,6 +113,7 @@ enum pageflags {
>   	PG_young,
>   	PG_idle,
>   #endif
> +	PG_movable,		/* MOVABLE */

Page flag space is also a rare resource and we shouldn't add new ones if 
it's possible to do otherwise - and it should be in this case.

Since Sergey already responded to the cover letter with links to the 
prior relevant series, I just wanted to point out the biggest reasons 
why this cannot be accepted technically.

Re: [PATCH 1/3] page: add new flags "PG_movable" and add interfaces to control these pages

[Kirill A. Shutemov]

On Thu, Oct 08, 2015 at 02:35:50PM +0800, Hui Zhu wrote:
> This patch add PG_movable to mark a page as movable.
> And when system call migrate function, it will call the interfaces isolate,
> put and migrate to control it.
> 
> There is a patch for page migrate interface in LKML.  But for zsmalloc,
> it is too deep inside the file system.  So I add another one.
> 
> Signed-off-by: Hui Zhu <zhuhui@xiaomi.com>
> ---
>  include/linux/mm_types.h   |  6 ++++++
>  include/linux/page-flags.h |  3 +++
>  mm/compaction.c            |  6 ++++++
>  mm/debug.c                 |  1 +
>  mm/migrate.c               | 17 +++++++++++++----
>  mm/vmscan.c                |  2 +-
>  6 files changed, 30 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> index 3d6baa7..132afb0 100644
> --- a/include/linux/mm_types.h
> +++ b/include/linux/mm_types.h
> @@ -12,6 +12,7 @@
>  #include <linux/cpumask.h>
>  #include <linux/uprobes.h>
>  #include <linux/page-flags-layout.h>
> +#include <linux/migrate_mode.h>
>  #include <asm/page.h>
>  #include <asm/mmu.h>
>  
> @@ -196,6 +197,11 @@ struct page {
>  #ifdef LAST_CPUPID_NOT_IN_PAGE_FLAGS
>  	int _last_cpupid;
>  #endif
> +
> +	int (*isolate)(struct page *page);
> +	void (*put)(struct page *page);
> +	int (*migrate)(struct page *page, struct page *newpage, int force,
> +		       enum migrate_mode mode);
>  }

That's no-go. We are not going to add three pointers to struct page. It
would cost ~0.5% of system memory.

NAK.

-- 
 Kirill A. Shutemov