[PATCH 0/3] zsmalloc: remove x86 dependency

[PATCH 0/3] zsmalloc: remove x86 dependency

[Seth Jennings]

This patchset continues/adapts Minchan Kim's work
to remove the x86 dependency from zsmalloc.

However, instead of whitelisting archs with support for
local_tlb_flush_kernel_range() in the zsmalloc Kconfig,
this patchset allows zsmalloc to work with all archs
through the addition of a generic/portable page
mapping methods (i.e. memcpy) when the required tlb
flushing functionality is not supported by the arch.

The arch advertises support for local_tlb_flush_kernel_range()
by defining __HAVE_LOCAL_FLUSH_TLB_KERNEL_RANGE

The third patch in the set adds local_tlb_flush_kernel_range()
support to x86.  In my single-threaded tests using zcache,
using the pte/tlb mapping method was 40% faster than the generic
method. So while the third patch is optional, it is highly
recommended.

Alex Shi is working on a large x86 patchset that includes
functionality similar to the third patch, however, it seems
that this patchset is getting very little attention and
includes much more than is needed for zsmalloc's purposes.

https://lkml.org/lkml/2012/6/12/116

Future work:
 - Add __HAVE_LOCAL_FLUSH_TLB_KERNEL_RANGE definition to
   archs that already have local_tlb_flush_kernel_range()
 - Add mapping mode flags (RO, WO, RW) to zs_map_object()
   to avoid unnecessary copies in the generic case

Based on Greg's staging-next.

Seth Jennings (3):
  zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC
  zsmalloc: add generic path and remove x86 dependency
  x86: add local_tlb_flush_kernel_range()

 arch/x86/include/asm/tlbflush.h          |   21 +++++
 drivers/staging/zcache/Kconfig           |    5 +-
 drivers/staging/zram/Kconfig             |    5 +-
 drivers/staging/zsmalloc/Kconfig         |    4 -
 drivers/staging/zsmalloc/zsmalloc-main.c |  136 ++++++++++++++++++++++++------
 drivers/staging/zsmalloc/zsmalloc_int.h  |    5 +-
 6 files changed, 138 insertions(+), 38 deletions(-)

-- 
1.7.9.5

[PATCH 0/3] zsmalloc: remove x86 dependency

[Seth Jennings]

This patchset continues/adapts Minchan Kim's work
to remove the x86 dependency from zsmalloc.

However, instead of whitelisting archs with support for
local_tlb_flush_kernel_range() in the zsmalloc Kconfig,
this patchset allows zsmalloc to work with all archs
through the addition of a generic/portable page
mapping methods (i.e. memcpy) when the required tlb
flushing functionality is not supported by the arch.

The arch advertises support for local_tlb_flush_kernel_range()
by defining __HAVE_LOCAL_FLUSH_TLB_KERNEL_RANGE

The third patch in the set adds local_tlb_flush_kernel_range()
support to x86.  In my single-threaded tests using zcache,
using the pte/tlb mapping method was 40% faster than the generic
method. So while the third patch is optional, it is highly
recommended.

Alex Shi is working on a large x86 patchset that includes
functionality similar to the third patch, however, it seems
that this patchset is getting very little attention and
includes much more than is needed for zsmalloc's purposes.

https://lkml.org/lkml/2012/6/12/116

Future work:
 - Add __HAVE_LOCAL_FLUSH_TLB_KERNEL_RANGE definition to
   archs that already have local_tlb_flush_kernel_range()
 - Add mapping mode flags (RO, WO, RW) to zs_map_object()
   to avoid unnecessary copies in the generic case

Based on Greg's staging-next.

Seth Jennings (3):
  zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC
  zsmalloc: add generic path and remove x86 dependency
  x86: add local_tlb_flush_kernel_range()

 arch/x86/include/asm/tlbflush.h          |   21 +++++
 drivers/staging/zcache/Kconfig           |    5 +-
 drivers/staging/zram/Kconfig             |    5 +-
 drivers/staging/zsmalloc/Kconfig         |    4 -
 drivers/staging/zsmalloc/zsmalloc-main.c |  136 ++++++++++++++++++++++++------
 drivers/staging/zsmalloc/zsmalloc_int.h  |    5 +-
 6 files changed, 138 insertions(+), 38 deletions(-)

-- 
1.7.9.5

--
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>

[PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC

[Seth Jennings]

This patch switches zcache and zram dependency to ZSMALLOC
rather than X86.  There is no net change since ZSMALLOC
depends on X86, however, this prevent further changes to
these files as zsmalloc dependencies change.

Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
---
 drivers/staging/zcache/Kconfig |    5 +----
 drivers/staging/zram/Kconfig   |    5 +----
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/zcache/Kconfig b/drivers/staging/zcache/Kconfig
index 7048e01..4881839 100644
--- a/drivers/staging/zcache/Kconfig
+++ b/drivers/staging/zcache/Kconfig
@@ -1,9 +1,6 @@
 config ZCACHE
 	bool "Dynamic compression of swap pages and clean pagecache pages"
-	# X86 dependency is because zsmalloc uses non-portable pte/tlb
-	# functions
-	depends on (CLEANCACHE || FRONTSWAP) && CRYPTO=y && X86
-	select ZSMALLOC
+	depends on (CLEANCACHE || FRONTSWAP) && CRYPTO=y && ZSMALLOC=y
 	select CRYPTO_LZO
 	default n
 	help
diff --git a/drivers/staging/zram/Kconfig b/drivers/staging/zram/Kconfig
index 9d11a4c..be5abe8 100644
--- a/drivers/staging/zram/Kconfig
+++ b/drivers/staging/zram/Kconfig
@@ -1,9 +1,6 @@
 config ZRAM
 	tristate "Compressed RAM block device support"
-	# X86 dependency is because zsmalloc uses non-portable pte/tlb
-	# functions
-	depends on BLOCK && SYSFS && X86
-	select ZSMALLOC
+	depends on BLOCK && SYSFS && ZSMALLOC
 	select LZO_COMPRESS
 	select LZO_DECOMPRESS
 	default n
-- 
1.7.9.5

[PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC

[Seth Jennings]

This patch switches zcache and zram dependency to ZSMALLOC
rather than X86.  There is no net change since ZSMALLOC
depends on X86, however, this prevent further changes to
these files as zsmalloc dependencies change.

Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
---
 drivers/staging/zcache/Kconfig |    5 +----
 drivers/staging/zram/Kconfig   |    5 +----
 2 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/staging/zcache/Kconfig b/drivers/staging/zcache/Kconfig
index 7048e01..4881839 100644
--- a/drivers/staging/zcache/Kconfig
+++ b/drivers/staging/zcache/Kconfig
@@ -1,9 +1,6 @@
 config ZCACHE
 	bool "Dynamic compression of swap pages and clean pagecache pages"
-	# X86 dependency is because zsmalloc uses non-portable pte/tlb
-	# functions
-	depends on (CLEANCACHE || FRONTSWAP) && CRYPTO=y && X86
-	select ZSMALLOC
+	depends on (CLEANCACHE || FRONTSWAP) && CRYPTO=y && ZSMALLOC=y
 	select CRYPTO_LZO
 	default n
 	help
diff --git a/drivers/staging/zram/Kconfig b/drivers/staging/zram/Kconfig
index 9d11a4c..be5abe8 100644
--- a/drivers/staging/zram/Kconfig
+++ b/drivers/staging/zram/Kconfig
@@ -1,9 +1,6 @@
 config ZRAM
 	tristate "Compressed RAM block device support"
-	# X86 dependency is because zsmalloc uses non-portable pte/tlb
-	# functions
-	depends on BLOCK && SYSFS && X86
-	select ZSMALLOC
+	depends on BLOCK && SYSFS && ZSMALLOC
 	select LZO_COMPRESS
 	select LZO_DECOMPRESS
 	default n
-- 
1.7.9.5

--
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>

Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC

[Minchan Kim]

On 06/26/2012 01:14 AM, Seth Jennings wrote:

> This patch switches zcache and zram dependency to ZSMALLOC
> rather than X86.  There is no net change since ZSMALLOC
> depends on X86, however, this prevent further changes to
> these files as zsmalloc dependencies change.
> 
> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>

Reviewed-by: Minchan Kim <minchan@kernel.org>

It could be merged regardless of other patches in this series.

-- 
Kind regards,
Minchan Kim

Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC

[Minchan Kim]

On 06/26/2012 01:14 AM, Seth Jennings wrote:

> This patch switches zcache and zram dependency to ZSMALLOC
> rather than X86.  There is no net change since ZSMALLOC
> depends on X86, however, this prevent further changes to
> these files as zsmalloc dependencies change.
> 
> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>

Reviewed-by: Minchan Kim <minchan@kernel.org>

It could be merged regardless of other patches in this series.

-- 
Kind regards,
Minchan Kim

--
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>

Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC

[Greg Kroah-Hartman]

On Wed, Jun 27, 2012 at 11:37:25AM +0900, Minchan Kim wrote:
> On 06/26/2012 01:14 AM, Seth Jennings wrote:
> 
> > This patch switches zcache and zram dependency to ZSMALLOC
> > rather than X86.  There is no net change since ZSMALLOC
> > depends on X86, however, this prevent further changes to
> > these files as zsmalloc dependencies change.
> > 
> > Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
> 
> Reviewed-by: Minchan Kim <minchan@kernel.org>
> 
> It could be merged regardless of other patches in this series.

I already did :)

Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC

[Greg Kroah-Hartman]

On Wed, Jun 27, 2012 at 11:37:25AM +0900, Minchan Kim wrote:
> On 06/26/2012 01:14 AM, Seth Jennings wrote:
> 
> > This patch switches zcache and zram dependency to ZSMALLOC
> > rather than X86.  There is no net change since ZSMALLOC
> > depends on X86, however, this prevent further changes to
> > these files as zsmalloc dependencies change.
> > 
> > Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
> 
> Reviewed-by: Minchan Kim <minchan@kernel.org>
> 
> It could be merged regardless of other patches in this series.

I already did :)

--
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>

Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC

[Minchan Kim]

Hi Greg,

On 06/27/2012 11:43 AM, Greg Kroah-Hartman wrote:

> On Wed, Jun 27, 2012 at 11:37:25AM +0900, Minchan Kim wrote:
>> On 06/26/2012 01:14 AM, Seth Jennings wrote:
>>
>>> This patch switches zcache and zram dependency to ZSMALLOC
>>> rather than X86.  There is no net change since ZSMALLOC
>>> depends on X86, however, this prevent further changes to
>>> these files as zsmalloc dependencies change.
>>>
>>> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
>>
>> Reviewed-by: Minchan Kim <minchan@kernel.org>
>>
>> It could be merged regardless of other patches in this series.
> 
> I already did :)


It would have been better if you send merge mail to Ccing people.
Anyway, Thanks!

-- 
Kind regards,
Minchan Kim

Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC

[Minchan Kim]

Hi Greg,

On 06/27/2012 11:43 AM, Greg Kroah-Hartman wrote:

> On Wed, Jun 27, 2012 at 11:37:25AM +0900, Minchan Kim wrote:
>> On 06/26/2012 01:14 AM, Seth Jennings wrote:
>>
>>> This patch switches zcache and zram dependency to ZSMALLOC
>>> rather than X86.  There is no net change since ZSMALLOC
>>> depends on X86, however, this prevent further changes to
>>> these files as zsmalloc dependencies change.
>>>
>>> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
>>
>> Reviewed-by: Minchan Kim <minchan@kernel.org>
>>
>> It could be merged regardless of other patches in this series.
> 
> I already did :)


It would have been better if you send merge mail to Ccing people.
Anyway, Thanks!

-- 
Kind regards,
Minchan Kim

--
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>

Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC

[Greg Kroah-Hartman]

On Wed, Jun 27, 2012 at 11:49:58AM +0900, Minchan Kim wrote:
> Hi Greg,
> 
> On 06/27/2012 11:43 AM, Greg Kroah-Hartman wrote:
> 
> > On Wed, Jun 27, 2012 at 11:37:25AM +0900, Minchan Kim wrote:
> >> On 06/26/2012 01:14 AM, Seth Jennings wrote:
> >>
> >>> This patch switches zcache and zram dependency to ZSMALLOC
> >>> rather than X86.  There is no net change since ZSMALLOC
> >>> depends on X86, however, this prevent further changes to
> >>> these files as zsmalloc dependencies change.
> >>>
> >>> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
> >>
> >> Reviewed-by: Minchan Kim <minchan@kernel.org>
> >>
> >> It could be merged regardless of other patches in this series.
> > 
> > I already did :)
> 
> 
> It would have been better if you send merge mail to Ccing people.
> Anyway, Thanks!

I do, for people on the cc: in the signed-off-by area of the patch.  For
me to manually add the people on the cc: of the email, I would have to
modify git to add them to the commit somehow, sorry.

greg k-h

Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC

[Greg Kroah-Hartman]

On Wed, Jun 27, 2012 at 11:49:58AM +0900, Minchan Kim wrote:
> Hi Greg,
> 
> On 06/27/2012 11:43 AM, Greg Kroah-Hartman wrote:
> 
> > On Wed, Jun 27, 2012 at 11:37:25AM +0900, Minchan Kim wrote:
> >> On 06/26/2012 01:14 AM, Seth Jennings wrote:
> >>
> >>> This patch switches zcache and zram dependency to ZSMALLOC
> >>> rather than X86.  There is no net change since ZSMALLOC
> >>> depends on X86, however, this prevent further changes to
> >>> these files as zsmalloc dependencies change.
> >>>
> >>> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
> >>
> >> Reviewed-by: Minchan Kim <minchan@kernel.org>
> >>
> >> It could be merged regardless of other patches in this series.
> > 
> > I already did :)
> 
> 
> It would have been better if you send merge mail to Ccing people.
> Anyway, Thanks!

I do, for people on the cc: in the signed-off-by area of the patch.  For
me to manually add the people on the cc: of the email, I would have to
modify git to add them to the commit somehow, sorry.

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>

Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC

[Konrad Rzeszutek Wilk]

On Tue, Jun 26, 2012 at 08:21:01PM -0700, Greg Kroah-Hartman wrote:
> On Wed, Jun 27, 2012 at 11:49:58AM +0900, Minchan Kim wrote:
> > Hi Greg,
> > 
> > On 06/27/2012 11:43 AM, Greg Kroah-Hartman wrote:
> > 
> > > On Wed, Jun 27, 2012 at 11:37:25AM +0900, Minchan Kim wrote:
> > >> On 06/26/2012 01:14 AM, Seth Jennings wrote:
> > >>
> > >>> This patch switches zcache and zram dependency to ZSMALLOC
> > >>> rather than X86.  There is no net change since ZSMALLOC
> > >>> depends on X86, however, this prevent further changes to
> > >>> these files as zsmalloc dependencies change.
> > >>>
> > >>> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
> > >>
> > >> Reviewed-by: Minchan Kim <minchan@kernel.org>
> > >>
> > >> It could be merged regardless of other patches in this series.
> > > 
> > > I already did :)
> > 
> > 
> > It would have been better if you send merge mail to Ccing people.
> > Anyway, Thanks!
> 
> I do, for people on the cc: in the signed-off-by area of the patch.  For
> me to manually add the people on the cc: of the email, I would have to
> modify git to add them to the commit somehow, sorry.

Is that some other script you have that does that?

Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC

[Konrad Rzeszutek Wilk]

