[PATCH] asm-generic/io.h: remove unnecessary include of linux/vmalloc.h

[PATCH] asm-generic/io.h: remove unnecessary include of linux/vmalloc.h

[Tobias Klauser]

Including linux/vmalloc.h in asm-generic/io.h isn't necessary since none
of the definitions are used in the header itself. Remove the include in
order to avoid potential header dependency problems if other headers
rely on implict inclusion of linux/vmalloc.h which means that changes
there could break unrelated parts.

Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
 include/asm-generic/io.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
index b4531e3b2120..d2d3bd163f5f 100644
--- a/include/asm-generic/io.h
+++ b/include/asm-generic/io.h
@@ -764,7 +764,6 @@ static inline void iowrite64_rep(volatile void __iomem *addr,
 
 #ifdef __KERNEL__
 
-#include <linux/vmalloc.h>
 #define __io_virt(x) ((void __force *)(x))
 
 #ifndef CONFIG_GENERIC_IOMAP
-- 
2.13.0

Re: [PATCH] asm-generic/io.h: remove unnecessary include of linux/vmalloc.h

[Arnd Bergmann]

On Tue, Sep 5, 2017 at 1:27 PM, Tobias Klauser <tklauser@distanz.ch> wrote:
> Including linux/vmalloc.h in asm-generic/io.h isn't necessary since none
> of the definitions are used in the header itself. Remove the include in
> order to avoid potential header dependency problems if other headers
> rely on implict inclusion of linux/vmalloc.h which means that changes
> there could break unrelated parts.
>
> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> ---
>  include/asm-generic/io.h | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> index b4531e3b2120..d2d3bd163f5f 100644
> --- a/include/asm-generic/io.h
> +++ b/include/asm-generic/io.h
> @@ -764,7 +764,6 @@ static inline void iowrite64_rep(volatile void __iomem *addr,
>
>  #ifdef __KERNEL__
>
> -#include <linux/vmalloc.h>
>  #define __io_virt(x) ((void __force *)(x))
>
>  #ifndef CONFIG_GENERIC_IOMAP

This seems like a good idea in principle, but I think it needs to be tested
well before we apply it, to avoid breaking random drivers that forgot to
add their own includes of that header.

I've added your patch to my testing queue, but not to the asm-generic
tree now. We should see if it leads to any randconfig build regressions
on the architectures I normally test.

Did you run into a specific problem with the #include, or did it just occur
to you that it might help in general?

      Arnd

Re: [PATCH] asm-generic/io.h: remove unnecessary include of linux/vmalloc.h

[Tobias Klauser]

On 2017-09-05 at 17:11:50 +0200, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tue, Sep 5, 2017 at 1:27 PM, Tobias Klauser <tklauser@distanz.ch> wrote:
> > Including linux/vmalloc.h in asm-generic/io.h isn't necessary since none
> > of the definitions are used in the header itself. Remove the include in
> > order to avoid potential header dependency problems if other headers
> > rely on implict inclusion of linux/vmalloc.h which means that changes
> > there could break unrelated parts.
> >
> > Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> > ---
> >  include/asm-generic/io.h | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> > index b4531e3b2120..d2d3bd163f5f 100644
> > --- a/include/asm-generic/io.h
> > +++ b/include/asm-generic/io.h
> > @@ -764,7 +764,6 @@ static inline void iowrite64_rep(volatile void __iomem *addr,
> >
> >  #ifdef __KERNEL__
> >
> > -#include <linux/vmalloc.h>
> >  #define __io_virt(x) ((void __force *)(x))
> >
> >  #ifndef CONFIG_GENERIC_IOMAP
> 
> This seems like a good idea in principle, but I think it needs to be tested
> well before we apply it, to avoid breaking random drivers that forgot to
> add their own includes of that header.

Yes, this certainly needs extensive testing. I already did several
randconfig builds on multiple platforms locally. Also, I sent the same
patch a while ago to LKML already as a fix (which wasn't sufficient) [1]
in order to get the kbuild test bot to test it ;)

  [1] https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1392578.html

And indeed, the kbuild test bot failed on multiple drivers as a result
of me submitting this patch back then and I successively sent patches to
fix the fallout. Now all of them are merged.

> I've added your patch to my testing queue, but not to the asm-generic
> tree now. We should see if it leads to any randconfig build regressions
> on the architectures I normally test.

Thanks!

> Did you run into a specific problem with the #include, or did it just occur
> to you that it might help in general?

See above. Sorry, I should have mentioned it in the original patch
submission but I forgot about the full history and only tried to
reconstruct it now that you asked ;)

Tobias

Re: [PATCH] asm-generic/io.h: remove unnecessary include of linux/vmalloc.h

[Arnd Bergmann]

On Tue, Sep 5, 2017 at 5:21 PM, Tobias Klauser <tklauser@distanz.ch> wrote:
> On 2017-09-05 at 17:11:50 +0200, Arnd Bergmann <arnd@arndb.de> wrote:
>> On Tue, Sep 5, 2017 at 1:27 PM, Tobias Klauser <tklauser@distanz.ch> wrote:
>> > Including linux/vmalloc.h in asm-generic/io.h isn't necessary since none
>> > of the definitions are used in the header itself. Remove the include in
>> > order to avoid potential header dependency problems if other headers
>> > rely on implict inclusion of linux/vmalloc.h which means that changes
>> > there could break unrelated parts.
>> >
>> > Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
>> > ---
>> >  include/asm-generic/io.h | 1 -
>> >  1 file changed, 1 deletion(-)
>> >
>> > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
>> > index b4531e3b2120..d2d3bd163f5f 100644
>> > --- a/include/asm-generic/io.h
>> > +++ b/include/asm-generic/io.h
>> > @@ -764,7 +764,6 @@ static inline void iowrite64_rep(volatile void __iomem *addr,
>> >
>> >  #ifdef __KERNEL__
>> >
>> > -#include <linux/vmalloc.h>
>> >  #define __io_virt(x) ((void __force *)(x))
>> >
>> >  #ifndef CONFIG_GENERIC_IOMAP
>>
>> This seems like a good idea in principle, but I think it needs to be tested
>> well before we apply it, to avoid breaking random drivers that forgot to
>> add their own includes of that header.
>
> Yes, this certainly needs extensive testing. I already did several
> randconfig builds on multiple platforms locally. Also, I sent the same
> patch a while ago to LKML already as a fix (which wasn't sufficient) [1]
> in order to get the kbuild test bot to test it ;)
>
>   [1] https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1392578.html
>
> And indeed, the kbuild test bot failed on multiple drivers as a result
> of me submitting this patch back then and I successively sent patches to
> fix the fallout. Now all of them are merged.

I've done around 200 randconfig builds (x86, arm and arm64) now and
found two new issues so far, there will probably be a couple more, but
I can take care of the ones in arch/arm/mach-*/.

       Arnd

commit ac5fa2421993e6372214b95b533592a8be65fb49
Author: Arnd Bergmann <arnd@arndb.de>
Date:   Tue Sep 5 21:13:58 2017 +0200

    ARM: add missing include statements

    After removing linux/vmalloc.h from asm-generic/io.h, a few other
    files fail:

    arch/arm/mach-shmobile/pm-r8a7779.c:35:13: error: expected '=',
',', ';', 'asm' or '__attribute__' before 'r8a7779_pm_init'
     void __init r8a7779_pm_init(void)
                 ^~~~~~~~~~~~~~~
    scripts/Makefile.build:311: recipe for target
'arch/arm/mach-shmobile/pm-r8a7779.o' failed
    In file included from arch/arm/mach-imx/devices/../mx3x.h:146:0,
                     from arch/arm/mach-imx/devices/../hardware.h:109,
                     from arch/arm/mach-imx/devices/platform-flexcan.c:8:
    arch/arm/include/asm/irq.h:39:50: error: unknown type name 'cpumask_t'

    Signed-off-by: Arnd Bergmann <arnd@arndb.de>

diff --git a/arch/arm/include/asm/irq.h b/arch/arm/include/asm/irq.h
index e53638c8ed8a..6095a1649865 100644
--- a/arch/arm/include/asm/irq.h
+++ b/arch/arm/include/asm/irq.h
@@ -22,6 +22,8 @@
 #endif

 #ifndef __ASSEMBLY__
+#include <linux/cpumask.h>
+
 struct irqaction;
 struct pt_regs;
 extern void migrate_irqs(void);
diff --git a/arch/arm/mach-shmobile/pm-r8a7779.c
b/arch/arm/mach-shmobile/pm-r8a7779.c
index 5c9a93f5e650..a891101e0a85 100644
--- a/arch/arm/mach-shmobile/pm-r8a7779.c
+++ b/arch/arm/mach-shmobile/pm-r8a7779.c
@@ -10,7 +10,7 @@
  */

 #include <linux/soc/renesas/rcar-sysc.h>
-
+#include <linux/init.h>
 #include <asm/io.h>

 #include "r8a7779.h"

Re: [PATCH] asm-generic/io.h: remove unnecessary include of linux/vmalloc.h

[Tobias Klauser]

Hi Arnd

On 2017-09-05 at 17:11:50 +0200, Arnd Bergmann <arnd@arndb.de> wrote:
> On Tue, Sep 5, 2017 at 1:27 PM, Tobias Klauser <tklauser@distanz.ch> wrote:
> > Including linux/vmalloc.h in asm-generic/io.h isn't necessary since none
> > of the definitions are used in the header itself. Remove the include in
> > order to avoid potential header dependency problems if other headers
> > rely on implict inclusion of linux/vmalloc.h which means that changes
> > there could break unrelated parts.
> >
> > Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> > ---
> >  include/asm-generic/io.h | 1 -
> >  1 file changed, 1 deletion(-)
> >
> > diff --git a/include/asm-generic/io.h b/include/asm-generic/io.h
> > index b4531e3b2120..d2d3bd163f5f 100644
> > --- a/include/asm-generic/io.h
> > +++ b/include/asm-generic/io.h
> > @@ -764,7 +764,6 @@ static inline void iowrite64_rep(volatile void __iomem *addr,
> >
> >  #ifdef __KERNEL__
> >
> > -#include <linux/vmalloc.h>
> >  #define __io_virt(x) ((void __force *)(x))
> >
> >  #ifndef CONFIG_GENERIC_IOMAP
> 
> This seems like a good idea in principle, but I think it needs to be tested
> well before we apply it, to avoid breaking random drivers that forgot to
> add their own includes of that header.
> 
> I've added your patch to my testing queue, but not to the asm-generic
> tree now. We should see if it leads to any randconfig build regressions
> on the architectures I normally test.

Did you see any other breakages caused by this patch in your testing
queue besides the ones you alredy reported/fixed? Would this be
something appropriate to submit during the next merge window? Or at
least to be included in linux-next so it gets some more coverage?

Thanks
Tobias

Re: [PATCH] asm-generic/io.h: remove unnecessary include of linux/vmalloc.h

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2335 bytes --]

Hi Tobias,

[auto build test WARNING on v4.13]
[also build test WARNING on next-20170907]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Tobias-Klauser/asm-generic-io-h-remove-unnecessary-include-of-linux-vmalloc-h/20170908-012111
config: openrisc-or1ksim_defconfig (attached as .config)
compiler: or1k-linux-gcc (GCC) 5.4.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=openrisc 

All warnings (new ones prefixed by >>):

   In file included from arch/openrisc/include/asm/io.h:33:0,
                    from include/linux/io.h:25,
                    from drivers/tty/serial/earlycon.c:19:
>> arch/openrisc/include/asm/pgtable.h:424:2: warning: 'struct vm_area_struct' declared inside parameter list
     unsigned long address, pte_t *pte)
     ^
>> arch/openrisc/include/asm/pgtable.h:424:2: warning: its scope is only this definition or declaration, which is probably not what you want

vim +424 arch/openrisc/include/asm/pgtable.h

61e85e36 Jonas Bonn 2011-06-04  416  
61e85e36 Jonas Bonn 2011-06-04  417  /*
61e85e36 Jonas Bonn 2011-06-04  418   * or32 doesn't have any external MMU info: the kernel page
61e85e36 Jonas Bonn 2011-06-04  419   * tables contain all the necessary information.
61e85e36 Jonas Bonn 2011-06-04  420   *
61e85e36 Jonas Bonn 2011-06-04  421   * Actually I am not sure on what this could be used for.
61e85e36 Jonas Bonn 2011-06-04  422   */
61e85e36 Jonas Bonn 2011-06-04  423  static inline void update_mmu_cache(struct vm_area_struct *vma,
61e85e36 Jonas Bonn 2011-06-04 @424  	unsigned long address, pte_t *pte)
61e85e36 Jonas Bonn 2011-06-04  425  {
61e85e36 Jonas Bonn 2011-06-04  426  }
61e85e36 Jonas Bonn 2011-06-04  427  

:::::: The code at line 424 was first introduced by commit
:::::: 61e85e367535a7b6385b404bef93928768140f96 OpenRISC: Memory management

:::::: TO: Jonas Bonn <jonas@southpole.se>
:::::: CC: Jonas Bonn <jonas@southpole.se>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 7592 bytes --]

Re: [PATCH] asm-generic/io.h: remove unnecessary include of linux/vmalloc.h

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2422 bytes --]

