Testing the arm chromebook config against the upstream
kernel produces a linker error for the zsmalloc module from
staging. The symbol flush_tlb_kernel_range is not available
there. Fix this by removing the reimplementation of
unmap_kernel_range in the zsmalloc module and using the
function directly.
Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/staging/zsmalloc/zsmalloc-main.c | 5 +----
mm/vmalloc.c | 1 +
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c
index e78d262..324e123 100644
--- a/drivers/staging/zsmalloc/zsmalloc-main.c
+++ b/drivers/staging/zsmalloc/zsmalloc-main.c
@@ -656,11 +656,8 @@ static inline void __zs_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);
- flush_tlb_kernel_range(addr, end);
+ unmap_kernel_range(addr, PAGE_SIZE * 2);
}
#else /* USE_PGTABLE_MAPPING */
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 0f751f2..f7cba11 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1266,6 +1266,7 @@ void unmap_kernel_range(unsigned long addr, unsigned long size)
vunmap_page_range(addr, end);
flush_tlb_kernel_range(addr, end);
}
+EXPORT_SYMBOL_GPL(unmap_kernel_range);
int map_vm_area(struct vm_struct *area, pgprot_t prot, struct page ***pages)
{
--
1.7.9.5
On Tue, Mar 26, 2013 at 11:33:52PM +0100, Joerg Roedel wrote:
> Testing the arm chromebook config against the upstream
> kernel produces a linker error for the zsmalloc module from
> staging. The symbol flush_tlb_kernel_range is not available
> there. Fix this by removing the reimplementation of
> unmap_kernel_range in the zsmalloc module and using the
> function directly.
>
> Signed-off-by: Joerg Roedel <[email protected]>
Why is this not an error for any other architecture? Why is arm
special?
> ---
> drivers/staging/zsmalloc/zsmalloc-main.c | 5 +----
> mm/vmalloc.c | 1 +
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c
> index e78d262..324e123 100644
> --- a/drivers/staging/zsmalloc/zsmalloc-main.c
> +++ b/drivers/staging/zsmalloc/zsmalloc-main.c
> @@ -656,11 +656,8 @@ static inline void __zs_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);
> - flush_tlb_kernel_range(addr, end);
> + unmap_kernel_range(addr, PAGE_SIZE * 2);
> }
>
> #else /* USE_PGTABLE_MAPPING */
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 0f751f2..f7cba11 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -1266,6 +1266,7 @@ void unmap_kernel_range(unsigned long addr, unsigned long size)
> vunmap_page_range(addr, end);
> flush_tlb_kernel_range(addr, end);
> }
> +EXPORT_SYMBOL_GPL(unmap_kernel_range);
I _really_ don't like adding core exports for a staging driver, there is
no guarantee that the staging driver will not just be deleted tomorrow.
So, if at all possible, I don't want to do this.
Perhaps, just make it so the staging code can't be a module?
greg k-h
On Tue, Mar 26, 2013 at 03:45:36PM -0700, Greg Kroah-Hartman wrote:
> On Tue, Mar 26, 2013 at 11:33:52PM +0100, Joerg Roedel wrote:
> > Testing the arm chromebook config against the upstream
> > kernel produces a linker error for the zsmalloc module from
> > staging. The symbol flush_tlb_kernel_range is not available
> > there. Fix this by removing the reimplementation of
> > unmap_kernel_range in the zsmalloc module and using the
> > function directly.
> >
> > Signed-off-by: Joerg Roedel <[email protected]>
>
> Why is this not an error for any other architecture? Why is arm
> special?
The version of the function __zs_unmap_object() which uses
flush_tlb_kernel_range() in the zsmalloc driver is only compiled in when
USE_PGTABLE_MAPPING is defined. And USE_PGTABLE_MAPPING is defined in
the same file only when CONFIG_ARM is defined. So this happens only on
ARM.
Regards,
Joerg
On Wed, Mar 27, 2013 at 12:03:59AM +0100, Joerg Roedel wrote:
> On Tue, Mar 26, 2013 at 03:45:36PM -0700, Greg Kroah-Hartman wrote:
> > On Tue, Mar 26, 2013 at 11:33:52PM +0100, Joerg Roedel wrote:
> > > Testing the arm chromebook config against the upstream
> > > kernel produces a linker error for the zsmalloc module from
> > > staging. The symbol flush_tlb_kernel_range is not available
> > > there. Fix this by removing the reimplementation of
> > > unmap_kernel_range in the zsmalloc module and using the
> > > function directly.
> > >
> > > Signed-off-by: Joerg Roedel <[email protected]>
> >
> > Why is this not an error for any other architecture? Why is arm
> > special?
>
> The version of the function __zs_unmap_object() which uses
> flush_tlb_kernel_range() in the zsmalloc driver is only compiled in when
> USE_PGTABLE_MAPPING is defined. And USE_PGTABLE_MAPPING is defined in
> the same file only when CONFIG_ARM is defined. So this happens only on
> ARM.
Then should I just mark the driver as broken on ARM?
Any reason for not including the driver authors on the Cc: for this
patch?
greg k-h
On Tue, Mar 26, 2013 at 04:09:10PM -0700, Greg Kroah-Hartman wrote:
> On Wed, Mar 27, 2013 at 12:03:59AM +0100, Joerg Roedel wrote:
> Then should I just mark the driver as broken on ARM?
Well, at least ARM on SMP, for !SMP the missing function is defined and
will be inlined.
> Any reason for not including the driver authors on the Cc: for this
> patch?
I wasn't sure who the driver author is in the long list of persons from
the get_maintainer script. So it was more or less lazyness :)
Should I try to get that patch through the driver authors instead of
you?
Joerg
On Wed, Mar 27, 2013 at 12:19:41AM +0100, Joerg Roedel wrote:
> On Tue, Mar 26, 2013 at 04:09:10PM -0700, Greg Kroah-Hartman wrote:
> > On Wed, Mar 27, 2013 at 12:03:59AM +0100, Joerg Roedel wrote:
>
> > Then should I just mark the driver as broken on ARM?
>
> Well, at least ARM on SMP, for !SMP the missing function is defined and
> will be inlined.
Then some Kconfig dependancies could be tweaked to try to make this work
properly, not building the driver where it will be broken.
> > Any reason for not including the driver authors on the Cc: for this
> > patch?
>
> I wasn't sure who the driver author is in the long list of persons from
> the get_maintainer script. So it was more or less lazyness :)
>
> Should I try to get that patch through the driver authors instead of
> you?
At least cc: them to have them weigh in on the issue, I know they are
trying to do something in this area, but I didn't think it was the same
as your patch.
thanks,
greg k-h
(Sending again with the zsmalloc driver writers on the list)
>From c8530ea21e65c21d27882fa334145c347b9a024b Mon Sep 17 00:00:00 2001
From: Joerg Roedel <[email protected]>
Date: Tue, 26 Mar 2013 23:24:22 +0100
Subject: [PATCH] staging: zsmalloc: Fix link error on ARM
Testing the arm chromebook config against the upstream
kernel produces a linker error for the zsmalloc module from
staging. The symbol flush_tlb_kernel_range is not available
there. Fix this by removing the reimplementation of
unmap_kernel_range in the zsmalloc module and using the
function directly.
Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/staging/zsmalloc/zsmalloc-main.c | 5 +----
mm/vmalloc.c | 1 +
2 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c
index e78d262..324e123 100644
--- a/drivers/staging/zsmalloc/zsmalloc-main.c
+++ b/drivers/staging/zsmalloc/zsmalloc-main.c
@@ -656,11 +656,8 @@ static inline void __zs_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);
- flush_tlb_kernel_range(addr, end);
+ unmap_kernel_range(addr, PAGE_SIZE * 2);
}
#else /* USE_PGTABLE_MAPPING */
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 0f751f2..f7cba11 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1266,6 +1266,7 @@ void unmap_kernel_range(unsigned long addr, unsigned long size)
vunmap_page_range(addr, end);
flush_tlb_kernel_range(addr, end);
}
+EXPORT_SYMBOL_GPL(unmap_kernel_range);
int map_vm_area(struct vm_struct *area, pgprot_t prot, struct page ***pages)
{
--
1.7.9.5
On Tue, Mar 26, 2013 at 11:33:52PM +0100, Joerg Roedel wrote:
> Testing the arm chromebook config against the upstream
> kernel produces a linker error for the zsmalloc module from
> staging. The symbol flush_tlb_kernel_range is not available
> there. Fix this by removing the reimplementation of
> unmap_kernel_range in the zsmalloc module and using the
> function directly.
>
> Signed-off-by: Joerg Roedel <[email protected]>
Oops, it was my fault. When I tested [1] on CONFIG_SMP machine on ARM,
it worked well. It means it's not always problem on every CONFIG_SMP
on ARM machine but some SMP machine define flush_tlb_kernel_range,
others don't.
At that time, Russell King already suggested same thing with your patch
and I meant to clean it up because the patch was already merged but I didn't.
Because we didn't catch up that it breaks build on some configuration
so I thought it's just clean up patch and Greg didn't want to accept
NOT-BUG patch of any z* family.
Now, it's BUG patch.
Remained problem is that Greg doesn't want to export core function for
staging driver and it's reasonable for me.
So my opinion is remove zsmalloc module build and could recover it with
making unmap_kernel_range exported function after we merged it into
mainline.
And please Cc stable.
[1] [99155188, zsmalloc: Fix TLB coherency and build problem]
> ---
> drivers/staging/zsmalloc/zsmalloc-main.c | 5 +----
> mm/vmalloc.c | 1 +
> 2 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/staging/zsmalloc/zsmalloc-main.c b/drivers/staging/zsmalloc/zsmalloc-main.c
> index e78d262..324e123 100644
> --- a/drivers/staging/zsmalloc/zsmalloc-main.c
> +++ b/drivers/staging/zsmalloc/zsmalloc-main.c
> @@ -656,11 +656,8 @@ static inline void __zs_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);
> - flush_tlb_kernel_range(addr, end);
> + unmap_kernel_range(addr, PAGE_SIZE * 2);
> }
>
> #else /* USE_PGTABLE_MAPPING */
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 0f751f2..f7cba11 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -1266,6 +1266,7 @@ void unmap_kernel_range(unsigned long addr, unsigned long size)
> vunmap_page_range(addr, end);
> flush_tlb_kernel_range(addr, end);
> }
> +EXPORT_SYMBOL_GPL(unmap_kernel_range);
>
> int map_vm_area(struct vm_struct *area, pgprot_t prot, struct page ***pages)
> {
> --
> 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>
--
Kind regards,
Minchan Kim
On Wed, Mar 27, 2013 at 09:05:52AM +0900, Minchan Kim wrote:
> Oops, it was my fault. When I tested [1] on CONFIG_SMP machine on ARM,
> it worked well. It means it's not always problem on every CONFIG_SMP
> on ARM machine but some SMP machine define flush_tlb_kernel_range,
> others don't.
>
> At that time, Russell King already suggested same thing with your patch
> and I meant to clean it up because the patch was already merged but I didn't.
> Because we didn't catch up that it breaks build on some configuration
> so I thought it's just clean up patch and Greg didn't want to accept
> NOT-BUG patch of any z* family.
>
> Now, it's BUG patch.
>
> Remained problem is that Greg doesn't want to export core function for
> staging driver and it's reasonable for me.
Okay, I see. So that is probably also the reason for the
reimplementation of unmap_kernel_range in the zsmalloc module :)
> So my opinion is remove zsmalloc module build and could recover it with
> making unmap_kernel_range exported function after we merged it into
> mainline.
Sounds reasonable, I update the patch to only allow zsmalloc to be
built-in. The benefit is that this still allows to use
unmap_kernel_range() in the driver.
Thanks,
Joerg
On Wed, Mar 27, 2013 at 09:05:52AM +0900, Minchan Kim wrote:
> And please Cc stable.
Okay, here it is. The result is compile-tested.
Changes since v1:
* Remove the module-export for unmap_kernel_range and make zsmalloc
built-in instead
Here is the patch:
>From 2b70502720b36909f9f39bdf27be21321a219c31 Mon Sep 17 00:00:00 2001
From: Joerg Roedel <[email protected]>
Date: Tue, 26 Mar 2013 23:24:22 +0100
Subject: [PATCH v2] staging: zsmalloc: Fix link error on ARM
Testing the arm chromebook config against the upstream
kernel produces a linker error for the zsmalloc module from
staging. The symbol flush_tlb_kernel_range is not available
there. Fix this by removing the reimplementation of
unmap_kernel_range in the zsmalloc module and using the
function directly. The unmap_kernel_range function is not
usable by modules, so also disallow building the driver as a
module for now.
Cc: [email protected]
Signed-off-by: Joerg Roedel <[email protected]>
---
drivers/staging/zsmalloc/Kconfig | 2 +-
drivers/staging/zsmalloc/zsmalloc-main.c | 5 +----
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/staging/zsmalloc/Kconfig b/drivers/staging/zsmalloc/Kconfig
index 9084565..7fab032 100644
--- a/drivers/staging/zsmalloc/Kconfig
+++ b/drivers/staging/zsmalloc/Kconfig
@@ -1,5 +1,5 @@
config ZSMALLOC
- tristate "Memory allocator for compressed pages"
+ bool "Memory allocator for compressed pages"
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 e78d262..324e123 100644
--- a/drivers/staging/zsmalloc/zsmalloc-main.c
+++ b/drivers/staging/zsmalloc/zsmalloc-main.c
@@ -656,11 +656,8 @@ static inline void __zs_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);
- flush_tlb_kernel_range(addr, end);
+ unmap_kernel_range(addr, PAGE_SIZE * 2);
}
#else /* USE_PGTABLE_MAPPING */
--
1.7.9.5
On Wed, Mar 27, 2013 at 01:43:14AM +0100, Joerg Roedel wrote:
> On Wed, Mar 27, 2013 at 09:05:52AM +0900, Minchan Kim wrote:
> > And please Cc stable.
>
> Okay, here it is. The result is compile-tested.
>
> Changes since v1:
>
> * Remove the module-export for unmap_kernel_range and make zsmalloc
> built-in instead
>
> Here is the patch:
>
> >From 2b70502720b36909f9f39bdf27be21321a219c31 Mon Sep 17 00:00:00 2001
> From: Joerg Roedel <[email protected]>
> Date: Tue, 26 Mar 2013 23:24:22 +0100
> Subject: [PATCH v2] staging: zsmalloc: Fix link error on ARM
>
> Testing the arm chromebook config against the upstream
> kernel produces a linker error for the zsmalloc module from
> staging. The symbol flush_tlb_kernel_range is not available
> there. Fix this by removing the reimplementation of
> unmap_kernel_range in the zsmalloc module and using the
> function directly. The unmap_kernel_range function is not
> usable by modules, so also disallow building the driver as a
> module for now.
>
> Cc: [email protected]
> Signed-off-by: Joerg Roedel <[email protected]>
Acked-by: Minchan Kim <[email protected]>
--
Kind regards,
Minchan Kim