[PATCH] zsmalloc: Add Kconfig for enabling PTE method

[PATCH] zsmalloc: Add Kconfig for enabling PTE method

[Minchan Kim]

Zsmalloc has two methods 1) copy-based and 2) pte based to access
allocations that span two pages.
You can see history why we supported two approach from [1].

But it was bad choice that adding hard coding to select architecture
which want to use pte based method. This patch removed it and adds
new Kconfig to select the approach.

This patch is based on next-20130202.

[1] https://lkml.org/lkml/2012/7/11/58

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Seth Jennings <sjenning@linux.vnet.ibm.com>
Cc: Nitin Gupta <ngupta@vflare.org>
Cc: Dan Magenheimer <dan.magenheimer@oracle.com>
Cc: Konrad Rzeszutek Wilk <konrad@darnok.org>
Signed-off-by: Minchan Kim <minchan@kernel.org>
---
 drivers/staging/zsmalloc/Kconfig         |   12 ++++++++++++
 drivers/staging/zsmalloc/zsmalloc-main.c |   11 -----------
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/zsmalloc/Kconfig b/drivers/staging/zsmalloc/Kconfig
index 9084565..2359123 100644
--- a/drivers/staging/zsmalloc/Kconfig
+++ b/drivers/staging/zsmalloc/Kconfig
@@ -8,3 +8,15 @@ config ZSMALLOC
 	  non-standard allocator interface where a handle, not a pointer, is
 	  returned by an alloc().  This handle must be mapped in order to
 	  access the allocated space.
+
+config ZSMALLOC_PGTABLE_MAPPING
+        bool "Use page table mapping to access allocations that span two pages"
+        depends on ZSMALLOC
+        default n
+        help
+	  By default, zsmalloc uses a copy-based object mapping method to access
+	  allocations that span two pages. However, if a particular architecture
+	  performs VM mapping faster than copying, then you should select this.
+	  This causes zsmalloc to use page table mapping rather than copying
+	  for object mapping. You can check speed with zsmalloc benchmark[1].
+	  [1] https://github.com/spartacus06/zsmalloc
diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c
index 06f73a9..b161ca1 100644
--- a/drivers/staging/zsmalloc/zsmalloc-main.c
+++ b/drivers/staging/zsmalloc/zsmalloc-main.c
@@ -218,17 +218,6 @@ struct zs_pool {
 #define CLASS_IDX_MASK	((1 << CLASS_IDX_BITS) - 1)
 #define FULLNESS_MASK	((1 << FULLNESS_BITS) - 1)
 
-/*
- * By default, zsmalloc uses a copy-based object mapping method to access
- * allocations that span two pages. However, if a particular architecture
- * performs VM mapping faster than copying, then it should be added here
- * so that USE_PGTABLE_MAPPING is defined. This causes zsmalloc to use
- * page table mapping rather than copying for object mapping.
-*/
-#if defined(CONFIG_ARM)
-#define USE_PGTABLE_MAPPING
-#endif
-
 struct mapping_area {
 #ifdef USE_PGTABLE_MAPPING
 	struct vm_struct *vm; /* vm area for mapping object that span pages */
-- 
1.7.9.5

Re: [PATCH] zsmalloc: Add Kconfig for enabling PTE method

[Simon Jeons]

On Mon, 2013-02-04 at 09:23 +0900, Minchan Kim wrote:
> Zsmalloc has two methods 1) copy-based and 2) pte based to access
> allocations that span two pages.
> You can see history why we supported two approach from [1].
> 
> But it was bad choice that adding hard coding to select architecture
> which want to use pte based method. This patch removed it and adds
> new Kconfig to select the approach.
> 
> This patch is based on next-20130202.

What's the meaning of 'zs' in zsmalloc? It's short for what?
 
