zsmalloc: Fix TLB coherency and build problem

PATCH In review Minchan Kim 4 participants 7 messages Jan 28, 2013 – Feb 19, 2013

Subsystems (3)

Stable branch Memory management Staging

lore

Share & tools

lore

Minchan Kim

author

2013-01-28 01:00 UTC #0 lore raw

PATCH 1/1 zsmalloc: Fix TLB coherency and build problem

Recently, Matt Sealey reported he fail to build zsmalloc caused by using of local_flush_tlb_kernel_range which are architecture dependent function so !CONFIG_SMP in ARM couldn't implement it so it ends up build error following as.

MODPOST 216 modules
LZMA arch/arm/boot/compressed/piggy.lzma
AS arch/arm/boot/compressed/lib1funcs.o
ERROR: "v7wbi_flush_kern_tlb_range"
[drivers/staging/zsmalloc/zsmalloc.ko] undefined!
make[1]: *** [__modpost] Error 1
make: *** [modules] Error 2
make: *** Waiting for unfinished jobs....

The reason we used that function is copy method by [1] was really slow in ARM but at that time.

More severe problem is ARM can prefetch speculatively on other CPUs so under us, other TLBs can have an entry only if we do flush local CPU. Russell King pointed that. Thanks!
We don't have many choices except using flush_tlb_kernel_range.

My experiment in ARMv7 processor 4 core didn't make any difference with zsmapbench[2] between local_flush_tlb_kernel_range and flush_tlb_kernel_range but still page-table based is much better than copy-based.

* bigger is better.

1. local_flush_tlb_kernel_range: 3918795 mappings
2. flush_tlb_kernel_range : 3989538 mappings
3. copy-based: 635158 mappings

This patch replace local_flush_tlb_kernel_range with flush_tlb_kernel_range which are avaialbe in all architectures because we already have used it in vmalloc allocator which are generic one so build problem should go away and performane loss shoud be void.

[1] f553646, zsmalloc: add page table mapping method
[2] https://github.com/spartacus06/zsmapbench

Cc [email protected]

Cc Dan Magenheimer <[email protected]>

Cc Russell King <[email protected]>

Cc Konrad Rzeszutek Wilk <[email protected]>

Cc Nitin Gupta <[email protected]>

Cc Seth Jennings <[email protected]>

Reported-by Matt Sealey <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---

Matt, Could you test this patch?

 drivers/staging/zsmalloc/zsmalloc-main.c |   10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

drivers/staging/zsmalloc/zsmalloc-main.c +4 -6