On Tue, Jun 26, 2012 at 08:21:01PM -0700, Greg Kroah-Hartman wrote:
> On Wed, Jun 27, 2012 at 11:49:58AM +0900, Minchan Kim wrote:
> > Hi Greg,
> > 
> > On 06/27/2012 11:43 AM, Greg Kroah-Hartman wrote:
> > 
> > > On Wed, Jun 27, 2012 at 11:37:25AM +0900, Minchan Kim wrote:
> > >> On 06/26/2012 01:14 AM, Seth Jennings wrote:
> > >>
> > >>> This patch switches zcache and zram dependency to ZSMALLOC
> > >>> rather than X86.  There is no net change since ZSMALLOC
> > >>> depends on X86, however, this prevent further changes to
> > >>> these files as zsmalloc dependencies change.
> > >>>
> > >>> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
> > >>
> > >> Reviewed-by: Minchan Kim <minchan@kernel.org>
> > >>
> > >> It could be merged regardless of other patches in this series.
> > > 
> > > I already did :)
> > 
> > 
> > It would have been better if you send merge mail to Ccing people.
> > Anyway, Thanks!
> 
> I do, for people on the cc: in the signed-off-by area of the patch.  For
> me to manually add the people on the cc: of the email, I would have to
> modify git to add them to the commit somehow, sorry.

Is that some other script you have that does that?

--
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>

Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC

[Greg Kroah-Hartman]

On Wed, Jun 27, 2012 at 11:40:03AM -0400, Konrad Rzeszutek Wilk wrote:
> On Tue, Jun 26, 2012 at 08:21:01PM -0700, Greg Kroah-Hartman wrote:
> > On Wed, Jun 27, 2012 at 11:49:58AM +0900, Minchan Kim wrote:
> > > Hi Greg,
> > > 
> > > On 06/27/2012 11:43 AM, Greg Kroah-Hartman wrote:
> > > 
> > > > On Wed, Jun 27, 2012 at 11:37:25AM +0900, Minchan Kim wrote:
> > > >> On 06/26/2012 01:14 AM, Seth Jennings wrote:
> > > >>
> > > >>> This patch switches zcache and zram dependency to ZSMALLOC
> > > >>> rather than X86.  There is no net change since ZSMALLOC
> > > >>> depends on X86, however, this prevent further changes to
> > > >>> these files as zsmalloc dependencies change.
> > > >>>
> > > >>> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
> > > >>
> > > >> Reviewed-by: Minchan Kim <minchan@kernel.org>
> > > >>
> > > >> It could be merged regardless of other patches in this series.
> > > > 
> > > > I already did :)
> > > 
> > > 
> > > It would have been better if you send merge mail to Ccing people.
> > > Anyway, Thanks!
> > 
> > I do, for people on the cc: in the signed-off-by area of the patch.  For
> > me to manually add the people on the cc: of the email, I would have to
> > modify git to add them to the commit somehow, sorry.
> 
> Is that some other script you have that does that?

I don't understand what you are asking here.

My workflow is:
	- apply patches to a branch and test
	- if good, 'git format-patch' to create the patches
	- merge to my upstream branch and push out publically
	- run script on the patches generated by 'git format-patch' to
	  say they have been applied.

My custom script is the last thing there, that takes the patches, finds
the email addresses in the patch, and notifies everyone.

Does that explain things better?

greg k-h

Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC

[Greg Kroah-Hartman]

On Wed, Jun 27, 2012 at 11:40:03AM -0400, Konrad Rzeszutek Wilk wrote:
> On Tue, Jun 26, 2012 at 08:21:01PM -0700, Greg Kroah-Hartman wrote:
> > On Wed, Jun 27, 2012 at 11:49:58AM +0900, Minchan Kim wrote:
> > > Hi Greg,
> > > 
> > > On 06/27/2012 11:43 AM, Greg Kroah-Hartman wrote:
> > > 
> > > > On Wed, Jun 27, 2012 at 11:37:25AM +0900, Minchan Kim wrote:
> > > >> On 06/26/2012 01:14 AM, Seth Jennings wrote:
> > > >>
> > > >>> This patch switches zcache and zram dependency to ZSMALLOC
> > > >>> rather than X86.  There is no net change since ZSMALLOC
> > > >>> depends on X86, however, this prevent further changes to
> > > >>> these files as zsmalloc dependencies change.
> > > >>>
> > > >>> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
> > > >>
> > > >> Reviewed-by: Minchan Kim <minchan@kernel.org>
> > > >>
> > > >> It could be merged regardless of other patches in this series.
> > > > 
> > > > I already did :)
> > > 
> > > 
> > > It would have been better if you send merge mail to Ccing people.
> > > Anyway, Thanks!
> > 
> > I do, for people on the cc: in the signed-off-by area of the patch.  For
> > me to manually add the people on the cc: of the email, I would have to
> > modify git to add them to the commit somehow, sorry.
> 
> Is that some other script you have that does that?

I don't understand what you are asking here.

My workflow is:
	- apply patches to a branch and test
	- if good, 'git format-patch' to create the patches
	- merge to my upstream branch and push out publically
	- run script on the patches generated by 'git format-patch' to
	  say they have been applied.

My custom script is the last thing there, that takes the patches, finds
the email addresses in the patch, and notifies everyone.

Does that explain things better?

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>

Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC

[Konrad Rzeszutek Wilk]

> > > > It would have been better if you send merge mail to Ccing people.
> > > > Anyway, Thanks!
> > > 
> > > I do, for people on the cc: in the signed-off-by area of the patch.  For
> > > me to manually add the people on the cc: of the email, I would have to
> > > modify git to add them to the commit somehow, sorry.
> > 
> > Is that some other script you have that does that?
> 
> I don't understand what you are asking here.
> 
> My workflow is:
> 	- apply patches to a branch and test
> 	- if good, 'git format-patch' to create the patches
> 	- merge to my upstream branch and push out publically
> 	- run script on the patches generated by 'git format-patch' to
> 	  say they have been applied.
> 
> My custom script is the last thing there, that takes the patches, finds
> the email addresses in the patch, and notifies everyone.

That custom script - is it available somewhere?

> 
> Does that explain things better?

Aye!
> 
> greg k-h

Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC

[Konrad Rzeszutek Wilk]

> > > > It would have been better if you send merge mail to Ccing people.
> > > > Anyway, Thanks!
> > > 
> > > I do, for people on the cc: in the signed-off-by area of the patch.  For
> > > me to manually add the people on the cc: of the email, I would have to
> > > modify git to add them to the commit somehow, sorry.
> > 
> > Is that some other script you have that does that?
> 
> I don't understand what you are asking here.
> 
> My workflow is:
> 	- apply patches to a branch and test
> 	- if good, 'git format-patch' to create the patches
> 	- merge to my upstream branch and push out publically
> 	- run script on the patches generated by 'git format-patch' to
> 	  say they have been applied.
> 
> My custom script is the last thing there, that takes the patches, finds
> the email addresses in the patch, and notifies everyone.

That custom script - is it available somewhere?

> 
> Does that explain things better?

Aye!
> 
> 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>

Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC

[Greg Kroah-Hartman]

On Wed, Jun 27, 2012 at 02:52:23PM -0400, Konrad Rzeszutek Wilk wrote:
> > > > > It would have been better if you send merge mail to Ccing people.
> > > > > Anyway, Thanks!
> > > > 
> > > > I do, for people on the cc: in the signed-off-by area of the patch.  For
> > > > me to manually add the people on the cc: of the email, I would have to
> > > > modify git to add them to the commit somehow, sorry.
> > > 
> > > Is that some other script you have that does that?
> > 
> > I don't understand what you are asking here.
> > 
> > My workflow is:
> > 	- apply patches to a branch and test
> > 	- if good, 'git format-patch' to create the patches
> > 	- merge to my upstream branch and push out publically
> > 	- run script on the patches generated by 'git format-patch' to
> > 	  say they have been applied.
> > 
> > My custom script is the last thing there, that takes the patches, finds
> > the email addresses in the patch, and notifies everyone.
> 
> That custom script - is it available somewhere?

It's right here:
	https://github.com/gregkh/gregkh-linux/blob/master/work/do.sh
along with other scripts that I use for kernel patch development, feel
free to use them if you like.

Note, this script does all of the above steps except apply the patches,
I got tired of running 3 scripts so I merged them into one.

greg k-h

Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC

[Greg Kroah-Hartman]

On Wed, Jun 27, 2012 at 02:52:23PM -0400, Konrad Rzeszutek Wilk wrote:
> > > > > It would have been better if you send merge mail to Ccing people.
> > > > > Anyway, Thanks!
> > > > 
> > > > I do, for people on the cc: in the signed-off-by area of the patch.  For
> > > > me to manually add the people on the cc: of the email, I would have to
> > > > modify git to add them to the commit somehow, sorry.
> > > 
> > > Is that some other script you have that does that?
> > 
> > I don't understand what you are asking here.
> > 
> > My workflow is:
> > 	- apply patches to a branch and test
> > 	- if good, 'git format-patch' to create the patches
> > 	- merge to my upstream branch and push out publically
> > 	- run script on the patches generated by 'git format-patch' to
> > 	  say they have been applied.
> > 
> > My custom script is the last thing there, that takes the patches, finds
> > the email addresses in the patch, and notifies everyone.
> 
> That custom script - is it available somewhere?

It's right here:
	https://github.com/gregkh/gregkh-linux/blob/master/work/do.sh
along with other scripts that I use for kernel patch development, feel
free to use them if you like.

Note, this script does all of the above steps except apply the patches,
I got tired of running 3 scripts so I merged them into one.

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>

[PATCH 2/3] zsmalloc: add generic path and remove x86 dependency

[Seth Jennings]

This patch adds generic pages mapping methods that
work on all archs in the absence of support for
local_tlb_flush_kernel_range() advertised by the
arch through __HAVE_LOCAL_TLB_FLUSH_KERNEL_RANGE

Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
---
 drivers/staging/zsmalloc/Kconfig         |    4 -
 drivers/staging/zsmalloc/zsmalloc-main.c |  136 ++++++++++++++++++++++++------
 drivers/staging/zsmalloc/zsmalloc_int.h  |    5 +-
 3 files changed, 115 insertions(+), 30 deletions(-)

diff --git a/drivers/staging/zsmalloc/Kconfig b/drivers/staging/zsmalloc/Kconfig
index a5ab720..9084565 100644
--- a/drivers/staging/zsmalloc/Kconfig
+++ b/drivers/staging/zsmalloc/Kconfig
@@ -1,9 +1,5 @@
 config ZSMALLOC
 	tristate "Memory allocator for compressed pages"
-	# X86 dependency is because of the use of __flush_tlb_one and set_pte
-	# in zsmalloc-main.c.
-	# TODO: convert these to portable functions
-	depends on X86
 	default n
 	help
 	  zsmalloc is a slab-based memory allocator designed to store
diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c
index 10b0d60..14f04d8 100644
--- a/drivers/staging/zsmalloc/zsmalloc-main.c
+++ b/drivers/staging/zsmalloc/zsmalloc-main.c
@@ -470,28 +470,116 @@ static struct page *find_get_zspage(struct size_class *class)
 	return page;
 }
 
+#ifdef __HAVE_LOCAL_FLUSH_TLB_KERNEL_RANGE
+static inline int zs_arch_cpu_up(struct mapping_area *area)
+{
+	if (area->vm)
+		return 0;
+	area->vm = alloc_vm_area(PAGE_SIZE * 2, NULL);
+	if (!area->vm)
+		return -ENOMEM;
+	return 0;
+}
+
+static inline void zs_arch_cpu_down(struct mapping_area *area)
+{
+	if (area->vm)
+		free_vm_area(area->vm);
+	area->vm = NULL;
+}
+
+static inline void zs_arch_map_object(struct mapping_area *area,
+				struct page *pages[2], int off, int size)
+{
+	BUG_ON(map_vm_area(area->vm, PAGE_KERNEL, &pages));
+	area->vm_addr = area->vm->addr;
+}
+
+static inline void zs_arch_unmap_object(struct mapping_area *area,
+				struct page *pages[2], int off, int size)
+{
+	unsigned long addr = (unsigned long)area->vm_addr;
+	unsigned long end = addr + (PAGE_SIZE * 2);
+
+	flush_cache_vunmap(addr, end);
+	unmap_kernel_range_noflush(addr, PAGE_SIZE * 2);
+	local_flush_tlb_kernel_range(addr, end);
+}
+#else
+static inline int zs_arch_cpu_up(struct mapping_area *area)
+{
+	if (area->vm_buf)
+		return 0;
+	area->vm_buf = (char *)__get_free_pages(GFP_KERNEL, 1);
+	if (!area->vm_buf)
+		return -ENOMEM;
+	return 0;
+}
+
+static inline void zs_arch_cpu_down(struct mapping_area *area)
+{
+	if (area->vm_buf)
+		free_pages((unsigned long)area->vm_buf, 1);
+	area->vm_buf = NULL;
+}
+
+static void zs_arch_map_object(struct mapping_area *area,
+				struct page *pages[2], int off, int size)
+{
+	int sizes[2];
+	char *buf = area->vm_buf + off;
+	void *addr;
+
+	sizes[0] = PAGE_SIZE - off;
+	sizes[1] = size - sizes[0];
+
+	/* copy object to temp buffer */
+	addr = kmap_atomic(pages[0]);
+	memcpy(buf, addr + off, sizes[0]);
+	kunmap_atomic(addr);
+	addr = kmap_atomic(pages[1]);
+	memcpy(buf + sizes[0], addr, sizes[1]);
+	kunmap_atomic(addr);
+	area->vm_addr = area->vm_buf;
+}
+
+static void zs_arch_unmap_object(struct mapping_area *area,
+				struct page *pages[2], int off, int size)
+{
+	int sizes[2];
+	char *buf = area->vm_buf + off;
+	void *addr;
+
+	sizes[0] = PAGE_SIZE - off;
+	sizes[1] = size - sizes[0];
+
+	/* copy temp buffer to obj*/
+	addr = kmap_atomic(pages[0]);
+	memcpy(addr + off, buf, sizes[0]);
+	kunmap_atomic(addr);
+	addr = kmap_atomic(pages[1]);
+	memcpy(addr, buf + sizes[0], sizes[1]);
+	kunmap_atomic(addr);
+}
+#endif
 
 static int zs_cpu_notifier(struct notifier_block *nb, unsigned long action,
 				void *pcpu)
 {
-	int cpu = (long)pcpu;
+	int ret, cpu = (long)pcpu;
 	struct mapping_area *area;
 
 	switch (action) {
 	case CPU_UP_PREPARE:
 		area = &per_cpu(zs_map_area, cpu);
-		if (area->vm)
-			break;
-		area->vm = alloc_vm_area(2 * PAGE_SIZE, area->vm_ptes);
-		if (!area->vm)
-			return notifier_from_errno(-ENOMEM);
+		ret = zs_arch_cpu_up(area);
+		if (ret)
+			return notifier_from_errno(ret);
 		break;
 	case CPU_DEAD:
 	case CPU_UP_CANCELED:
 		area = &per_cpu(zs_map_area, cpu);
-		if (area->vm)
-			free_vm_area(area->vm);
-		area->vm = NULL;
+		zs_arch_cpu_down(area);
 		break;
 	}
 
@@ -716,19 +804,14 @@ void *zs_map_object(struct zs_pool *pool, unsigned long handle)
 		area->vm_addr = kmap_atomic(page);
 	} else {
 		/* this object spans two pages */
-		struct page *nextp;
-
-		nextp = get_next_page(page);
-		BUG_ON(!nextp);
+		struct page *pages[2];
 
+		pages[0] = page;
+		pages[1] = get_next_page(page);
+		BUG_ON(!pages[1]);
 
-		set_pte(area->vm_ptes[0], mk_pte(page, PAGE_KERNEL));
-		set_pte(area->vm_ptes[1], mk_pte(nextp, PAGE_KERNEL));
-
-		/* We pre-allocated VM area so mapping can never fail */
-		area->vm_addr = area->vm->addr;
+		zs_arch_map_object(area, pages, off, class->size);
 	}
