2012-05-14 08:45:12

by Minchan Kim

[permalink] [raw]
Subject: [PATCH 1/3] zsmalloc: support zsmalloc to ARM, MIPS, SUPERH

zsmalloc uses set_pte and __flush_tlb_one for performance but
many architecture don't support it. so this patch removes
set_pte and __flush_tlb_one which are x86 dependency.
Instead of it, use local_flush_tlb_kernel_range which are available
by more architectures. It would be better than supporting only x86
and last patch in series will enable again with supporting
local_flush_tlb_kernel_range in x86.

About local_flush_tlb_kernel_range,
If architecture is very smart, it could flush only tlb entries related to vaddr.
If architecture is smart, it could flush only tlb entries related to a CPU.
If architecture is _NOT_ smart, it could flush all entries of all CPUs.
So, it would be best to support both portability and performance.

Cc: Russell King <[email protected]>
Cc: Ralf Baechle <[email protected]>
Cc: Paul Mundt <[email protected]>
Cc: Guan Xuetao <[email protected]>
Cc: Chen Liqin <[email protected]>
Signed-off-by: Minchan Kim <[email protected]>
---

Need double check about supporting local_flush_tlb_kernel_range
in ARM, MIPS, SUPERH maintainers. And I will Ccing unicore32 and
score maintainers because arch directory in those arch have
local_flush_tlb_kernel_range, too but I'm very unfamiliar with those
architecture so pass it to maintainers.
I didn't coded up dumb local_flush_tlb_kernel_range which flush
all cpus. I expect someone need ZSMALLOC will implement it easily in future.
Seth might support it in PowerPC. :)

drivers/staging/zsmalloc/Kconfig | 6 ++---
drivers/staging/zsmalloc/zsmalloc-main.c | 36 +++++++++++++++++++++---------
drivers/staging/zsmalloc/zsmalloc_int.h | 1 -
3 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/drivers/staging/zsmalloc/Kconfig b/drivers/staging/zsmalloc/Kconfig
index a5ab720..def2483 100644
--- a/drivers/staging/zsmalloc/Kconfig
+++ b/drivers/staging/zsmalloc/Kconfig
@@ -1,9 +1,9 @@
config ZSMALLOC
tristate "Memory allocator for compressed pages"
- # X86 dependency is because of the use of __flush_tlb_one and set_pte
+ # arch dependency is because of the use of local_unmap_kernel_range
# in zsmalloc-main.c.
- # TODO: convert these to portable functions
- depends on X86
+ # TODO: implement local_unmap_kernel_range in all architecture.
+ depends on (ARM || MIPS || SUPERH)
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 4496737..8a8b08f 100644
--- a/drivers/staging/zsmalloc/zsmalloc-main.c
+++ b/drivers/staging/zsmalloc/zsmalloc-main.c
@@ -442,7 +442,7 @@ static int zs_cpu_notifier(struct notifier_block *nb, unsigned long action,
area = &per_cpu(zs_map_area, cpu);
if (area->vm)
break;
- area->vm = alloc_vm_area(2 * PAGE_SIZE, area->vm_ptes);
+ area->vm = alloc_vm_area(2 * PAGE_SIZE, NULL);
if (!area->vm)
return notifier_from_errno(-ENOMEM);
break;
@@ -696,13 +696,22 @@ void *zs_map_object(struct zs_pool *pool, void *handle)
} else {
/* this object spans two pages */
struct page *nextp;
+ struct page *pages[2];
+ struct page **page_array = &pages[0];
+ int err;

nextp = get_next_page(page);
BUG_ON(!nextp);

+ page_array[0] = page;
+ page_array[1] = nextp;

- set_pte(area->vm_ptes[0], mk_pte(page, PAGE_KERNEL));
- set_pte(area->vm_ptes[1], mk_pte(nextp, PAGE_KERNEL));
+ /*
+ * map_vm_area never fail because we already allocated
+ * pages for page table in alloc_vm_area.
+ */
+ err = map_vm_area(area->vm, PAGE_KERNEL, &page_array);
+ BUG_ON(err);

/* We pre-allocated VM area so mapping can never fail */
area->vm_addr = area->vm->addr;
@@ -712,6 +721,15 @@ void *zs_map_object(struct zs_pool *pool, void *handle)
}
EXPORT_SYMBOL_GPL(zs_map_object);

