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

[Minchan Kim]

On Tue, Feb 05, 2013 at 06:28:54PM -0800, Greg Kroah-Hartman wrote:
> On Wed, Feb 06, 2013 at 11:17:08AM +0900, Minchan Kim wrote:
> > diff --git a/drivers/staging/zsmalloc/Kconfig b/drivers/staging/zsmalloc/Kconfig
> > index 9084565..232b3b6 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 PGTABLE_MAPPING
> > +        bool "Use page table mapping to access allocations that span two pages"
> 
> No tabs?
> 
> Please also put "ZSmalloc somewhere in the text here, otherwise it
> really doesn't make much sense when seeing it in a menu.
> 
> > +        depends on ZSMALLOC
> > +        default n
> 
> That's the default, so it can be dropped.
> 
> > +        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
> 
> Care to specify exactly _what_ architectures this should be set for or
> not?  That will help the distros out a lot in determining if this should
> be enabled or not.
> 
> > diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c
> > index 06f73a9..2c1805c 100644
> > --- a/drivers/staging/zsmalloc/zsmalloc-main.c
> > +++ b/drivers/staging/zsmalloc/zsmalloc-main.c
> > @@ -207,6 +207,7 @@ struct zs_pool {
> >  	struct size_class size_class[ZS_SIZE_CLASSES];
> >  
> >  	gfp_t flags;	/* allocation flags used when growing pool */
> > +
> >  };
> >  
> >  /*
> 
> Why add this extra line?
> 
> thanks,
> 
> greg k-h

Sorry for bothering you.
I fixed all you pointed out.
Thanks for the review, Greg!

Here it goes.

------------------- 8< -------------------