[PATCH] sh: Add EXPORT_SYMBOL(min_low_pfn) and EXPORT_SYMBOL(max_low_pfn) to sh_ksyms_32.c

[PATCH] sh: Add EXPORT_SYMBOL(min_low_pfn) and EXPORT_SYMBOL(max_low_pfn) to sh_ksyms_32.c

[Nobuhiro Iwamatsu]

Min_low_pfn and max_low_pfn were used in pfn_valid macro if defined
CONFIG_FLATMEM. When the functions that use the pfn_valid is used in driver
module, max_low_pfn and min_low_pfn is to undefined, and fail to build.

----
ERROR: "min_low_pfn" [drivers/block/aoe/aoe.ko] undefined!
ERROR: "max_low_pfn" [drivers/block/aoe/aoe.ko] undefined!
make[2]: *** [__modpost] Error 1
make[1]: *** [modules] Error 2
----

This patch fix this problem.

Signed-off-by: Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com>
---
 arch/sh/kernel/sh_ksyms_32.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/sh/kernel/sh_ksyms_32.c b/arch/sh/kernel/sh_ksyms_32.c
index 2a0a596..a7d3b1b 100644
--- a/arch/sh/kernel/sh_ksyms_32.c
+++ b/arch/sh/kernel/sh_ksyms_32.c
@@ -20,6 +20,11 @@ EXPORT_SYMBOL(csum_partial_copy_generic);
 EXPORT_SYMBOL(copy_page);
 EXPORT_SYMBOL(__clear_user);
 EXPORT_SYMBOL(empty_zero_page);
+#ifdef CONFIG_FLATMEM
+/* need in pfn_valid macro */
+EXPORT_SYMBOL(min_low_pfn);
+EXPORT_SYMBOL(max_low_pfn);
+#endif
 
 #define DECLARE_EXPORT(name)		\
 	extern void name(void);EXPORT_SYMBOL(name)
-- 
1.8.5.1

Re: [PATCH] sh: Add EXPORT_SYMBOL(min_low_pfn) and EXPORT_SYMBOL(max_low_pfn) to sh_ksyms_32.c

[Kuninori Morimoto]

Hi

> Min_low_pfn and max_low_pfn were used in pfn_valid macro if defined
> CONFIG_FLATMEM. When the functions that use the pfn_valid is used in driver
> module, max_low_pfn and min_low_pfn is to undefined, and fail to build.
> 
> ----
> ERROR: "min_low_pfn" [drivers/block/aoe/aoe.ko] undefined!
> ERROR: "max_low_pfn" [drivers/block/aoe/aoe.ko] undefined!
> make[2]: *** [__modpost] Error 1
> make[1]: *** [modules] Error 2
> ----
(snip)
> +#ifdef CONFIG_FLATMEM
> +/* need in pfn_valid macro */
> +EXPORT_SYMBOL(min_low_pfn);
> +EXPORT_SYMBOL(max_low_pfn);
> +#endif

I'm not sure detail of min/max_low_pfn,
but these exist under ${LINUX}/mm/[no]bootmem.c ?

EXPORT_SYMBOL() on arch/sh/kernel/sh_ksyms_32.c
seems strange for me.

Best regards
---
Kuninori Morimoto

Re: [PATCH] sh: Add EXPORT_SYMBOL(min_low_pfn) and EXPORT_SYMBOL(max_low_pfn) to sh_ksyms_32.c

[Nobuhiro Iwamatsu]

Hi,

(2013/12/13 10:12), Kuninori Morimoto wrote:
> 
> Hi
> 
>> Min_low_pfn and max_low_pfn were used in pfn_valid macro if defined
>> CONFIG_FLATMEM. When the functions that use the pfn_valid is used in driver
>> module, max_low_pfn and min_low_pfn is to undefined, and fail to build.
>>
>> ----
>> ERROR: "min_low_pfn" [drivers/block/aoe/aoe.ko] undefined!
>> ERROR: "max_low_pfn" [drivers/block/aoe/aoe.ko] undefined!
>> make[2]: *** [__modpost] Error 1
>> make[1]: *** [modules] Error 2
>> ----
> (snip)
>> +#ifdef CONFIG_FLATMEM
>> +/* need in pfn_valid macro */
>> +EXPORT_SYMBOL(min_low_pfn);
>> +EXPORT_SYMBOL(max_low_pfn);
>> +#endif
> 
> I'm not sure detail of min/max_low_pfn,
> but these exist under ${LINUX}/mm/[no]bootmem.c ?
> 