> 
> [1] https://lkml.org/lkml/2012/7/11/58
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Seth Jennings <sjenning@linux.vnet.ibm.com>
> Cc: Nitin Gupta <ngupta@vflare.org>
> Cc: Dan Magenheimer <dan.magenheimer@oracle.com>
> Cc: Konrad Rzeszutek Wilk <konrad@darnok.org>
> Signed-off-by: Minchan Kim <minchan@kernel.org>
> ---
>  drivers/staging/zsmalloc/Kconfig         |   12 ++++++++++++
>  drivers/staging/zsmalloc/zsmalloc-main.c |   11 -----------
>  2 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/zsmalloc/Kconfig b/drivers/staging/zsmalloc/Kconfig
> index 9084565..2359123 100644
> --- a/drivers/staging/zsmalloc/Kconfig
> +++ b/drivers/staging/zsmalloc/Kconfig
> @@ -8,3 +8,15 @@ config ZSMALLOC
>  	  non-standard allocator interface where a handle, not a pointer, is
>  	  returned by an alloc().  This handle must be mapped in order to
>  	  access the allocated space.
> +
> +config ZSMALLOC_PGTABLE_MAPPING
> +        bool "Use page table mapping to access allocations that span two pages"
> +        depends on ZSMALLOC
> +        default n
> +        help
> +	  By default, zsmalloc uses a copy-based object mapping method to access
> +	  allocations that span two pages. However, if a particular architecture
> +	  performs VM mapping faster than copying, then you should select this.
> +	  This causes zsmalloc to use page table mapping rather than copying
> +	  for object mapping. You can check speed with zsmalloc benchmark[1].
> +	  [1] https://github.com/spartacus06/zsmalloc
> diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c
> index 06f73a9..b161ca1 100644
> --- a/drivers/staging/zsmalloc/zsmalloc-main.c
> +++ b/drivers/staging/zsmalloc/zsmalloc-main.c
> @@ -218,17 +218,6 @@ struct zs_pool {
>  #define CLASS_IDX_MASK	((1 << CLASS_IDX_BITS) - 1)
>  #define FULLNESS_MASK	((1 << FULLNESS_BITS) - 1)
>  
> -/*
> - * By default, zsmalloc uses a copy-based object mapping method to access
> - * allocations that span two pages. However, if a particular architecture
> - * performs VM mapping faster than copying, then it should be added here
> - * so that USE_PGTABLE_MAPPING is defined. This causes zsmalloc to use
> - * page table mapping rather than copying for object mapping.
> -*/
> -#if defined(CONFIG_ARM)
> -#define USE_PGTABLE_MAPPING
> -#endif
> -
>  struct mapping_area {
>  #ifdef USE_PGTABLE_MAPPING
>  	struct vm_struct *vm; /* vm area for mapping object that span pages */

Re: [PATCH] zsmalloc: Add Kconfig for enabling PTE method

[Minchan Kim]

On Sun, Feb 03, 2013 at 08:02:09PM -0600, Simon Jeons wrote:
> On Mon, 2013-02-04 at 09:23 +0900, Minchan Kim wrote:
> > Zsmalloc has two methods 1) copy-based and 2) pte based to access
> > allocations that span two pages.
> > You can see history why we supported two approach from [1].
> > 
> > But it was bad choice that adding hard coding to select architecture
> > which want to use pte based method. This patch removed it and adds
> > new Kconfig to select the approach.
> > 
> > This patch is based on next-20130202.
> 
> What's the meaning of 'zs' in zsmalloc? It's short for what?

I'm not right person to answer but I guess it stands for compressed slab.

-- 
Kind regards,
Minchan Kim

Re: [PATCH] zsmalloc: Add Kconfig for enabling PTE method

[Greg Kroah-Hartman]