Hi Tobias,

[auto build test ERROR on v4.13]
[also build test ERROR on next-20170907]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Tobias-Klauser/asm-generic-io-h-remove-unnecessary-include-of-linux-vmalloc-h/20170908-012111
config: arm-shmobile_defconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> arch/arm/mach-shmobile/pm-r8a7779.c:24:20: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'r8a7779_sysc_init'
    static void __init r8a7779_sysc_init(void)
                       ^~~~~~~~~~~~~~~~~
>> arch/arm/mach-shmobile/pm-r8a7779.c:35:13: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'r8a7779_pm_init'
    void __init r8a7779_pm_init(void)
                ^~~~~~~~~~~~~~~

vim +24 arch/arm/mach-shmobile/pm-r8a7779.c

f40aaf6da Magnus Damm        2012-01-10  23  
a662c0826 Magnus Damm        2012-01-10 @24  static void __init r8a7779_sysc_init(void)
a662c0826 Magnus Damm        2012-01-10  25  {
053239987 Geert Uytterhoeven 2016-06-28  26  	rcar_sysc_init(0xffd85000, 0x0131000e);
a662c0826 Magnus Damm        2012-01-10  27  }
a662c0826 Magnus Damm        2012-01-10  28  
f40aaf6da Magnus Damm        2012-01-10  29  #else /* CONFIG_PM || CONFIG_SMP */
f40aaf6da Magnus Damm        2012-01-10  30  
f40aaf6da Magnus Damm        2012-01-10  31  static inline void r8a7779_sysc_init(void) {}
f40aaf6da Magnus Damm        2012-01-10  32  
f40aaf6da Magnus Damm        2012-01-10  33  #endif /* CONFIG_PM || CONFIG_SMP */
f40aaf6da Magnus Damm        2012-01-10  34  
a662c0826 Magnus Damm        2012-01-10 @35  void __init r8a7779_pm_init(void)