+static void local_unmap_kernel_range(unsigned long addr, unsigned long size)
+{
+ unsigned long end = addr + size;
+
+ flush_cache_vunmap(addr, end);
+ unmap_kernel_range_noflush(addr, size);
+ local_flush_tlb_kernel_range(addr, end);
+}
+
void zs_unmap_object(struct zs_pool *pool, void *handle)
{
struct page *page;
@@ -730,14 +748,12 @@ void zs_unmap_object(struct zs_pool *pool, void *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
+ local_unmap_kernel_range((unsigned long)area->vm->addr,
+ PAGE_SIZE * 2);
+
put_cpu_var(zs_map_area);
}
EXPORT_SYMBOL_GPL(zs_unmap_object);
diff --git a/drivers/staging/zsmalloc/zsmalloc_int.h b/drivers/staging/zsmalloc/zsmalloc_int.h
index 6fd32a9..eaec845 100644
--- a/drivers/staging/zsmalloc/zsmalloc_int.h
+++ b/drivers/staging/zsmalloc/zsmalloc_int.h
@@ -111,7 +111,6 @@ static const int fullness_threshold_frac = 4;

struct mapping_area {
struct vm_struct *vm;
- pte_t *vm_ptes[2];
char *vm_addr;
};

--
1.7.9.5


2012-05-14 08:45:14

by Minchan Kim

[permalink] [raw]
Subject: [PATCH 3/3] x86: Support local_flush_tlb_kernel_range

The zsmalloc [un]maps non-physical contiguos pages to contiguous
virual address frequently so it needs frequent tlb-flush.
Now x86 doesn't support common utility function for flushing just
a few tlb entries so zsmalloc have been used set_pte and __flush_tlb_one
which are x86 specific functions. It means zsmalloc have a dependency
with x86.

This patch adds new function, local_flush_tlb_kernel_range which
are good candidate for being common utility function because other
architecture(ex, MIPS, sh, unicore32, arm, score) already have
supportd it.

Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Tejun Heo <[email protected]>
Cc: David Howells <[email protected]>
Cc: [email protected]
Signed-off-by: Minchan Kim <[email protected]>
---
arch/x86/include/asm/tlbflush.h | 12 ++++++++++++
drivers/staging/zsmalloc/Kconfig | 2 +-
2 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/tlbflush.h b/arch/x86/include/asm/tlbflush.h
index 4ece077..6e1253a 100644
--- a/arch/x86/include/asm/tlbflush.h
+++ b/arch/x86/include/asm/tlbflush.h
@@ -172,4 +172,16 @@ static inline void flush_tlb_kernel_range(unsigned long start,
flush_tlb_all();
}

+static inline void local_flush_tlb_kernel_range(unsigned long start,
+ unsigned long end)
+{
+ if (cpu_has_invlpg) {
+ while (start < end) {
+ __flush_tlb_single(start);
+ start += PAGE_SIZE;
+ }
+ } else
+ local_flush_tlb();
+}
+
#endif /* _ASM_X86_TLBFLUSH_H */
diff --git a/drivers/staging/zsmalloc/Kconfig b/drivers/staging/zsmalloc/Kconfig
index def2483..29819b8 100644
--- a/drivers/staging/zsmalloc/Kconfig
+++ b/drivers/staging/zsmalloc/Kconfig
@@ -3,7 +3,7 @@ config ZSMALLOC
# arch dependency is because of the use of local_unmap_kernel_range
# in zsmalloc-main.c.
# TODO: implement local_unmap_kernel_range in all architecture.
- depends on (ARM || MIPS || SUPERH)
+ depends on (ARM || MIPS || SUPERH || X86)
default n
help
zsmalloc is a slab-based memory allocator designed to store
--
1.7.9.5

2012-05-14 08:45:43

by Minchan Kim

[permalink] [raw]
Subject: [PATCH 2/3] zram: remove comment in Kconfig

Exactly speaking, zram should has dependency with
zsmalloc, not x86. So x86 dependeny check is redundant.

Signed-off-by: Minchan Kim <[email protected]>
---
drivers/staging/zram/Kconfig | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/staging/zram/Kconfig b/drivers/staging/zram/Kconfig
index 9d11a4c..ee23a86 100644
--- a/drivers/staging/zram/Kconfig
+++ b/drivers/staging/zram/Kconfig
@@ -1,8 +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
+ depends on BLOCK && SYSFS
select ZSMALLOC
select LZO_COMPRESS
select LZO_DECOMPRESS
--
1.7.9.5

2012-05-14 14:42:34

by Seth Jennings

[permalink] [raw]
Subject: Re: [PATCH 2/3] zram: remove comment in Kconfig

On 05/14/2012 03:45 AM, Minchan Kim wrote:

> Exactly speaking, zram should has dependency with
> zsmalloc, not x86. So x86 dependeny check is redundant.
>
> Signed-off-by: Minchan Kim <[email protected]>
> ---
> drivers/staging/zram/Kconfig | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/drivers/staging/zram/Kconfig b/drivers/staging/zram/Kconfig
> index 9d11a4c..ee23a86 100644
> --- a/drivers/staging/zram/Kconfig
> +++ b/drivers/staging/zram/Kconfig
> @@ -1,8 +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
> + depends on BLOCK && SYSFS


Two comments here:

1) zram should really depend on ZSMALLOC instead of selecting it
because, as the patch has it, zram could be selected on an arch that
zsmalloc doesn't support.

