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 <[email protected]>
---
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
* Jaswinder Singh Rajput <[email protected]> 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
Hello Ingo,
On Sun, Jan 4, 2009 at 10:51 PM, Ingo Molnar <[email protected]> wrote:
>
> * Jaswinder Singh Rajput <[email protected]> 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.
On Sun, Jan 4, 2009 at 10:51 PM, Ingo Molnar <[email protected]> wrote:
>
> * Jaswinder Singh Rajput <[email protected]> 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
* Jaswinder Singh Rajput <[email protected]> wrote:
> On Sun, Jan 4, 2009 at 10:51 PM, Ingo Molnar <[email protected]> wrote:
> >
> > * Jaswinder Singh Rajput <[email protected]> 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
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 <[email protected]>
---
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
* Jaswinder Singh Rajput <[email protected]> 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
On Mon, Jan 5, 2009 at 6:45 PM, Ingo Molnar <[email protected]> wrote:
>
> * Jaswinder Singh Rajput <[email protected]> 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
* Jaswinder Singh Rajput <[email protected]> wrote:
> On Mon, Jan 5, 2009 at 6:45 PM, Ingo Molnar <[email protected]> wrote:
> >
> > * Jaswinder Singh Rajput <[email protected]> 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
On Mon, Jan 5, 2009 at 7:20 PM, Ingo Molnar <[email protected]> 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
On Sun, Jan 4, 2009 at 10:51 PM, Ingo Molnar <[email protected]> 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
* Jaswinder Singh Rajput <[email protected]> wrote:
> On Sun, Jan 4, 2009 at 10:51 PM, Ingo Molnar <[email protected]> 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