-
 	return area->vm_addr + off;
 }
 EXPORT_SYMBOL_GPL(zs_map_object);
@@ -751,13 +834,16 @@ void zs_unmap_object(struct zs_pool *pool, unsigned long handle)
 	off = obj_idx_to_offset(page, obj_idx, class->size);
 
 	area = &__get_cpu_var(zs_map_area);
-	if (off + class->size <= PAGE_SIZE) {
+	if (off + class->size <= PAGE_SIZE)
 		kunmap_atomic(area->vm_addr);
-	} else {
-		set_pte(area->vm_ptes[0], __pte(0));
-		set_pte(area->vm_ptes[1], __pte(0));
-		__flush_tlb_one((unsigned long)area->vm_addr);
-		__flush_tlb_one((unsigned long)area->vm_addr + PAGE_SIZE);
+	else {
+		struct page *pages[2];
+
+		pages[0] = page;
+		pages[1] = get_next_page(page);
+		BUG_ON(!pages[1]);
+
+		zs_arch_unmap_object(area, pages, off, class->size);
 	}
 	put_cpu_var(zs_map_area);
 }
diff --git a/drivers/staging/zsmalloc/zsmalloc_int.h b/drivers/staging/zsmalloc/zsmalloc_int.h
index 6fd32a9..8a6887e 100644
--- a/drivers/staging/zsmalloc/zsmalloc_int.h
+++ b/drivers/staging/zsmalloc/zsmalloc_int.h
@@ -110,8 +110,11 @@ enum fullness_group {
 static const int fullness_threshold_frac = 4;
 
 struct mapping_area {
+#ifdef __HAVE_LOCAL_FLUSH_TLB_KERNEL_RANGE
 	struct vm_struct *vm;
-	pte_t *vm_ptes[2];
+#else
+	char *vm_buf;
+#endif
 	char *vm_addr;
 };
 
-- 
1.7.9.5

[PATCH 2/3] zsmalloc: add generic path and remove x86 dependency

[Seth Jennings]

This patch adds generic pages mapping methods that
work on all archs in the absence of support for
local_tlb_flush_kernel_range() advertised by the
arch through __HAVE_LOCAL_TLB_FLUSH_KERNEL_RANGE

Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
---
 drivers/staging/zsmalloc/Kconfig         |    4 -
 drivers/staging/zsmalloc/zsmalloc-main.c |  136 ++++++++++++++++++++++++------
 drivers/staging/zsmalloc/zsmalloc_int.h  |    5 +-
 3 files changed, 115 insertions(+), 30 deletions(-)

diff --git a/drivers/staging/zsmalloc/Kconfig b/drivers/staging/zsmalloc/Kconfig
index a5ab720..9084565 100644
--- a/drivers/staging/zsmalloc/Kconfig
+++ b/drivers/staging/zsmalloc/Kconfig
@@ -1,9 +1,5 @@
 config ZSMALLOC
 	tristate "Memory allocator for compressed pages"
-	# X86 dependency is because of the use of __flush_tlb_one and set_pte
-	# in zsmalloc-main.c.
-	# TODO: convert these to portable functions
-	depends on X86
 	default n
 	help
 	  zsmalloc is a slab-based memory allocator designed to store
diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c
index 10b0d60..14f04d8 100644
--- a/drivers/staging/zsmalloc/zsmalloc-main.c
+++ b/drivers/staging/zsmalloc/zsmalloc-main.c
@@ -470,28 +470,116 @@ static struct page *find_get_zspage(struct size_class *class)
 	return page;
 }
 
+#ifdef __HAVE_LOCAL_FLUSH_TLB_KERNEL_RANGE
+static inline int zs_arch_cpu_up(struct mapping_area *area)
+{
+	if (area->vm)
+		return 0;
+	area->vm = alloc_vm_area(PAGE_SIZE * 2, NULL);
+	if (!area->vm)
+		return -ENOMEM;
+	return 0;
+}
+
+static inline void zs_arch_cpu_down(struct mapping_area *area)
+{
+	if (area->vm)
+		free_vm_area(area->vm);
+	area->vm = NULL;
+}
+
+static inline void zs_arch_map_object(struct mapping_area *area,
+				struct page *pages[2], int off, int size)
+{
+	BUG_ON(map_vm_area(area->vm, PAGE_KERNEL, &pages));
+	area->vm_addr = area->vm->addr;
+}
+
+static inline void zs_arch_unmap_object(struct mapping_area *area,
+				struct page *pages[2], int off, int size)
+{
+	unsigned long addr = (unsigned long)area->vm_addr;
+	unsigned long end = addr + (PAGE_SIZE * 2);
+
+	flush_cache_vunmap(addr, end);
+	unmap_kernel_range_noflush(addr, PAGE_SIZE * 2);
+	local_flush_tlb_kernel_range(addr, end);
+}
+#else
+static inline int zs_arch_cpu_up(struct mapping_area *area)
+{
+	if (area->vm_buf)
+		return 0;
+	area->vm_buf = (char *)__get_free_pages(GFP_KERNEL, 1);
+	if (!area->vm_buf)
+		return -ENOMEM;
+	return 0;
+}
+
+static inline void zs_arch_cpu_down(struct mapping_area *area)
+{
+	if (area->vm_buf)
+		free_pages((unsigned long)area->vm_buf, 1);
+	area->vm_buf = NULL;
+}
+
+static void zs_arch_map_object(struct mapping_area *area,
+				struct page *pages[2], int off, int size)
+{
+	int sizes[2];
+	char *buf = area->vm_buf + off;
+	void *addr;
+
+	sizes[0] = PAGE_SIZE - off;
+	sizes[1] = size - sizes[0];
+
+	/* copy object to temp buffer */
+	addr = kmap_atomic(pages[0]);
+	memcpy(buf, addr + off, sizes[0]);
+	kunmap_atomic(addr);
+	addr = kmap_atomic(pages[1]);
+	memcpy(buf + sizes[0], addr, sizes[1]);
+	kunmap_atomic(addr);
+	area->vm_addr = area->vm_buf;
+}
+
+static void zs_arch_unmap_object(struct mapping_area *area,
+				struct page *pages[2], int off, int size)
+{
+	int sizes[2];
+	char *buf = area->vm_buf + off;
+	void *addr;
+
+	sizes[0] = PAGE_SIZE - off;
+	sizes[1] = size - sizes[0];
+
+	/* copy temp buffer to obj*/
+	addr = kmap_atomic(pages[0]);
+	memcpy(addr + off, buf, sizes[0]);
+	kunmap_atomic(addr);
+	addr = kmap_atomic(pages[1]);
+	memcpy(addr, buf + sizes[0], sizes[1]);
+	kunmap_atomic(addr);
+}
+#endif
 
 static int zs_cpu_notifier(struct notifier_block *nb, unsigned long action,
 				void *pcpu)
 {
-	int cpu = (long)pcpu;
+	int ret, cpu = (long)pcpu;
 	struct mapping_area *area;
 
 	switch (action) {
 	case CPU_UP_PREPARE:
 		area = &per_cpu(zs_map_area, cpu);
-		if (area->vm)
-			break;
-		area->vm = alloc_vm_area(2 * PAGE_SIZE, area->vm_ptes);
-		if (!area->vm)
-			return notifier_from_errno(-ENOMEM);
+		ret = zs_arch_cpu_up(area);
+		if (ret)
+			return notifier_from_errno(ret);
 		break;
 	case CPU_DEAD:
 	case CPU_UP_CANCELED:
 		area = &per_cpu(zs_map_area, cpu);
-		if (area->vm)
-			free_vm_area(area->vm);
-		area->vm = NULL;
+		zs_arch_cpu_down(area);
 		break;
 	}
 
@@ -716,19 +804,14 @@ void *zs_map_object(struct zs_pool *pool, unsigned long handle)
 		area->vm_addr = kmap_atomic(page);
 	} else {
 		/* this object spans two pages */
-		struct page *nextp;
-
-		nextp = get_next_page(page);
-		BUG_ON(!nextp);
+		struct page *pages[2];
 
+		pages[0] = page;
+		pages[1] = get_next_page(page);
+		BUG_ON(!pages[1]);
 
-		set_pte(area->vm_ptes[0], mk_pte(page, PAGE_KERNEL));
-		set_pte(area->vm_ptes[1], mk_pte(nextp, PAGE_KERNEL));
-
-		/* We pre-allocated VM area so mapping can never fail */
-		area->vm_addr = area->vm->addr;
+		zs_arch_map_object(area, pages, off, class->size);
 	}
-
 	return area->vm_addr + off;
 }
 EXPORT_SYMBOL_GPL(zs_map_object);
@@ -751,13 +834,16 @@ void zs_unmap_object(struct zs_pool *pool, unsigned long handle)
 	off = obj_idx_to_offset(page, obj_idx, class->size);
 
 	area = &__get_cpu_var(zs_map_area);
-	if (off + class->size <= PAGE_SIZE) {
+	if (off + class->size <= PAGE_SIZE)
 		kunmap_atomic(area->vm_addr);
-	} else {
-		set_pte(area->vm_ptes[0], __pte(0));
-		set_pte(area->vm_ptes[1], __pte(0));
-		__flush_tlb_one((unsigned long)area->vm_addr);
-		__flush_tlb_one((unsigned long)area->vm_addr + PAGE_SIZE);
+	else {
+		struct page *pages[2];
+
+		pages[0] = page;
+		pages[1] = get_next_page(page);
+		BUG_ON(!pages[1]);
+
+		zs_arch_unmap_object(area, pages, off, class->size);
 	}
 	put_cpu_var(zs_map_area);
 }
