[PATCH] staging: zsmalloc: Fix link error on ARM

[PATCH] staging: zsmalloc: Fix link error on ARM

[Joerg Roedel]

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 <joro@8bytes.org>
---
 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 majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

Re: [PATCH] staging: zsmalloc: Fix link error on ARM

[Greg Kroah-Hartman]

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 <joro@8bytes.org>

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

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

Re: [PATCH] staging: zsmalloc: Fix link error on ARM

[Joerg Roedel]

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 <joro@8bytes.org>
> 
> 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


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

Re: [PATCH] staging: zsmalloc: Fix link error on ARM

[Greg Kroah-Hartman]

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 <joro@8bytes.org>
> > 
> > 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

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

Re: [PATCH] staging: zsmalloc: Fix link error on ARM

[Joerg Roedel]

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


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

Re: [PATCH] staging: zsmalloc: Fix link error on ARM

[Greg Kroah-Hartman]

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

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

Re: [PATCH] staging: zsmalloc: Fix link error on ARM

[Joerg Roedel]

(Sending again with the zsmalloc driver writers on the list)

Re: [PATCH] staging: zsmalloc: Fix link error on ARM

[Minchan Kim]

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 <joro@8bytes.org>


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

-- 
Kind regards,
Minchan Kim

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

Re: [PATCH] staging: zsmalloc: Fix link error on ARM

[Joerg Roedel]

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


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

Re: [PATCH] staging: zsmalloc: Fix link error on ARM

[Joerg Roedel]

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:

Re: [PATCH] staging: zsmalloc: Fix link error on ARM

[Minchan Kim]

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 <joro@8bytes.org>
> 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: stable@vger.kernel.org
> Signed-off-by: Joerg Roedel <joro@8bytes.org>
Acked-by: Minchan Kim <minchan@kernel.org>

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