[PATCH -tip] x86: setup.c fix style problems

[PATCH -tip] x86: setup.c fix style problems

[Jaswinder Singh Rajput]

Impact: cleanup, fix style problems, more readable

Fixes style problems:
WARNING: Use #include <linux/smp.h> instead of <asm/smp.h>
WARNING: Use #include <linux/percpu.h> instead of <asm/percpu.h>
WARNING: Use #include <linux/topology.h> instead of <asm/topology.h>
WARNING: externs should be avoided in .c files
ERROR: code indent should use tabs where possible

total: 2 errors, 4 warnings

Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
---
 arch/x86/kernel/setup.c |   11 ++++-------
 1 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index ae0d804..d8fa2bb 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -66,6 +66,8 @@
 
 #include <linux/percpu.h>
 #include <linux/crash_dump.h>
+#include <linux/smp.h>
+#include <linux/topology.h>
 
 #include <video/edid.h>
 
@@ -89,7 +91,6 @@
 
 #include <asm/system.h>
 #include <asm/vsyscall.h>
-#include <asm/smp.h>
 #include <asm/desc.h>
 #include <asm/dma.h>
 #include <asm/iommu.h>
@@ -101,8 +102,6 @@
 #include <asm/paravirt.h>
 #include <asm/hypervisor.h>
 
-#include <asm/percpu.h>
-#include <asm/topology.h>
 #include <asm/apicdef.h>
 #ifdef CONFIG_X86_64
 #include <asm/numa_64.h>
@@ -216,8 +215,6 @@ EXPORT_SYMBOL(screen_info);
 struct edid_info edid_info;
 EXPORT_SYMBOL_GPL(edid_info);
 
-extern int root_mountflags;
-
 unsigned long saved_video_mode;
 
 #define RAMDISK_IMAGE_START_MASK	0x07FF
@@ -823,8 +820,8 @@ void __init setup_arch(char **cmdline_p)
 #else
 	num_physpages = max_pfn;
 
- 	if (cpu_has_x2apic)
- 		check_x2apic();
+	if (cpu_has_x2apic)
+		check_x2apic();
 
 	/* How many end-of-memory variables you have, grandma! */
 	/* need this before calling reserve_initrd */
-- 
1.5.5.1

Re: [PATCH -tip] x86: setup.c fix style problems

[Ingo Molnar]

* Jaswinder Singh Rajput <jaswinder@infradead.org> wrote:

> Impact: cleanup, fix style problems, more readable
> 
> Fixes style problems:
> WARNING: Use #include <linux/smp.h> instead of <asm/smp.h>
> WARNING: Use #include <linux/percpu.h> instead of <asm/percpu.h>
> WARNING: Use #include <linux/topology.h> instead of <asm/topology.h>
> WARNING: externs should be avoided in .c files
> ERROR: code indent should use tabs where possible
> 
> total: 2 errors, 4 warnings

doesnt build on some configs:

 arch/x86/kernel/setup.c:943: error: implicit declaration of function 'prefill_possible_map'

the x86-local declarations in smp.h should probably be moved into a 
separate smp_x86.h file - to avoid such trouble.

	Ingo

Re: [PATCH -tip] x86: setup.c fix style problems

[Jaswinder Singh Rajput]

Hello Ingo,

On Sun, Jan 4, 2009 at 10:51 PM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Jaswinder Singh Rajput <jaswinder@infradead.org> wrote:
>
>> Impact: cleanup, fix style problems, more readable
>>
>> Fixes style problems:
>> WARNING: Use #include <linux/smp.h> instead of <asm/smp.h>
>> WARNING: Use #include <linux/percpu.h> instead of <asm/percpu.h>
>> WARNING: Use #include <linux/topology.h> instead of <asm/topology.h>
>> WARNING: externs should be avoided in .c files
>> ERROR: code indent should use tabs where possible
>>
>> total: 2 errors, 4 warnings
>
> doesnt build on some configs:
>

Very strange. I have tested this on X86_32 (UP and SMP) and X86_64 SMP

Can you please provide me config file where it fails.

Thank you,

JSR.

Re: [PATCH -tip] x86: setup.c fix style problems

[Jaswinder Singh Rajput]

On Sun, Jan 4, 2009 at 10:51 PM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Jaswinder Singh Rajput <jaswinder@infradead.org> wrote:
>
>> Impact: cleanup, fix style problems, more readable
>>
>> Fixes style problems:
>> WARNING: Use #include <linux/smp.h> instead of <asm/smp.h>
>> WARNING: Use #include <linux/percpu.h> instead of <asm/percpu.h>
>> WARNING: Use #include <linux/topology.h> instead of <asm/topology.h>
>> WARNING: externs should be avoided in .c files
>> ERROR: code indent should use tabs where possible
>>
>> total: 2 errors, 4 warnings
>
> doesnt build on some configs:
>
>  arch/x86/kernel/setup.c:943: error: implicit declaration of function 'prefill_possible_map'
>