Yes, I know.

> EXPORT_SYMBOL() on arch/sh/kernel/sh_ksyms_32.c
> seems strange for me.

Handling of these depends on CPUs.
And in SH32, it may be referred to from modules.
Therefore, I have feeling that it is not amusing that sh_ksyms_32.c defines these using EXPORT_SYMBOL.

Would you explain the reason that you thought to be strange?

> 
> Best regards
> ---
> Kuninori Morimoto

Best regards,
  Nobuhiro

Re: [PATCH] sh: Add EXPORT_SYMBOL(min_low_pfn) and EXPORT_SYMBOL(max_low_pfn) to sh_ksyms_32.c

[Kuninori Morimoto]

Hi


> > EXPORT_SYMBOL() on arch/sh/kernel/sh_ksyms_32.c
> > seems strange for me.
> 
> Handling of these depends on CPUs.
> And in SH32, it may be referred to from modules.
> Therefore, I have feeling that it is not amusing that sh_ksyms_32.c defines these using EXPORT_SYMBOL.
> 
> Would you explain the reason that you thought to be strange?

I think EXPORT_SYMBOL() should be called
from ${LINUX}/mm/[no]bootmem.c, not sh_ksyms_32.c

Best regards
---
Kuninori Morimoto

Re: [PATCH] sh: Add EXPORT_SYMBOL(min_low_pfn) and EXPORT_SYMBOL(max_low_pfn) to sh_ksyms_32.c

[Nobuhiro Iwamatsu]

Hi,

(2013/12/13 12:17), Kuninori Morimoto wrote:
>
> Hi
>
>
>>> EXPORT_SYMBOL() on arch/sh/kernel/sh_ksyms_32.c
>>> seems strange for me.
>>
>> Handling of these depends on CPUs.
>> And in SH32, it may be referred to from modules.
>> Therefore, I have feeling that it is not amusing that sh_ksyms_32.c defines these using EXPORT_SYMBOL.
>>
>> Would you explain the reason that you thought to be strange?
>
> I think EXPORT_SYMBOL() should be called
> from ${LINUX}/mm/[no]bootmem.c, not sh_ksyms_32.c

The usage of max_low_pfn and min_low_pfn depend on the architecture as I have described above.
You do not need to set as EXPORT_SYMBOL these all architectures.

Best regards,
   Nobuhiro

Re: [PATCH] sh: Add EXPORT_SYMBOL(min_low_pfn) and EXPORT_SYMBOL(max_low_pfn) to sh_ksyms_32.c

[Andrew Morton]

On Fri, 13 Dec 2013 09:10:38 +0900 Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com> wrote:

> Min_low_pfn and max_low_pfn were used in pfn_valid macro if defined
> CONFIG_FLATMEM. When the functions that use the pfn_valid is used in driver
> module, max_low_pfn and min_low_pfn is to undefined, and fail to build.
> 
> ----
> ERROR: "min_low_pfn" [drivers/block/aoe/aoe.ko] undefined!
> ERROR: "max_low_pfn" [drivers/block/aoe/aoe.ko] undefined!

This could be a sign that the aoe driver is doing something which it
shouldn't be doing.