index eb00772..82e627c 100644
@@ -222,11 +222,9 @@ struct zs_pool {
 /*
  * By default, zsmalloc uses a copy-based object mapping method to access
  * allocations that span two pages. However, if a particular architecture
- * 1) Implements local_flush_tlb_kernel_range() and 2) Performs VM mapping
- * faster than copying, then it should be added here so that
- * USE_PGTABLE_MAPPING is defined. This causes zsmalloc to use page table
- * mapping rather than copying
- * for object mapping.
+ * performs VM mapping faster than copying, then it should be added here
+ * so that USE_PGTABLE_MAPPING is defined. This causes zsmalloc to use
+ * page table mapping rather than copying for object mapping.
 */
 #if defined(CONFIG_ARM)
 #define USE_PGTABLE_MAPPING

@@ -663,7 +661,7 @@ static inline void __zs_unmap_object(struct mapping_area *area,
 
 	flush_cache_vunmap(addr, end);
 	unmap_kernel_range_noflush(addr, PAGE_SIZE * 2);
-	local_flush_tlb_kernel_range(addr, end);
+	flush_tlb_kernel_range(addr, end);
 }
 
 #else /* USE_PGTABLE_MAPPING */

Signature · 1 line

1.7.9.5

Minchan Kim

author

2013-01-30 02:48 UTC #1 lore raw

Ping

Quoted 92 lines · Minchan Kim

Recently, Matt Sealey reported he fail to build zsmalloc caused by using of local_flush_tlb_kernel_range which are architecture dependent function so !CONFIG_SMP in ARM couldn't implement it so it ends up build error following as.
MODPOST 216 modules
LZMA arch/arm/boot/compressed/piggy.lzma
AS arch/arm/boot/compressed/lib1funcs.o
ERROR: "v7wbi_flush_kern_tlb_range"
[drivers/staging/zsmalloc/zsmalloc.ko] undefined!
make[1]: *** [__modpost] Error 1
make: *** [modules] Error 2
make: *** Waiting for unfinished jobs....
The reason we used that function is copy method by [1] was really slow in ARM but at that time.
More severe problem is ARM can prefetch speculatively on other CPUs so under us, other TLBs can have an entry only if we do flush local CPU. Russell King pointed that. Thanks!
We don't have many choices except using flush_tlb_kernel_range.
My experiment in ARMv7 processor 4 core didn't make any difference with zsmapbench[2] between local_flush_tlb_kernel_range and flush_tlb_kernel_range but still page-table based is much better than copy-based.
* bigger is better.
1. local_flush_tlb_kernel_range: 3918795 mappings
2. flush_tlb_kernel_range : 3989538 mappings
3. copy-based: 635158 mappings
This patch replace local_flush_tlb_kernel_range with flush_tlb_kernel_range which are avaialbe in all architectures because we already have used it in vmalloc allocator which are generic one so build problem should go away and performane loss shoud be void.
[1] f553646, zsmalloc: add page table mapping method
[2] https://github.com/spartacus06/zsmapbench
Cc [email protected]
Cc Dan Magenheimer <[email protected]>
Cc Russell King <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Nitin Gupta <[email protected]>
Cc Seth Jennings <[email protected]>
Reported-by Matt Sealey <[email protected]>
Signed-off-by Minchan Kim <[email protected]>
---
Matt, Could you test this patch?
 drivers/staging/zsmalloc/zsmalloc-main.c |   10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
drivers/staging/zsmalloc/zsmalloc-main.c +4 -6
index eb00772..82e627c 100644
@@ -222,11 +222,9 @@ struct zs_pool {
 /*
  * By default, zsmalloc uses a copy-based object mapping method to access
  * allocations that span two pages. However, if a particular architecture
- * 1) Implements local_flush_tlb_kernel_range() and 2) Performs VM mapping
- * faster than copying, then it should be added here so that
- * USE_PGTABLE_MAPPING is defined. This causes zsmalloc to use page table
- * mapping rather than copying
- * for object mapping.
+ * performs VM mapping faster than copying, then it should be added here
+ * so that USE_PGTABLE_MAPPING is defined. This causes zsmalloc to use
+ * page table mapping rather than copying for object mapping.
 */
 #if defined(CONFIG_ARM)
 #define USE_PGTABLE_MAPPING
@@ -663,7 +661,7 @@ static inline void __zs_unmap_object(struct mapping_area *area,
 
 	flush_cache_vunmap(addr, end);
 	unmap_kernel_range_noflush(addr, PAGE_SIZE * 2);
-	local_flush_tlb_kernel_range(addr, end);
+	flush_tlb_kernel_range(addr, end);
 }
 
 #else /* USE_PGTABLE_MAPPING */
Signature · 7 lines
1.7.9.5

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected].  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Signature · 2 lines

Kind regards,
Minchan Kim

Konrad Rzeszutek Wilk

Acked-by on v1 1/1

2013-02-01 13:02 UTC #2 lore raw

In reply to Minchan Kim

Quoted 38 lines · Minchan Kim

Recently, Matt Sealey reported he fail to build zsmalloc caused by using of local_flush_tlb_kernel_range which are architecture dependent function so !CONFIG_SMP in ARM couldn't implement it so it ends up build error following as.
MODPOST 216 modules
LZMA arch/arm/boot/compressed/piggy.lzma
AS arch/arm/boot/compressed/lib1funcs.o
ERROR: "v7wbi_flush_kern_tlb_range"
[drivers/staging/zsmalloc/zsmalloc.ko] undefined!
make[1]: *** [__modpost] Error 1
make: *** [modules] Error 2
make: *** Waiting for unfinished jobs....
The reason we used that function is copy method by [1] was really slow in ARM but at that time.
More severe problem is ARM can prefetch speculatively on other CPUs so under us, other TLBs can have an entry only if we do flush local CPU. Russell King pointed that. Thanks!
We don't have many choices except using flush_tlb_kernel_range.
My experiment in ARMv7 processor 4 core didn't make any difference with zsmapbench[2] between local_flush_tlb_kernel_range and flush_tlb_kernel_range but still page-table based is much better than copy-based.
* bigger is better.
1. local_flush_tlb_kernel_range: 3918795 mappings
2. flush_tlb_kernel_range : 3989538 mappings
3. copy-based: 635158 mappings
This patch replace local_flush_tlb_kernel_range with flush_tlb_kernel_range which are avaialbe in all architectures because we already have used it in vmalloc allocator which are generic one so build problem should go away and performane loss shoud be void.

Quoted 7 lines

[1] f553646, zsmalloc: add page table mapping method
[2] https://github.com/spartacus06/zsmapbench
Cc [email protected]
Cc Dan Magenheimer <[email protected]>
Cc Russell King <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>

Acked-by Konrad Rzeszutek Wilk <[email protected]>

Quoted 42 lines

Cc Nitin Gupta <[email protected]>

Cc Seth Jennings <[email protected]>

Reported-by Matt Sealey <[email protected]>

Signed-off-by Minchan Kim <[email protected]>

---

Matt, Could you test this patch?

 drivers/staging/zsmalloc/zsmalloc-main.c |   10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

drivers/staging/zsmalloc/zsmalloc-main.c +4 -6

index eb00772..82e627c 100644
@@ -222,11 +222,9 @@ struct zs_pool {
 /*
  * By default, zsmalloc uses a copy-based object mapping method to access
  * allocations that span two pages. However, if a particular architecture
- * 1) Implements local_flush_tlb_kernel_range() and 2) Performs VM mapping
- * faster than copying, then it should be added here so that
- * USE_PGTABLE_MAPPING is defined. This causes zsmalloc to use page table
- * mapping rather than copying
- * for object mapping.
+ * performs VM mapping faster than copying, then it should be added here
+ * so that USE_PGTABLE_MAPPING is defined. This causes zsmalloc to use
+ * page table mapping rather than copying for object mapping.
 */
 #if defined(CONFIG_ARM)
 #define USE_PGTABLE_MAPPING

@@ -663,7 +661,7 @@ static inline void __zs_unmap_object(struct mapping_area *area,
 
 	flush_cache_vunmap(addr, end);
 	unmap_kernel_range_noflush(addr, PAGE_SIZE * 2);
-	local_flush_tlb_kernel_range(addr, end);
+	flush_tlb_kernel_range(addr, end);
 }
 
 #else /* USE_PGTABLE_MAPPING */

Signature · 1 line

1.7.9.5

Russell King - ARM Linux

2013-02-01 14:02 UTC #3 lore raw

In reply to Minchan Kim

drivers/staging/zsmalloc/zsmalloc-main.c static inline void __zs_unmap_object(struct mapping_area *area, · line 664

Quoted 6 lines · Minchan Kim

drivers/staging/zsmalloc/zsmalloc-main.c +1 -1

@@ -663,7 +661,7 @@ static inline void __zs_unmap_object(struct mapping_area *area,
 
 	flush_cache_vunmap(addr, end);
 	unmap_kernel_range_noflush(addr, PAGE_SIZE * 2);
-	local_flush_tlb_kernel_range(addr, end);
+	flush_tlb_kernel_range(addr, end);

void unmap_kernel_range_noflush(unsigned long addr, unsigned long size)

{
        vunmap_page_range(addr, addr + size);
}

void unmap_kernel_range(unsigned long addr, unsigned long size)

{
        unsigned long end = addr + size;

        flush_cache_vunmap(addr, end);
        vunmap_page_range(addr, end);
        flush_tlb_kernel_range(addr, end);
}

So, given the above, what would be different between:

	unsigned long end = addr + (PAGE_SIZE * 2);

	flush_cache_vunmap(addr, end);
	unmap_kernel_range_noflush(addr, PAGE_SIZE * 2);
	flush_tlb_kernel_range(addr, end);

(which is what it becomes after your change) and

	unmap_kernel_range(addr, PAGE_SIZE * 2);

Minchan Kim

author

2013-02-03 23:50 UTC #4 lore raw

Quoted 30 lines · Russell King - ARM Linux

drivers/staging/zsmalloc/zsmalloc-main.c static inline void __zs_unmap_object(struct mapping_area *area, · line 664
Quoted 6 lines · Minchan Kim
drivers/staging/zsmalloc/zsmalloc-main.c +1 -1
@@ -663,7 +661,7 @@ static inline void __zs_unmap_object(struct mapping_area *area,
 
 	flush_cache_vunmap(addr, end);
 	unmap_kernel_range_noflush(addr, PAGE_SIZE * 2);
-	local_flush_tlb_kernel_range(addr, end);
+	flush_tlb_kernel_range(addr, end);
void unmap_kernel_range_noflush(unsigned long addr, unsigned long size)
{
        vunmap_page_range(addr, addr + size);
}
void unmap_kernel_range(unsigned long addr, unsigned long size)
{
        unsigned long end = addr + size;

        flush_cache_vunmap(addr, end);
        vunmap_page_range(addr, end);
        flush_tlb_kernel_range(addr, end);
}
So, given the above, what would be different between:
	unsigned long end = addr + (PAGE_SIZE * 2);

	flush_cache_vunmap(addr, end);
	unmap_kernel_range_noflush(addr, PAGE_SIZE * 2);
	flush_tlb_kernel_range(addr, end);

Quoted 5 lines

(which is what it becomes after your change) and
	unmap_kernel_range(addr, PAGE_SIZE * 2);
?

Good point. I will clean it up.
Thanks.

Quoted 6 lines

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Signature · 2 lines

Kind regards,
Minchan Kim

Simon Jeons

2013-02-19 10:07 UTC #5 lore raw

In reply to Minchan Kim

Quoted 4 lines · Minchan Kim

Recently, Matt Sealey reported he fail to build zsmalloc caused by using of local_flush_tlb_kernel_range which are architecture dependent function so !CONFIG_SMP in ARM couldn't implement it so it ends up build error following as.

Confuse me!

1) Why I see flush_tlb_kernel_range is different in different architecture?
2) Does local here means local cpu? If the answer is yes, why ARM doesn't support it?

Quoted 80 lines

MODPOST 216 modules
LZMA arch/arm/boot/compressed/piggy.lzma
AS arch/arm/boot/compressed/lib1funcs.o
ERROR: "v7wbi_flush_kern_tlb_range"
[drivers/staging/zsmalloc/zsmalloc.ko] undefined!
make[1]: *** [__modpost] Error 1
make: *** [modules] Error 2
make: *** Waiting for unfinished jobs....
The reason we used that function is copy method by [1] was really slow in ARM but at that time.
More severe problem is ARM can prefetch speculatively on other CPUs so under us, other TLBs can have an entry only if we do flush local CPU. Russell King pointed that. Thanks!
We don't have many choices except using flush_tlb_kernel_range.
My experiment in ARMv7 processor 4 core didn't make any difference with zsmapbench[2] between local_flush_tlb_kernel_range and flush_tlb_kernel_range but still page-table based is much better than copy-based.
* bigger is better.
1. local_flush_tlb_kernel_range: 3918795 mappings
2. flush_tlb_kernel_range : 3989538 mappings
3. copy-based: 635158 mappings
This patch replace local_flush_tlb_kernel_range with flush_tlb_kernel_range which are avaialbe in all architectures because we already have used it in vmalloc allocator which are generic one so build problem should go away and performane loss shoud be void.
[1] f553646, zsmalloc: add page table mapping method
[2] https://github.com/spartacus06/zsmapbench
Cc [email protected]
Cc Dan Magenheimer <[email protected]>
Cc Russell King <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Nitin Gupta <[email protected]>
Cc Seth Jennings <[email protected]>
Reported-by Matt Sealey <[email protected]>
Signed-off-by Minchan Kim <[email protected]>
---
Matt, Could you test this patch?
  drivers/staging/zsmalloc/zsmalloc-main.c |   10 ++++------
  1 file changed, 4 insertions(+), 6 deletions(-)
drivers/staging/zsmalloc/zsmalloc-main.c +4 -6
index eb00772..82e627c 100644
@@ -222,11 +222,9 @@ struct zs_pool {
  /*
   * By default, zsmalloc uses a copy-based object mapping method to access
   * allocations that span two pages. However, if a particular architecture
- * 1) Implements local_flush_tlb_kernel_range() and 2) Performs VM mapping
- * faster than copying, then it should be added here so that
- * USE_PGTABLE_MAPPING is defined. This causes zsmalloc to use page table
- * mapping rather than copying
- * for object mapping.
+ * performs VM mapping faster than copying, then it should be added here
+ * so that USE_PGTABLE_MAPPING is defined. This causes zsmalloc to use
+ * page table mapping rather than copying for object mapping.
  */
  #if defined(CONFIG_ARM)
  #define USE_PGTABLE_MAPPING
@@ -663,7 +661,7 @@ static inline void __zs_unmap_object(struct mapping_area *area,
 
  	flush_cache_vunmap(addr, end);
  	unmap_kernel_range_noflush(addr, PAGE_SIZE * 2);
-	local_flush_tlb_kernel_range(addr, end);
+	flush_tlb_kernel_range(addr, end);
  }
 
  #else /* USE_PGTABLE_MAPPING */

Minchan Kim

author

2013-02-19 23:45 UTC #6 lore raw

Quoted 9 lines · Simon Jeons

Quoted 4 lines · Minchan Kim
Recently, Matt Sealey reported he fail to build zsmalloc caused by using of local_flush_tlb_kernel_range which are architecture dependent function so !CONFIG_SMP in ARM couldn't implement it so it ends up build error following as.
Confuse me!
1) Why I see flush_tlb_kernel_range is different in different architecture?

IMHO, all architecture can do their best effort by thier own way.

Quoted 1 line

2) Does local here means local cpu? If the answer is yes, why ARM

Yes.

Quoted 1 line

doesn't support it?

ARM supports it for some configuration and CPUs.
The thing is that every architecture doesn't support so it's not a generic API. It means we should avoid it in general layer.

Quoted 85 lines

Quoted 73 lines
MODPOST 216 modules
LZMA arch/arm/boot/compressed/piggy.lzma
AS arch/arm/boot/compressed/lib1funcs.o
ERROR: "v7wbi_flush_kern_tlb_range"
[drivers/staging/zsmalloc/zsmalloc.ko] undefined!
make[1]: *** [__modpost] Error 1
make: *** [modules] Error 2
make: *** Waiting for unfinished jobs....
The reason we used that function is copy method by [1] was really slow in ARM but at that time.
More severe problem is ARM can prefetch speculatively on other CPUs so under us, other TLBs can have an entry only if we do flush local CPU. Russell King pointed that. Thanks!
We don't have many choices except using flush_tlb_kernel_range.
My experiment in ARMv7 processor 4 core didn't make any difference with zsmapbench[2] between local_flush_tlb_kernel_range and flush_tlb_kernel_range but still page-table based is much better than copy-based.
* bigger is better.
1. local_flush_tlb_kernel_range: 3918795 mappings
2. flush_tlb_kernel_range : 3989538 mappings
3. copy-based: 635158 mappings
This patch replace local_flush_tlb_kernel_range with flush_tlb_kernel_range which are avaialbe in all architectures because we already have used it in vmalloc allocator which are generic one so build problem should go away and performane loss shoud be void.
[1] f553646, zsmalloc: add page table mapping method
[2] https://github.com/spartacus06/zsmapbench
Cc [email protected]
Cc Dan Magenheimer <[email protected]>
Cc Russell King <[email protected]>
Cc Konrad Rzeszutek Wilk <[email protected]>
Cc Nitin Gupta <[email protected]>
Cc Seth Jennings <[email protected]>
Reported-by Matt Sealey <[email protected]>
Signed-off-by Minchan Kim <[email protected]>
---
Matt, Could you test this patch?
 drivers/staging/zsmalloc/zsmalloc-main.c |   10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
drivers/staging/zsmalloc/zsmalloc-main.c +3 -5
index eb00772..82e627c 100644
@@ -222,11 +222,9 @@ struct zs_pool {
 /*
  * By default, zsmalloc uses a copy-based object mapping method to access
  * allocations that span two pages. However, if a particular architecture
- * 1) Implements local_flush_tlb_kernel_range() and 2) Performs VM mapping
- * faster than copying, then it should be added here so that
- * USE_PGTABLE_MAPPING is defined. This causes zsmalloc to use page table
- * mapping rather than copying
- * for object mapping.
+ * performs VM mapping faster than copying, then it should be added here
+ * so that USE_PGTABLE_MAPPING is defined. This causes zsmalloc to use
+ * page table mapping rather than copying for object mapping.
 */
 #if defined(CONFIG_ARM)
 #define USE_PGTABLE_MAPPING
drivers/staging/zsmalloc/zsmalloc-main.c static inline void __zs_unmap_object(struct mapping_area *area, · line 665
Quoted 6 lines
drivers/staging/zsmalloc/zsmalloc-main.c +1 -1
@@ -663,7 +661,7 @@ static inline void __zs_unmap_object(struct mapping_area *area,
 	flush_cache_vunmap(addr, end);
 	unmap_kernel_range_noflush(addr, PAGE_SIZE * 2);
-	local_flush_tlb_kernel_range(addr, end);
+	flush_tlb_kernel_range(addr, end);
 }
 #else /* USE_PGTABLE_MAPPING */
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>

Signature · 2 lines

Kind regards,
Minchan Kim