diff --git a/drivers/staging/zsmalloc/zsmalloc_int.h b/drivers/staging/zsmalloc/zsmalloc_int.h
index 6fd32a9..8a6887e 100644
--- a/drivers/staging/zsmalloc/zsmalloc_int.h
+++ b/drivers/staging/zsmalloc/zsmalloc_int.h
@@ -110,8 +110,11 @@ enum fullness_group {
 static const int fullness_threshold_frac = 4;
 
 struct mapping_area {
+#ifdef __HAVE_LOCAL_FLUSH_TLB_KERNEL_RANGE
 	struct vm_struct *vm;
-	pte_t *vm_ptes[2];
+#else
+	char *vm_buf;
+#endif
 	char *vm_addr;
 };
 
-- 
1.7.9.5

--
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>

Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency

[Greg Kroah-Hartman]

On Mon, Jun 25, 2012 at 11:14:37AM -0500, Seth Jennings wrote:
> This patch adds generic pages mapping methods that
> work on all archs in the absence of support for
> local_tlb_flush_kernel_range() advertised by the
> arch through __HAVE_LOCAL_TLB_FLUSH_KERNEL_RANGE

Is this #define something that other arches define now?  Or is this
something new that you are adding here?

thanks,

greg k-h

Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency

[Greg Kroah-Hartman]

On Mon, Jun 25, 2012 at 11:14:37AM -0500, Seth Jennings wrote:
> This patch adds generic pages mapping methods that
> work on all archs in the absence of support for
> local_tlb_flush_kernel_range() advertised by the
> arch through __HAVE_LOCAL_TLB_FLUSH_KERNEL_RANGE

Is this #define something that other arches define now?  Or is this
something new that you are adding here?

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>

Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency

[Seth Jennings]

On 06/25/2012 11:59 AM, Greg Kroah-Hartman wrote:
> On Mon, Jun 25, 2012 at 11:14:37AM -0500, Seth Jennings wrote:
>> This patch adds generic pages mapping methods that
>> work on all archs in the absence of support for
>> local_tlb_flush_kernel_range() advertised by the
>> arch through __HAVE_LOCAL_TLB_FLUSH_KERNEL_RANGE
> 
> Is this #define something that other arches define now?  Or is this
> something new that you are adding here?

Something new I'm adding.

The precedent for this approach is the __HAVE_ARCH_* defines
that let the arch independent stuff know if a generic
function needs to be defined or if there is an arch specific
function.

You can "grep -R __HAVE_ARCH_* arch/x86/" to see the ones
that already exist.

I guess I should have called it
__HAVE_ARCH_LOCAL_TLB_FLUSH_KERNEL_RANGE though, not
__HAVE_LOCAL_TLB_FLUSH_KERNEL_RANGE.

--
Seth

Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency

[Seth Jennings]

On 06/25/2012 11:59 AM, Greg Kroah-Hartman wrote:
> On Mon, Jun 25, 2012 at 11:14:37AM -0500, Seth Jennings wrote:
>> This patch adds generic pages mapping methods that
>> work on all archs in the absence of support for
>> local_tlb_flush_kernel_range() advertised by the
>> arch through __HAVE_LOCAL_TLB_FLUSH_KERNEL_RANGE
> 
> Is this #define something that other arches define now?  Or is this
> something new that you are adding here?

Something new I'm adding.

The precedent for this approach is the __HAVE_ARCH_* defines
that let the arch independent stuff know if a generic
function needs to be defined or if there is an arch specific
function.

You can "grep -R __HAVE_ARCH_* arch/x86/" to see the ones
that already exist.

I guess I should have called it
__HAVE_ARCH_LOCAL_TLB_FLUSH_KERNEL_RANGE though, not
__HAVE_LOCAL_TLB_FLUSH_KERNEL_RANGE.

--
Seth

--
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>

Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency

[Greg Kroah-Hartman]

On Mon, Jun 25, 2012 at 12:10:57PM -0500, Seth Jennings wrote:
> On 06/25/2012 11:59 AM, Greg Kroah-Hartman wrote:
> > On Mon, Jun 25, 2012 at 11:14:37AM -0500, Seth Jennings wrote:
> >> This patch adds generic pages mapping methods that
> >> work on all archs in the absence of support for
> >> local_tlb_flush_kernel_range() advertised by the
> >> arch through __HAVE_LOCAL_TLB_FLUSH_KERNEL_RANGE
> > 
> > Is this #define something that other arches define now?  Or is this
> > something new that you are adding here?
> 
> Something new I'm adding.

Ah, ok.

> The precedent for this approach is the __HAVE_ARCH_* defines
> that let the arch independent stuff know if a generic
> function needs to be defined or if there is an arch specific
> function.
> 
> You can "grep -R __HAVE_ARCH_* arch/x86/" to see the ones
> that already exist.
> 
> I guess I should have called it
> __HAVE_ARCH_LOCAL_TLB_FLUSH_KERNEL_RANGE though, not
> __HAVE_LOCAL_TLB_FLUSH_KERNEL_RANGE.

You need to get the mm developers to agree with this before I can take
it.

But, why even depend on this?  Can't you either live without it, or
just implement it for all arches somehow?

thanks,

greg k-h

Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency

[Greg Kroah-Hartman]

On Mon, Jun 25, 2012 at 12:10:57PM -0500, Seth Jennings wrote:
> On 06/25/2012 11:59 AM, Greg Kroah-Hartman wrote:
> > On Mon, Jun 25, 2012 at 11:14:37AM -0500, Seth Jennings wrote:
> >> This patch adds generic pages mapping methods that
> >> work on all archs in the absence of support for
> >> local_tlb_flush_kernel_range() advertised by the
> >> arch through __HAVE_LOCAL_TLB_FLUSH_KERNEL_RANGE
> > 
> > Is this #define something that other arches define now?  Or is this
> > something new that you are adding here?
> 
> Something new I'm adding.

Ah, ok.

> The precedent for this approach is the __HAVE_ARCH_* defines
> that let the arch independent stuff know if a generic
> function needs to be defined or if there is an arch specific
> function.
> 
> You can "grep -R __HAVE_ARCH_* arch/x86/" to see the ones
> that already exist.
> 
> I guess I should have called it
> __HAVE_ARCH_LOCAL_TLB_FLUSH_KERNEL_RANGE though, not
> __HAVE_LOCAL_TLB_FLUSH_KERNEL_RANGE.

You need to get the mm developers to agree with this before I can take
it.

But, why even depend on this?  Can't you either live without it, or
just implement it for all arches somehow?

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>

Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency

[Seth Jennings]

On 06/25/2012 12:19 PM, Greg Kroah-Hartman wrote:
> On Mon, Jun 25, 2012 at 12:10:57PM -0500, Seth Jennings wrote:
>> On 06/25/2012 11:59 AM, Greg Kroah-Hartman wrote:
>>> On Mon, Jun 25, 2012 at 11:14:37AM -0500, Seth Jennings wrote:
>>>> This patch adds generic pages mapping methods that
>>>> work on all archs in the absence of support for
>>>> local_tlb_flush_kernel_range() advertised by the
>>>> arch through __HAVE_LOCAL_TLB_FLUSH_KERNEL_RANGE
>>>
>>> Is this #define something that other arches define now?  Or is this
>>> something new that you are adding here?
>>
>> Something new I'm adding.
> 
> Ah, ok.
> 
>> The precedent for this approach is the __HAVE_ARCH_* defines
>> that let the arch independent stuff know if a generic
>> function needs to be defined or if there is an arch specific
>> function.
>>
>> You can "grep -R __HAVE_ARCH_* arch/x86/" to see the ones
>> that already exist.
>>
>> I guess I should have called it
>> __HAVE_ARCH_LOCAL_TLB_FLUSH_KERNEL_RANGE though, not
>> __HAVE_LOCAL_TLB_FLUSH_KERNEL_RANGE.
> 
> You need to get the mm developers to agree with this before I can take
> it.
> 
> But, why even depend on this?  Can't you either live without it

The whole point of the patch is _not_ to depend on it.  It
just performs worse without it.  We could just rip out all
the the page table assisted page mapping, but, for the
arches that have support for it, we'd be degrading
performance in exchange for portability.  Why choose when we
can have both?

> , or just implement it for all arches somehow?

It can be implemented for some arches and already is for
some (MIPS, ARM, at least).  But for some arches, I imagine
this can't be implemented due to hardware limitations.

A benefit of this approach is the arches opt-in to the
optimized zsmalloc by implementing
local_tlb_flush_kernel_range() without having to change
anything in zsmalloc.

--
Seth

Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency

[Seth Jennings]

On 06/25/2012 12:19 PM, Greg Kroah-Hartman wrote:
> On Mon, Jun 25, 2012 at 12:10:57PM -0500, Seth Jennings wrote:
>> On 06/25/2012 11:59 AM, Greg Kroah-Hartman wrote:
>>> On Mon, Jun 25, 2012 at 11:14:37AM -0500, Seth Jennings wrote:
>>>> This patch adds generic pages mapping methods that
>>>> work on all archs in the absence of support for
>>>> local_tlb_flush_kernel_range() advertised by the
>>>> arch through __HAVE_LOCAL_TLB_FLUSH_KERNEL_RANGE
>>>
>>> Is this #define something that other arches define now?  Or is this
>>> something new that you are adding here?
>>
>> Something new I'm adding.
> 
> Ah, ok.
> 
>> The precedent for this approach is the __HAVE_ARCH_* defines
>> that let the arch independent stuff know if a generic
>> function needs to be defined or if there is an arch specific
>> function.
>>
>> You can "grep -R __HAVE_ARCH_* arch/x86/" to see the ones
>> that already exist.
>>
>> I guess I should have called it
>> __HAVE_ARCH_LOCAL_TLB_FLUSH_KERNEL_RANGE though, not
>> __HAVE_LOCAL_TLB_FLUSH_KERNEL_RANGE.
> 
> You need to get the mm developers to agree with this before I can take
> it.
> 
> But, why even depend on this?  Can't you either live without it

The whole point of the patch is _not_ to depend on it.  It
just performs worse without it.  We could just rip out all
the the page table assisted page mapping, but, for the
arches that have support for it, we'd be degrading
performance in exchange for portability.  Why choose when we
can have both?

> , or just implement it for all arches somehow?

It can be implemented for some arches and already is for
some (MIPS, ARM, at least).  But for some arches, I imagine
this can't be implemented due to hardware limitations.

A benefit of this approach is the arches opt-in to the
optimized zsmalloc by implementing
local_tlb_flush_kernel_range() without having to change
anything in zsmalloc.

--
Seth

--
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>

Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency

[Greg Kroah-Hartman]

On Mon, Jun 25, 2012 at 01:24:29PM -0500, Seth Jennings wrote:
> On 06/25/2012 12:19 PM, Greg Kroah-Hartman wrote:
> > On Mon, Jun 25, 2012 at 12:10:57PM -0500, Seth Jennings wrote:
> >> On 06/25/2012 11:59 AM, Greg Kroah-Hartman wrote:
> >>> On Mon, Jun 25, 2012 at 11:14:37AM -0500, Seth Jennings wrote:
> >>>> This patch adds generic pages mapping methods that
> >>>> work on all archs in the absence of support for
> >>>> local_tlb_flush_kernel_range() advertised by the
> >>>> arch through __HAVE_LOCAL_TLB_FLUSH_KERNEL_RANGE
> >>>
> >>> Is this #define something that other arches define now?  Or is this
> >>> something new that you are adding here?
> >>
> >> Something new I'm adding.
> > 
> > Ah, ok.
> > 
> >> The precedent for this approach is the __HAVE_ARCH_* defines
> >> that let the arch independent stuff know if a generic
> >> function needs to be defined or if there is an arch specific
> >> function.
> >>
> >> You can "grep -R __HAVE_ARCH_* arch/x86/" to see the ones
> >> that already exist.
> >>
> >> I guess I should have called it
> >> __HAVE_ARCH_LOCAL_TLB_FLUSH_KERNEL_RANGE though, not
> >> __HAVE_LOCAL_TLB_FLUSH_KERNEL_RANGE.
> > 
> > You need to get the mm developers to agree with this before I can take
> > it.
> > 
> > But, why even depend on this?  Can't you either live without it
> 
> The whole point of the patch is _not_ to depend on it.  It
> just performs worse without it.  We could just rip out all
> the the page table assisted page mapping, but, for the
> arches that have support for it, we'd be degrading
> performance in exchange for portability.  Why choose when we
> can have both?

Ok, I'll let you fight it out with the mm people before applying these 2
patches, I've applied the first one only for now.

greg k-h

Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency

[Greg Kroah-Hartman]

On Mon, Jun 25, 2012 at 01:24:29PM -0500, Seth Jennings wrote:
> On 06/25/2012 12:19 PM, Greg Kroah-Hartman wrote:
> > On Mon, Jun 25, 2012 at 12:10:57PM -0500, Seth Jennings wrote:
> >> On 06/25/2012 11:59 AM, Greg Kroah-Hartman wrote:
> >>> On Mon, Jun 25, 2012 at 11:14:37AM -0500, Seth Jennings wrote:
> >>>> This patch adds generic pages mapping methods that
> >>>> work on all archs in the absence of support for
> >>>> local_tlb_flush_kernel_range() advertised by the
> >>>> arch through __HAVE_LOCAL_TLB_FLUSH_KERNEL_RANGE
> >>>
> >>> Is this #define something that other arches define now?  Or is this
> >>> something new that you are adding here?
> >>
> >> Something new I'm adding.
> > 
> > Ah, ok.
> > 
> >> The precedent for this approach is the __HAVE_ARCH_* defines
> >> that let the arch independent stuff know if a generic
> >> function needs to be defined or if there is an arch specific
> >> function.
> >>
> >> You can "grep -R __HAVE_ARCH_* arch/x86/" to see the ones
> >> that already exist.
> >>
> >> I guess I should have called it
> >> __HAVE_ARCH_LOCAL_TLB_FLUSH_KERNEL_RANGE though, not
> >> __HAVE_LOCAL_TLB_FLUSH_KERNEL_RANGE.
> > 
> > You need to get the mm developers to agree with this before I can take
> > it.
> > 
> > But, why even depend on this?  Can't you either live without it
> 
> The whole point of the patch is _not_ to depend on it.  It
> just performs worse without it.  We could just rip out all
> the the page table assisted page mapping, but, for the
> arches that have support for it, we'd be degrading
> performance in exchange for portability.  Why choose when we
> can have both?

Ok, I'll let you fight it out with the mm people before applying these 2
patches, I've applied the first one only for now.

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>

Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency

[Minchan Kim]

On 06/26/2012 01:14 AM, Seth Jennings wrote:

> This patch adds generic pages mapping methods that
> work on all archs in the absence of support for
> local_tlb_flush_kernel_range() advertised by the
> arch through __HAVE_LOCAL_TLB_FLUSH_KERNEL_RANGE
> 
> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>


Sorry for handling this issue recently.
I like the patch.
Some comment below.

> ---
>  drivers/staging/zsmalloc/Kconfig         |    4 -
>  drivers/staging/zsmalloc/zsmalloc-main.c |  136 ++++++++++++++++++++++++------
>  drivers/staging/zsmalloc/zsmalloc_int.h  |    5 +-
>  3 files changed, 115 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/staging/zsmalloc/Kconfig b/drivers/staging/zsmalloc/Kconfig
> index a5ab720..9084565 100644
> --- a/drivers/staging/zsmalloc/Kconfig
> +++ b/drivers/staging/zsmalloc/Kconfig
> @@ -1,9 +1,5 @@
>  config ZSMALLOC
>  	tristate "Memory allocator for compressed pages"
> -	# X86 dependency is because of the use of __flush_tlb_one and set_pte
> -	# in zsmalloc-main.c.
> -	# TODO: convert these to portable functions
> -	depends on X86
>  	default n
>  	help
>  	  zsmalloc is a slab-based memory allocator designed to store
> diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c
> index 10b0d60..14f04d8 100644
> --- a/drivers/staging/zsmalloc/zsmalloc-main.c
> +++ b/drivers/staging/zsmalloc/zsmalloc-main.c
> @@ -470,28 +470,116 @@ static struct page *find_get_zspage(struct size_class *class)
>  	return page;
>  }
>  
> +#ifdef __HAVE_LOCAL_FLUSH_TLB_KERNEL_RANGE


As you already mentioned, __HAVE_ARCH_LOCAL_XXX

> +static inline int zs_arch_cpu_up(struct mapping_area *area)


Why should function's name represent arch dependent?
IMHO, it would be better to use just zs_cpu_up.


> +{
> +	if (area->vm)
> +		return 0;


Just out of curiosity.
When do we need above check?

> +	area->vm = alloc_vm_area(PAGE_SIZE * 2, NULL);
> +	if (!area->vm)
> +		return -ENOMEM;
> +	return 0;
> +}
> +
> +static inline void zs_arch_cpu_down(struct mapping_area *area)


Ditto.

> +{
> +	if (area->vm)
> +		free_vm_area(area->vm);
> +	area->vm = NULL;
> +}
> +
> +static inline void zs_arch_map_object(struct mapping_area *area,
> +				struct page *pages[2], int off, int size)
> +{
> +	BUG_ON(map_vm_area(area->vm, PAGE_KERNEL, &pages));


I think we need some comment about why map_vm_area must not fail.
I used below comment in my patch.
+               /*
+                * map_vm_area never fail because we already allocated
+                * pages for page table in alloc_vm_area.
+                */

> +	area->vm_addr = area->vm->addr;
> +}
> +
> +static inline void zs_arch_unmap_object(struct mapping_area *area,
> +				struct page *pages[2], int off, int size)
> +{
> +	unsigned long addr = (unsigned long)area->vm_addr;
> +	unsigned long end = addr + (PAGE_SIZE * 2);
> +
> +	flush_cache_vunmap(addr, end);
> +	unmap_kernel_range_noflush(addr, PAGE_SIZE * 2);
> +	local_flush_tlb_kernel_range(addr, end);
> +}
> +#else
> +static inline int zs_arch_cpu_up(struct mapping_area *area)
> +{
> +	if (area->vm_buf)
> +		return 0;
> +	area->vm_buf = (char *)__get_free_pages(GFP_KERNEL, 1);

> +	if (!area->vm_buf)

> +		return -ENOMEM;
> +	return 0;
> +}
> +
> +static inline void zs_arch_cpu_down(struct mapping_area *area)
> +{
> +	if (area->vm_buf)
> +		free_pages((unsigned long)area->vm_buf, 1);
> +	area->vm_buf = NULL;
> +}
> +
> +static void zs_arch_map_object(struct mapping_area *area,
> +				struct page *pages[2], int off, int size)


How about just void __zs_map_object?
Anyway, it's just preference and I am not strong against.