On Mon, Feb 04, 2013 at 09:23:41AM +0900, Minchan Kim wrote:
> Zsmalloc has two methods 1) copy-based and 2) pte based to access
> allocations that span two pages.
> You can see history why we supported two approach from [1].
> 
> But it was bad choice that adding hard coding to select architecture
> which want to use pte based method. This patch removed it and adds
> new Kconfig to select the approach.
> 
> This patch is based on next-20130202.
> 
> [1] https://lkml.org/lkml/2012/7/11/58
> 
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Seth Jennings <sjenning@linux.vnet.ibm.com>
> Cc: Nitin Gupta <ngupta@vflare.org>
> Cc: Dan Magenheimer <dan.magenheimer@oracle.com>
> Cc: Konrad Rzeszutek Wilk <konrad@darnok.org>
> Signed-off-by: Minchan Kim <minchan@kernel.org>
> ---
>  drivers/staging/zsmalloc/Kconfig         |   12 ++++++++++++
>  drivers/staging/zsmalloc/zsmalloc-main.c |   11 -----------
>  2 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/staging/zsmalloc/Kconfig b/drivers/staging/zsmalloc/Kconfig
> index 9084565..2359123 100644
> --- a/drivers/staging/zsmalloc/Kconfig
> +++ b/drivers/staging/zsmalloc/Kconfig
> @@ -8,3 +8,15 @@ config ZSMALLOC
>  	  non-standard allocator interface where a handle, not a pointer, is
>  	  returned by an alloc().  This handle must be mapped in order to
>  	  access the allocated space.
> +
> +config ZSMALLOC_PGTABLE_MAPPING
> +        bool "Use page table mapping to access allocations that span two pages"
> +        depends on ZSMALLOC
> +        default n
> +        help
> +	  By default, zsmalloc uses a copy-based object mapping method to access
> +	  allocations that span two pages. However, if a particular architecture
> +	  performs VM mapping faster than copying, then you should select this.
> +	  This causes zsmalloc to use page table mapping rather than copying
> +	  for object mapping. You can check speed with zsmalloc benchmark[1].
> +	  [1] https://github.com/spartacus06/zsmalloc
> diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c
> index 06f73a9..b161ca1 100644
> --- a/drivers/staging/zsmalloc/zsmalloc-main.c
> +++ b/drivers/staging/zsmalloc/zsmalloc-main.c
> @@ -218,17 +218,6 @@ struct zs_pool {
>  #define CLASS_IDX_MASK	((1 << CLASS_IDX_BITS) - 1)
>  #define FULLNESS_MASK	((1 << FULLNESS_BITS) - 1)
>  
> -/*
> - * By default, zsmalloc uses a copy-based object mapping method to access
> - * allocations that span two pages. However, if a particular architecture
> - * performs VM mapping faster than copying, then it should be added here
> - * so that USE_PGTABLE_MAPPING is defined. This causes zsmalloc to use
> - * page table mapping rather than copying for object mapping.
> -*/
> -#if defined(CONFIG_ARM)
> -#define USE_PGTABLE_MAPPING
> -#endif

Did you test this?  I don't see the new config value you added actually
do anything in this code.  Also, if I select it incorrectly on ARM, or
or other platforms, what is keeping this from doing bad things?

thanks,

greg k-h

Re: [PATCH] zsmalloc: Add Kconfig for enabling PTE method

[Minchan Kim]

Hi Greg,

On Mon, Feb 04, 2013 at 10:51:46AM -0800, Greg Kroah-Hartman wrote:
> On Mon, Feb 04, 2013 at 09:23:41AM +0900, Minchan Kim wrote:
> > Zsmalloc has two methods 1) copy-based and 2) pte based to access
> > allocations that span two pages.
> > You can see history why we supported two approach from [1].
> > 
> > But it was bad choice that adding hard coding to select architecture
> > which want to use pte based method. This patch removed it and adds
> > new Kconfig to select the approach.
> > 
> > This patch is based on next-20130202.
> > 
> > [1] https://lkml.org/lkml/2012/7/11/58
> > 
> > Cc: Andrew Morton <akpm@linux-foundation.org>
> > Cc: Seth Jennings <sjenning@linux.vnet.ibm.com>
> > Cc: Nitin Gupta <ngupta@vflare.org>
> > Cc: Dan Magenheimer <dan.magenheimer@oracle.com>
> > Cc: Konrad Rzeszutek Wilk <konrad@darnok.org>
> > Signed-off-by: Minchan Kim <minchan@kernel.org>
> > ---
> >  drivers/staging/zsmalloc/Kconfig         |   12 ++++++++++++
> >  drivers/staging/zsmalloc/zsmalloc-main.c |   11 -----------
> >  2 files changed, 12 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/staging/zsmalloc/Kconfig b/drivers/staging/zsmalloc/Kconfig
> > index 9084565..2359123 100644
> > --- a/drivers/staging/zsmalloc/Kconfig
> > +++ b/drivers/staging/zsmalloc/Kconfig
> > @@ -8,3 +8,15 @@ config ZSMALLOC
> >  	  non-standard allocator interface where a handle, not a pointer, is
> >  	  returned by an alloc().  This handle must be mapped in order to
> >  	  access the allocated space.
> > +
> > +config ZSMALLOC_PGTABLE_MAPPING
> > +        bool "Use page table mapping to access allocations that span two pages"
> > +        depends on ZSMALLOC
> > +        default n
> > +        help
> > +	  By default, zsmalloc uses a copy-based object mapping method to access
> > +	  allocations that span two pages. However, if a particular architecture
> > +	  performs VM mapping faster than copying, then you should select this.
> > +	  This causes zsmalloc to use page table mapping rather than copying
> > +	  for object mapping. You can check speed with zsmalloc benchmark[1].
> > +	  [1] https://github.com/spartacus06/zsmalloc
> > diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c
> > index 06f73a9..b161ca1 100644
> > --- a/drivers/staging/zsmalloc/zsmalloc-main.c
> > +++ b/drivers/staging/zsmalloc/zsmalloc-main.c
> > @@ -218,17 +218,6 @@ struct zs_pool {
> >  #define CLASS_IDX_MASK	((1 << CLASS_IDX_BITS) - 1)
> >  #define FULLNESS_MASK	((1 << FULLNESS_BITS) - 1)
> >  
> > -/*
> > - * By default, zsmalloc uses a copy-based object mapping method to access
> > - * allocations that span two pages. However, if a particular architecture
> > - * performs VM mapping faster than copying, then it should be added here
> > - * so that USE_PGTABLE_MAPPING is defined. This causes zsmalloc to use
> > - * page table mapping rather than copying for object mapping.
> > -*/
> > -#if defined(CONFIG_ARM)
> > -#define USE_PGTABLE_MAPPING
> > -#endif
> 
> Did you test this?  I don't see the new config value you added actually
> do anything in this code.  Also, if I select it incorrectly on ARM, or

*slaps self*

> or other platforms, what is keeping this from doing bad things?

There is no way to prevent it now.
I thought a way to detect it dynamically by testing performance
both approaches in booting/module-loading time and select the best choice.
For it, we should add benchmark code and delay booting/module-loading,
it's not good for embedded system because they are fighting with 300msec all
day long.
So I think best choice we can do is that pass the decision to user by Kconfig
which includes pointing the benchmark. I intionally removed "ARM" word in help
because we checked the performance in just three devices of all ARM CPU
so we can't make sure it does makse sense all ARM CPU.

Of course, I'm open for suggestion. Do you have better idea?


> 
> thanks,
> 
> greg k-h
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

-- 
Kind regards,
Minchan Kim

Re: [PATCH] zsmalloc: Add Kconfig for enabling PTE method

[Greg Kroah-Hartman]

On Tue, Feb 05, 2013 at 09:08:54AM +0900, Minchan Kim wrote:
> Hi Greg,
> 
> On Mon, Feb 04, 2013 at 10:51:46AM -0800, Greg Kroah-Hartman wrote:
> > On Mon, Feb 04, 2013 at 09:23:41AM +0900, Minchan Kim wrote:
> > > Zsmalloc has two methods 1) copy-based and 2) pte based to access
> > > allocations that span two pages.
> > > You can see history why we supported two approach from [1].
> > > 
> > > But it was bad choice that adding hard coding to select architecture
> > > which want to use pte based method. This patch removed it and adds
> > > new Kconfig to select the approach.
> > > 
> > > This patch is based on next-20130202.
> > > 
> > > [1] https://lkml.org/lkml/2012/7/11/58
> > > 
> > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > Cc: Seth Jennings <sjenning@linux.vnet.ibm.com>
> > > Cc: Nitin Gupta <ngupta@vflare.org>
> > > Cc: Dan Magenheimer <dan.magenheimer@oracle.com>
> > > Cc: Konrad Rzeszutek Wilk <konrad@darnok.org>
> > > Signed-off-by: Minchan Kim <minchan@kernel.org>
> > > ---
> > >  drivers/staging/zsmalloc/Kconfig         |   12 ++++++++++++
> > >  drivers/staging/zsmalloc/zsmalloc-main.c |   11 -----------
> > >  2 files changed, 12 insertions(+), 11 deletions(-)
> > > 
> > > diff --git a/drivers/staging/zsmalloc/Kconfig b/drivers/staging/zsmalloc/Kconfig
> > > index 9084565..2359123 100644
> > > --- a/drivers/staging/zsmalloc/Kconfig
> > > +++ b/drivers/staging/zsmalloc/Kconfig
> > > @@ -8,3 +8,15 @@ config ZSMALLOC
> > >  	  non-standard allocator interface where a handle, not a pointer, is
> > >  	  returned by an alloc().  This handle must be mapped in order to
> > >  	  access the allocated space.
> > > +
> > > +config ZSMALLOC_PGTABLE_MAPPING
> > > +        bool "Use page table mapping to access allocations that span two pages"
> > > +        depends on ZSMALLOC
> > > +        default n
> > > +        help
> > > +	  By default, zsmalloc uses a copy-based object mapping method to access
> > > +	  allocations that span two pages. However, if a particular architecture
> > > +	  performs VM mapping faster than copying, then you should select this.
> > > +	  This causes zsmalloc to use page table mapping rather than copying
> > > +	  for object mapping. You can check speed with zsmalloc benchmark[1].
> > > +	  [1] https://github.com/spartacus06/zsmalloc
> > > diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c
> > > index 06f73a9..b161ca1 100644
> > > --- a/drivers/staging/zsmalloc/zsmalloc-main.c
> > > +++ b/drivers/staging/zsmalloc/zsmalloc-main.c
> > > @@ -218,17 +218,6 @@ struct zs_pool {
> > >  #define CLASS_IDX_MASK	((1 << CLASS_IDX_BITS) - 1)
> > >  #define FULLNESS_MASK	((1 << FULLNESS_BITS) - 1)
> > >  
> > > -/*
> > > - * By default, zsmalloc uses a copy-based object mapping method to access
> > > - * allocations that span two pages. However, if a particular architecture
> > > - * performs VM mapping faster than copying, then it should be added here
> > > - * so that USE_PGTABLE_MAPPING is defined. This causes zsmalloc to use
> > > - * page table mapping rather than copying for object mapping.
> > > -*/
> > > -#if defined(CONFIG_ARM)
> > > -#define USE_PGTABLE_MAPPING
> > > -#endif
> > 
> > Did you test this?  I don't see the new config value you added actually
> > do anything in this code.  Also, if I select it incorrectly on ARM, or
> 
> *slaps self*

Ok, so I'll drop this patch now.  As for what to do instead, I have no
idea, sorry, but the others should.

thanks,

greg k-h

Re: [PATCH] zsmalloc: Add Kconfig for enabling PTE method

[Minchan Kim]

On Tue, Feb 05, 2013 at 11:25:20AM -0800, Greg Kroah-Hartman wrote:
> On Tue, Feb 05, 2013 at 09:08:54AM +0900, Minchan Kim wrote:
> > Hi Greg,
> > 
> > On Mon, Feb 04, 2013 at 10:51:46AM -0800, Greg Kroah-Hartman wrote:
> > > On Mon, Feb 04, 2013 at 09:23:41AM +0900, Minchan Kim wrote:
> > > > Zsmalloc has two methods 1) copy-based and 2) pte based to access
> > > > allocations that span two pages.
> > > > You can see history why we supported two approach from [1].
> > > > 
> > > > But it was bad choice that adding hard coding to select architecture
> > > > which want to use pte based method. This patch removed it and adds
> > > > new Kconfig to select the approach.
> > > > 
> > > > This patch is based on next-20130202.
> > > > 
> > > > [1] https://lkml.org/lkml/2012/7/11/58
> > > > 
> > > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > > Cc: Seth Jennings <sjenning@linux.vnet.ibm.com>
> > > > Cc: Nitin Gupta <ngupta@vflare.org>
> > > > Cc: Dan Magenheimer <dan.magenheimer@oracle.com>
> > > > Cc: Konrad Rzeszutek Wilk <konrad@darnok.org>
> > > > Signed-off-by: Minchan Kim <minchan@kernel.org>
> > > > ---
> > > >  drivers/staging/zsmalloc/Kconfig         |   12 ++++++++++++
> > > >  drivers/staging/zsmalloc/zsmalloc-main.c |   11 -----------
> > > >  2 files changed, 12 insertions(+), 11 deletions(-)
> > > > 
> > > > diff --git a/drivers/staging/zsmalloc/Kconfig b/drivers/staging/zsmalloc/Kconfig
> > > > index 9084565..2359123 100644
> > > > --- a/drivers/staging/zsmalloc/Kconfig
> > > > +++ b/drivers/staging/zsmalloc/Kconfig
> > > > @@ -8,3 +8,15 @@ config ZSMALLOC
> > > >  	  non-standard allocator interface where a handle, not a pointer, is
> > > >  	  returned by an alloc().  This handle must be mapped in order to
> > > >  	  access the allocated space.
> > > > +
> > > > +config ZSMALLOC_PGTABLE_MAPPING
> > > > +        bool "Use page table mapping to access allocations that span two pages"
> > > > +        depends on ZSMALLOC
> > > > +        default n
> > > > +        help
> > > > +	  By default, zsmalloc uses a copy-based object mapping method to access
> > > > +	  allocations that span two pages. However, if a particular architecture
> > > > +	  performs VM mapping faster than copying, then you should select this.
> > > > +	  This causes zsmalloc to use page table mapping rather than copying
> > > > +	  for object mapping. You can check speed with zsmalloc benchmark[1].
> > > > +	  [1] https://github.com/spartacus06/zsmalloc
> > > > diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c
> > > > index 06f73a9..b161ca1 100644
> > > > --- a/drivers/staging/zsmalloc/zsmalloc-main.c
> > > > +++ b/drivers/staging/zsmalloc/zsmalloc-main.c
> > > > @@ -218,17 +218,6 @@ struct zs_pool {
> > > >  #define CLASS_IDX_MASK	((1 << CLASS_IDX_BITS) - 1)
> > > >  #define FULLNESS_MASK	((1 << FULLNESS_BITS) - 1)
> > > >  
> > > > -/*
> > > > - * By default, zsmalloc uses a copy-based object mapping method to access
> > > > - * allocations that span two pages. However, if a particular architecture
> > > > - * performs VM mapping faster than copying, then it should be added here
> > > > - * so that USE_PGTABLE_MAPPING is defined. This causes zsmalloc to use
> > > > - * page table mapping rather than copying for object mapping.
> > > > -*/
> > > > -#if defined(CONFIG_ARM)
> > > > -#define USE_PGTABLE_MAPPING
> > > > -#endif
> > > 
> > > Did you test this?  I don't see the new config value you added actually
> > > do anything in this code.  Also, if I select it incorrectly on ARM, or
> > 
> > *slaps self*
> 
> Ok, so I'll drop this patch now.  As for what to do instead, I have no
> idea, sorry, but the others should.

Okay. Then, let's discuss further.
The history we introuced copy-based method is due to portability casused by
set_pte and __flush_tlb_one usage in young zsmalloc age. They are gone now
so there isn't issue any more. But we found copy-based method is 3 times faster
than pte-based in VM so I expect you guys don't want to give up it for just
portability. Of course,
I can't give up pte-based model as you know well, it's 6 times faster than
copy-based model in ARM.

Hard-coding for some arch like now isn't good and Kconfig for selecting choice
was rejected by Greg as you can see above.

Remained thing is new Kconfig ZSMALLOC_SMART_CHOICE and adding
new boot/module paramter. If admin enable it, In booting and module loading time,
zsmalloc start benchmark both model, find best in the system and select it.
If it is different with thing admin selected or default method, zsmalloc can warn
about it so he can select right choice next time without enabling ZSMALLOC_SMART_CHOICE
so he don't lose booting time and code size.

For it, we should add some code for benchamrk and selecting model dynamically.
Frankly speaking, I feel it's overkill. zsmalloc is in staging now so IMHO,
I don't see any problem in my patch.

Anyway, we should conclude agreeement and Andrew, too because he asked to add
Kconfig model to promote zsmalloc.

Andrew, Nitin, Dan, Seth and Konrad?
What do you think about it?

> 
> thanks,
> 
> greg k-h
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

-- 
Kind regards,
Minchan Kim

Re: [PATCH] zsmalloc: Add Kconfig for enabling PTE method

[Greg Kroah-Hartman]

On Wed, Feb 06, 2013 at 10:17:21AM +0900, Minchan Kim wrote:
> > > > Did you test this?  I don't see the new config value you added actually
> > > > do anything in this code.  Also, if I select it incorrectly on ARM, or
> > > 
> > > *slaps self*
> > 
> > Ok, so I'll drop this patch now.  As for what to do instead, I have no
> > idea, sorry, but the others should.
> 
> Okay. Then, let's discuss further.
> The history we introuced copy-based method is due to portability casused by
> set_pte and __flush_tlb_one usage in young zsmalloc age. They are gone now
> so there isn't issue any more. But we found copy-based method is 3 times faster
> than pte-based in VM so I expect you guys don't want to give up it for just
> portability. Of course,
> I can't give up pte-based model as you know well, it's 6 times faster than
> copy-based model in ARM.
> 
> Hard-coding for some arch like now isn't good and Kconfig for selecting choice
> was rejected by Greg as you can see above.

I rejected your patch because it did not do anything, why would I accept
it?

What would you have done in my situation?

It's not an issue of "portability" or "speed" or anything other than
"the patch you sent was obviously not correct."

greg k-h

Re: [PATCH] zsmalloc: Add Kconfig for enabling PTE method

[Minchan Kim]

Hi Greg,

On Tue, Feb 05, 2013 at 05:42:59PM -0800, Greg Kroah-Hartman wrote:
> On Wed, Feb 06, 2013 at 10:17:21AM +0900, Minchan Kim wrote:
> > > > > Did you test this?  I don't see the new config value you added actually
> > > > > do anything in this code.  Also, if I select it incorrectly on ARM, or
> > > > 
> > > > *slaps self*
> > > 
> > > Ok, so I'll drop this patch now.  As for what to do instead, I have no
> > > idea, sorry, but the others should.
> > 
> > Okay. Then, let's discuss further.
> > The history we introuced copy-based method is due to portability casused by
> > set_pte and __flush_tlb_one usage in young zsmalloc age. They are gone now
> > so there isn't issue any more. But we found copy-based method is 3 times faster
> > than pte-based in VM so I expect you guys don't want to give up it for just
> > portability. Of course,
> > I can't give up pte-based model as you know well, it's 6 times faster than
> > copy-based model in ARM.
> > 
> > Hard-coding for some arch like now isn't good and Kconfig for selecting choice
> > was rejected by Greg as you can see above.
> 
> I rejected your patch because it did not do anything, why would I accept
> it?
> 
> What would you have done in my situation?
> 
> It's not an issue of "portability" or "speed" or anything other than
> "the patch you sent was obviously not correct."

I totally misunderstood that you'd like to solve this issue.
"Also, if I select it incorrectly on ARM, or
or other platforms, what is keeping this from doing bad things?"
Then, I will resend it soon.

Thanks.

> 
> greg k-h
> 
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majordomo@kvack.org.  For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

-- 
Kind regards,
Minchan Kim

Re: [PATCH] zsmalloc: Add Kconfig for enabling PTE method

[Ric Mason]

On 02/04/2013 08:23 AM, Minchan Kim wrote:
> Zsmalloc has two methods 1) copy-based and 2) pte based to access
> allocations that span two pages.
> You can see history why we supported two approach from [1].
>
> But it was bad choice that adding hard coding to select architecture
> which want to use pte based method. This patch removed it and adds
> new Kconfig to select the approach.
>
> This patch is based on next-20130202.
>
> [1] https://lkml.org/lkml/2012/7/11/58
>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Seth Jennings <sjenning@linux.vnet.ibm.com>
> Cc: Nitin Gupta <ngupta@vflare.org>
> Cc: Dan Magenheimer <dan.magenheimer@oracle.com>
> Cc: Konrad Rzeszutek Wilk <konrad@darnok.org>
> Signed-off-by: Minchan Kim <minchan@kernel.org>
> ---
>   drivers/staging/zsmalloc/Kconfig         |   12 ++++++++++++
>   drivers/staging/zsmalloc/zsmalloc-main.c |   11 -----------
>   2 files changed, 12 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/staging/zsmalloc/Kconfig b/drivers/staging/zsmalloc/Kconfig
> index 9084565..2359123 100644
> --- a/drivers/staging/zsmalloc/Kconfig
> +++ b/drivers/staging/zsmalloc/Kconfig
> @@ -8,3 +8,15 @@ config ZSMALLOC
>   	  non-standard allocator interface where a handle, not a pointer, is
>   	  returned by an alloc().  This handle must be mapped in order to
>   	  access the allocated space.
> +
> +config ZSMALLOC_PGTABLE_MAPPING
> +        bool "Use page table mapping to access allocations that span two pages"
> +        depends on ZSMALLOC
> +        default n
> +        help
> +	  By default, zsmalloc uses a copy-based object mapping method to access
> +	  allocations that span two pages. However, if a particular architecture
> +	  performs VM mapping faster than copying, then you should select this.
> +	  This causes zsmalloc to use page table mapping rather than copying
> +	  for object mapping. You can check speed with zsmalloc benchmark[1].
> +	  [1] https://github.com/spartacus06/zsmalloc

