Re: [PATCH v8 1/2] kbuild: Allow asm-specific compiler_types.h

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 [off-list ref]:

quoted

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