* [PATCH 0/3] zsmalloc: remove x86 dependency @ 2012-06-25 16:14 ` Seth Jennings 0 siblings, 0 replies; 68+ messages in thread From: Seth Jennings @ 2012-06-25 16:14 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Seth Jennings, Andrew Morton, Dan Magenheimer, Konrad Rzeszutek Wilk, Nitin Gupta, Minchan Kim, Robert Jennings, linux-mm, devel, linux-kernel 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 ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH 0/3] zsmalloc: remove x86 dependency @ 2012-06-25 16:14 ` Seth Jennings 0 siblings, 0 replies; 68+ messages in thread From: Seth Jennings @ 2012-06-25 16:14 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Seth Jennings, Andrew Morton, Dan Magenheimer, Konrad Rzeszutek Wilk, Nitin Gupta, Minchan Kim, Robert Jennings, linux-mm, devel, linux-kernel 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC 2012-06-25 16:14 ` Seth Jennings @ 2012-06-25 16:14 ` Seth Jennings -1 siblings, 0 replies; 68+ messages in thread From: Seth Jennings @ 2012-06-25 16:14 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Seth Jennings, Andrew Morton, Dan Magenheimer, Konrad Rzeszutek Wilk, Nitin Gupta, Minchan Kim, Robert Jennings, linux-mm, devel, linux-kernel 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 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC @ 2012-06-25 16:14 ` Seth Jennings 0 siblings, 0 replies; 68+ messages in thread From: Seth Jennings @ 2012-06-25 16:14 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Seth Jennings, Andrew Morton, Dan Magenheimer, Konrad Rzeszutek Wilk, Nitin Gupta, Minchan Kim, Robert Jennings, linux-mm, devel, linux-kernel 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> ^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC 2012-06-25 16:14 ` Seth Jennings @ 2012-06-27 2:37 ` Minchan Kim -1 siblings, 0 replies; 68+ messages in thread From: Minchan Kim @ 2012-06-27 2:37 UTC (permalink / raw) To: Seth Jennings Cc: Greg Kroah-Hartman, devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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 ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC @ 2012-06-27 2:37 ` Minchan Kim 0 siblings, 0 replies; 68+ messages in thread From: Minchan Kim @ 2012-06-27 2:37 UTC (permalink / raw) To: Seth Jennings Cc: Greg Kroah-Hartman, devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC 2012-06-27 2:37 ` Minchan Kim @ 2012-06-27 2:43 ` Greg Kroah-Hartman -1 siblings, 0 replies; 68+ messages in thread From: Greg Kroah-Hartman @ 2012-06-27 2:43 UTC (permalink / raw) To: Minchan Kim Cc: Seth Jennings, devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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 :) ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC @ 2012-06-27 2:43 ` Greg Kroah-Hartman 0 siblings, 0 replies; 68+ messages in thread From: Greg Kroah-Hartman @ 2012-06-27 2:43 UTC (permalink / raw) To: Minchan Kim Cc: Seth Jennings, devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC 2012-06-27 2:43 ` Greg Kroah-Hartman @ 2012-06-27 2:49 ` Minchan Kim -1 siblings, 0 replies; 68+ messages in thread From: Minchan Kim @ 2012-06-27 2:49 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Seth Jennings, devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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 ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC @ 2012-06-27 2:49 ` Minchan Kim 0 siblings, 0 replies; 68+ messages in thread From: Minchan Kim @ 2012-06-27 2:49 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Seth Jennings, devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC 2012-06-27 2:49 ` Minchan Kim @ 2012-06-27 3:21 ` Greg Kroah-Hartman -1 siblings, 0 replies; 68+ messages in thread From: Greg Kroah-Hartman @ 2012-06-27 3:21 UTC (permalink / raw) To: Minchan Kim Cc: Seth Jennings, devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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 ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC @ 2012-06-27 3:21 ` Greg Kroah-Hartman 0 siblings, 0 replies; 68+ messages in thread From: Greg Kroah-Hartman @ 2012-06-27 3:21 UTC (permalink / raw) To: Minchan Kim Cc: Seth Jennings, devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC 2012-06-27 3:21 ` Greg Kroah-Hartman @ 2012-06-27 15:40 ` Konrad Rzeszutek Wilk -1 siblings, 0 replies; 68+ messages in thread From: Konrad Rzeszutek Wilk @ 2012-06-27 15:40 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Minchan Kim, Seth Jennings, devel, Dan Magenheimer, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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? ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC @ 2012-06-27 15:40 ` Konrad Rzeszutek Wilk 0 siblings, 0 replies; 68+ messages in thread From: Konrad Rzeszutek Wilk @ 2012-06-27 15:40 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Minchan Kim, Seth Jennings, devel, Dan Magenheimer, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC 2012-06-27 15:40 ` Konrad Rzeszutek Wilk @ 2012-06-27 18:55 ` Greg Kroah-Hartman -1 siblings, 0 replies; 68+ messages in thread From: Greg Kroah-Hartman @ 2012-06-27 18:55 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: devel, Seth Jennings, Dan Magenheimer, linux-mm, linux-kernel, Minchan Kim, Andrew Morton, Robert Jennings, Nitin Gupta 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 ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC @ 2012-06-27 18:55 ` Greg Kroah-Hartman 0 siblings, 0 replies; 68+ messages in thread From: Greg Kroah-Hartman @ 2012-06-27 18:55 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: devel, Seth Jennings, Dan Magenheimer, linux-mm, linux-kernel, Minchan Kim, Andrew Morton, Robert Jennings, Nitin Gupta 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC 2012-06-27 18:55 ` Greg Kroah-Hartman @ 2012-06-27 18:52 ` Konrad Rzeszutek Wilk -1 siblings, 0 replies; 68+ messages in thread From: Konrad Rzeszutek Wilk @ 2012-06-27 18:52 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: devel, Seth Jennings, Dan Magenheimer, linux-mm, linux-kernel, Minchan Kim, Andrew Morton, Robert Jennings, Nitin Gupta > > > > 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 ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC @ 2012-06-27 18:52 ` Konrad Rzeszutek Wilk 0 siblings, 0 replies; 68+ messages in thread From: Konrad Rzeszutek Wilk @ 2012-06-27 18:52 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: devel, Seth Jennings, Dan Magenheimer, linux-mm, linux-kernel, Minchan Kim, Andrew Morton, Robert Jennings, Nitin Gupta > > > > 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC 2012-06-27 18:52 ` Konrad Rzeszutek Wilk @ 2012-06-27 19:29 ` Greg Kroah-Hartman -1 siblings, 0 replies; 68+ messages in thread From: Greg Kroah-Hartman @ 2012-06-27 19:29 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: devel, Seth Jennings, Dan Magenheimer, linux-mm, linux-kernel, Minchan Kim, Andrew Morton, Robert Jennings, Nitin Gupta 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 ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC @ 2012-06-27 19:29 ` Greg Kroah-Hartman 0 siblings, 0 replies; 68+ messages in thread From: Greg Kroah-Hartman @ 2012-06-27 19:29 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: devel, Seth Jennings, Dan Magenheimer, linux-mm, linux-kernel, Minchan Kim, Andrew Morton, Robert Jennings, Nitin Gupta 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency 2012-06-25 16:14 ` Seth Jennings @ 2012-06-25 16:14 ` Seth Jennings -1 siblings, 0 replies; 68+ messages in thread From: Seth Jennings @ 2012-06-25 16:14 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Seth Jennings, Andrew Morton, Dan Magenheimer, Konrad Rzeszutek Wilk, Nitin Gupta, Minchan Kim, Robert Jennings, linux-mm, devel, linux-kernel 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 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency @ 2012-06-25 16:14 ` Seth Jennings 0 siblings, 0 replies; 68+ messages in thread From: Seth Jennings @ 2012-06-25 16:14 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Seth Jennings, Andrew Morton, Dan Magenheimer, Konrad Rzeszutek Wilk, Nitin Gupta, Minchan Kim, Robert Jennings, linux-mm, devel, linux-kernel 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> ^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency 2012-06-25 16:14 ` Seth Jennings @ 2012-06-25 16:59 ` Greg Kroah-Hartman -1 siblings, 0 replies; 68+ messages in thread From: Greg Kroah-Hartman @ 2012-06-25 16:59 UTC (permalink / raw) To: Seth Jennings Cc: devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Minchan Kim, Andrew Morton, Robert Jennings, Nitin Gupta 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 ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency @ 2012-06-25 16:59 ` Greg Kroah-Hartman 0 siblings, 0 replies; 68+ messages in thread From: Greg Kroah-Hartman @ 2012-06-25 16:59 UTC (permalink / raw) To: Seth Jennings Cc: devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Minchan Kim, Andrew Morton, Robert Jennings, Nitin Gupta 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency 2012-06-25 16:59 ` Greg Kroah-Hartman @ 2012-06-25 17:10 ` Seth Jennings -1 siblings, 0 replies; 68+ messages in thread From: Seth Jennings @ 2012-06-25 17:10 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Minchan Kim, Andrew Morton, Robert Jennings, Nitin Gupta 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 ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency @ 2012-06-25 17:10 ` Seth Jennings 0 siblings, 0 replies; 68+ messages in thread From: Seth Jennings @ 2012-06-25 17:10 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Minchan Kim, Andrew Morton, Robert Jennings, Nitin Gupta 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency 2012-06-25 17:10 ` Seth Jennings @ 2012-06-25 17:19 ` Greg Kroah-Hartman -1 siblings, 0 replies; 68+ messages in thread From: Greg Kroah-Hartman @ 2012-06-25 17:19 UTC (permalink / raw) To: Seth Jennings Cc: devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Minchan Kim, Andrew Morton, Robert Jennings, Nitin Gupta 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 ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency @ 2012-06-25 17:19 ` Greg Kroah-Hartman 0 siblings, 0 replies; 68+ messages in thread From: Greg Kroah-Hartman @ 2012-06-25 17:19 UTC (permalink / raw) To: Seth Jennings Cc: devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Minchan Kim, Andrew Morton, Robert Jennings, Nitin Gupta 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency 2012-06-25 17:19 ` Greg Kroah-Hartman @ 2012-06-25 18:24 ` Seth Jennings -1 siblings, 0 replies; 68+ messages in thread From: Seth Jennings @ 2012-06-25 18:24 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Minchan Kim, Andrew Morton, Robert Jennings, Nitin Gupta 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 ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency @ 2012-06-25 18:24 ` Seth Jennings 0 siblings, 0 replies; 68+ messages in thread From: Seth Jennings @ 2012-06-25 18:24 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Minchan Kim, Andrew Morton, Robert Jennings, Nitin Gupta 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency 2012-06-25 18:24 ` Seth Jennings @ 2012-06-25 23:37 ` Greg Kroah-Hartman -1 siblings, 0 replies; 68+ messages in thread From: Greg Kroah-Hartman @ 2012-06-25 23:37 UTC (permalink / raw) To: Seth Jennings Cc: devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Minchan Kim, Andrew Morton, Robert Jennings, Nitin Gupta 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 ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency @ 2012-06-25 23:37 ` Greg Kroah-Hartman 0 siblings, 0 replies; 68+ messages in thread From: Greg Kroah-Hartman @ 2012-06-25 23:37 UTC (permalink / raw) To: Seth Jennings Cc: devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Minchan Kim, Andrew Morton, Robert Jennings, Nitin Gupta 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency 2012-06-25 16:14 ` Seth Jennings @ 2012-06-27 5:28 ` Minchan Kim -1 siblings, 0 replies; 68+ messages in thread From: Minchan Kim @ 2012-06-27 5:28 UTC (permalink / raw) To: Seth Jennings Cc: Greg Kroah-Hartman, devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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 ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency @ 2012-06-27 5:28 ` Minchan Kim 0 siblings, 0 replies; 68+ messages in thread From: Minchan Kim @ 2012-06-27 5:28 UTC (permalink / raw) To: Seth Jennings Cc: Greg Kroah-Hartman, devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency 2012-06-27 5:28 ` Minchan Kim @ 2012-06-27 19:09 ` Seth Jennings -1 siblings, 0 replies; 68+ messages in thread From: Seth Jennings @ 2012-06-27 19:09 UTC (permalink / raw) To: Minchan Kim Cc: Greg Kroah-Hartman, devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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 ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency @ 2012-06-27 19:09 ` Seth Jennings 0 siblings, 0 replies; 68+ messages in thread From: Seth Jennings @ 2012-06-27 19:09 UTC (permalink / raw) To: Minchan Kim Cc: Greg Kroah-Hartman, devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency 2012-06-27 19:09 ` Seth Jennings @ 2012-06-28 0:20 ` Minchan Kim -1 siblings, 0 replies; 68+ messages in thread From: Minchan Kim @ 2012-06-28 0:20 UTC (permalink / raw) To: Seth Jennings Cc: Greg Kroah-Hartman, devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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 ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency @ 2012-06-28 0:20 ` Minchan Kim 0 siblings, 0 replies; 68+ messages in thread From: Minchan Kim @ 2012-06-28 0:20 UTC (permalink / raw) To: Seth Jennings Cc: Greg Kroah-Hartman, devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* [PATCH 3/3] x86: add local_tlb_flush_kernel_range() 2012-06-25 16:14 ` Seth Jennings @ 2012-06-25 16:14 ` Seth Jennings -1 siblings, 0 replies; 68+ messages in thread From: Seth Jennings @ 2012-06-25 16:14 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Seth Jennings, Andrew Morton, Dan Magenheimer, Konrad Rzeszutek Wilk, Nitin Gupta, Minchan Kim, Robert Jennings, linux-mm, devel, linux-kernel 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 ^ permalink raw reply related [flat|nested] 68+ messages in thread
* [PATCH 3/3] x86: add local_tlb_flush_kernel_range() @ 2012-06-25 16:14 ` Seth Jennings 0 siblings, 0 replies; 68+ messages in thread From: Seth Jennings @ 2012-06-25 16:14 UTC (permalink / raw) To: Greg Kroah-Hartman Cc: Seth Jennings, Andrew Morton, Dan Magenheimer, Konrad Rzeszutek Wilk, Nitin Gupta, Minchan Kim, Robert Jennings, linux-mm, devel, linux-kernel 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> ^ permalink raw reply related [flat|nested] 68+ messages in thread
* Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() 2012-06-25 16:14 ` Seth Jennings @ 2012-06-25 23:01 ` Konrad Rzeszutek Wilk -1 siblings, 0 replies; 68+ messages in thread From: Konrad Rzeszutek Wilk @ 2012-06-25 23:01 UTC (permalink / raw) To: Seth Jennings Cc: Greg Kroah-Hartman, Andrew Morton, Dan Magenheimer, Konrad Rzeszutek Wilk, Nitin Gupta, Minchan Kim, Robert Jennings, linux-mm, devel, linux-kernel 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> > ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() @ 2012-06-25 23:01 ` Konrad Rzeszutek Wilk 0 siblings, 0 replies; 68+ messages in thread From: Konrad Rzeszutek Wilk @ 2012-06-25 23:01 UTC (permalink / raw) To: Seth Jennings Cc: Greg Kroah-Hartman, Andrew Morton, Dan Magenheimer, Konrad Rzeszutek Wilk, Nitin Gupta, Minchan Kim, Robert Jennings, linux-mm, devel, linux-kernel 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() 2012-06-25 23:01 ` Konrad Rzeszutek Wilk @ 2012-06-26 13:39 ` Seth Jennings -1 siblings, 0 replies; 68+ messages in thread From: Seth Jennings @ 2012-06-26 13:39 UTC (permalink / raw) To: konrad Cc: Greg Kroah-Hartman, Andrew Morton, Dan Magenheimer, Konrad Rzeszutek Wilk, Nitin Gupta, Minchan Kim, Robert Jennings, linux-mm, devel, linux-kernel 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 ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() @ 2012-06-26 13:39 ` Seth Jennings 0 siblings, 0 replies; 68+ messages in thread From: Seth Jennings @ 2012-06-26 13:39 UTC (permalink / raw) To: konrad Cc: Greg Kroah-Hartman, Andrew Morton, Dan Magenheimer, Konrad Rzeszutek Wilk, Nitin Gupta, Minchan Kim, Robert Jennings, linux-mm, devel, linux-kernel 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() 2012-06-25 16:14 ` Seth Jennings @ 2012-06-27 5:53 ` Minchan Kim -1 siblings, 0 replies; 68+ messages in thread From: Minchan Kim @ 2012-06-27 5:53 UTC (permalink / raw) To: Seth Jennings Cc: Greg Kroah-Hartman, devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta, Alex Shi 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 ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() @ 2012-06-27 5:53 ` Minchan Kim 0 siblings, 0 replies; 68+ messages in thread From: Minchan Kim @ 2012-06-27 5:53 UTC (permalink / raw) To: Seth Jennings Cc: Greg Kroah-Hartman, devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta, Alex Shi 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() 2012-06-27 5:53 ` Minchan Kim @ 2012-06-27 6:14 ` Alex Shi -1 siblings, 0 replies; 68+ messages in thread From: Alex Shi @ 2012-06-27 6:14 UTC (permalink / raw) To: Minchan Kim Cc: Seth Jennings, Greg Kroah-Hartman, devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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. ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() @ 2012-06-27 6:14 ` Alex Shi 0 siblings, 0 replies; 68+ messages in thread From: Alex Shi @ 2012-06-27 6:14 UTC (permalink / raw) To: Minchan Kim Cc: Seth Jennings, Greg Kroah-Hartman, devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() 2012-06-27 6:14 ` Alex Shi @ 2012-06-27 6:26 ` Minchan Kim -1 siblings, 0 replies; 68+ messages in thread From: Minchan Kim @ 2012-06-27 6:26 UTC (permalink / raw) To: Alex Shi Cc: Seth Jennings, Greg Kroah-Hartman, devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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 ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() @ 2012-06-27 6:26 ` Minchan Kim 0 siblings, 0 replies; 68+ messages in thread From: Minchan Kim @ 2012-06-27 6:26 UTC (permalink / raw) To: Alex Shi Cc: Seth Jennings, Greg Kroah-Hartman, devel, Dan Magenheimer, Konrad Rzeszutek Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* RE: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() 2012-06-27 6:26 ` Minchan Kim @ 2012-06-27 15:12 ` Dan Magenheimer -1 siblings, 0 replies; 68+ messages in thread From: Dan Magenheimer @ 2012-06-27 15:12 UTC (permalink / raw) To: Minchan Kim, Alex Shi Cc: Seth Jennings, Greg Kroah-Hartman, devel, Konrad Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta > 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. ^ permalink raw reply [flat|nested] 68+ messages in thread
* RE: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() @ 2012-06-27 15:12 ` Dan Magenheimer 0 siblings, 0 replies; 68+ messages in thread From: Dan Magenheimer @ 2012-06-27 15:12 UTC (permalink / raw) To: Minchan Kim, Alex Shi Cc: Seth Jennings, Greg Kroah-Hartman, devel, Konrad Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta > 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() 2012-06-27 15:12 ` Dan Magenheimer @ 2012-06-27 15:39 ` Konrad Rzeszutek Wilk -1 siblings, 0 replies; 68+ messages in thread From: Konrad Rzeszutek Wilk @ 2012-06-27 15:39 UTC (permalink / raw) To: Dan Magenheimer Cc: Minchan Kim, Alex Shi, Seth Jennings, Greg Kroah-Hartman, devel, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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 :-) ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() @ 2012-06-27 15:39 ` Konrad Rzeszutek Wilk 0 siblings, 0 replies; 68+ messages in thread From: Konrad Rzeszutek Wilk @ 2012-06-27 15:39 UTC (permalink / raw) To: Dan Magenheimer Cc: Minchan Kim, Alex Shi, Seth Jennings, Greg Kroah-Hartman, devel, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() 2012-06-27 15:39 ` Konrad Rzeszutek Wilk @ 2012-06-27 18:35 ` Seth Jennings -1 siblings, 0 replies; 68+ messages in thread From: Seth Jennings @ 2012-06-27 18:35 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: Dan Magenheimer, Minchan Kim, Alex Shi, Greg Kroah-Hartman, devel, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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 ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() @ 2012-06-27 18:35 ` Seth Jennings 0 siblings, 0 replies; 68+ messages in thread From: Seth Jennings @ 2012-06-27 18:35 UTC (permalink / raw) To: Konrad Rzeszutek Wilk Cc: Dan Magenheimer, Minchan Kim, Alex Shi, Greg Kroah-Hartman, devel, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() 2012-06-27 15:12 ` Dan Magenheimer @ 2012-06-27 18:33 ` Seth Jennings -1 siblings, 0 replies; 68+ messages in thread From: Seth Jennings @ 2012-06-27 18:33 UTC (permalink / raw) To: Dan Magenheimer Cc: Minchan Kim, Alex Shi, Greg Kroah-Hartman, devel, Konrad Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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 ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() @ 2012-06-27 18:33 ` Seth Jennings 0 siblings, 0 replies; 68+ messages in thread From: Seth Jennings @ 2012-06-27 18:33 UTC (permalink / raw) To: Dan Magenheimer Cc: Minchan Kim, Alex Shi, Greg Kroah-Hartman, devel, Konrad Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* RE: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() 2012-06-27 18:33 ` Seth Jennings @ 2012-06-27 21:15 ` Dan Magenheimer -1 siblings, 0 replies; 68+ messages in thread From: Dan Magenheimer @ 2012-06-27 21:15 UTC (permalink / raw) To: Seth Jennings Cc: Minchan Kim, Alex Shi, Greg Kroah-Hartman, devel, Konrad Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta > 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 ^ permalink raw reply [flat|nested] 68+ messages in thread
* RE: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() @ 2012-06-27 21:15 ` Dan Magenheimer 0 siblings, 0 replies; 68+ messages in thread From: Dan Magenheimer @ 2012-06-27 21:15 UTC (permalink / raw) To: Seth Jennings Cc: Minchan Kim, Alex Shi, Greg Kroah-Hartman, devel, Konrad Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta > 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() 2012-06-27 21:15 ` Dan Magenheimer @ 2012-06-27 21:41 ` Seth Jennings -1 siblings, 0 replies; 68+ messages in thread From: Seth Jennings @ 2012-06-27 21:41 UTC (permalink / raw) To: Dan Magenheimer Cc: Minchan Kim, Alex Shi, Greg Kroah-Hartman, devel, Konrad Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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 ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() @ 2012-06-27 21:41 ` Seth Jennings 0 siblings, 0 replies; 68+ messages in thread From: Seth Jennings @ 2012-06-27 21:41 UTC (permalink / raw) To: Dan Magenheimer Cc: Minchan Kim, Alex Shi, Greg Kroah-Hartman, devel, Konrad Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() 2012-06-27 18:33 ` Seth Jennings @ 2012-06-28 2:03 ` Alex Shi -1 siblings, 0 replies; 68+ messages in thread From: Alex Shi @ 2012-06-28 2:03 UTC (permalink / raw) To: Seth Jennings Cc: Dan Magenheimer, Minchan Kim, Greg Kroah-Hartman, devel, Konrad Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta, H. Peter Anvin 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 ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() @ 2012-06-28 2:03 ` Alex Shi 0 siblings, 0 replies; 68+ messages in thread From: Alex Shi @ 2012-06-28 2:03 UTC (permalink / raw) To: Seth Jennings Cc: Dan Magenheimer, Minchan Kim, Greg Kroah-Hartman, devel, Konrad Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta, H. Peter Anvin 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() 2012-06-28 2:03 ` Alex Shi @ 2012-06-28 15:21 ` Seth Jennings -1 siblings, 0 replies; 68+ messages in thread From: Seth Jennings @ 2012-06-28 15:21 UTC (permalink / raw) To: Alex Shi Cc: Dan Magenheimer, Minchan Kim, Greg Kroah-Hartman, devel, Konrad Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta, H. Peter Anvin 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 ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() @ 2012-06-28 15:21 ` Seth Jennings 0 siblings, 0 replies; 68+ messages in thread From: Seth Jennings @ 2012-06-28 15:21 UTC (permalink / raw) To: Alex Shi Cc: Dan Magenheimer, Minchan Kim, Greg Kroah-Hartman, devel, Konrad Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta, H. Peter Anvin 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() 2012-06-28 15:21 ` Seth Jennings @ 2012-06-29 0:19 ` Alex Shi -1 siblings, 0 replies; 68+ messages in thread From: Alex Shi @ 2012-06-29 0:19 UTC (permalink / raw) To: Seth Jennings Cc: Dan Magenheimer, Minchan Kim, Greg Kroah-Hartman, devel, Konrad Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta, H. Peter Anvin 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 > ^ permalink raw reply [flat|nested] 68+ messages in thread
* Re: [PATCH 3/3] x86: add local_tlb_flush_kernel_range() @ 2012-06-29 0:19 ` Alex Shi 0 siblings, 0 replies; 68+ messages in thread From: Alex Shi @ 2012-06-29 0:19 UTC (permalink / raw) To: Seth Jennings Cc: Dan Magenheimer, Minchan Kim, Greg Kroah-Hartman, devel, Konrad Wilk, linux-kernel, linux-mm, Andrew Morton, Robert Jennings, Nitin Gupta, H. Peter Anvin 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> ^ permalink raw reply [flat|nested] 68+ messages in thread
end of thread, other threads:[~2012-06-29 0:21 UTC | newest] Thread overview: 68+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-06-25 16:14 [PATCH 0/3] zsmalloc: remove x86 dependency Seth Jennings 2012-06-25 16:14 ` Seth Jennings 2012-06-25 16:14 ` [PATCH 1/3] zram/zcache: swtich Kconfig dependency from X86 to ZSMALLOC Seth Jennings 2012-06-25 16:14 ` Seth Jennings 2012-06-27 2:37 ` Minchan Kim 2012-06-27 2:37 ` Minchan Kim 2012-06-27 2:43 ` Greg Kroah-Hartman 2012-06-27 2:43 ` Greg Kroah-Hartman 2012-06-27 2:49 ` Minchan Kim 2012-06-27 2:49 ` Minchan Kim 2012-06-27 3:21 ` Greg Kroah-Hartman 2012-06-27 3:21 ` Greg Kroah-Hartman 2012-06-27 15:40 ` Konrad Rzeszutek Wilk 2012-06-27 15:40 ` Konrad Rzeszutek Wilk 2012-06-27 18:55 ` Greg Kroah-Hartman 2012-06-27 18:55 ` Greg Kroah-Hartman 2012-06-27 18:52 ` Konrad Rzeszutek Wilk 2012-06-27 18:52 ` Konrad Rzeszutek Wilk 2012-06-27 19:29 ` Greg Kroah-Hartman 2012-06-27 19:29 ` Greg Kroah-Hartman 2012-06-25 16:14 ` [PATCH 2/3] zsmalloc: add generic path and remove x86 dependency Seth Jennings 2012-06-25 16:14 ` Seth Jennings 2012-06-25 16:59 ` Greg Kroah-Hartman 2012-06-25 16:59 ` Greg Kroah-Hartman 2012-06-25 17:10 ` Seth Jennings 2012-06-25 17:10 ` Seth Jennings 2012-06-25 17:19 ` Greg Kroah-Hartman 2012-06-25 17:19 ` Greg Kroah-Hartman 2012-06-25 18:24 ` Seth Jennings 2012-06-25 18:24 ` Seth Jennings 2012-06-25 23:37 ` Greg Kroah-Hartman 2012-06-25 23:37 ` Greg Kroah-Hartman 2012-06-27 5:28 ` Minchan Kim 2012-06-27 5:28 ` Minchan Kim 2012-06-27 19:09 ` Seth Jennings 2012-06-27 19:09 ` Seth Jennings 2012-06-28 0:20 ` Minchan Kim 2012-06-28 0:20 ` Minchan Kim 2012-06-25 16:14 ` [PATCH 3/3] x86: add local_tlb_flush_kernel_range() Seth Jennings 2012-06-25 16:14 ` Seth Jennings 2012-06-25 23:01 ` Konrad Rzeszutek Wilk 2012-06-25 23:01 ` Konrad Rzeszutek Wilk 2012-06-26 13:39 ` Seth Jennings 2012-06-26 13:39 ` Seth Jennings 2012-06-27 5:53 ` Minchan Kim 2012-06-27 5:53 ` Minchan Kim 2012-06-27 6:14 ` Alex Shi 2012-06-27 6:14 ` Alex Shi 2012-06-27 6:26 ` Minchan Kim 2012-06-27 6:26 ` Minchan Kim 2012-06-27 15:12 ` Dan Magenheimer 2012-06-27 15:12 ` Dan Magenheimer 2012-06-27 15:39 ` Konrad Rzeszutek Wilk 2012-06-27 15:39 ` Konrad Rzeszutek Wilk 2012-06-27 18:35 ` Seth Jennings 2012-06-27 18:35 ` Seth Jennings 2012-06-27 18:33 ` Seth Jennings 2012-06-27 18:33 ` Seth Jennings 2012-06-27 21:15 ` Dan Magenheimer 2012-06-27 21:15 ` Dan Magenheimer 2012-06-27 21:41 ` Seth Jennings 2012-06-27 21:41 ` Seth Jennings 2012-06-28 2:03 ` Alex Shi 2012-06-28 2:03 ` Alex Shi 2012-06-28 15:21 ` Seth Jennings 2012-06-28 15:21 ` Seth Jennings 2012-06-29 0:19 ` Alex Shi 2012-06-29 0:19 ` Alex Shi
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.