Is there benchmark to test zcache? eg. internal fragmentation level ...
> diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c
> index 06f73a9..b161ca1 100644
> --- a/drivers/staging/zsmalloc/zsmalloc-main.c
> +++ b/drivers/staging/zsmalloc/zsmalloc-main.c
> @@ -218,17 +218,6 @@ struct zs_pool {
>   #define CLASS_IDX_MASK	((1 << CLASS_IDX_BITS) - 1)
>   #define FULLNESS_MASK	((1 << FULLNESS_BITS) - 1)
>   
> -/*
> - * By default, zsmalloc uses a copy-based object mapping method to access
> - * allocations that span two pages. However, if a particular architecture
> - * performs VM mapping faster than copying, then it should be added here
> - * so that USE_PGTABLE_MAPPING is defined. This causes zsmalloc to use
> - * page table mapping rather than copying for object mapping.
> -*/
> -#if defined(CONFIG_ARM)
> -#define USE_PGTABLE_MAPPING
> -#endif
> -
>   struct mapping_area {
>   #ifdef USE_PGTABLE_MAPPING
>   	struct vm_struct *vm; /* vm area for mapping object that span pages */

Re: [PATCH] zsmalloc: Add Kconfig for enabling PTE method

[Seth Jennings]

On 02/16/2013 12:28 AM, Ric Mason wrote:
> On 02/04/2013 08:23 AM, Minchan Kim wrote:
>> Zsmalloc has two methods 1) copy-based and 2) pte based to access
>> allocations that span two pages.
>> You can see history why we supported two approach from [1].
>>
>> But it was bad choice that adding hard coding to select architecture
>> which want to use pte based method. This patch removed it and adds
>> new Kconfig to select the approach.
>>
>> This patch is based on next-20130202.
>>
>> [1] https://lkml.org/lkml/2012/7/11/58
>>
>> Cc: Andrew Morton <akpm@linux-foundation.org>
>> Cc: Seth Jennings <sjenning@linux.vnet.ibm.com>
>> Cc: Nitin Gupta <ngupta@vflare.org>
>> Cc: Dan Magenheimer <dan.magenheimer@oracle.com>
>> Cc: Konrad Rzeszutek Wilk <konrad@darnok.org>
>> Signed-off-by: Minchan Kim <minchan@kernel.org>
>> ---
>>   drivers/staging/zsmalloc/Kconfig         |   12 ++++++++++++
>>   drivers/staging/zsmalloc/zsmalloc-main.c |   11 -----------
>>   2 files changed, 12 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/staging/zsmalloc/Kconfig
>> b/drivers/staging/zsmalloc/Kconfig
>> index 9084565..2359123 100644
>> --- a/drivers/staging/zsmalloc/Kconfig
>> +++ b/drivers/staging/zsmalloc/Kconfig
>> @@ -8,3 +8,15 @@ config ZSMALLOC
>>         non-standard allocator interface where a handle, not a
>> pointer, is
>>         returned by an alloc().  This handle must be mapped in order to
>>         access the allocated space.
>> +
>> +config ZSMALLOC_PGTABLE_MAPPING
>> +        bool "Use page table mapping to access allocations that
>> span two pages"
>> +        depends on ZSMALLOC
>> +        default n
>> +        help
>> +      By default, zsmalloc uses a copy-based object mapping method
>> to access
>> +      allocations that span two pages. However, if a particular
>> architecture
>> +      performs VM mapping faster than copying, then you should
>> select this.
>> +      This causes zsmalloc to use page table mapping rather than
>> copying
>> +      for object mapping. You can check speed with zsmalloc
>> benchmark[1].
>> +      [1] https://github.com/spartacus06/zsmalloc
> 
> Is there benchmark to test zcache? eg. internal fragmentation level ...