z:/usr/src/25> grep pfn drivers/block/aoe/*.[ch]
z:/usr/src/25> 

Confused.  Can you please work out precisely where in AOE this
reference is occurring?

Re: [PATCH] sh: Add EXPORT_SYMBOL(min_low_pfn) and EXPORT_SYMBOL(max_low_pfn) to sh_ksyms_32.c

[Geert Uytterhoeven]

On Fri, Dec 13, 2013 at 8:56 AM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Fri, 13 Dec 2013 09:10:38 +0900 Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com> wrote:
>
>> Min_low_pfn and max_low_pfn were used in pfn_valid macro if defined
>> CONFIG_FLATMEM. When the functions that use the pfn_valid is used in driver
>> module, max_low_pfn and min_low_pfn is to undefined, and fail to build.
>>
>> ----
>> ERROR: "min_low_pfn" [drivers/block/aoe/aoe.ko] undefined!
>> ERROR: "max_low_pfn" [drivers/block/aoe/aoe.ko] undefined!
>
> This could be a sign that the aoe driver is doing something which it
> shouldn't be doing.
>
> z:/usr/src/25> grep pfn drivers/block/aoe/*.[ch]
> z:/usr/src/25>
>
> Confused.  Can you please work out precisely where in AOE this
> reference is occurring?

From my patch for mips, it's due to virt_addr_valid(), which calls pfn_valid()
on most architectures, cfr.
http://marc.info/?l=linux-kernel&m\x135672726823050&w=1.

Note that Ralf decided to fix it differently, by uninlining virt_addr_valid:

commit d3ce88431892b703b04769566338a89eda6b0477
Author: Ralf Baechle <ralf@linux-mips.org>
Date:   Fri Dec 28 15:34:40 2012 +0100

    MIPS: Fix modpost error in modules attepting to use virt_addr_valid().

    ERROR: "min_low_pfn" [drivers/block/aoe/aoe.ko] undefined!

    Fixed by moving the implementation of virt_addr_valid() into the kernel
    proper and exporting it which removes the pains of an inline or macro
    implementation.

    Signed-off-by: Ralf Baechle <ralf@linux-mips.org>

diff --git a/arch/mips/include/asm/page.h b/arch/mips/include/asm/page.h
index bf84e48ad669..dbaec94046da 100644
--- a/arch/mips/include/asm/page.h
+++ b/arch/mips/include/asm/page.h
@@ -198,7 +198,10 @@ typedef struct { unsigned long pgprot; } pgprot_t;
 #endif

 #define virt_to_page(kaddr)    pfn_to_page(PFN_DOWN(virt_to_phys(kaddr)))
-#define virt_addr_valid(kaddr) pfn_valid(PFN_DOWN(virt_to_phys(kaddr)))
+
+extern int __virt_addr_valid(const volatile void *kaddr);
+#define virt_addr_valid(kaddr)                                         \
+       __virt_addr_valid((const volatile void *) (kaddr))

 #define VM_DATA_DEFAULT_FLAGS  (VM_READ | VM_WRITE | VM_EXEC | \
                                 VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
diff --git a/arch/mips/mm/ioremap.c b/arch/mips/mm/ioremap.c
index cacfd31e8ec9..7657fd21cd3f 100644
--- a/arch/mips/mm/ioremap.c
+++ b/arch/mips/mm/ioremap.c
@@ -190,3 +190,9 @@ void __iounmap(const volatile void __iomem *addr)

 EXPORT_SYMBOL(__ioremap);
 EXPORT_SYMBOL(__iounmap);
+
+int __virt_addr_valid(const volatile void *kaddr)
+{
+       return pfn_valid(PFN_DOWN(virt_to_phys(kaddr)));
+}
+EXPORT_SYMBOL_GPL(__virt_addr_valid);

BTW, ia64 exports both min_low_pfn and max_low_pfn,
metag exports min_low_pfn.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] sh: Add EXPORT_SYMBOL(min_low_pfn) and EXPORT_SYMBOL(max_low_pfn) to sh_ksyms_32.c

[Andrew Morton]

On Fri, 13 Dec 2013 09:51:00 +0100 Geert Uytterhoeven <geert@linux-m68k.org> wrote:

> Note that Ralf decided to fix it differently, by uninlining virt_addr_valid:

geeze, what a mess this is.

I suppose that the proposed sh patch is reasonable - it's the sh
implementation of virt_addr_valid() whcih chooses to use these symbols
so sh should export them to support that implementation.

Re: [PATCH] sh: Add EXPORT_SYMBOL(min_low_pfn) and EXPORT_SYMBOL(max_low_pfn) to sh_ksyms_32.c

[Nobuhiro Iwamatsu]

2013/12/13 Andrew Morton <akpm@linux-foundation.org>:
> On Fri, 13 Dec 2013 09:10:38 +0900 Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@renesas.com> wrote:
>
>> Min_low_pfn and max_low_pfn were used in pfn_valid macro if defined
>> CONFIG_FLATMEM. When the functions that use the pfn_valid is used in driver
>> module, max_low_pfn and min_low_pfn is to undefined, and fail to build.
>>
>> ----
>> ERROR: "min_low_pfn" [drivers/block/aoe/aoe.ko] undefined!
>> ERROR: "max_low_pfn" [drivers/block/aoe/aoe.ko] undefined!
>
> This could be a sign that the aoe driver is doing something which it
> shouldn't be doing.
>
> z:/usr/src/25> grep pfn drivers/block/aoe/*.[ch]
> z:/usr/src/25>
>
> Confused.  Can you please work out precisely where in AOE this
> reference is occurring?

min_low_pfn and max_low_pfn are referred from pfn_valid() in
./arch/sh/include/asm/page.h.

-- 
Nobuhiro Iwamatsu

Re: [PATCH] sh: Add EXPORT_SYMBOL(min_low_pfn) and EXPORT_SYMBOL(max_low_pfn) to sh_ksyms_32.c

[Nobuhiro Iwamatsu]

2013/12/14 Andrew Morton <akpm@linux-foundation.org>:
> On Fri, 13 Dec 2013 09:51:00 +0100 Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>
>> Note that Ralf decided to fix it differently, by uninlining virt_addr_valid:
>
> geeze, what a mess this is.
>
> I suppose that the proposed sh patch is reasonable - it's the sh
> implementation of virt_addr_valid() whcih chooses to use these symbols
> so sh should export them to support that implementation.

Andrew, thanks for your comment.

Geert, do you think about this?

Best regards,
  Nobuhiro
-- 
Nobuhiro Iwamatsu

Re: [PATCH] sh: Add EXPORT_SYMBOL(min_low_pfn) and EXPORT_SYMBOL(max_low_pfn) to sh_ksyms_32.c

[Geert Uytterhoeven]

On Mon, Dec 16, 2013 at 2:28 AM, Nobuhiro Iwamatsu
<nobuhiro.iwamatsu.yj@renesas.com> wrote:
> 2013/12/14 Andrew Morton <akpm@linux-foundation.org>:
>> On Fri, 13 Dec 2013 09:51:00 +0100 Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>
>>> Note that Ralf decided to fix it differently, by uninlining virt_addr_valid:
>>
>> geeze, what a mess this is.
>>
>> I suppose that the proposed sh patch is reasonable - it's the sh
>> implementation of virt_addr_valid() whcih chooses to use these symbols
>> so sh should export them to support that implementation.
>
> Andrew, thanks for your comment.
>
> Geert, do you think about this?

Both are valid solutions: either you fix it at the high level, like MIPS
does, or at the low level. It's just about what you want to expose, and
if you want to risk people touching too much innards.

Anyway:

Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH] sh: Add EXPORT_SYMBOL(min_low_pfn) and EXPORT_SYMBOL(max_low_pfn) to sh_ksyms_32.c

[Nobuhiro Iwamatsu]

2013/12/16 Geert Uytterhoeven <geert@linux-m68k.org>:
> On Mon, Dec 16, 2013 at 2:28 AM, Nobuhiro Iwamatsu
> <nobuhiro.iwamatsu.yj@renesas.com> wrote:
>> 2013/12/14 Andrew Morton <akpm@linux-foundation.org>:
>>> On Fri, 13 Dec 2013 09:51:00 +0100 Geert Uytterhoeven <geert@linux-m68k.org> wrote:
>>>
>>>> Note that Ralf decided to fix it differently, by uninlining virt_addr_valid:
>>>
>>> geeze, what a mess this is.
>>>
>>> I suppose that the proposed sh patch is reasonable - it's the sh
>>> implementation of virt_addr_valid() whcih chooses to use these symbols
>>> so sh should export them to support that implementation.
>>
>> Andrew, thanks for your comment.
>>
>> Geert, do you think about this?
>
> Both are valid solutions: either you fix it at the high level, like MIPS
> does, or at the low level. It's just about what you want to expose, and
> if you want to risk people touching too much innards.
>

I see.  I understood

> Anyway:
>
> Acked-by: Geert Uytterhoeven <geert@linux-m68k.org>

Thanks.

>
> Gr{oetje,eeting}s,
>
>                         Geert
>

Best regards,
  Nobuhiro