> +{
> +	int sizes[2];
> +	char *buf = area->vm_buf + off;
> +	void *addr;
> +
> +	sizes[0] = PAGE_SIZE - off;
> +	sizes[1] = size - sizes[0];
> +
> +	/* copy object to temp buffer */
> +	addr = kmap_atomic(pages[0]);
> +	memcpy(buf, addr + off, sizes[0]);
> +	kunmap_atomic(addr);
> +	addr = kmap_atomic(pages[1]);
> +	memcpy(buf + sizes[0], addr, sizes[1]);
> +	kunmap_atomic(addr);
> +	area->vm_addr = area->vm_buf;
> +}
> +
> +static void zs_arch_unmap_object(struct mapping_area *area,
> +				struct page *pages[2], int off, int size)
> +{
> +	int sizes[2];
> +	char *buf = area->vm_buf + off;
> +	void *addr;
> +
> +	sizes[0] = PAGE_SIZE - off;
> +	sizes[1] = size - sizes[0];
> +
> +	/* copy temp buffer to obj*/
> +	addr = kmap_atomic(pages[0]);
> +	memcpy(addr + off, buf, sizes[0]);
> +	kunmap_atomic(addr);
> +	addr = kmap_atomic(pages[1]);
> +	memcpy(addr, buf + sizes[0], sizes[1]);
> +	kunmap_atomic(addr);
> +}
> +#endif
>  
>  static int zs_cpu_notifier(struct notifier_block *nb, unsigned long action,
>  				void *pcpu)
>  {
> -	int cpu = (long)pcpu;
> +	int ret, cpu = (long)pcpu;
>  	struct mapping_area *area;
>  
>  	switch (action) {
>  	case CPU_UP_PREPARE:
>  		area = &per_cpu(zs_map_area, cpu);
> -		if (area->vm)
> -			break;
> -		area->vm = alloc_vm_area(2 * PAGE_SIZE, area->vm_ptes);
> -		if (!area->vm)
> -			return notifier_from_errno(-ENOMEM);
> +		ret = zs_arch_cpu_up(area);
> +		if (ret)
> +			return notifier_from_errno(ret);
>  		break;
>  	case CPU_DEAD:
>  	case CPU_UP_CANCELED:
>  		area = &per_cpu(zs_map_area, cpu);
> -		if (area->vm)
> -			free_vm_area(area->vm);
> -		area->vm = NULL;
> +		zs_arch_cpu_down(area);
>  		break;
>  	}
>  
> @@ -716,19 +804,14 @@ void *zs_map_object(struct zs_pool *pool, unsigned long handle)
>  		area->vm_addr = kmap_atomic(page);
>  	} else {
>  		/* this object spans two pages */
> -		struct page *nextp;
> -
> -		nextp = get_next_page(page);
> -		BUG_ON(!nextp);
> +		struct page *pages[2];
>  
> +		pages[0] = page;
> +		pages[1] = get_next_page(page);
> +		BUG_ON(!pages[1]);
>  
> -		set_pte(area->vm_ptes[0], mk_pte(page, PAGE_KERNEL));
> -		set_pte(area->vm_ptes[1], mk_pte(nextp, PAGE_KERNEL));
> -
> -		/* We pre-allocated VM area so mapping can never fail */
> -		area->vm_addr = area->vm->addr;
> +		zs_arch_map_object(area, pages, off, class->size);
>  	}
> -
>  	return area->vm_addr + off;
>  }
>  EXPORT_SYMBOL_GPL(zs_map_object);
> @@ -751,13 +834,16 @@ void zs_unmap_object(struct zs_pool *pool, unsigned long handle)
>  	off = obj_idx_to_offset(page, obj_idx, class->size);
>  
>  	area = &__get_cpu_var(zs_map_area);
> -	if (off + class->size <= PAGE_SIZE) {
> +	if (off + class->size <= PAGE_SIZE)
>  		kunmap_atomic(area->vm_addr);
> -	} else {
> -		set_pte(area->vm_ptes[0], __pte(0));
> -		set_pte(area->vm_ptes[1], __pte(0));
> -		__flush_tlb_one((unsigned long)area->vm_addr);
> -		__flush_tlb_one((unsigned long)area->vm_addr + PAGE_SIZE);
> +	else {
> +		struct page *pages[2];
> +
> +		pages[0] = page;
> +		pages[1] = get_next_page(page);
> +		BUG_ON(!pages[1]);
> +
> +		zs_arch_unmap_object(area, pages, off, class->size);
>  	}
>  	put_cpu_var(zs_map_area);
>  }
> diff --git a/drivers/staging/zsmalloc/zsmalloc_int.h b/drivers/staging/zsmalloc/zsmalloc_int.h
> index 6fd32a9..8a6887e 100644
> --- a/drivers/staging/zsmalloc/zsmalloc_int.h
> +++ b/drivers/staging/zsmalloc/zsmalloc_int.h
> @@ -110,8 +110,11 @@ enum fullness_group {
>  static const int fullness_threshold_frac = 4;
>  
>  struct mapping_area {
> +#ifdef __HAVE_LOCAL_FLUSH_TLB_KERNEL_RANGE
>  	struct vm_struct *vm;
> -	pte_t *vm_ptes[2];
> +#else
> +	char *vm_buf;
> +#endif
>  	char *vm_addr;
>  };


Need comment about vm_buf and vm_addr.

Thanks, Seth.

-- 
Kind regards,
Minchan Kim

Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency

[Minchan Kim]

On 06/26/2012 01:14 AM, Seth Jennings wrote:

> This patch adds generic pages mapping methods that
> work on all archs in the absence of support for
> local_tlb_flush_kernel_range() advertised by the
> arch through __HAVE_LOCAL_TLB_FLUSH_KERNEL_RANGE
> 
> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>


Sorry for handling this issue recently.
I like the patch.
Some comment below.

> ---
>  drivers/staging/zsmalloc/Kconfig         |    4 -
>  drivers/staging/zsmalloc/zsmalloc-main.c |  136 ++++++++++++++++++++++++------
>  drivers/staging/zsmalloc/zsmalloc_int.h  |    5 +-
>  3 files changed, 115 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/staging/zsmalloc/Kconfig b/drivers/staging/zsmalloc/Kconfig
> index a5ab720..9084565 100644
> --- a/drivers/staging/zsmalloc/Kconfig
> +++ b/drivers/staging/zsmalloc/Kconfig
> @@ -1,9 +1,5 @@
>  config ZSMALLOC
>  	tristate "Memory allocator for compressed pages"
> -	# X86 dependency is because of the use of __flush_tlb_one and set_pte
> -	# in zsmalloc-main.c.
> -	# TODO: convert these to portable functions
> -	depends on X86
>  	default n
>  	help
>  	  zsmalloc is a slab-based memory allocator designed to store
> diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c
> index 10b0d60..14f04d8 100644
> --- a/drivers/staging/zsmalloc/zsmalloc-main.c
> +++ b/drivers/staging/zsmalloc/zsmalloc-main.c
> @@ -470,28 +470,116 @@ static struct page *find_get_zspage(struct size_class *class)
>  	return page;
>  }
>  
> +#ifdef __HAVE_LOCAL_FLUSH_TLB_KERNEL_RANGE


As you already mentioned, __HAVE_ARCH_LOCAL_XXX

> +static inline int zs_arch_cpu_up(struct mapping_area *area)


Why should function's name represent arch dependent?
IMHO, it would be better to use just zs_cpu_up.


> +{
> +	if (area->vm)
> +		return 0;


Just out of curiosity.
When do we need above check?

> +	area->vm = alloc_vm_area(PAGE_SIZE * 2, NULL);
> +	if (!area->vm)
> +		return -ENOMEM;
> +	return 0;
> +}
> +
> +static inline void zs_arch_cpu_down(struct mapping_area *area)


Ditto.

> +{
> +	if (area->vm)
> +		free_vm_area(area->vm);
> +	area->vm = NULL;
> +}
> +
> +static inline void zs_arch_map_object(struct mapping_area *area,
> +				struct page *pages[2], int off, int size)
> +{
> +	BUG_ON(map_vm_area(area->vm, PAGE_KERNEL, &pages));


I think we need some comment about why map_vm_area must not fail.
I used below comment in my patch.
+               /*
+                * map_vm_area never fail because we already allocated
+                * pages for page table in alloc_vm_area.
+                */

> +	area->vm_addr = area->vm->addr;
> +}
> +
> +static inline void zs_arch_unmap_object(struct mapping_area *area,
> +				struct page *pages[2], int off, int size)
> +{
> +	unsigned long addr = (unsigned long)area->vm_addr;
> +	unsigned long end = addr + (PAGE_SIZE * 2);
> +
> +	flush_cache_vunmap(addr, end);
> +	unmap_kernel_range_noflush(addr, PAGE_SIZE * 2);
> +	local_flush_tlb_kernel_range(addr, end);
> +}
> +#else
> +static inline int zs_arch_cpu_up(struct mapping_area *area)
> +{
> +	if (area->vm_buf)
> +		return 0;
> +	area->vm_buf = (char *)__get_free_pages(GFP_KERNEL, 1);

> +	if (!area->vm_buf)

> +		return -ENOMEM;
> +	return 0;
> +}
> +
> +static inline void zs_arch_cpu_down(struct mapping_area *area)
> +{
> +	if (area->vm_buf)
> +		free_pages((unsigned long)area->vm_buf, 1);
> +	area->vm_buf = NULL;
> +}
> +
> +static void zs_arch_map_object(struct mapping_area *area,
> +				struct page *pages[2], int off, int size)


How about just void __zs_map_object?
Anyway, it's just preference and I am not strong against.

> +{
> +	int sizes[2];
> +	char *buf = area->vm_buf + off;
> +	void *addr;
> +
> +	sizes[0] = PAGE_SIZE - off;
> +	sizes[1] = size - sizes[0];
> +
> +	/* copy object to temp buffer */
> +	addr = kmap_atomic(pages[0]);
> +	memcpy(buf, addr + off, sizes[0]);
> +	kunmap_atomic(addr);
> +	addr = kmap_atomic(pages[1]);
> +	memcpy(buf + sizes[0], addr, sizes[1]);
> +	kunmap_atomic(addr);
> +	area->vm_addr = area->vm_buf;
> +}
> +
> +static void zs_arch_unmap_object(struct mapping_area *area,
> +				struct page *pages[2], int off, int size)
> +{
> +	int sizes[2];
> +	char *buf = area->vm_buf + off;
> +	void *addr;
> +
> +	sizes[0] = PAGE_SIZE - off;
> +	sizes[1] = size - sizes[0];
> +
> +	/* copy temp buffer to obj*/
> +	addr = kmap_atomic(pages[0]);
> +	memcpy(addr + off, buf, sizes[0]);
> +	kunmap_atomic(addr);
> +	addr = kmap_atomic(pages[1]);
> +	memcpy(addr, buf + sizes[0], sizes[1]);
> +	kunmap_atomic(addr);
> +}
> +#endif
>  
>  static int zs_cpu_notifier(struct notifier_block *nb, unsigned long action,
>  				void *pcpu)
>  {
> -	int cpu = (long)pcpu;
> +	int ret, cpu = (long)pcpu;
>  	struct mapping_area *area;
>  
>  	switch (action) {
>  	case CPU_UP_PREPARE:
>  		area = &per_cpu(zs_map_area, cpu);
> -		if (area->vm)
> -			break;
> -		area->vm = alloc_vm_area(2 * PAGE_SIZE, area->vm_ptes);
> -		if (!area->vm)
> -			return notifier_from_errno(-ENOMEM);
> +		ret = zs_arch_cpu_up(area);
> +		if (ret)
> +			return notifier_from_errno(ret);
>  		break;
>  	case CPU_DEAD:
>  	case CPU_UP_CANCELED:
>  		area = &per_cpu(zs_map_area, cpu);
> -		if (area->vm)
> -			free_vm_area(area->vm);
> -		area->vm = NULL;
> +		zs_arch_cpu_down(area);
>  		break;
>  	}
>  
> @@ -716,19 +804,14 @@ void *zs_map_object(struct zs_pool *pool, unsigned long handle)
>  		area->vm_addr = kmap_atomic(page);
>  	} else {
>  		/* this object spans two pages */
> -		struct page *nextp;
> -
> -		nextp = get_next_page(page);
> -		BUG_ON(!nextp);
> +		struct page *pages[2];
>  
> +		pages[0] = page;
> +		pages[1] = get_next_page(page);
> +		BUG_ON(!pages[1]);
>  
> -		set_pte(area->vm_ptes[0], mk_pte(page, PAGE_KERNEL));
> -		set_pte(area->vm_ptes[1], mk_pte(nextp, PAGE_KERNEL));
> -
> -		/* We pre-allocated VM area so mapping can never fail */
> -		area->vm_addr = area->vm->addr;
> +		zs_arch_map_object(area, pages, off, class->size);
>  	}
> -
>  	return area->vm_addr + off;
>  }
>  EXPORT_SYMBOL_GPL(zs_map_object);
> @@ -751,13 +834,16 @@ void zs_unmap_object(struct zs_pool *pool, unsigned long handle)
>  	off = obj_idx_to_offset(page, obj_idx, class->size);
>  
>  	area = &__get_cpu_var(zs_map_area);
> -	if (off + class->size <= PAGE_SIZE) {
> +	if (off + class->size <= PAGE_SIZE)
>  		kunmap_atomic(area->vm_addr);
> -	} else {
> -		set_pte(area->vm_ptes[0], __pte(0));
> -		set_pte(area->vm_ptes[1], __pte(0));
> -		__flush_tlb_one((unsigned long)area->vm_addr);
> -		__flush_tlb_one((unsigned long)area->vm_addr + PAGE_SIZE);
> +	else {
> +		struct page *pages[2];
> +
> +		pages[0] = page;
> +		pages[1] = get_next_page(page);
> +		BUG_ON(!pages[1]);
> +
> +		zs_arch_unmap_object(area, pages, off, class->size);
>  	}
>  	put_cpu_var(zs_map_area);
>  }
> diff --git a/drivers/staging/zsmalloc/zsmalloc_int.h b/drivers/staging/zsmalloc/zsmalloc_int.h
> index 6fd32a9..8a6887e 100644
> --- a/drivers/staging/zsmalloc/zsmalloc_int.h
> +++ b/drivers/staging/zsmalloc/zsmalloc_int.h
> @@ -110,8 +110,11 @@ enum fullness_group {
>  static const int fullness_threshold_frac = 4;
>  
>  struct mapping_area {
> +#ifdef __HAVE_LOCAL_FLUSH_TLB_KERNEL_RANGE
>  	struct vm_struct *vm;
> -	pte_t *vm_ptes[2];
> +#else
> +	char *vm_buf;
> +#endif
>  	char *vm_addr;
>  };


Need comment about vm_buf and vm_addr.

Thanks, Seth.

-- 
Kind regards,
Minchan Kim

--
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>

Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency

[Seth Jennings]

On 06/27/2012 12:28 AM, Minchan Kim wrote:
>> +{
>> +	if (area->vm)
>> +		return 0;
> 
> 
> Just out of curiosity.
> When do we need above check?

I did this in the case that there was a race between the for
loop in zs_init(), calling zs_cpu_notifier(), and a CPU
coming online.  I've never seen the condition hit, but if it
did, it would leak memory without this check.

I would move the cpu notifier registration after the loop in
zs_init(), but then I could miss a cpu up event and we
wouldn't have the needed per-cpu resources for mapping.

All other suggestions are accepted.  Thanks for the feedback!

--
Seth

Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency

[Seth Jennings]

On 06/27/2012 12:28 AM, Minchan Kim wrote:
>> +{
>> +	if (area->vm)
>> +		return 0;
> 
> 
> Just out of curiosity.
> When do we need above check?

I did this in the case that there was a race between the for
loop in zs_init(), calling zs_cpu_notifier(), and a CPU
coming online.  I've never seen the condition hit, but if it
did, it would leak memory without this check.

I would move the cpu notifier registration after the loop in
zs_init(), but then I could miss a cpu up event and we
wouldn't have the needed per-cpu resources for mapping.

All other suggestions are accepted.  Thanks for the feedback!

--
Seth

--
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>

Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency

[Minchan Kim]

On 06/28/2012 04:09 AM, Seth Jennings wrote:

> On 06/27/2012 12:28 AM, Minchan Kim wrote:
>>> +{
>>> +	if (area->vm)
>>> +		return 0;
>>
>>
>> Just out of curiosity.
>> When do we need above check?
> 
> I did this in the case that there was a race between the for
> loop in zs_init(), calling zs_cpu_notifier(), and a CPU
> coming online.  I've never seen the condition hit, but if it
> did, it would leak memory without this check.
> 


Could you add this as a comment?



> I would move the cpu notifier registration after the loop in
> zs_init(), but then I could miss a cpu up event and we
> wouldn't have the needed per-cpu resources for mapping.
> 
> All other suggestions are accepted.  Thanks for the feedback!
> 



Thanks!

-- 
Kind regards,
Minchan Kim

Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency

[Minchan Kim]

On 06/28/2012 04:09 AM, Seth Jennings wrote:

> On 06/27/2012 12:28 AM, Minchan Kim wrote:
>>> +{
>>> +	if (area->vm)
>>> +		return 0;
>>
>>
>> Just out of curiosity.
>> When do we need above check?
> 
> I did this in the case that there was a race between the for
> loop in zs_init(), calling zs_cpu_notifier(), and a CPU
> coming online.  I've never seen the condition hit, but if it
> did, it would leak memory without this check.
> 


Could you add this as a comment?



> I would move the cpu notifier registration after the loop in
> zs_init(), but then I could miss a cpu up event and we
> wouldn't have the needed per-cpu resources for mapping.
> 
> All other suggestions are accepted.  Thanks for the feedback!
> 



Thanks!

-- 
Kind regards,
Minchan Kim

--
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>

[PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Seth Jennings]

This patch adds support for a local_tlb_flush_kernel_range()
function for the x86 arch.  This function allows for CPU-local
TLB flushing, potentially using invlpg for single entry flushing,
using an arch independent function name.

Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
---
 arch/x86/include/asm/tlbflush.h |   21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index 36a1a2a..92a280b 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -168,4 +168,25 @@ static inline void flush_tlb_kernel_range(unsigned long start,
 	flush_tlb_all();
 }
 
+#define __HAVE_LOCAL_FLUSH_TLB_KERNEL_RANGE
+/*
+ * INVLPG_BREAK_EVEN_PAGES is the number of pages after which single tlb
+ * flushing becomes more costly than just doing a complete tlb flush.
+ * While this break even point varies among x86 hardware, tests have shown
+ * that 8 is a good generic value.
+*/
+#define INVLPG_BREAK_EVEN_PAGES 8
+static inline void local_flush_tlb_kernel_range(unsigned long start,
+		unsigned long end)
+{
+	if (cpu_has_invlpg &&
+		(end - start)/PAGE_SIZE <= INVLPG_BREAK_EVEN_PAGES) {
+		while (start < end) {
+			__flush_tlb_single(start);
+			start += PAGE_SIZE;
+		}
+	} else
+		local_flush_tlb();
+}
+
 #endif /* _ASM_X86_TLBFLUSH_H */
-- 
1.7.9.5

[PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Seth Jennings]

This patch adds support for a local_tlb_flush_kernel_range()
function for the x86 arch.  This function allows for CPU-local
TLB flushing, potentially using invlpg for single entry flushing,
using an arch independent function name.

Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
---
 arch/x86/include/asm/tlbflush.h |   21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index 36a1a2a..92a280b 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -168,4 +168,25 @@ static inline void flush_tlb_kernel_range(unsigned long start,
 	flush_tlb_all();
 }
 
+#define __HAVE_LOCAL_FLUSH_TLB_KERNEL_RANGE
+/*
+ * INVLPG_BREAK_EVEN_PAGES is the number of pages after which single tlb
+ * flushing becomes more costly than just doing a complete tlb flush.
+ * While this break even point varies among x86 hardware, tests have shown
+ * that 8 is a good generic value.
+*/
+#define INVLPG_BREAK_EVEN_PAGES 8
+static inline void local_flush_tlb_kernel_range(unsigned long start,
+		unsigned long end)
+{
+	if (cpu_has_invlpg &&
+		(end - start)/PAGE_SIZE <= INVLPG_BREAK_EVEN_PAGES) {
+		while (start < end) {
+			__flush_tlb_single(start);
+			start += PAGE_SIZE;
+		}
+	} else
+		local_flush_tlb();
+}
+
 #endif /* _ASM_X86_TLBFLUSH_H */
-- 
1.7.9.5

--
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>

Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Konrad Rzeszutek Wilk]

On Mon, Jun 25, 2012 at 12:14 PM, Seth Jennings
<sjenning@linux.vnet.ibm.com> wrote:
> This patch adds support for a local_tlb_flush_kernel_range()
> function for the x86 arch.  This function allows for CPU-local
> TLB flushing, potentially using invlpg for single entry flushing,
> using an arch independent function name.

What x86 hardware did you use to figure the optimal number?

>
> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
> ---
>  arch/x86/include/asm/tlbflush.h |   21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
> index 36a1a2a..92a280b 100644
> --- a/arch/x86/include/asm/tlbflush.h
> +++ b/arch/x86/include/asm/tlbflush.h
> @@ -168,4 +168,25 @@ static inline void flush_tlb_kernel_range(unsigned long start,
>        flush_tlb_all();
>  }
>
> +#define __HAVE_LOCAL_FLUSH_TLB_KERNEL_RANGE
> +/*
> + * INVLPG_BREAK_EVEN_PAGES is the number of pages after which single tlb
> + * flushing becomes more costly than just doing a complete tlb flush.
> + * While this break even point varies among x86 hardware, tests have shown
> + * that 8 is a good generic value.
> +*/
> +#define INVLPG_BREAK_EVEN_PAGES 8
> +static inline void local_flush_tlb_kernel_range(unsigned long start,
> +               unsigned long end)
> +{
> +       if (cpu_has_invlpg &&
> +               (end - start)/PAGE_SIZE <= INVLPG_BREAK_EVEN_PAGES) {
> +               while (start < end) {
> +                       __flush_tlb_single(start);
> +                       start += PAGE_SIZE;
> +               }
> +       } else
> +               local_flush_tlb();
> +}
> +
>  #endif /* _ASM_X86_TLBFLUSH_H */
> --
> 1.7.9.5
>
> --
> 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>
>

Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Konrad Rzeszutek Wilk]

On Mon, Jun 25, 2012 at 12:14 PM, Seth Jennings
<sjenning@linux.vnet.ibm.com> wrote:
> This patch adds support for a local_tlb_flush_kernel_range()
> function for the x86 arch.  This function allows for CPU-local
> TLB flushing, potentially using invlpg for single entry flushing,
> using an arch independent function name.

What x86 hardware did you use to figure the optimal number?

>
> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
> ---
>  arch/x86/include/asm/tlbflush.h |   21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
> index 36a1a2a..92a280b 100644
> --- a/arch/x86/include/asm/tlbflush.h
> +++ b/arch/x86/include/asm/tlbflush.h
> @@ -168,4 +168,25 @@ static inline void flush_tlb_kernel_range(unsigned long start,
>        flush_tlb_all();
>  }
>
> +#define __HAVE_LOCAL_FLUSH_TLB_KERNEL_RANGE
> +/*
> + * INVLPG_BREAK_EVEN_PAGES is the number of pages after which single tlb
> + * flushing becomes more costly than just doing a complete tlb flush.
> + * While this break even point varies among x86 hardware, tests have shown
> + * that 8 is a good generic value.
> +*/
> +#define INVLPG_BREAK_EVEN_PAGES 8
> +static inline void local_flush_tlb_kernel_range(unsigned long start,
> +               unsigned long end)
> +{
> +       if (cpu_has_invlpg &&
> +               (end - start)/PAGE_SIZE <= INVLPG_BREAK_EVEN_PAGES) {
> +               while (start < end) {
> +                       __flush_tlb_single(start);
> +                       start += PAGE_SIZE;
> +               }
> +       } else
> +               local_flush_tlb();
> +}
> +
>  #endif /* _ASM_X86_TLBFLUSH_H */
> --
> 1.7.9.5
>
> --
> 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>
>

--
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>

Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Seth Jennings]

On 06/25/2012 06:01 PM, Konrad Rzeszutek Wilk wrote:
> On Mon, Jun 25, 2012 at 12:14 PM, Seth Jennings
> <sjenning@linux.vnet.ibm.com> wrote:
>> This patch adds support for a local_tlb_flush_kernel_range()
>> function for the x86 arch.  This function allows for CPU-local
>> TLB flushing, potentially using invlpg for single entry flushing,
>> using an arch independent function name.
> 
> What x86 hardware did you use to figure the optimal number?

Actually I didn't.  I used Alex Shi's numbers.

https://lkml.org/lkml/2012/6/25/39

"Like some machine in my hands, balance points is 16 entries
on Romely-EP; while it is at 8 entries on Bloomfield NHM-EP;
and is 256 on IVB mobile CPU. but on model 15 core2 Xeon
using invlpg has nothing help.

For untested machine, do a conservative optimization, same
as NHM CPU."

--
Seth

Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Seth Jennings]

On 06/25/2012 06:01 PM, Konrad Rzeszutek Wilk wrote:
> On Mon, Jun 25, 2012 at 12:14 PM, Seth Jennings
> <sjenning@linux.vnet.ibm.com> wrote:
>> This patch adds support for a local_tlb_flush_kernel_range()
>> function for the x86 arch.  This function allows for CPU-local
>> TLB flushing, potentially using invlpg for single entry flushing,
>> using an arch independent function name.
> 
> What x86 hardware did you use to figure the optimal number?

Actually I didn't.  I used Alex Shi's numbers.

https://lkml.org/lkml/2012/6/25/39

"Like some machine in my hands, balance points is 16 entries
on Romely-EP; while it is at 8 entries on Bloomfield NHM-EP;
and is 256 on IVB mobile CPU. but on model 15 core2 Xeon
using invlpg has nothing help.

For untested machine, do a conservative optimization, same
as NHM CPU."

--
Seth

--
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>

Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Minchan Kim]