First, zsmalloc is not used in zcache right now so just wanted to say
that.  It is used in zram and the proposed zswap
(https://lwn.net/Articles/528817/)

There is not an official benchmark.  However anything that generates
activity that will hit the frontswap or cleancache hooks will do.
These are workloads that overcommit memory and use swap, or access
file sets whose size is larger that the system page cache.

The closest thing to a fragmentation metric is an effective
compression ratio that can be calculated with debugfs attributes:

zcache_[eph|pers]_zbytes / (zcache_[eph|pers]_pageframes * PAGE_SIZE)

eph for cleancache, and pers for frontswap.

Seth

RE: [PATCH] zsmalloc: Add Kconfig for enabling PTE method

[Dan Magenheimer]

> From: Seth Jennings [mailto:sjenning@linux.vnet.ibm.com]
> Subject: Re: [PATCH] zsmalloc: Add Kconfig for enabling PTE method
> 
> On 02/16/2013 12:28 AM, Ric Mason wrote:
> > On 02/04/2013 08:23 AM, Minchan Kim wrote:
> >> +      for object mapping. You can check speed with zsmalloc
> >> benchmark[1].
> >> +      [1] https://github.com/spartacus06/zsmalloc
> >
> > Is there benchmark to test zcache? eg. internal fragmentation level ...
> 
> First, zsmalloc is not used in zcache right now so just wanted to say
> that.  It is used in zram and the proposed zswap
> (https://lwn.net/Articles/528817/)
> 
> There is not an official benchmark.  However anything that generates
> activity that will hit the frontswap or cleancache hooks will do.
> These are workloads that overcommit memory and use swap, or access
> file sets whose size is larger that the system page cache.

I think it's important to note that the question "is there
a benchmark" is a very deep and difficult question for any
compression solution because it is so workload-dependent.
Unlike many benchmarks that simply synthesize a _quantity_
of data, zcache/zswap/zram all are very sensitive to the
actual contents of that data as the compression ratio
varies widely depending on the data.  So we need to ensure
that the data used by any benchmark has similar "entropy"
to real world workloads.  I'm not sure how we can do that.

So it may or may not be useful to measure zcache/zswap/zram using
standard benchmarks (including things like SPECjbb).  At least
kernbench is something that kernel developers do every day,
so it is definitely a real world workload... but adding
parallel compiles (via "make -jN") until the system thrashes,
and then showing zcache/zswap/zram reduces the thrashing may
not be at all representative of a broad range of workloads
that cause memory pressure... kernbench is just convenient for
us developers to demonstrate that the mechanism works.

Ideas welcome... well-thought out ideas anyway!

Dan