2) This change would need to be done in zcache as well.

Seth

2012-05-15 02:30:56

by Minchan Kim

[permalink] [raw]
Subject: Re: [PATCH 2/3] zram: remove comment in Kconfig

On 05/14/2012 11:42 PM, Seth Jennings wrote:

> On 05/14/2012 03:45 AM, Minchan Kim wrote:
>
>> Exactly speaking, zram should has dependency with
>> zsmalloc, not x86. So x86 dependeny check is redundant.
>>
>> Signed-off-by: Minchan Kim <[email protected]>
>> ---
>> drivers/staging/zram/Kconfig | 4 +---
>> 1 file changed, 1 insertion(+), 3 deletions(-)
>>
>> diff --git a/drivers/staging/zram/Kconfig b/drivers/staging/zram/Kconfig
>> index 9d11a4c..ee23a86 100644
>> --- a/drivers/staging/zram/Kconfig
>> +++ b/drivers/staging/zram/Kconfig
>> @@ -1,8 +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
>> + depends on BLOCK && SYSFS
>
>
> Two comments here:
>
> 1) zram should really depend on ZSMALLOC instead of selecting it
> because, as the patch has it, zram could be selected on an arch that
> zsmalloc doesn't support.


Argh, Totally my mistake. my patch didn't match with my comment, either. :(

>
> 2) This change would need to be done in zcache as well.


I see.
Seth, Thanks.

send v2.

== CUT_HERE ==

>From be81aec5a4f35139aae2bf3d18139fbc114897ca Mon Sep 17 00:00:00 2001
From: Minchan Kim <[email protected]>
Date: Tue, 15 May 2012 11:26:48 +0900
Subject: [PATCH] [zram,zcache] remove dependency with x86

Exactly saying, [zram|zcache] should has a dependency with
zsmalloc, not x86. So replace x86 dependeny with ZSMALLOC.