This problem will only arise if CONFIG_SMP is not defined. Then
linux/smp.h will not include asm/smp.h

But why we are keeping non SMP stuff in asm/smp.h, we need to fix this.

JSR

Re: [PATCH -tip] x86: setup.c fix style problems

[Ingo Molnar]

* Jaswinder Singh Rajput <jaswinderlinux@gmail.com> wrote:

> On Sun, Jan 4, 2009 at 10:51 PM, Ingo Molnar <mingo@elte.hu> wrote:
> >
> > * Jaswinder Singh Rajput <jaswinder@infradead.org> wrote:
> >
> >> Impact: cleanup, fix style problems, more readable
> >>
> >> Fixes style problems:
> >> WARNING: Use #include <linux/smp.h> instead of <asm/smp.h>
> >> WARNING: Use #include <linux/percpu.h> instead of <asm/percpu.h>
> >> WARNING: Use #include <linux/topology.h> instead of <asm/topology.h>
> >> WARNING: externs should be avoided in .c files
> >> ERROR: code indent should use tabs where possible
> >>
> >> total: 2 errors, 4 warnings
> >
> > doesnt build on some configs:
> >
> >  arch/x86/kernel/setup.c:943: error: implicit declaration of function 'prefill_possible_map'
> >
> 
> This problem will only arise if CONFIG_SMP is not defined. Then 
> linux/smp.h will not include asm/smp.h
> 
> But why we are keeping non SMP stuff in asm/smp.h, we need to fix this.

yeah - i mentioned it to you before that there are such cases.

	Ingo

Re: [PATCH -tip] x86: setup.c fix style problems

[Jaswinder Singh Rajput]

On Sun, 2009-01-04 at 21:34 +0100, Ingo Molnar wrote:
> yeah - i mentioned it to you before that there are such cases.
> 

OK, so here is new patch excluding smp.h:

[PATCH]  x86: setup.c fix style problems

Impact: cleanup

Fix:

WARNING: Use #include <linux/percpu.h> instead of <asm/percpu.h>
WARNING: Use #include <linux/topology.h> instead of <asm/topology.h>
WARNING: externs should be avoided in .c files
ERROR: code indent should use tabs where possible

total: 2 errors, 3 warnings

Signed-off-by: Jaswinder Singh Rajput <jaswinderrajput@gmail.com>
---
 arch/x86/kernel/setup.c |    9 +++------
 1 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/arch/x86/kernel/setup.c b/arch/x86/kernel/setup.c
index ae0d804..57e76ca 100644
--- a/arch/x86/kernel/setup.c
+++ b/arch/x86/kernel/setup.c
@@ -66,6 +66,7 @@
 
 #include <linux/percpu.h>
 #include <linux/crash_dump.h>
+#include <linux/topology.h>
 
 #include <video/edid.h>
 
@@ -101,8 +102,6 @@
 #include <asm/paravirt.h>
 #include <asm/hypervisor.h>
 
-#include <asm/percpu.h>
-#include <asm/topology.h>
 #include <asm/apicdef.h>
 #ifdef CONFIG_X86_64
 #include <asm/numa_64.h>
@@ -216,8 +215,6 @@ EXPORT_SYMBOL(screen_info);
 struct edid_info edid_info;
 EXPORT_SYMBOL_GPL(edid_info);
 
-extern int root_mountflags;
-
 unsigned long saved_video_mode;
 
 #define RAMDISK_IMAGE_START_MASK	0x07FF
@@ -823,8 +820,8 @@ void __init setup_arch(char **cmdline_p)
 #else
 	num_physpages = max_pfn;
 
- 	if (cpu_has_x2apic)
- 		check_x2apic();
+	if (cpu_has_x2apic)
+		check_x2apic();
 
 	/* How many end-of-memory variables you have, grandma! */
 	/* need this before calling reserve_initrd */
-- 
1.5.6.6

Re: [PATCH -tip] x86: setup.c fix style problems

[Ingo Molnar]

* Jaswinder Singh Rajput <jaswinder@infradead.org> wrote:

> On Sun, 2009-01-04 at 21:34 +0100, Ingo Molnar wrote:
> > yeah - i mentioned it to you before that there are such cases.
> > 
> 
> OK, so here is new patch excluding smp.h:

Could you please instead fix the smp.h assumption so that we dont hit this 
in the future? Then we can apply your original patch. [ Do you need the 
specific .config that hits this? Can dig it out if necessary. ]

	Ingo

Re: [PATCH -tip] x86: setup.c fix style problems

[Jaswinder Singh Rajput]