:::::: The code at line 24 was first introduced by commit
:::::: a662c08260fac126059a148cbd1061e71e806b4a ARM: mach-shmobile: r8a7779 power domain support V2

:::::: TO: Magnus Damm <damm@opensource.se>
:::::: CC: Paul Mundt <lethal@linux-sh.org>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 27647 bytes --]

Re: [PATCH] asm-generic/io.h: remove unnecessary include of linux/vmalloc.h

[Tobias Klauser]

On 2017-09-07 at 21:36:45 +0200, kbuild test robot <lkp@intel.com> wrote:
> Hi Tobias,
> 
> [auto build test ERROR on v4.13]
> [also build test ERROR on next-20170907]
> [if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
> 
> url:    https://github.com/0day-ci/linux/commits/Tobias-Klauser/asm-generic-io-h-remove-unnecessary-include-of-linux-vmalloc-h/20170908-012111
> config: arm-shmobile_defconfig (attached as .config)
> compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
> reproduce:
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # save the attached .config to linux build tree
>         make.cross ARCH=arm 
> 
> All errors (new ones prefixed by >>):
> 
> >> arch/arm/mach-shmobile/pm-r8a7779.c:24:20: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'r8a7779_sysc_init'
>     static void __init r8a7779_sysc_init(void)
>                        ^~~~~~~~~~~~~~~~~
> >> arch/arm/mach-shmobile/pm-r8a7779.c:35:13: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'r8a7779_pm_init'
>     void __init r8a7779_pm_init(void)
>                 ^~~~~~~~~~~~~~~

This warning/error is fixed by Arnd's patch posted in
https://marc.info/?l=linux-kernel&m=150464459009788&w=2