From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Minchan Kim <minchan@kernel.org>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
Andrew Morton <akpm@linux-foundation.org>,
Seth Jennings <sjenning@linux.vnet.ibm.com>,
Nitin Gupta <ngupta@vflare.org>,
Dan Magenheimer <dan.magenheimer@oracle.com>,
Konrad Rzeszutek Wilk <konrad@darnok.org>
Subject: Re: [PATCH] zsmalloc: Add Kconfig for enabling PTE method
Date: Tue, 5 Feb 2013 11:25:20 -0800 [thread overview]
Message-ID: <20130205192520.GA8441@kroah.com> (raw)
In-Reply-To: <20130205000854.GC2610@blaptop>
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
--
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>
WARNING: multiple messages have this Message-ID (diff)
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Minchan Kim <minchan@kernel.org>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
Andrew Morton <akpm@linux-foundation.org>,
Seth Jennings <sjenning@linux.vnet.ibm.com>,
Nitin Gupta <ngupta@vflare.org>,
Dan Magenheimer <dan.magenheimer@oracle.com>,
Konrad Rzeszutek Wilk <konrad@darnok.org>
Subject: Re: [PATCH] zsmalloc: Add Kconfig for enabling PTE method
Date: Tue, 5 Feb 2013 11:25:20 -0800 [thread overview]
Message-ID: <20130205192520.GA8441@kroah.com> (raw)
In-Reply-To: <20130205000854.GC2610@blaptop>
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
next prev parent reply other threads:[~2013-02-05 19:23 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-02-04 0:23 [PATCH] zsmalloc: Add Kconfig for enabling PTE method Minchan Kim
2013-02-04 0:23 ` Minchan Kim
2013-02-04 2:02 ` Simon Jeons
2013-02-04 2:02 ` Simon Jeons
2013-02-04 2:58 ` Minchan Kim
2013-02-04 2:58 ` Minchan Kim
2013-02-04 18:51 ` Greg Kroah-Hartman
2013-02-04 18:51 ` Greg Kroah-Hartman
2013-02-05 0:08 ` Minchan Kim
2013-02-05 0:08 ` Minchan Kim
2013-02-05 19:25 ` Greg Kroah-Hartman [this message]
2013-02-05 19:25 ` Greg Kroah-Hartman
2013-02-06 1:17 ` Minchan Kim
2013-02-06 1:17 ` Minchan Kim
2013-02-06 1:42 ` Greg Kroah-Hartman
2013-02-06 1:42 ` Greg Kroah-Hartman
2013-02-06 1:44 ` Minchan Kim
2013-02-06 1:44 ` Minchan Kim
2013-02-16 6:28 ` Ric Mason
2013-02-16 6:28 ` Ric Mason
2013-02-18 18:24 ` Seth Jennings
2013-02-18 18:24 ` Seth Jennings
2013-02-18 23:48 ` Dan Magenheimer
2013-02-18 23:48 ` Dan Magenheimer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130205192520.GA8441@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=akpm@linux-foundation.org \
--cc=dan.magenheimer@oracle.com \
--cc=konrad@darnok.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=minchan@kernel.org \
--cc=ngupta@vflare.org \
--cc=sjenning@linux.vnet.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.