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 <[email protected]>
---
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
On Tue, Sep 5, 2017 at 1:27 PM, Tobias Klauser <[email protected]> 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 <[email protected]>
> ---
> 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
On 2017-09-05 at 17:11:50 +0200, Arnd Bergmann <[email protected]> wrote:
> On Tue, Sep 5, 2017 at 1:27 PM, Tobias Klauser <[email protected]> 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 <[email protected]>
> > ---
> > 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/[email protected]/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
On Tue, Sep 5, 2017 at 5:21 PM, Tobias Klauser <[email protected]> wrote:
> On 2017-09-05 at 17:11:50 +0200, Arnd Bergmann <[email protected]> wrote:
>> On Tue, Sep 5, 2017 at 1:27 PM, Tobias Klauser <[email protected]> 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 <[email protected]>
>> > ---
>> > 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/[email protected]/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 <[email protected]>
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 <[email protected]>
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"
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 <[email protected]>
:::::: CC: Jonas Bonn <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
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 <[email protected]>
:::::: CC: Paul Mundt <[email protected]>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
On 2017-09-07 at 21:36:45 +0200, kbuild test robot <[email protected]> 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
Hi Arnd
On 2017-09-05 at 17:11:50 +0200, Arnd Bergmann <[email protected]> wrote:
> On Tue, Sep 5, 2017 at 1:27 PM, Tobias Klauser <[email protected]> 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 <[email protected]>
> > ---
> > 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
From 1577959051597032249@xxx Fri Sep 08 08:23:39 +0000 2017
X-GM-THRID: 1577699186507194076
X-Gmail-Labels: Inbox,Category Forums