| Message ID | 20180818181017.1246-2-paul.burton@mips.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | MIPS: Override barrier_before_unreachable() to fix microMIPS | expand |
Hi Paul, 2018-08-19 3:10 GMT+09:00 Paul Burton <paul.burton@mips.com>: > We have a need to override the definition of > barrier_before_unreachable() for MIPS, which means we either need to add > architecture-specific code into linux/compiler-gcc.h or we need to allow > the architecture to provide a header that can define the macro before > the generic definition. The latter seems like the better approach. > > A straightforward approach to the per-arch header is to make use of > asm-generic to provide a default empty header & adjust architectures > which don't need anything specific to make use of that by adding the > header to generic-y. Unfortunately this doesn't work so well due to > commit a95b37e20db9 ("kbuild: get <linux/compiler_types.h> out of > <linux/kconfig.h>") which moved the inclusion of linux/compiler.h to > cflags using the -include compiler flag. > > Because the -include flag is present for all C files we compile, we need > the architecture-provided header to be present before any C files are > compiled. If any C files can be compiled prior to the asm-generic header > wrappers being generated then we hit a build failure due to missing > header. Such cases do exist - one pointed out by the kbuild test robot > is the compilation of arch/ia64/kernel/nr-irqs.c, which occurs as part > of the archprepare target [1]. > > This leaves us with a few options: > > 1) Use generic-y & fix any build failures we find by enforcing > ordering such that the asm-generic target occurs before any C > compilation, such that linux/compiler_types.h can always include > the generated asm-generic wrapper which in turn includes the empty > asm-generic header. This would rely on us finding all the > problematic cases - I don't know for sure that the ia64 issue is > the only one. > > 2) Add an actual empty header to each architecture, so that we don't > need the generated asm-generic wrapper. This seems messy. > > 3) Give up & add #ifdef CONFIG_MIPS or similar to > linux/compiler_types.h. This seems messy too. > > 4) Include the arch header only when it's actually needed, removing > the need for the asm-generic wrapper for all other architectures. > > This patch allows us to use approach 4, by including an > asm/compiler_types.h header using the -include flag in the same way we > do for linux/compiler_types.h, but only if the header actually exists. I agree with the approach 4), but I am of two minds about how to implement it. I guess the cost of evaluating 'wildcard' for each C file is unnoticeable level, but I am slightly in favor of including <asm/compilr_types.h> from <linux/compiler_types.h> conditionally. I am not sure about the CONFIG name, but for example, like this. #ifdef CONFIG_HAVE_ARCH_COMPILER_TYPES #include <asm/compiler_types.h> #endif What do you think? > [1] https://lists.01.org/pipermail/kbuild-all/2018-August/051175.html > > Signed-off-by: Paul Burton <paul.burton@mips.com> > Cc: Arnd Bergmann <arnd@arndb.de> > Cc: James Hogan <jhogan@kernel.org> > Cc: Masahiro Yamada <yamada.masahiro@socionext.com> > Cc: Ralf Baechle <ralf@linux-mips.org> > Cc: linux-arch@vger.kernel.org > Cc: linux-kbuild@vger.kernel.org > Cc: linux-mips@linux-mips.org > > --- > Any thoughts anyone? > > This isn't the prettiest it could possibly be but it's a small change & > clearly shouldn't break anything, which are good qualities for a patch > fixing build failures that we'd ideally backport as far as 4.16. > > Changes in v8: > - New patch. > > scripts/Makefile.lib | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > index 1bb594fcfe12..4e7b41ef029b 100644 > --- a/scripts/Makefile.lib > +++ b/scripts/Makefile.lib > @@ -151,8 +151,11 @@ __a_flags = $(call flags,_a_flags) > __cpp_flags = $(call flags,_cpp_flags) > endif > > +c_includes = $(wildcard $(srctree)/arch/$(SRCARCH)/include/asm/compiler_types.h) > +c_includes += $(srctree)/include/linux/compiler_types.h > + > c_flags = -Wp,-MD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) \ > - -include $(srctree)/include/linux/compiler_types.h \ > + $(addprefix -include ,$(c_includes)) \ > $(__c_flags) $(modkern_cflags) \ > $(basename_flags) $(modname_flags) > > -- > 2.18.0 >
Hi Masahiro, On Mon, Aug 20, 2018 at 02:04:24PM +0900, Masahiro Yamada wrote: > 2018-08-19 3:10 GMT+09:00 Paul Burton <paul.burton@mips.com>: > > We have a need to override the definition of > > barrier_before_unreachable() for MIPS, which means we either need to add > > architecture-specific code into linux/compiler-gcc.h or we need to allow > > the architecture to provide a header that can define the macro before > > the generic definition. The latter seems like the better approach. > > > > A straightforward approach to the per-arch header is to make use of > > asm-generic to provide a default empty header & adjust architectures > > which don't need anything specific to make use of that by adding the > > header to generic-y. Unfortunately this doesn't work so well due to > > commit a95b37e20db9 ("kbuild: get <linux/compiler_types.h> out of > > <linux/kconfig.h>") which moved the inclusion of linux/compiler.h to > > cflags using the -include compiler flag. > > > > Because the -include flag is present for all C files we compile, we need > > the architecture-provided header to be present before any C files are > > compiled. If any C files can be compiled prior to the asm-generic header > > wrappers being generated then we hit a build failure due to missing > > header. Such cases do exist - one pointed out by the kbuild test robot > > is the compilation of arch/ia64/kernel/nr-irqs.c, which occurs as part > > of the archprepare target [1]. > > > > This leaves us with a few options: > > > > 1) Use generic-y & fix any build failures we find by enforcing > > ordering such that the asm-generic target occurs before any C > > compilation, such that linux/compiler_types.h can always include > > the generated asm-generic wrapper which in turn includes the empty > > asm-generic header. This would rely on us finding all the > > problematic cases - I don't know for sure that the ia64 issue is > > the only one. > > > > 2) Add an actual empty header to each architecture, so that we don't > > need the generated asm-generic wrapper. This seems messy. > > > > 3) Give up & add #ifdef CONFIG_MIPS or similar to > > linux/compiler_types.h. This seems messy too. > > > > 4) Include the arch header only when it's actually needed, removing > > the need for the asm-generic wrapper for all other architectures. > > > > This patch allows us to use approach 4, by including an > > asm/compiler_types.h header using the -include flag in the same way we > > do for linux/compiler_types.h, but only if the header actually exists. > > I agree with the approach 4), > but I am of two minds about how to implement it. > > I guess the cost of evaluating 'wildcard' for each C file > is unnoticeable level, but I am slightly in favor of > including <asm/compilr_types.h> from <linux/compiler_types.h> > conditionally. > > I am not sure about the CONFIG name, but for example, like this. > > #ifdef CONFIG_HAVE_ARCH_COMPILER_TYPES > #include <asm/compiler_types.h> > #endif > > What do you think? That sounds fine to me - I'll submit a v9 shortly :) Thanks, Paul
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 1bb594fcfe12..4e7b41ef029b 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -151,8 +151,11 @@ __a_flags = $(call flags,_a_flags) __cpp_flags = $(call flags,_cpp_flags) endif +c_includes = $(wildcard $(srctree)/arch/$(SRCARCH)/include/asm/compiler_types.h) +c_includes += $(srctree)/include/linux/compiler_types.h + c_flags = -Wp,-MD,$(depfile) $(NOSTDINC_FLAGS) $(LINUXINCLUDE) \ - -include $(srctree)/include/linux/compiler_types.h \ + $(addprefix -include ,$(c_includes)) \ $(__c_flags) $(modkern_cflags) \ $(basename_flags) $(modname_flags)
We have a need to override the definition of barrier_before_unreachable() for MIPS, which means we either need to add architecture-specific code into linux/compiler-gcc.h or we need to allow the architecture to provide a header that can define the macro before the generic definition. The latter seems like the better approach. A straightforward approach to the per-arch header is to make use of asm-generic to provide a default empty header & adjust architectures which don't need anything specific to make use of that by adding the header to generic-y. Unfortunately this doesn't work so well due to commit a95b37e20db9 ("kbuild: get <linux/compiler_types.h> out of <linux/kconfig.h>") which moved the inclusion of linux/compiler.h to cflags using the -include compiler flag. Because the -include flag is present for all C files we compile, we need the architecture-provided header to be present before any C files are compiled. If any C files can be compiled prior to the asm-generic header wrappers being generated then we hit a build failure due to missing header. Such cases do exist - one pointed out by the kbuild test robot is the compilation of arch/ia64/kernel/nr-irqs.c, which occurs as part of the archprepare target [1]. This leaves us with a few options: 1) Use generic-y & fix any build failures we find by enforcing ordering such that the asm-generic target occurs before any C compilation, such that linux/compiler_types.h can always include the generated asm-generic wrapper which in turn includes the empty asm-generic header. This would rely on us finding all the problematic cases - I don't know for sure that the ia64 issue is the only one. 2) Add an actual empty header to each architecture, so that we don't need the generated asm-generic wrapper. This seems messy. 3) Give up & add #ifdef CONFIG_MIPS or similar to linux/compiler_types.h. This seems messy too. 4) Include the arch header only when it's actually needed, removing the need for the asm-generic wrapper for all other architectures. This patch allows us to use approach 4, by including an asm/compiler_types.h header using the -include flag in the same way we do for linux/compiler_types.h, but only if the header actually exists. [1] https://lists.01.org/pipermail/kbuild-all/2018-August/051175.html Signed-off-by: Paul Burton <paul.burton@mips.com> Cc: Arnd Bergmann <arnd@arndb.de> Cc: James Hogan <jhogan@kernel.org> Cc: Masahiro Yamada <yamada.masahiro@socionext.com> Cc: Ralf Baechle <ralf@linux-mips.org> Cc: linux-arch@vger.kernel.org Cc: linux-kbuild@vger.kernel.org Cc: linux-mips@linux-mips.org --- Any thoughts anyone? This isn't the prettiest it could possibly be but it's a small change & clearly shouldn't break anything, which are good qualities for a patch fixing build failures that we'd ideally backport as far as 4.16. Changes in v8: - New patch. scripts/Makefile.lib | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)