On Mon, Jan 5, 2009 at 6:45 PM, Ingo Molnar <mingo@elte.hu> wrote:
>
> * Jaswinder Singh Rajput <jaswinder@infradead.org> wrote:
>
>> On Sun, 2009-01-04 at 21:34 +0100, Ingo Molnar wrote:
>> > yeah - i mentioned it to you before that there are such cases.
>> >
>>
>> OK, so here is new patch excluding smp.h:
>
> Could you please instead fix the smp.h assumption so that we dont hit this
> in the future? Then we can apply your original patch.

Sure, I will do it.

> [ Do you need the
> specific .config that hits this? Can dig it out if necessary. ]

If you provide me that .config file then it will be very helpful for
cross-checking.

JSR

Re: [PATCH -tip] x86: setup.c fix style problems

[Ingo Molnar]

* Jaswinder Singh Rajput <jaswinderlinux@gmail.com> wrote:

> On Mon, Jan 5, 2009 at 6:45 PM, Ingo Molnar <mingo@elte.hu> wrote:
> >
> > * Jaswinder Singh Rajput <jaswinder@infradead.org> wrote:
> >
> >> On Sun, 2009-01-04 at 21:34 +0100, Ingo Molnar wrote:
> >> > yeah - i mentioned it to you before that there are such cases.
> >> >
> >>
> >> OK, so here is new patch excluding smp.h:
> >
> > Could you please instead fix the smp.h assumption so that we dont hit this
> > in the future? Then we can apply your original patch.
> 
> Sure, I will do it.
> 
> > [ Do you need the
> > specific .config that hits this? Can dig it out if necessary. ]
> 
> If you provide me that .config file then it will be very helpful for
> cross-checking.

hm, cannot find it - there's been too many other bugs in that timeframe so 
i'd have to try a dozen configs to find out which one it was.

It's not a big problem i think: i'd suggest you fix the problem pinpointed 
by the build error i sent to you - and if anything else happens with newer 
patches i'll send you a full config for those problems. This area is thick 
include file spaghetti that will need a few iterations to sort out.

	Ingo

Re: [PATCH -tip] x86: setup.c fix style problems

[Jaswinder Singh Rajput]

On Mon, Jan 5, 2009 at 7:20 PM, Ingo Molnar <mingo@elte.hu> wrote:
>
> hm, cannot find it - there's been too many other bugs in that timeframe so
> i'd have to try a dozen configs to find out which one it was.
>

OK, no problem :-)

JSR

Re: [PATCH -tip] x86: setup.c fix style problems

[Jaswinder Singh Rajput]

On Sun, Jan 4, 2009 at 10:51 PM, Ingo Molnar <mingo@elte.hu> wrote:
>
> the x86-local declarations in smp.h should probably be moved into a
> separate smp_x86.h file - to avoid such trouble.
>

I think we do not need to make any new files only thing we need is to
rearrange stuff between existing files.

Currently there is no such protocol, If we follow this logic then it
will very easy for us to maintain things:

CONFIG_SMP -> asm/smp.h
CONFIG_X86_LOCAL_APIC -> asm/apic.h
CONFIG_X86_IO_APIC -> asm/io_apic.h

And if you want then I will give strict instructions so that we will
never face these problems again, like :-

asm/smp.h:

#ifndef CONFIG_SMP
#error "Including wrong file, why you need smp.h for Uniprocessor"
#else
...
#endif

Similarly for asm/apic.h and asm/io_apic.h

--
JSR

Re: [PATCH -tip] x86: setup.c fix style problems

[Ingo Molnar]

* Jaswinder Singh Rajput <jaswinderlinux@gmail.com> wrote:

> On Sun, Jan 4, 2009 at 10:51 PM, Ingo Molnar <mingo@elte.hu> wrote:
> >
> > the x86-local declarations in smp.h should probably be moved into a
> > separate smp_x86.h file - to avoid such trouble.
> >
> 
> I think we do not need to make any new files only thing we need is to
> rearrange stuff between existing files.
> 
> Currently there is no such protocol, If we follow this logic then it
> will very easy for us to maintain things:
> 
> CONFIG_SMP -> asm/smp.h
> CONFIG_X86_LOCAL_APIC -> asm/apic.h
> CONFIG_X86_IO_APIC -> asm/io_apic.h

yep.

> And if you want then I will give strict instructions so that we will 
> never face these problems again, like :-
> 
> asm/smp.h:
> 
> #ifndef CONFIG_SMP
> #error "Including wrong file, why you need smp.h for Uniprocessor"
> #else
> ...
> #endif
> 
> Similarly for asm/apic.h and asm/io_apic.h

the smp.h change would work, agreed - but it's not really good for things 
like apic.h and io_apic.h - often apic.h is included to pick up init and 
various callback prototypes - which exist on !CONFIG_X96_IO_APIC too.

	Ingo