On 06/26/2012 01:14 AM, Seth Jennings wrote:

> This patch adds support for a local_tlb_flush_kernel_range()
> function for the x86 arch.  This function allows for CPU-local
> TLB flushing, potentially using invlpg for single entry flushing,
> using an arch independent function name.
> 
> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>


Anyway, we don't matter INVLPG_BREAK_EVEN_PAGES's optimization point is 8 or something.
We do care only it works without big problem.
I believe after Alex's patch is settle down, we can fix it if it's wrong.
But it shouldn't prevent merging this patch.

Acked-by: Minchan Kim <minchan@kernel.org>

Nitpick: I expect you change __HAVE_LOCAL_XXX to __HAVE_ARCH_LOCAL_XXX in next spin. :)


> ---
>  arch/x86/include/asm/tlbflush.h |   21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
> index 36a1a2a..92a280b 100644
> --- a/arch/x86/include/asm/tlbflush.h
> +++ b/arch/x86/include/asm/tlbflush.h
> @@ -168,4 +168,25 @@ static inline void flush_tlb_kernel_range(unsigned long start,
>  	flush_tlb_all();
>  }
>  
> +#define __HAVE_LOCAL_FLUSH_TLB_KERNEL_RANGE
> +/*
> + * INVLPG_BREAK_EVEN_PAGES is the number of pages after which single tlb
> + * flushing becomes more costly than just doing a complete tlb flush.
> + * While this break even point varies among x86 hardware, tests have shown
> + * that 8 is a good generic value.
> +*/
> +#define INVLPG_BREAK_EVEN_PAGES 8
> +static inline void local_flush_tlb_kernel_range(unsigned long start,
> +		unsigned long end)
> +{
> +	if (cpu_has_invlpg &&
> +		(end - start)/PAGE_SIZE <= INVLPG_BREAK_EVEN_PAGES) {
> +		while (start < end) {
> +			__flush_tlb_single(start);
> +			start += PAGE_SIZE;
> +		}
> +	} else
> +		local_flush_tlb();
> +}
> +
>  #endif /* _ASM_X86_TLBFLUSH_H */



-- 
Kind regards,
Minchan Kim

Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Minchan Kim]

On 06/26/2012 01:14 AM, Seth Jennings wrote:

> This patch adds support for a local_tlb_flush_kernel_range()
> function for the x86 arch.  This function allows for CPU-local
> TLB flushing, potentially using invlpg for single entry flushing,
> using an arch independent function name.
> 
> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>


Anyway, we don't matter INVLPG_BREAK_EVEN_PAGES's optimization point is 8 or something.
We do care only it works without big problem.
I believe after Alex's patch is settle down, we can fix it if it's wrong.
But it shouldn't prevent merging this patch.

Acked-by: Minchan Kim <minchan@kernel.org>

Nitpick: I expect you change __HAVE_LOCAL_XXX to __HAVE_ARCH_LOCAL_XXX in next spin. :)


> ---
>  arch/x86/include/asm/tlbflush.h |   21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
> index 36a1a2a..92a280b 100644
> --- a/arch/x86/include/asm/tlbflush.h
> +++ b/arch/x86/include/asm/tlbflush.h
> @@ -168,4 +168,25 @@ static inline void flush_tlb_kernel_range(unsigned long start,
>  	flush_tlb_all();
>  }
>  
> +#define __HAVE_LOCAL_FLUSH_TLB_KERNEL_RANGE
> +/*
> + * INVLPG_BREAK_EVEN_PAGES is the number of pages after which single tlb
> + * flushing becomes more costly than just doing a complete tlb flush.
> + * While this break even point varies among x86 hardware, tests have shown
> + * that 8 is a good generic value.
> +*/
> +#define INVLPG_BREAK_EVEN_PAGES 8
> +static inline void local_flush_tlb_kernel_range(unsigned long start,
> +		unsigned long end)
> +{
> +	if (cpu_has_invlpg &&
> +		(end - start)/PAGE_SIZE <= INVLPG_BREAK_EVEN_PAGES) {
> +		while (start < end) {
> +			__flush_tlb_single(start);
> +			start += PAGE_SIZE;
> +		}
> +	} else
> +		local_flush_tlb();
> +}
> +
>  #endif /* _ASM_X86_TLBFLUSH_H */



-- 
Kind regards,
Minchan Kim

--
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>

Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Alex Shi]

On 06/27/2012 01:53 PM, Minchan Kim wrote:

> On 06/26/2012 01:14 AM, Seth Jennings wrote:
> 
>> This patch adds support for a local_tlb_flush_kernel_range()
>> function for the x86 arch.  This function allows for CPU-local
>> TLB flushing, potentially using invlpg for single entry flushing,
>> using an arch independent function name.
>>
>> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
> 
> 
> Anyway, we don't matter INVLPG_BREAK_EVEN_PAGES's optimization point is 8 or something.


Different CPU type has different balance point on the invlpg replacing
flush all. and some CPU never get benefit from invlpg, So, it's better
to use different value for different CPU, not a fixed
INVLPG_BREAK_EVEN_PAGES.

Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Alex Shi]

On 06/27/2012 01:53 PM, Minchan Kim wrote:

> On 06/26/2012 01:14 AM, Seth Jennings wrote:
> 
>> This patch adds support for a local_tlb_flush_kernel_range()
>> function for the x86 arch.  This function allows for CPU-local
>> TLB flushing, potentially using invlpg for single entry flushing,
>> using an arch independent function name.
>>
>> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
> 
> 
> Anyway, we don't matter INVLPG_BREAK_EVEN_PAGES's optimization point is 8 or something.


Different CPU type has different balance point on the invlpg replacing
flush all. and some CPU never get benefit from invlpg, So, it's better
to use different value for different CPU, not a fixed
INVLPG_BREAK_EVEN_PAGES.

--
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>

Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Minchan Kim]

Hello,

On 06/27/2012 03:14 PM, Alex Shi wrote:

> On 06/27/2012 01:53 PM, Minchan Kim wrote:
> 
>> On 06/26/2012 01:14 AM, Seth Jennings wrote:
>>
>>> This patch adds support for a local_tlb_flush_kernel_range()
>>> function for the x86 arch.  This function allows for CPU-local
>>> TLB flushing, potentially using invlpg for single entry flushing,
>>> using an arch independent function name.
>>>
>>> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
>>
>>
>> Anyway, we don't matter INVLPG_BREAK_EVEN_PAGES's optimization point is 8 or something.
> 
> 
> Different CPU type has different balance point on the invlpg replacing
> flush all. and some CPU never get benefit from invlpg, So, it's better
> to use different value for different CPU, not a fixed
> INVLPG_BREAK_EVEN_PAGES.


I think it could be another patch as further step and someone who are
very familiar with architecture could do better than.
So I hope it could be merged if it doesn't have real big problem.

Thanks for the comment, Alex.


-- 
Kind regards,
Minchan Kim

Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Minchan Kim]

Hello,

On 06/27/2012 03:14 PM, Alex Shi wrote:

> On 06/27/2012 01:53 PM, Minchan Kim wrote:
> 
>> On 06/26/2012 01:14 AM, Seth Jennings wrote:
>>
>>> This patch adds support for a local_tlb_flush_kernel_range()
>>> function for the x86 arch.  This function allows for CPU-local
>>> TLB flushing, potentially using invlpg for single entry flushing,
>>> using an arch independent function name.
>>>
>>> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
>>
>>
>> Anyway, we don't matter INVLPG_BREAK_EVEN_PAGES's optimization point is 8 or something.
> 
> 
> Different CPU type has different balance point on the invlpg replacing
> flush all. and some CPU never get benefit from invlpg, So, it's better
> to use different value for different CPU, not a fixed
> INVLPG_BREAK_EVEN_PAGES.