Signed-off-by: Minchan Kim <[email protected]>
---
drivers/staging/zcache/Kconfig | 3 +--
drivers/staging/zram/Kconfig | 3 +--
2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/zcache/Kconfig b/drivers/staging/zcache/Kconfig
index 7048e01..ceb7f28 100644
--- a/drivers/staging/zcache/Kconfig
+++ b/drivers/staging/zcache/Kconfig
@@ -2,8 +2,7 @@ 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
select CRYPTO_LZO
default n
help
diff --git a/drivers/staging/zram/Kconfig b/drivers/staging/zram/Kconfig
index 9d11a4c..e3ac62d 100644
--- a/drivers/staging/zram/Kconfig
+++ b/drivers/staging/zram/Kconfig
@@ -2,8 +2,7 @@ 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



--
Kind regards,
Minchan Kim

2012-05-15 14:37:24

by Konrad Rzeszutek Wilk

[permalink] [raw]
Subject: Re: [PATCH 2/3] zram: remove comment in Kconfig

>
> == CUT_HERE ==
>
> >From be81aec5a4f35139aae2bf3d18139fbc114897ca Mon Sep 17 00:00:00 2001
> From: Minchan Kim <[email protected]>
> Date: Tue, 15 May 2012 11:26:48 +0900
> Subject: [PATCH] [zram,zcache] remove dependency with x86
>
> Exactly saying, [zram|zcache] should has a dependency with
> zsmalloc, not x86. So replace x86 dependeny with ZSMALLOC.
>
> Signed-off-by: Minchan Kim <[email protected]>

Reviewed-by: Konrad Rzeszutek Wilk <[email protected]>

> ---
> ?drivers/staging/zcache/Kconfig | ? ?3 +--
> ?drivers/staging/zram/Kconfig ? | ? ?3 +--
> ?2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/zcache/Kconfig b/drivers/staging/zcache/Kconfig
> index 7048e01..ceb7f28 100644
> --- a/drivers/staging/zcache/Kconfig
> +++ b/drivers/staging/zcache/Kconfig
> @@ -2,8 +2,7 @@ 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
> ? ? ? ?select CRYPTO_LZO
> ? ? ? ?default n
> ? ? ? ?help
> diff --git a/drivers/staging/zram/Kconfig b/drivers/staging/zram/Kconfig
> index 9d11a4c..e3ac62d 100644
> --- a/drivers/staging/zram/Kconfig
> +++ b/drivers/staging/zram/Kconfig
> @@ -2,8 +2,7 @@ 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
>
>
>
> --
> Kind regards,
> Minchan Kim
>
> --
> 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/ .
> Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
> Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
>

2012-05-15 15:55:24

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 2/3] zram: remove comment in Kconfig

On Tue, May 15, 2012 at 11:31:24AM +0900, Minchan Kim wrote:
> On 05/14/2012 11:42 PM, Seth Jennings wrote:
>
> > On 05/14/2012 03:45 AM, Minchan Kim wrote:
> >
> >> Exactly speaking, zram should has dependency with
> >> zsmalloc, not x86. So x86 dependeny check is redundant.
> >>
> >> Signed-off-by: Minchan Kim <[email protected]>
> >> ---
> >> drivers/staging/zram/Kconfig | 4 +---
> >> 1 file changed, 1 insertion(+), 3 deletions(-)
> >>
> >> diff --git a/drivers/staging/zram/Kconfig b/drivers/staging/zram/Kconfig
> >> index 9d11a4c..ee23a86 100644
> >> --- a/drivers/staging/zram/Kconfig
> >> +++ b/drivers/staging/zram/Kconfig
> >> @@ -1,8 +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
> >> + depends on BLOCK && SYSFS
> >
> >
> > Two comments here:
> >
> > 1) zram should really depend on ZSMALLOC instead of selecting it
> > because, as the patch has it, zram could be selected on an arch that
> > zsmalloc doesn't support.
>
>
> Argh, Totally my mistake. my patch didn't match with my comment, either. :(
>
> >
> > 2) This change would need to be done in zcache as well.
>
>
> I see.
> Seth, Thanks.
>
> send v2.

It's all messed up with tabs and spaces, care to resend?

thanks,

greg k-h