Re: [PATCH 21/21] [PATCH] finish processor.h integration

Glauber de Oliveira Costa
2007-12-18T13:50:50Z

Ingo Molnar wrote:
> * Ingo Molnar <mingo@elte.hu> wrote:
>
>>> What's left in processor_32.h and processor_64.h cannot be cleanly
>>> integrated. However, it's just a couple of definitions. They are
>>> moved to processor.h around ifdefs, and the original files are
>>> deleted. Note that there's much less headers included in the final
>>> version.
>> and this patch breaks the build on the attached config, with:
>>
>> CC arch/x86/mm/boot_ioremap_32.o
>> In file included from include/asm/fixmap_32.h:28,
>> from include/asm/fixmap.h:2,
>> from include/asm/pgtable_32.h:16,
>> from include/asm/pgtable.h:2,
>> from arch/x86/mm/boot_ioremap_32.c:21:
>> include/asm/acpi.h:159: error: expected '=', ',', ';', 'asm' or '__attribute__' before 'acpi_fake_nodes'
>> make[1]: *** [arch/x86/mm/boot_ioremap_32.o] Error 1
>> make: *** [arch/x86/mm/boot_ioremap_32.o] Error 2
>
> really, please do _much_ more careful unification. We unified two full
> architectures in .24-rc1 and there was _not a single regression_ due to
> that unification. Not a single build failure, not a single boot or
> runtime failure.
IIRC, 24-rc1 was a different story. Most, if not all patches, had no
binary diff, etc. This is not the case here.

> here the problem is apparently caused by your patch, a careless
> 'unification' of include file sections. 32-bit had this:

Point is this patches do unification, but they are not just that, as you
can see. I am attempting to cleanup headers that appears not to be used,
I am effectivelly adding a new feature to x86_64, and so on. I am
testing everything, all I sent works in my configs, and your testing,
and point out the configs that breaks is really much appreciated.

> -#include <asm/vm86.h>
> -#include <asm/math_emu.h>
> -#include <asm/segment.h>
> -#include <asm/page.h>
> -#include <asm/types.h>
> -#include <asm/sigcontext.h>
> -#include <asm/cpufeature.h>
> -#include <asm/msr.h>
> -#include <asm/system.h>
> -#include <linux/threads.h>
> -#include <linux/init.h>
> -#include <asm/desc_defs.h>
>
> 64-bit had this:
>
> -#include <asm/segment.h>
> -#include <asm/page.h>
> -#include <asm/types.h>
> -#include <asm/sigcontext.h>
> -#include <asm/cpufeature.h>
> -#include <linux/threads.h>
> -#include <asm/msr.h>
> -#include <asm/current.h>
> -#include <asm/system.h>
> -#include <linux/personality.h>
> -#include <asm/desc_defs.h>
>
> and the 'unified' processor.h has:
>
> +#include <asm/desc_defs.h>
> +#include <asm/msr.h>
> #include <asm/page.h>
> #include <asm/percpu.h>
> #include <asm/system.h>
> #include <asm/percpu.h>
> #include <linux/cpumask.h>
> #include <linux/cache.h>
> +#include <linux/personality.h>
>
> Those are visible, _crutial_ differences totally unmentioned in the
> patch.
>
> yes, our include file dependencies are a jungle, the differences between
> 32-bit and 64-bit are arbitrary in 80% of the cases, but still there's
> no reason why this couldnt be done correctly. The patch below is a quick
> bandaid that adds the missing bits.

And that's because I'm trying to drop headers that are not used anymore.
Again, getting test for other people but me, helps making sure it's
correct. I am working on updating the series now, and will send an
updated version.

> Ingo
>
> ------------------>
> Subject: x86: fix include file mess
> From: Ingo Molnar <mingo@elte.hu>
>
> fix include file mess.
>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> ---
> include/asm-x86/processor.h | 16 ++++++++++++++--
> 1 file changed, 14 insertions(+), 2 deletions(-)
>
> Index: linux-x86.q/include/asm-x86/processor.h
> ===================================================================
> --- linux-x86.q.orig/include/asm-x86/processor.h
> +++ linux-x86.q/include/asm-x86/processor.h
> @@ -6,15 +6,27 @@
> struct task_struct;
> struct mm_struct;
>
> +#ifdef CONFIG_X86_32
> +# include <asm/math_emu.h>
> +# include <asm/vm86.h>
> +#endif
> +
> +#include <asm/cpufeature.h>
> +#include <asm/current.h>
> #include <asm/desc_defs.h>
> #include <asm/msr.h>
> #include <asm/page.h>
> #include <asm/percpu.h>
> +#include <asm/segment.h>
> +#include <asm/sigcontext.h>
> #include <asm/system.h>
> -#include <asm/percpu.h>
> -#include <linux/cpumask.h>
> +#include <asm/types.h>
> +
> #include <linux/cache.h>
> +#include <linux/cpumask.h>
> +#include <linux/init.h>
> #include <linux/personality.h>
> +#include <linux/threads.h>
>
> /*
> * Default implementation of macro that returns current