I think it could be another patch as further step and someone who are
very familiar with architecture could do better than.
So I hope it could be merged if it doesn't have real big problem.

Thanks for the comment, Alex.


-- 
Kind regards,
Minchan Kim

--
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>

RE: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Dan Magenheimer]

> From: Minchan Kim [mailto:minchan@kernel.org]
> Subject: Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()
> 
> Hello,
> 
> On 06/27/2012 03:14 PM, Alex Shi wrote:
> 
> > On 06/27/2012 01:53 PM, Minchan Kim wrote:
> >
> >> On 06/26/2012 01:14 AM, Seth Jennings wrote:
> >>
> >>> This patch adds support for a local_tlb_flush_kernel_range()
> >>> function for the x86 arch.  This function allows for CPU-local
> >>> TLB flushing, potentially using invlpg for single entry flushing,
> >>> using an arch independent function name.
> >>>
> >>> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
> >>
> >>
> >> Anyway, we don't matter INVLPG_BREAK_EVEN_PAGES's optimization point is 8 or something.
> >
> >
> > Different CPU type has different balance point on the invlpg replacing
> > flush all. and some CPU never get benefit from invlpg, So, it's better
> > to use different value for different CPU, not a fixed
> > INVLPG_BREAK_EVEN_PAGES.
> 
> I think it could be another patch as further step and someone who are
> very familiar with architecture could do better than.
> So I hope it could be merged if it doesn't have real big problem.
> 
> Thanks for the comment, Alex.

Just my opinion, but I have to agree with Alex.  Hardcoding
behavior that is VERY processor-specific is a bad idea.  TLBs should
only be messed with when absolutely necessary, not for the
convenience of defending an abstraction that is nice-to-have
but, in current OS kernel code, unnecessary.

IIUC, zsmalloc only cares that the breakeven point is greater
than two.  An arch-specific choice of (A) two page flushes
vs (B) one all-TLB flush should be all that is necessary right
now.  (And, per separate discussion, even this isn't really
necessary either.)

If zsmalloc _ever_ gets extended to support items that might
span three or more pages, a more generic TLB flush-pages-vs-flush-all
approach may be warranted and, by then, may already exist in some
future kernel.  Until then, IMHO, keep it simple.

RE: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Dan Magenheimer]

> From: Minchan Kim [mailto:minchan@kernel.org]
> Subject: Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()
> 
> Hello,
> 
> On 06/27/2012 03:14 PM, Alex Shi wrote:
> 
> > On 06/27/2012 01:53 PM, Minchan Kim wrote:
> >
> >> On 06/26/2012 01:14 AM, Seth Jennings wrote:
> >>
> >>> This patch adds support for a local_tlb_flush_kernel_range()
> >>> function for the x86 arch.  This function allows for CPU-local
> >>> TLB flushing, potentially using invlpg for single entry flushing,
> >>> using an arch independent function name.
> >>>
> >>> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
> >>
> >>
> >> Anyway, we don't matter INVLPG_BREAK_EVEN_PAGES's optimization point is 8 or something.
> >
> >
> > Different CPU type has different balance point on the invlpg replacing
> > flush all. and some CPU never get benefit from invlpg, So, it's better
> > to use different value for different CPU, not a fixed
> > INVLPG_BREAK_EVEN_PAGES.
> 
> I think it could be another patch as further step and someone who are
> very familiar with architecture could do better than.
> So I hope it could be merged if it doesn't have real big problem.
> 
> Thanks for the comment, Alex.

Just my opinion, but I have to agree with Alex.  Hardcoding
behavior that is VERY processor-specific is a bad idea.  TLBs should
only be messed with when absolutely necessary, not for the
convenience of defending an abstraction that is nice-to-have
but, in current OS kernel code, unnecessary.

IIUC, zsmalloc only cares that the breakeven point is greater
than two.  An arch-specific choice of (A) two page flushes
vs (B) one all-TLB flush should be all that is necessary right
now.  (And, per separate discussion, even this isn't really
necessary either.)

If zsmalloc _ever_ gets extended to support items that might
span three or more pages, a more generic TLB flush-pages-vs-flush-all
approach may be warranted and, by then, may already exist in some
future kernel.  Until then, IMHO, keep it simple.

--
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>

Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Konrad Rzeszutek Wilk]

On Wed, Jun 27, 2012 at 08:12:56AM -0700, Dan Magenheimer wrote:
> > From: Minchan Kim [mailto:minchan@kernel.org]
> > Subject: Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()
> > 
> > Hello,
> > 
> > On 06/27/2012 03:14 PM, Alex Shi wrote:
> > 
> > > On 06/27/2012 01:53 PM, Minchan Kim wrote:
> > >
> > >> On 06/26/2012 01:14 AM, Seth Jennings wrote:
> > >>
> > >>> This patch adds support for a local_tlb_flush_kernel_range()
> > >>> function for the x86 arch.  This function allows for CPU-local
> > >>> TLB flushing, potentially using invlpg for single entry flushing,
> > >>> using an arch independent function name.
> > >>>
> > >>> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
> > >>
> > >>
> > >> Anyway, we don't matter INVLPG_BREAK_EVEN_PAGES's optimization point is 8 or something.
> > >
> > >
> > > Different CPU type has different balance point on the invlpg replacing
> > > flush all. and some CPU never get benefit from invlpg, So, it's better
> > > to use different value for different CPU, not a fixed
> > > INVLPG_BREAK_EVEN_PAGES.
> > 
> > I think it could be another patch as further step and someone who are
> > very familiar with architecture could do better than.
> > So I hope it could be merged if it doesn't have real big problem.
> > 
> > Thanks for the comment, Alex.
> 
> Just my opinion, but I have to agree with Alex.  Hardcoding
> behavior that is VERY processor-specific is a bad idea.  TLBs should
> only be messed with when absolutely necessary, not for the
> convenience of defending an abstraction that is nice-to-have
> but, in current OS kernel code, unnecessary.

At least put a big fat comment in the patch saying:
"This is based on research done by Alex, where ...


This needs to be redone where it is automatically figured
out based on the CPUID, but ." [include what Dan just
said about breakeven point]


> 
> IIUC, zsmalloc only cares that the breakeven point is greater
> than two.  An arch-specific choice of (A) two page flushes
> vs (B) one all-TLB flush should be all that is necessary right
> now.  (And, per separate discussion, even this isn't really
> necessary either.)
> 
> If zsmalloc _ever_ gets extended to support items that might
> span three or more pages, a more generic TLB flush-pages-vs-flush-all
> approach may be warranted and, by then, may already exist in some
> future kernel.  Until then, IMHO, keep it simple.

Comments are simple :-)

Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Konrad Rzeszutek Wilk]

On Wed, Jun 27, 2012 at 08:12:56AM -0700, Dan Magenheimer wrote:
> > From: Minchan Kim [mailto:minchan@kernel.org]
> > Subject: Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()
> > 
> > Hello,
> > 
> > On 06/27/2012 03:14 PM, Alex Shi wrote:
> > 
> > > On 06/27/2012 01:53 PM, Minchan Kim wrote:
> > >
> > >> On 06/26/2012 01:14 AM, Seth Jennings wrote:
> > >>
> > >>> This patch adds support for a local_tlb_flush_kernel_range()
> > >>> function for the x86 arch.  This function allows for CPU-local
> > >>> TLB flushing, potentially using invlpg for single entry flushing,
> > >>> using an arch independent function name.
> > >>>
> > >>> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
> > >>
> > >>
> > >> Anyway, we don't matter INVLPG_BREAK_EVEN_PAGES's optimization point is 8 or something.
> > >
> > >
> > > Different CPU type has different balance point on the invlpg replacing
> > > flush all. and some CPU never get benefit from invlpg, So, it's better
> > > to use different value for different CPU, not a fixed
> > > INVLPG_BREAK_EVEN_PAGES.
> > 
> > I think it could be another patch as further step and someone who are
> > very familiar with architecture could do better than.
> > So I hope it could be merged if it doesn't have real big problem.
> > 
> > Thanks for the comment, Alex.
> 
> Just my opinion, but I have to agree with Alex.  Hardcoding
> behavior that is VERY processor-specific is a bad idea.  TLBs should
> only be messed with when absolutely necessary, not for the
> convenience of defending an abstraction that is nice-to-have
> but, in current OS kernel code, unnecessary.

At least put a big fat comment in the patch saying:
"This is based on research done by Alex, where ...


This needs to be redone where it is automatically figured
out based on the CPUID, but ." [include what Dan just
said about breakeven point]


> 
> IIUC, zsmalloc only cares that the breakeven point is greater
> than two.  An arch-specific choice of (A) two page flushes
> vs (B) one all-TLB flush should be all that is necessary right
> now.  (And, per separate discussion, even this isn't really
> necessary either.)
> 
> If zsmalloc _ever_ gets extended to support items that might
> span three or more pages, a more generic TLB flush-pages-vs-flush-all
> approach may be warranted and, by then, may already exist in some
> future kernel.  Until then, IMHO, keep it simple.

Comments are simple :-)

--
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>

Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Seth Jennings]

On 06/27/2012 10:39 AM, Konrad Rzeszutek Wilk wrote:
> On Wed, Jun 27, 2012 at 08:12:56AM -0700, Dan Magenheimer wrote:
>>> From: Minchan Kim [mailto:minchan@kernel.org]
>>> Subject: Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()
>>>
>>> Hello,
>>>
>>> On 06/27/2012 03:14 PM, Alex Shi wrote:
>>>
>>>> On 06/27/2012 01:53 PM, Minchan Kim wrote:
>>>>
>>>>> On 06/26/2012 01:14 AM, Seth Jennings wrote:
>>>>>
>>>>>> This patch adds support for a local_tlb_flush_kernel_range()
>>>>>> function for the x86 arch.  This function allows for CPU-local
>>>>>> TLB flushing, potentially using invlpg for single entry flushing,
>>>>>> using an arch independent function name.
>>>>>>
>>>>>> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
>>>>>
>>>>>
>>>>> Anyway, we don't matter INVLPG_BREAK_EVEN_PAGES's optimization point is 8 or something.
>>>>
>>>>
>>>> Different CPU type has different balance point on the invlpg replacing
>>>> flush all. and some CPU never get benefit from invlpg, So, it's better
>>>> to use different value for different CPU, not a fixed
>>>> INVLPG_BREAK_EVEN_PAGES.
>>>
>>> I think it could be another patch as further step and someone who are
>>> very familiar with architecture could do better than.
>>> So I hope it could be merged if it doesn't have real big problem.
>>>
>>> Thanks for the comment, Alex.
>>
>> Just my opinion, but I have to agree with Alex.  Hardcoding
>> behavior that is VERY processor-specific is a bad idea.  TLBs should
>> only be messed with when absolutely necessary, not for the
>> convenience of defending an abstraction that is nice-to-have
>> but, in current OS kernel code, unnecessary.
> 
> At least put a big fat comment in the patch saying:
> "This is based on research done by Alex, where ...

I can do this.

--
Seth

Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Seth Jennings]

On 06/27/2012 10:39 AM, Konrad Rzeszutek Wilk wrote:
> On Wed, Jun 27, 2012 at 08:12:56AM -0700, Dan Magenheimer wrote:
>>> From: Minchan Kim [mailto:minchan@kernel.org]
>>> Subject: Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()
>>>
>>> Hello,
>>>
>>> On 06/27/2012 03:14 PM, Alex Shi wrote:
>>>
>>>> On 06/27/2012 01:53 PM, Minchan Kim wrote:
>>>>
>>>>> On 06/26/2012 01:14 AM, Seth Jennings wrote:
>>>>>
>>>>>> This patch adds support for a local_tlb_flush_kernel_range()
>>>>>> function for the x86 arch.  This function allows for CPU-local
>>>>>> TLB flushing, potentially using invlpg for single entry flushing,
>>>>>> using an arch independent function name.
>>>>>>
>>>>>> Signed-off-by: Seth Jennings <sjenning@linux.vnet.ibm.com>
>>>>>
>>>>>
>>>>> Anyway, we don't matter INVLPG_BREAK_EVEN_PAGES's optimization point is 8 or something.
>>>>
>>>>
>>>> Different CPU type has different balance point on the invlpg replacing
>>>> flush all. and some CPU never get benefit from invlpg, So, it's better
>>>> to use different value for different CPU, not a fixed
>>>> INVLPG_BREAK_EVEN_PAGES.
>>>
>>> I think it could be another patch as further step and someone who are
>>> very familiar with architecture could do better than.
>>> So I hope it could be merged if it doesn't have real big problem.
>>>
>>> Thanks for the comment, Alex.
>>
>> Just my opinion, but I have to agree with Alex.  Hardcoding
>> behavior that is VERY processor-specific is a bad idea.  TLBs should
>> only be messed with when absolutely necessary, not for the
>> convenience of defending an abstraction that is nice-to-have
>> but, in current OS kernel code, unnecessary.
> 
> At least put a big fat comment in the patch saying:
> "This is based on research done by Alex, where ...

I can do this.

--
Seth

--
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>

Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Seth Jennings]

On 06/27/2012 10:12 AM, Dan Magenheimer wrote:
>> From: Minchan Kim [mailto:minchan@kernel.org]
>> Subject: Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()
>>
>> On 06/27/2012 03:14 PM, Alex Shi wrote:
>>>
>>> On 06/27/2012 01:53 PM, Minchan Kim wrote:
>>> Different CPU type has different balance point on the invlpg replacing
>>> flush all. and some CPU never get benefit from invlpg, So, it's better
>>> to use different value for different CPU, not a fixed
>>> INVLPG_BREAK_EVEN_PAGES.
>>
>> I think it could be another patch as further step and someone who are
>> very familiar with architecture could do better than.
>> So I hope it could be merged if it doesn't have real big problem.
>>
>> Thanks for the comment, Alex.
> 
> Just my opinion, but I have to agree with Alex.  Hardcoding
> behavior that is VERY processor-specific is a bad idea.  TLBs should
> only be messed with when absolutely necessary, not for the
> convenience of defending an abstraction that is nice-to-have
> but, in current OS kernel code, unnecessary.

I agree that it's not optimal.  The selection based on CPUID
is part of Alex's patchset, and I'll be glad to use that
code when it gets integrated.

But the real discussion is are we going to:
1) wait until Alex's patches to be integrated, degrading
zsmalloc in the meantime or
2) put in some simple temporary logic that works well (not
best) for most cases

> IIUC, zsmalloc only cares that the breakeven point is greater
> than two.  An arch-specific choice of (A) two page flushes
> vs (B) one all-TLB flush should be all that is necessary right
> now.  (And, per separate discussion, even this isn't really
> necessary either.)
> 
> If zsmalloc _ever_ gets extended to support items that might
> span three or more pages, a more generic TLB flush-pages-vs-flush-all
> approach may be warranted and, by then, may already exist in some
> future kernel.  Until then, IMHO, keep it simple.

I guess I'm not following.  Are you supporting the removal
of the "break even" logic?  I added that logic as a
compromise for Peter's feedback:

http://lkml.org/lkml/2012/5/17/177

--
Seth

Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Seth Jennings]

On 06/27/2012 10:12 AM, Dan Magenheimer wrote:
>> From: Minchan Kim [mailto:minchan@kernel.org]
>> Subject: Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()
>>
>> On 06/27/2012 03:14 PM, Alex Shi wrote:
>>>
>>> On 06/27/2012 01:53 PM, Minchan Kim wrote:
>>> Different CPU type has different balance point on the invlpg replacing
>>> flush all. and some CPU never get benefit from invlpg, So, it's better
>>> to use different value for different CPU, not a fixed
>>> INVLPG_BREAK_EVEN_PAGES.
>>
>> I think it could be another patch as further step and someone who are
>> very familiar with architecture could do better than.
>> So I hope it could be merged if it doesn't have real big problem.
>>
>> Thanks for the comment, Alex.
> 
> Just my opinion, but I have to agree with Alex.  Hardcoding
> behavior that is VERY processor-specific is a bad idea.  TLBs should
> only be messed with when absolutely necessary, not for the
> convenience of defending an abstraction that is nice-to-have
> but, in current OS kernel code, unnecessary.

I agree that it's not optimal.  The selection based on CPUID
is part of Alex's patchset, and I'll be glad to use that
code when it gets integrated.

But the real discussion is are we going to:
1) wait until Alex's patches to be integrated, degrading
zsmalloc in the meantime or
2) put in some simple temporary logic that works well (not
best) for most cases

> IIUC, zsmalloc only cares that the breakeven point is greater
> than two.  An arch-specific choice of (A) two page flushes
> vs (B) one all-TLB flush should be all that is necessary right
> now.  (And, per separate discussion, even this isn't really
> necessary either.)
> 
> If zsmalloc _ever_ gets extended to support items that might
> span three or more pages, a more generic TLB flush-pages-vs-flush-all
> approach may be warranted and, by then, may already exist in some
> future kernel.  Until then, IMHO, keep it simple.

I guess I'm not following.  Are you supporting the removal
of the "break even" logic?  I added that logic as a
compromise for Peter's feedback:

http://lkml.org/lkml/2012/5/17/177

--
Seth

--
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>

RE: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Dan Magenheimer]

> From: Seth Jennings [mailto:sjenning@linux.vnet.ibm.com]
> Sent: Wednesday, June 27, 2012 12:34 PM
> To: Dan Magenheimer
> Cc: Minchan Kim; Alex Shi; Greg Kroah-Hartman; devel@driverdev.osuosl.org; Konrad Wilk; linux-
> kernel@vger.kernel.org; linux-mm@kvack.org; Andrew Morton; Robert Jennings; Nitin Gupta
> Subject: Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()
> 
> On 06/27/2012 10:12 AM, Dan Magenheimer wrote:
> >> From: Minchan Kim [mailto:minchan@kernel.org]
> >> Subject: Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()
> >>
> >> On 06/27/2012 03:14 PM, Alex Shi wrote:
> >>>
> >>> On 06/27/2012 01:53 PM, Minchan Kim wrote:
> >>> Different CPU type has different balance point on the invlpg replacing
> >>> flush all. and some CPU never get benefit from invlpg, So, it's better
> >>> to use different value for different CPU, not a fixed
> >>> INVLPG_BREAK_EVEN_PAGES.
> >>
> >> I think it could be another patch as further step and someone who are
> >> very familiar with architecture could do better than.
> >> So I hope it could be merged if it doesn't have real big problem.
> >>
> >> Thanks for the comment, Alex.
> >
> > Just my opinion, but I have to agree with Alex.  Hardcoding
> > behavior that is VERY processor-specific is a bad idea.  TLBs should
> > only be messed with when absolutely necessary, not for the
> > convenience of defending an abstraction that is nice-to-have
> > but, in current OS kernel code, unnecessary.
> 
> I agree that it's not optimal.  The selection based on CPUID
> is part of Alex's patchset, and I'll be glad to use that
> code when it gets integrated.
> 
> But the real discussion is are we going to:
> 1) wait until Alex's patches to be integrated, degrading
> zsmalloc in the meantime or
> 2) put in some simple temporary logic that works well (not
> best) for most cases
> 
> > IIUC, zsmalloc only cares that the breakeven point is greater
> > than two.  An arch-specific choice of (A) two page flushes
> > vs (B) one all-TLB flush should be all that is necessary right
> > now.  (And, per separate discussion, even this isn't really
> > necessary either.)
> >
> > If zsmalloc _ever_ gets extended to support items that might
> > span three or more pages, a more generic TLB flush-pages-vs-flush-all
> > approach may be warranted and, by then, may already exist in some
> > future kernel.  Until then, IMHO, keep it simple.
> 
> I guess I'm not following.  Are you supporting the removal
> of the "break even" logic?  I added that logic as a
> compromise for Peter's feedback:
> 
> http://lkml.org/lkml/2012/5/17/177

Yes, as long as I am correct that zsmalloc never has to map/flush
more than two pages at a time, I think dealing with the break-even
logic is overkill.  I see Peter isn't on this dist list... maybe
you should ask him if he agrees, as long as we are only always
talking about flush-two-TLB-pages vs flush-all.

(And, of course, per previous discussion, I think even mapping/flushing
two TLB pages is unnecessary and overkill required only for protecting an
abstraction, but will stop beating that dead horse. ;-)

Dan

RE: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Dan Magenheimer]

> From: Seth Jennings [mailto:sjenning@linux.vnet.ibm.com]
> Sent: Wednesday, June 27, 2012 12:34 PM
> To: Dan Magenheimer
> Cc: Minchan Kim; Alex Shi; Greg Kroah-Hartman; devel@driverdev.osuosl.org; Konrad Wilk; linux-
> kernel@vger.kernel.org; linux-mm@kvack.org; Andrew Morton; Robert Jennings; Nitin Gupta
> Subject: Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()
> 
> On 06/27/2012 10:12 AM, Dan Magenheimer wrote:
> >> From: Minchan Kim [mailto:minchan@kernel.org]
> >> Subject: Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()
> >>
> >> On 06/27/2012 03:14 PM, Alex Shi wrote:
> >>>
> >>> On 06/27/2012 01:53 PM, Minchan Kim wrote:
> >>> Different CPU type has different balance point on the invlpg replacing
> >>> flush all. and some CPU never get benefit from invlpg, So, it's better
> >>> to use different value for different CPU, not a fixed
> >>> INVLPG_BREAK_EVEN_PAGES.
> >>
> >> I think it could be another patch as further step and someone who are
> >> very familiar with architecture could do better than.
> >> So I hope it could be merged if it doesn't have real big problem.
> >>
> >> Thanks for the comment, Alex.
> >
> > Just my opinion, but I have to agree with Alex.  Hardcoding
> > behavior that is VERY processor-specific is a bad idea.  TLBs should
> > only be messed with when absolutely necessary, not for the
> > convenience of defending an abstraction that is nice-to-have
> > but, in current OS kernel code, unnecessary.
> 
> I agree that it's not optimal.  The selection based on CPUID
> is part of Alex's patchset, and I'll be glad to use that
> code when it gets integrated.
> 
> But the real discussion is are we going to:
> 1) wait until Alex's patches to be integrated, degrading
> zsmalloc in the meantime or
> 2) put in some simple temporary logic that works well (not
> best) for most cases
> 
> > IIUC, zsmalloc only cares that the breakeven point is greater
> > than two.  An arch-specific choice of (A) two page flushes
> > vs (B) one all-TLB flush should be all that is necessary right
> > now.  (And, per separate discussion, even this isn't really
> > necessary either.)
> >
> > If zsmalloc _ever_ gets extended to support items that might
> > span three or more pages, a more generic TLB flush-pages-vs-flush-all
> > approach may be warranted and, by then, may already exist in some
> > future kernel.  Until then, IMHO, keep it simple.
> 
> I guess I'm not following.  Are you supporting the removal
> of the "break even" logic?  I added that logic as a
> compromise for Peter's feedback:
> 
> http://lkml.org/lkml/2012/5/17/177

Yes, as long as I am correct that zsmalloc never has to map/flush
more than two pages at a time, I think dealing with the break-even
logic is overkill.  I see Peter isn't on this dist list... maybe
you should ask him if he agrees, as long as we are only always
talking about flush-two-TLB-pages vs flush-all.

(And, of course, per previous discussion, I think even mapping/flushing
two TLB pages is unnecessary and overkill required only for protecting an
abstraction, but will stop beating that dead horse. ;-)

Dan

--
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>

Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Seth Jennings]

On 06/27/2012 04:15 PM, Dan Magenheimer wrote:
>> From: Seth Jennings [mailto:sjenning@linux.vnet.ibm.com]
>> I guess I'm not following.  Are you supporting the removal
>> of the "break even" logic?  I added that logic as a
>> compromise for Peter's feedback:
>>
>> http://lkml.org/lkml/2012/5/17/177
> 
> Yes, as long as I am correct that zsmalloc never has to map/flush
> more than two pages at a time, I think dealing with the break-even
> logic is overkill.

The implementation of local_flush_tlb_kernel_range()
shouldn't be influenced by zsmalloc at all.  Additionally,
we can't assume that zsmalloc will always be the only user
of this function.

> I see Peter isn't on this dist list... maybe
> you should ask him if he agrees, as long as we are only always
> talking about flush-two-TLB-pages vs flush-all.

Yes, I'm planning to send out the next version of patches
tomorrow (minus the first that has already been accepted)
and I'll include him like I should have the first time :-/

> (And, of course, per previous discussion, I think even mapping/flushing
> two TLB pages is unnecessary and overkill required only for protecting an
> abstraction, but will stop beating that dead horse. ;-)

With this patchset, I actually quantified the the
performance gain with page table assisted mapping vs mapping
via copy, and there is a significant 40% difference in
single-threaded performance.

You can do the test yourself by commenting out the
#define __HAVE_ARCH_LOCAL_FLUSH_TLB_KERNEL_RANGE
in tlbflush.h which will cause the new mapping via copy
method to be used.

--
Seth

Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Seth Jennings]

On 06/27/2012 04:15 PM, Dan Magenheimer wrote:
>> From: Seth Jennings [mailto:sjenning@linux.vnet.ibm.com]
>> I guess I'm not following.  Are you supporting the removal
>> of the "break even" logic?  I added that logic as a
>> compromise for Peter's feedback:
>>
>> http://lkml.org/lkml/2012/5/17/177
> 
> Yes, as long as I am correct that zsmalloc never has to map/flush
> more than two pages at a time, I think dealing with the break-even
> logic is overkill.

The implementation of local_flush_tlb_kernel_range()
shouldn't be influenced by zsmalloc at all.  Additionally,
we can't assume that zsmalloc will always be the only user
of this function.

> I see Peter isn't on this dist list... maybe
> you should ask him if he agrees, as long as we are only always
> talking about flush-two-TLB-pages vs flush-all.

Yes, I'm planning to send out the next version of patches
tomorrow (minus the first that has already been accepted)
and I'll include him like I should have the first time :-/

> (And, of course, per previous discussion, I think even mapping/flushing
> two TLB pages is unnecessary and overkill required only for protecting an
> abstraction, but will stop beating that dead horse. ;-)

With this patchset, I actually quantified the the
performance gain with page table assisted mapping vs mapping
via copy, and there is a significant 40% difference in
single-threaded performance.

You can do the test yourself by commenting out the
#define __HAVE_ARCH_LOCAL_FLUSH_TLB_KERNEL_RANGE
in tlbflush.h which will cause the new mapping via copy
method to be used.

--
Seth

--
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>

Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Alex Shi]

On 06/28/2012 02:33 AM, Seth Jennings wrote:

> On 06/27/2012 10:12 AM, Dan Magenheimer wrote:
>>> From: Minchan Kim [mailto:minchan@kernel.org]
>>> Subject: Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()
>>>
>>> On 06/27/2012 03:14 PM, Alex Shi wrote:
>>>>
>>>> On 06/27/2012 01:53 PM, Minchan Kim wrote:
>>>> Different CPU type has different balance point on the invlpg replacing
>>>> flush all. and some CPU never get benefit from invlpg, So, it's better
>>>> to use different value for different CPU, not a fixed
>>>> INVLPG_BREAK_EVEN_PAGES.
>>>
>>> I think it could be another patch as further step and someone who are
>>> very familiar with architecture could do better than.
>>> So I hope it could be merged if it doesn't have real big problem.
>>>
>>> Thanks for the comment, Alex.
>>
>> Just my opinion, but I have to agree with Alex.  Hardcoding
>> behavior that is VERY processor-specific is a bad idea.  TLBs should
>> only be messed with when absolutely necessary, not for the
>> convenience of defending an abstraction that is nice-to-have
>> but, in current OS kernel code, unnecessary.
> 
> I agree that it's not optimal.  The selection based on CPUID
> is part of Alex's patchset, and I'll be glad to use that
> code when it gets integrated.
> 
> But the real discussion is are we going to:
> 1) wait until Alex's patches to be integrated, degrading
> zsmalloc in the meantime or


Peter Anvin is merging my TLB patch set into tip tree, x86/mm branch.

> 2) put in some simple temporary logic that works well (not
> best) for most cases

Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Alex Shi]

On 06/28/2012 02:33 AM, Seth Jennings wrote:

> On 06/27/2012 10:12 AM, Dan Magenheimer wrote:
>>> From: Minchan Kim [mailto:minchan@kernel.org]
>>> Subject: Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()
>>>
>>> On 06/27/2012 03:14 PM, Alex Shi wrote:
>>>>
>>>> On 06/27/2012 01:53 PM, Minchan Kim wrote:
>>>> Different CPU type has different balance point on the invlpg replacing
>>>> flush all. and some CPU never get benefit from invlpg, So, it's better
>>>> to use different value for different CPU, not a fixed
>>>> INVLPG_BREAK_EVEN_PAGES.
>>>
>>> I think it could be another patch as further step and someone who are
>>> very familiar with architecture could do better than.
>>> So I hope it could be merged if it doesn't have real big problem.
>>>
>>> Thanks for the comment, Alex.
>>
>> Just my opinion, but I have to agree with Alex.  Hardcoding
>> behavior that is VERY processor-specific is a bad idea.  TLBs should
>> only be messed with when absolutely necessary, not for the
>> convenience of defending an abstraction that is nice-to-have
>> but, in current OS kernel code, unnecessary.
> 
> I agree that it's not optimal.  The selection based on CPUID
> is part of Alex's patchset, and I'll be glad to use that
> code when it gets integrated.
> 
> But the real discussion is are we going to:
> 1) wait until Alex's patches to be integrated, degrading
> zsmalloc in the meantime or


Peter Anvin is merging my TLB patch set into tip tree, x86/mm branch.

> 2) put in some simple temporary logic that works well (not
> best) for most cases


--
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>

Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Seth Jennings]

On 06/27/2012 09:03 PM, Alex Shi wrote:
> Peter Anvin is merging my TLB patch set into tip tree, x86/mm branch.

Great! I don't know the integration path of this tree.  Will
these patches go into mainline in the next merge window from
here?

--
Seth

Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Seth Jennings]

On 06/27/2012 09:03 PM, Alex Shi wrote:
> Peter Anvin is merging my TLB patch set into tip tree, x86/mm branch.

Great! I don't know the integration path of this tree.  Will
these patches go into mainline in the next merge window from
here?

--
Seth

--
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>

Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Alex Shi]

On 06/28/2012 11:21 PM, Seth Jennings wrote:

> On 06/27/2012 09:03 PM, Alex Shi wrote:
>> Peter Anvin is merging my TLB patch set into tip tree, x86/mm branch.
> 
> Great! I don't know the integration path of this tree.  Will
> these patches go into mainline in the next merge window from
> here?
> 


$git clone git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git

and get the branch of x86/mm

> --
> Seth
> 

Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range()

[Alex Shi]

On 06/28/2012 11:21 PM, Seth Jennings wrote:

> On 06/27/2012 09:03 PM, Alex Shi wrote:
>> Peter Anvin is merging my TLB patch set into tip tree, x86/mm branch.
> 
> Great! I don't know the integration path of this tree.  Will
> these patches go into mainline in the next merge window from
> here?
> 


$git clone git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git

and get the branch of x86/mm

> --
> Seth
> 


--
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>