Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Tue, Jan 15, 2008 at 11:14:09PM +0100, Ingo Molnar wrote: > > * Adrian Bunk <[EMAIL PROTECTED]> wrote: > > > > make ARCH=x86_64 allyesconfig > > > will set CONFIG_64BIT for you - no? > > > > Yes. > > > > But this still leaves the fact that when someone says 'allyesconfig' > > it's no longer clear which configuration he has. And no, I do not > > consider it funny that this now implies two hours of compilation twice > > just for seeing one bug. > > sorry - i should have mentioned that it's 64-bit. (and i even > mis-remembered having seen it on 32-bit as well) This bug seems to also be present on 32bit, but allyesconfig minus CONFIG_HOTPLUG is not among the configurations where it's present on 32bit. > Ingo cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
* Adrian Bunk <[EMAIL PROTECTED]> wrote: > > make ARCH=x86_64 allyesconfig > > will set CONFIG_64BIT for you - no? > > Yes. > > But this still leaves the fact that when someone says 'allyesconfig' > it's no longer clear which configuration he has. And no, I do not > consider it funny that this now implies two hours of compilation twice > just for seeing one bug. sorry - i should have mentioned that it's 64-bit. (and i even mis-remembered having seen it on 32-bit as well) Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Tue, Jan 15, 2008 at 07:21:42PM +0100, Sam Ravnborg wrote: >... > > > And I will add a config option to: > > > - set -fno-unit-at-a-time > > > > I was told future gcc versions would remove that. Why do you > > want it? > Are there any better way to tell gcc no to inline so agressively? I haven't tested it, but -fno-inline-functions-called-once alone _might_ be enough. > Sam cu Adrian BTW: I hope it's clear for everyone that disabling such optimizations should only be used for debugging section mismatches, not for production kernels. -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Tue, Jan 15, 2008 at 07:29:04PM +0100, Andi Kleen wrote:
> On Tue, Jan 15, 2008 at 07:21:42PM +0100, Sam Ravnborg wrote:
> > With default options to gcc my .config produces ~65 warnings
> > but with -fno-unit-a-time I get 112 warnings.
> > Solely due to less inlining done by gcc.
> >
> > So there are two sources for the 'randomization':
> > a) The actual config
> > b) The sometimes agressive inlining
>
> Inlining should not be random. And how does inlining cause such a warning?
Consider:
static int __init foo()
{
// ...
}
static int bar()
{
// ...
if (foo())
// ...
}
gcc will often inline foo into bar - and then all code are suddenly
part of .text and no section mismatch.
But you add anohter call to foo() somewhere so gcc decide no longer
to inline foo() and we then have a reference from .text to .init.text.
> >
> > a) will be addressed by having separate sections for each
> > __init* type that is at link time combined where it belongs.
>
> One problem I ran into the past was that older binutils seem
> to have some exponential behaviour with a lot of named sections
> and run very slowly.
This is more the -ffunction-section issue I guess.
What we are dealig with here is ~20 more sections and the kernel has
~100 section today (or more). So not a huge increase.
Sam
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Tue, Jan 15, 2008 at 07:21:42PM +0100, Sam Ravnborg wrote: > With default options to gcc my .config produces ~65 warnings > but with -fno-unit-a-time I get 112 warnings. > Solely due to less inlining done by gcc. > > So there are two sources for the 'randomization': > a) The actual config > b) The sometimes agressive inlining Inlining should not be random. And how does inlining cause such a warning? > > a) will be addressed by having separate sections for each > __init* type that is at link time combined where it belongs. One problem I ran into the past was that older binutils seem to have some exponential behaviour with a lot of named sections and run very slowly. > > b) is addressed by a Kernel Hacking option which >1) uses -fno-unit-at-a-time to get less gcc inlining >2) maybe make all __*init function no-inline >3) maybe disable inlining globally > > > > And I will add a config option to: > > > - set -fno-unit-at-a-time > > > > I was told future gcc versions would remove that. Why do you > > want it? > Are there any better way to tell gcc no to inline so agressively? You can either sprinkle noinlines or set specific --params to throttle back the inliner. The later is very gcc version specific unfortunately. -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
> > > > The plan is to let section mismatch warnings become errors > > after the merge window - so we hit -mm first. > > A lot of those I look at seem to be not really bugs; also my > impression is that they sometimes crop up randomly. e.g. you > change something completely unrelated and suddenly you get > a section warning somewhere else. I have fixed a lot of these warnings. And when I look closer at them they are explainable. The warnings are today very dependent on the configuration and the inlining that gcc uses. With default options to gcc my .config produces ~65 warnings but with -fno-unit-a-time I get 112 warnings. Solely due to less inlining done by gcc. So there are two sources for the 'randomization': a) The actual config b) The sometimes agressive inlining a) will be addressed by having separate sections for each __init* type that is at link time combined where it belongs. b) is addressed by a Kernel Hacking option which 1) uses -fno-unit-at-a-time to get less gcc inlining 2) maybe make all __*init function no-inline 3) maybe disable inlining globally > > And I will add a config option to: > > - set -fno-unit-at-a-time > > I was told future gcc versions would remove that. Why do you > want it? Are there any better way to tell gcc no to inline so agressively? Sam -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Tue, Jan 15, 2008 at 06:11:46PM +0100, Andi Kleen wrote: > On Tue, Jan 15, 2008 at 05:25:13PM +0100, Sam Ravnborg wrote: > > On Tue, Jan 15, 2008 at 04:17:42PM +0100, Ingo Molnar wrote: > > > > > > * Sam Ravnborg <[EMAIL PROTECTED]> wrote: > > > > > > > > find below the current set of warnings on -git. There are 62. > > > > > > > > The correct figure is 112. > > > > > > > > You need to do a: > > > > make KCFLAGS=-fno-unit-at-a-time > > > > build to see them all. > > > > > > btw., please add a .config option to trigger the -fno-unit-at-a-time > > > flags. Something like CONFIG_SECTION_ERRORS=y - plus perhaps combine it > > > with the patch below that turns such section bugs into detectable build > > > errors. A distro does not want to build a kernel that could potentially > > > corrupt kernel memory. (it's a security risk as well.) If we make the > > > err=1 dependent on CONFIG_SECTION_ERRORS then we'll have this > > > configurable. > > > > The plan is to let section mismatch warnings become errors > > after the merge window - so we hit -mm first. > > A lot of those I look at seem to be not really bugs; Half of them are possible Oopses, and the other half are about wasted memory. The warnings in the kernel that are not really bugs you can count with your fingers. > also my > impression is that they sometimes crop up randomly. e.g. you > change something completely unrelated and suddenly you get > a section warning somewhere else. >... Not all errors are always visible, they might depend on CONFIG_ options, and sometimes gcc optimizes such bugs away. > -Andi cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Tue, Jan 15, 2008 at 05:25:13PM +0100, Sam Ravnborg wrote: > On Tue, Jan 15, 2008 at 04:17:42PM +0100, Ingo Molnar wrote: > > > > * Sam Ravnborg <[EMAIL PROTECTED]> wrote: > > > > > > find below the current set of warnings on -git. There are 62. > > > > > > The correct figure is 112. > > > > > > You need to do a: > > > make KCFLAGS=-fno-unit-at-a-time > > > build to see them all. > > > > btw., please add a .config option to trigger the -fno-unit-at-a-time > > flags. Something like CONFIG_SECTION_ERRORS=y - plus perhaps combine it > > with the patch below that turns such section bugs into detectable build > > errors. A distro does not want to build a kernel that could potentially > > corrupt kernel memory. (it's a security risk as well.) If we make the > > err=1 dependent on CONFIG_SECTION_ERRORS then we'll have this > > configurable. > > The plan is to let section mismatch warnings become errors > after the merge window - so we hit -mm first. A lot of those I look at seem to be not really bugs; also my impression is that they sometimes crop up randomly. e.g. you change something completely unrelated and suddenly you get a section warning somewhere else. > And I will add a config option to: > - set -fno-unit-at-a-time I was told future gcc versions would remove that. Why do you want it? > - add no-inline to all functions marked __init* > - and maybe disable __inline if that gives additional errors I sometimes do that for debugging "define static noinline" in specific files or similar because it's easier to make sense of oopses when functions not inlined. Not sure it would work as a global option though because if you do it globally then all the inlines in all .hs would be affected and that might lead to immense code bloat. -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Tue, Jan 15, 2008 at 04:17:42PM +0100, Ingo Molnar wrote: > > * Sam Ravnborg <[EMAIL PROTECTED]> wrote: > > > > find below the current set of warnings on -git. There are 62. > > > > The correct figure is 112. > > > > You need to do a: > > make KCFLAGS=-fno-unit-at-a-time > > build to see them all. > > btw., please add a .config option to trigger the -fno-unit-at-a-time > flags. Something like CONFIG_SECTION_ERRORS=y - plus perhaps combine it > with the patch below that turns such section bugs into detectable build > errors. A distro does not want to build a kernel that could potentially > corrupt kernel memory. (it's a security risk as well.) If we make the > err=1 dependent on CONFIG_SECTION_ERRORS then we'll have this > configurable. The plan is to let section mismatch warnings become errors after the merge window - so we hit -mm first. And I will add a config option to: - set -fno-unit-at-a-time - add no-inline to all functions marked __init* - and maybe disable __inline if that gives additional errors Slowly getting there. Need to beat modpost in shape to get much less configuration dependent warnings/errors first. Sam -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
* Sam Ravnborg <[EMAIL PROTECTED]> wrote:
> > find below the current set of warnings on -git. There are 62.
>
> The correct figure is 112.
>
> You need to do a:
> make KCFLAGS=-fno-unit-at-a-time
> build to see them all.
btw., please add a .config option to trigger the -fno-unit-at-a-time
flags. Something like CONFIG_SECTION_ERRORS=y - plus perhaps combine it
with the patch below that turns such section bugs into detectable build
errors. A distro does not want to build a kernel that could potentially
corrupt kernel memory. (it's a security risk as well.) If we make the
err=1 dependent on CONFIG_SECTION_ERRORS then we'll have this
configurable.
Ingo
>
Subject: x86: link mismatch error
From: Ingo Molnar <[EMAIL PROTECTED]>
turn the build warning into a build error.
Signed-off-by: Ingo Molnar <[EMAIL PROTECTED]>
---
scripts/mod/modpost.c | 63 ++
1 file changed, 39 insertions(+), 24 deletions(-)
Index: linux/scripts/mod/modpost.c
===
--- linux.orig/scripts/mod/modpost.c
+++ linux/scripts/mod/modpost.c
@@ -863,8 +863,8 @@ static void find_symbols_between(struct
* Try to find symbols near it so user can find it.
* Check whitelist before warning - it may be a false positive.
**/
-static void warn_sec_mismatch(const char *modname, const char *fromsec,
- struct elf_info *elf, Elf_Sym *sym, Elf_Rela r)
+static int error_sec_mismatch(const char *modname, const char *fromsec,
+struct elf_info *elf, Elf_Sym *sym, Elf_Rela r)
{
const char *refsymname = "";
Elf_Sym *before, *after;
@@ -874,6 +874,7 @@ static void warn_sec_mismatch(const char
const char *secstrings = (void *)hdr +
sechdrs[hdr->e_shstrndx].sh_offset;
const char *secname = secstrings + sechdrs[sym->st_shndx].sh_name;
+ int err = 0;
find_symbols_between(elf, r.r_offset, fromsec, , );
@@ -885,32 +886,38 @@ static void warn_sec_mismatch(const char
if (secref_whitelist(modname, secname, fromsec,
before ? elf->strtab + before->st_name : "",
refsymname))
- return;
+ goto out;
if (before && after) {
- warn("%s(%s+0x%llx): Section mismatch: reference to %s:%s "
+ merror("%s(%s+0x%llx): Section mismatch: reference to %s:%s "
"(between '%s' and '%s')\n",
modname, fromsec, (unsigned long long)r.r_offset,
secname, refsymname,
elf->strtab + before->st_name,
elf->strtab + after->st_name);
+ err = 1;
} else if (before) {
- warn("%s(%s+0x%llx): Section mismatch: reference to %s:%s "
+ merror("%s(%s+0x%llx): Section mismatch: reference to %s:%s "
"(after '%s')\n",
modname, fromsec, (unsigned long long)r.r_offset,
secname, refsymname,
elf->strtab + before->st_name);
+ err = 1;
} else if (after) {
- warn("%s(%s+0x%llx): Section mismatch: reference to %s:%s "
+ merror("%s(%s+0x%llx): Section mismatch: reference to %s:%s "
"before '%s' (at offset -0x%llx)\n",
modname, fromsec, (unsigned long long)r.r_offset,
secname, refsymname,
elf->strtab + after->st_name);
+ err = 1;
} else {
- warn("%s(%s+0x%llx): Section mismatch: reference to %s:%s\n",
+ merror("%s(%s+0x%llx): Section mismatch: reference to %s:%s\n",
modname, fromsec, (unsigned long long)r.r_offset,
secname, refsymname);
+ err = 1;
}
+out:
+ return err;
}
static unsigned int *reloc_location(struct elf_info *elf,
@@ -997,10 +1004,10 @@ static int addend_mips_rel(struct elf_in
* to find all references to a section that reference a section that will
* be discarded and warns about it.
**/
-static void check_sec_ref(struct module *mod, const char *modname,
- struct elf_info *elf,
- int section(const char*),
- int section_ref_ok(const char *))
+static int check_sec_ref(struct module *mod, const char *modname,
+struct elf_info *elf,
+int section(const char*),
+int section_ref_ok(const char *))
{
int i;
Elf_Sym *sym;
@@ -1049,9 +1056,11 @@ static void check_sec_ref(struct module
secname = secstrings +
sechdrs[sym->st_shndx].sh_name;
- if
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
* Sam Ravnborg [EMAIL PROTECTED] wrote:
find below the current set of warnings on -git. There are 62.
The correct figure is 112.
You need to do a:
make KCFLAGS=-fno-unit-at-a-time
build to see them all.
btw., please add a .config option to trigger the -fno-unit-at-a-time
flags. Something like CONFIG_SECTION_ERRORS=y - plus perhaps combine it
with the patch below that turns such section bugs into detectable build
errors. A distro does not want to build a kernel that could potentially
corrupt kernel memory. (it's a security risk as well.) If we make the
err=1 dependent on CONFIG_SECTION_ERRORS then we'll have this
configurable.
Ingo
Subject: x86: link mismatch error
From: Ingo Molnar [EMAIL PROTECTED]
turn the build warning into a build error.
Signed-off-by: Ingo Molnar [EMAIL PROTECTED]
---
scripts/mod/modpost.c | 63 ++
1 file changed, 39 insertions(+), 24 deletions(-)
Index: linux/scripts/mod/modpost.c
===
--- linux.orig/scripts/mod/modpost.c
+++ linux/scripts/mod/modpost.c
@@ -863,8 +863,8 @@ static void find_symbols_between(struct
* Try to find symbols near it so user can find it.
* Check whitelist before warning - it may be a false positive.
**/
-static void warn_sec_mismatch(const char *modname, const char *fromsec,
- struct elf_info *elf, Elf_Sym *sym, Elf_Rela r)
+static int error_sec_mismatch(const char *modname, const char *fromsec,
+struct elf_info *elf, Elf_Sym *sym, Elf_Rela r)
{
const char *refsymname = ;
Elf_Sym *before, *after;
@@ -874,6 +874,7 @@ static void warn_sec_mismatch(const char
const char *secstrings = (void *)hdr +
sechdrs[hdr-e_shstrndx].sh_offset;
const char *secname = secstrings + sechdrs[sym-st_shndx].sh_name;
+ int err = 0;
find_symbols_between(elf, r.r_offset, fromsec, before, after);
@@ -885,32 +886,38 @@ static void warn_sec_mismatch(const char
if (secref_whitelist(modname, secname, fromsec,
before ? elf-strtab + before-st_name : ,
refsymname))
- return;
+ goto out;
if (before after) {
- warn(%s(%s+0x%llx): Section mismatch: reference to %s:%s
+ merror(%s(%s+0x%llx): Section mismatch: reference to %s:%s
(between '%s' and '%s')\n,
modname, fromsec, (unsigned long long)r.r_offset,
secname, refsymname,
elf-strtab + before-st_name,
elf-strtab + after-st_name);
+ err = 1;
} else if (before) {
- warn(%s(%s+0x%llx): Section mismatch: reference to %s:%s
+ merror(%s(%s+0x%llx): Section mismatch: reference to %s:%s
(after '%s')\n,
modname, fromsec, (unsigned long long)r.r_offset,
secname, refsymname,
elf-strtab + before-st_name);
+ err = 1;
} else if (after) {
- warn(%s(%s+0x%llx): Section mismatch: reference to %s:%s
+ merror(%s(%s+0x%llx): Section mismatch: reference to %s:%s
before '%s' (at offset -0x%llx)\n,
modname, fromsec, (unsigned long long)r.r_offset,
secname, refsymname,
elf-strtab + after-st_name);
+ err = 1;
} else {
- warn(%s(%s+0x%llx): Section mismatch: reference to %s:%s\n,
+ merror(%s(%s+0x%llx): Section mismatch: reference to %s:%s\n,
modname, fromsec, (unsigned long long)r.r_offset,
secname, refsymname);
+ err = 1;
}
+out:
+ return err;
}
static unsigned int *reloc_location(struct elf_info *elf,
@@ -997,10 +1004,10 @@ static int addend_mips_rel(struct elf_in
* to find all references to a section that reference a section that will
* be discarded and warns about it.
**/
-static void check_sec_ref(struct module *mod, const char *modname,
- struct elf_info *elf,
- int section(const char*),
- int section_ref_ok(const char *))
+static int check_sec_ref(struct module *mod, const char *modname,
+struct elf_info *elf,
+int section(const char*),
+int section_ref_ok(const char *))
{
int i;
Elf_Sym *sym;
@@ -1049,9 +1056,11 @@ static void check_sec_ref(struct module
secname = secstrings +
sechdrs[sym-st_shndx].sh_name;
- if (section(secname))
-
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Tue, Jan 15, 2008 at 04:17:42PM +0100, Ingo Molnar wrote: * Sam Ravnborg [EMAIL PROTECTED] wrote: find below the current set of warnings on -git. There are 62. The correct figure is 112. You need to do a: make KCFLAGS=-fno-unit-at-a-time build to see them all. btw., please add a .config option to trigger the -fno-unit-at-a-time flags. Something like CONFIG_SECTION_ERRORS=y - plus perhaps combine it with the patch below that turns such section bugs into detectable build errors. A distro does not want to build a kernel that could potentially corrupt kernel memory. (it's a security risk as well.) If we make the err=1 dependent on CONFIG_SECTION_ERRORS then we'll have this configurable. The plan is to let section mismatch warnings become errors after the merge window - so we hit -mm first. And I will add a config option to: - set -fno-unit-at-a-time - add no-inline to all functions marked __init* - and maybe disable __inline if that gives additional errors Slowly getting there. Need to beat modpost in shape to get much less configuration dependent warnings/errors first. Sam -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Tue, Jan 15, 2008 at 05:25:13PM +0100, Sam Ravnborg wrote: On Tue, Jan 15, 2008 at 04:17:42PM +0100, Ingo Molnar wrote: * Sam Ravnborg [EMAIL PROTECTED] wrote: find below the current set of warnings on -git. There are 62. The correct figure is 112. You need to do a: make KCFLAGS=-fno-unit-at-a-time build to see them all. btw., please add a .config option to trigger the -fno-unit-at-a-time flags. Something like CONFIG_SECTION_ERRORS=y - plus perhaps combine it with the patch below that turns such section bugs into detectable build errors. A distro does not want to build a kernel that could potentially corrupt kernel memory. (it's a security risk as well.) If we make the err=1 dependent on CONFIG_SECTION_ERRORS then we'll have this configurable. The plan is to let section mismatch warnings become errors after the merge window - so we hit -mm first. A lot of those I look at seem to be not really bugs; also my impression is that they sometimes crop up randomly. e.g. you change something completely unrelated and suddenly you get a section warning somewhere else. And I will add a config option to: - set -fno-unit-at-a-time I was told future gcc versions would remove that. Why do you want it? - add no-inline to all functions marked __init* - and maybe disable __inline if that gives additional errors I sometimes do that for debugging define static noinline in specific files or similar because it's easier to make sense of oopses when functions not inlined. Not sure it would work as a global option though because if you do it globally then all the inlines in all .hs would be affected and that might lead to immense code bloat. -Andi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Tue, Jan 15, 2008 at 06:11:46PM +0100, Andi Kleen wrote: On Tue, Jan 15, 2008 at 05:25:13PM +0100, Sam Ravnborg wrote: On Tue, Jan 15, 2008 at 04:17:42PM +0100, Ingo Molnar wrote: * Sam Ravnborg [EMAIL PROTECTED] wrote: find below the current set of warnings on -git. There are 62. The correct figure is 112. You need to do a: make KCFLAGS=-fno-unit-at-a-time build to see them all. btw., please add a .config option to trigger the -fno-unit-at-a-time flags. Something like CONFIG_SECTION_ERRORS=y - plus perhaps combine it with the patch below that turns such section bugs into detectable build errors. A distro does not want to build a kernel that could potentially corrupt kernel memory. (it's a security risk as well.) If we make the err=1 dependent on CONFIG_SECTION_ERRORS then we'll have this configurable. The plan is to let section mismatch warnings become errors after the merge window - so we hit -mm first. A lot of those I look at seem to be not really bugs; Half of them are possible Oopses, and the other half are about wasted memory. The warnings in the kernel that are not really bugs you can count with your fingers. also my impression is that they sometimes crop up randomly. e.g. you change something completely unrelated and suddenly you get a section warning somewhere else. ... Not all errors are always visible, they might depend on CONFIG_ options, and sometimes gcc optimizes such bugs away. -Andi cu Adrian -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
The plan is to let section mismatch warnings become errors after the merge window - so we hit -mm first. A lot of those I look at seem to be not really bugs; also my impression is that they sometimes crop up randomly. e.g. you change something completely unrelated and suddenly you get a section warning somewhere else. I have fixed a lot of these warnings. And when I look closer at them they are explainable. The warnings are today very dependent on the configuration and the inlining that gcc uses. With default options to gcc my .config produces ~65 warnings but with -fno-unit-a-time I get 112 warnings. Solely due to less inlining done by gcc. So there are two sources for the 'randomization': a) The actual config b) The sometimes agressive inlining a) will be addressed by having separate sections for each __init* type that is at link time combined where it belongs. b) is addressed by a Kernel Hacking option which 1) uses -fno-unit-at-a-time to get less gcc inlining 2) maybe make all __*init function no-inline 3) maybe disable inlining globally And I will add a config option to: - set -fno-unit-at-a-time I was told future gcc versions would remove that. Why do you want it? Are there any better way to tell gcc no to inline so agressively? Sam -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Tue, Jan 15, 2008 at 07:21:42PM +0100, Sam Ravnborg wrote: With default options to gcc my .config produces ~65 warnings but with -fno-unit-a-time I get 112 warnings. Solely due to less inlining done by gcc. So there are two sources for the 'randomization': a) The actual config b) The sometimes agressive inlining Inlining should not be random. And how does inlining cause such a warning? a) will be addressed by having separate sections for each __init* type that is at link time combined where it belongs. One problem I ran into the past was that older binutils seem to have some exponential behaviour with a lot of named sections and run very slowly. b) is addressed by a Kernel Hacking option which 1) uses -fno-unit-at-a-time to get less gcc inlining 2) maybe make all __*init function no-inline 3) maybe disable inlining globally And I will add a config option to: - set -fno-unit-at-a-time I was told future gcc versions would remove that. Why do you want it? Are there any better way to tell gcc no to inline so agressively? You can either sprinkle noinlines or set specific --params to throttle back the inliner. The later is very gcc version specific unfortunately. -Andi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Tue, Jan 15, 2008 at 07:29:04PM +0100, Andi Kleen wrote:
On Tue, Jan 15, 2008 at 07:21:42PM +0100, Sam Ravnborg wrote:
With default options to gcc my .config produces ~65 warnings
but with -fno-unit-a-time I get 112 warnings.
Solely due to less inlining done by gcc.
So there are two sources for the 'randomization':
a) The actual config
b) The sometimes agressive inlining
Inlining should not be random. And how does inlining cause such a warning?
Consider:
static int __init foo()
{
// ...
}
static int bar()
{
// ...
if (foo())
// ...
}
gcc will often inline foo into bar - and then all code are suddenly
part of .text and no section mismatch.
But you add anohter call to foo() somewhere so gcc decide no longer
to inline foo() and we then have a reference from .text to .init.text.
a) will be addressed by having separate sections for each
__init* type that is at link time combined where it belongs.
One problem I ran into the past was that older binutils seem
to have some exponential behaviour with a lot of named sections
and run very slowly.
This is more the -ffunction-section issue I guess.
What we are dealig with here is ~20 more sections and the kernel has
~100 section today (or more). So not a huge increase.
Sam
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Tue, Jan 15, 2008 at 07:21:42PM +0100, Sam Ravnborg wrote: ... And I will add a config option to: - set -fno-unit-at-a-time I was told future gcc versions would remove that. Why do you want it? Are there any better way to tell gcc no to inline so agressively? I haven't tested it, but -fno-inline-functions-called-once alone _might_ be enough. Sam cu Adrian BTW: I hope it's clear for everyone that disabling such optimizations should only be used for debugging section mismatches, not for production kernels. -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
* Adrian Bunk [EMAIL PROTECTED] wrote: make ARCH=x86_64 allyesconfig will set CONFIG_64BIT for you - no? Yes. But this still leaves the fact that when someone says 'allyesconfig' it's no longer clear which configuration he has. And no, I do not consider it funny that this now implies two hours of compilation twice just for seeing one bug. sorry - i should have mentioned that it's 64-bit. (and i even mis-remembered having seen it on 32-bit as well) Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Tue, Jan 15, 2008 at 11:14:09PM +0100, Ingo Molnar wrote: * Adrian Bunk [EMAIL PROTECTED] wrote: make ARCH=x86_64 allyesconfig will set CONFIG_64BIT for you - no? Yes. But this still leaves the fact that when someone says 'allyesconfig' it's no longer clear which configuration he has. And no, I do not consider it funny that this now implies two hours of compilation twice just for seeing one bug. sorry - i should have mentioned that it's 64-bit. (and i even mis-remembered having seen it on 32-bit as well) This bug seems to also be present on 32bit, but allyesconfig minus CONFIG_HOTPLUG is not among the configurations where it's present on 32bit. Ingo cu Adrian -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Mon, Jan 14, 2008 at 09:27:40PM +0100, Sam Ravnborg wrote: > > > > Can you fix the bug that menuconfig does not let me enable CONFIG_64BIT? > > > > > > make ARCH=x86_64 allyesconfig > > > will set CONFIG_64BIT for you - no? > > > > Yes. > > > > But this still leaves the fact that when someone says 'allyesconfig' > > it's no longer clear which configuration he has. > Assuming you are on an x86 box... > > make allyesconfig will give you a config for same OS bit-size as you run. > make ARCH=i386 allyesconfig gives you a 32 bit config > make ARCH=x86_64 allyesconfig gives you a 64 bit config > > make ARCH=x86 allyesconfig gives you a 32 bit config where you can select 64 > bit. > > It was seen as the best solution when unifying 32 and 64 bit stuff > and introducing support for ARCH=x86. > > No-one has complained until now so most people seems to get it > or maybe they are just silent. Or I'm the only person doing kernel development on a 32bit x86 machine? Even oldconfig is busted since the Kconfig choice is not available without explicitely setting ARCH. > Sam cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
> > > Can you fix the bug that menuconfig does not let me enable CONFIG_64BIT? > > > > make ARCH=x86_64 allyesconfig > > will set CONFIG_64BIT for you - no? > > Yes. > > But this still leaves the fact that when someone says 'allyesconfig' > it's no longer clear which configuration he has. Assuming you are on an x86 box... make allyesconfig will give you a config for same OS bit-size as you run. make ARCH=i386 allyesconfig gives you a 32 bit config make ARCH=x86_64 allyesconfig gives you a 64 bit config make ARCH=x86 allyesconfig gives you a 32 bit config where you can select 64 bit. It was seen as the best solution when unifying 32 and 64 bit stuff and introducing support for ARCH=x86. No-one has complained until now so most people seems to get it or maybe they are just silent. Sam -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Mon, Jan 14, 2008 at 09:01:26PM +0100, Sam Ravnborg wrote: > On Mon, Jan 14, 2008 at 09:52:58PM +0200, Adrian Bunk wrote: > > On Mon, Jan 14, 2008 at 08:10:09PM +0100, Ingo Molnar wrote: > > > > > > * Adrian Bunk <[EMAIL PROTECTED]> wrote: > > > > > > > > for example, in current -git, could you tell me why this triggers: > > > > > > > > > > WARNING: vmlinux.o(.text+0x87e2a): Section mismatch: reference to > > > > > .init.text: (between 'process_zones' and > > > > > 'setup_per_cpu_pageset') > > > > > > > > > > and how to resolve it? I had a quick look and it was not obvious to > > > > > me. > > > > > > > > Please send the .config. > > > > > > 'make allyesconfig' and disable CONFIG_HOTPLUG=y. > > > > Works for me without the warning. > > > > Wait, I remember something @[EMAIL PROTECTED]: > > 'make allyesconfig' on x86 became ambiguously. > > > > If you have by chance a 64bit CPU that would explain why I didn't get it. > > > > @Sam: > > Can you fix the bug that menuconfig does not let me enable CONFIG_64BIT? > > make ARCH=x86_64 allyesconfig > will set CONFIG_64BIT for you - no? Yes. But this still leaves the fact that when someone says 'allyesconfig' it's no longer clear which configuration he has. And no, I do not consider it funny that this now implies two hours of compilation twice just for seeing one bug. And the fact that I have to set ARCH only for seeing the 32/64bit Kconfig choice is clearly bullshit. > Sam cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Mon, Jan 14, 2008 at 04:24:40PM +0100, Ingo Molnar wrote: > > * Ingo Molnar <[EMAIL PROTECTED]> wrote: > > > also, in theory we've got a pretty reliable set of the following > > information: > > > > function X references symbol Y > > > > and we know what type of sections they are in, right? > > > > could -ffunction-sections be used to delay the categorization of > > functions to the link stage, and in essence remove the need to mark > > functions via any of the __init markers? > > find below the current set of warnings on -git. There are 62. The correct figure is 112. You need to do a: make KCFLAGS=-fno-unit-at-a-time build to see them all. > > for example, instead of the rather cryptic: > > WARNING: vmlinux.o(.text+0x1815e): Section mismatch: reference to > .init.text:calibrate_delay (between 'smp_callin' and '__cpu_up') > > the following output would be more informative: > > -> > | function: > | > | ./init/calibrate.c:void __devinit calibrate_delay(void) > | > | calls: > | > | ./arch/x86/kernel/smpboot_32.c:static void __cpuinit smp_callin(void) > | > | but calibrate_delay() is __devinit while smp_callin() is __cpuinit. > | Change smp_callin() to __devinit to resolve this warning. > |> > > would result in a lot faster cycles of fixing this. > > do we have all the info to print this? Not yet. The patch I posted on lkml bring us one step further. At modpost time today we only see .init.text and .text but with the suggested patch we will be able to see what section a function belong to and we are several steps closer to better error messages. So let me get that into shape and I will revisit the messages. In this case I would not know if calibrate_delay() should be __cpuinit or smp_callin() should be __devinit looking at the information modpost has available without additional digging. But a quick grep tells me that the right fix is to annotate calibrate_delay() with __cpuinit because is is used from either __cpuinit annotated or __init annotated functions. Sam -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Mon, Jan 14, 2008 at 09:52:58PM +0200, Adrian Bunk wrote: > On Mon, Jan 14, 2008 at 08:10:09PM +0100, Ingo Molnar wrote: > > > > * Adrian Bunk <[EMAIL PROTECTED]> wrote: > > > > > > for example, in current -git, could you tell me why this triggers: > > > > > > > > WARNING: vmlinux.o(.text+0x87e2a): Section mismatch: reference to > > > > .init.text: (between 'process_zones' and > > > > 'setup_per_cpu_pageset') > > > > > > > > and how to resolve it? I had a quick look and it was not obvious to me. > > > > > > Please send the .config. > > > > 'make allyesconfig' and disable CONFIG_HOTPLUG=y. > > Works for me without the warning. > > Wait, I remember something @[EMAIL PROTECTED]: > 'make allyesconfig' on x86 became ambiguously. > > If you have by chance a 64bit CPU that would explain why I didn't get it. > > @Sam: > Can you fix the bug that menuconfig does not let me enable CONFIG_64BIT? make ARCH=x86_64 allyesconfig will set CONFIG_64BIT for you - no? Sam -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Mon, Jan 14, 2008 at 04:01:03PM +0100, Ingo Molnar wrote:
>
> * Ingo Molnar <[EMAIL PROTECTED]> wrote:
>
> > > Would be great to have them automated - just dunno how to do it. Do
> > > you see a feasible way to do it?
> >
> > a good starting point would be to make the warnings a lot more
> > self-explanatory. Right now it's often non-obvious trying to figure
> > out how the dependencies are structured and which one should be
> > changed to get rid of the bug.
>
> for example, in current -git, could you tell me why this triggers:
>
> WARNING: vmlinux.o(.text+0x87e2a): Section mismatch: reference to
> .init.text: (between 'process_zones' and 'setup_per_cpu_pageset')
>
> and how to resolve it? I had a quick look and it was not obvious to me.
I was confused by your error message - it looked all wrong.
process_zones is .text but setup_per_cpu_pageset is __init. I expect you
had some local changes.
So I tried myself and got this warning:
WARNING: mm/built-in.o(.text+0x6864): Section mismatch: reference to
.init.text: (between 'process_zones' and 'pageset_cpuup_callback')
This made much more sense.
So I looked closely at process_zones() and the first
thing I always do is to check all the local functions.
I noticed that we use the function zone_batchsize() which is marked __devinit.
A function marked __cpuinit may use other functions marked __cpuinit, data
marked
__cpuinitdata and .text/.data. But references to __devinit is not ok.
I furthermore noticed that zone_batchsize() were used in anohter
function marked __meminit.
So the simple fix for this warning is to remove the annotation of
zone_batchsize.
It looks like a real opps candidate to me..
Why modpost did not pick up the zone_batchsize symbol is anohter matter.
It is present in the file:
$ objdump --syms vmlinux.o | grep zone_batchsize
00016929 l F .init.text 0053 zone_batchsize
Debugging modpost I could see that we had an addend value of 695,
but the addr of the symbol is 699. So somehow we point 4 bytes wrong.
Strange...
Anyway - here follows the patch.
Sam
[PATCH] mm: fix section mismatch warning in page_alloc.c
With CONFIG_HOTPLUG=n and CONFIG_HOTPLUG_CPU=y we saw
following warning:
WARNING: mm/built-in.o(.text+0x6864): Section mismatch: reference to
.init.text: (between 'process_zones' and 'pageset_cpuup_callback')
The culprint was zone_batchsize() which were annotated
__devinit but used from process_zones() which is annotated __cpuinit.
zone_batchsize() are used from another function annotated __meminit
so the only valid option is to drop the annotation of
zone_batchsize() so we know it is always valid to use it.
Signed-off-by: Sam Ravnborg <[EMAIL PROTECTED]>
---
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index e1028fa..b2838c2 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2566,7 +2566,7 @@ static void __meminit zone_init_free_lists(struct
pglist_data *pgdat,
memmap_init_zone((size), (nid), (zone), (start_pfn), MEMMAP_EARLY)
#endif
-static int __devinit zone_batchsize(struct zone *zone)
+static int zone_batchsize(struct zone *zone)
{
int batch;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Mon, Jan 14, 2008 at 08:10:09PM +0100, Ingo Molnar wrote: > > * Adrian Bunk <[EMAIL PROTECTED]> wrote: > > > > for example, in current -git, could you tell me why this triggers: > > > > > > WARNING: vmlinux.o(.text+0x87e2a): Section mismatch: reference to > > > .init.text: (between 'process_zones' and > > > 'setup_per_cpu_pageset') > > > > > > and how to resolve it? I had a quick look and it was not obvious to me. > > > > Please send the .config. > > 'make allyesconfig' and disable CONFIG_HOTPLUG=y. Works for me without the warning. Wait, I remember something @[EMAIL PROTECTED]: 'make allyesconfig' on x86 became ambiguously. If you have by chance a 64bit CPU that would explain why I didn't get it. @Sam: Can you fix the bug that menuconfig does not let me enable CONFIG_64BIT? > Ingo cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
* Adrian Bunk <[EMAIL PROTECTED]> wrote: > > for example, in current -git, could you tell me why this triggers: > > > > WARNING: vmlinux.o(.text+0x87e2a): Section mismatch: reference to > > .init.text: (between 'process_zones' and 'setup_per_cpu_pageset') > > > > and how to resolve it? I had a quick look and it was not obvious to me. > > Please send the .config. 'make allyesconfig' and disable CONFIG_HOTPLUG=y. Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Mon, Jan 14, 2008 at 04:01:03PM +0100, Ingo Molnar wrote: > > * Ingo Molnar <[EMAIL PROTECTED]> wrote: > > > > Would be great to have them automated - just dunno how to do it. Do > > > you see a feasible way to do it? > > > > a good starting point would be to make the warnings a lot more > > self-explanatory. Right now it's often non-obvious trying to figure > > out how the dependencies are structured and which one should be > > changed to get rid of the bug. > > for example, in current -git, could you tell me why this triggers: > > WARNING: vmlinux.o(.text+0x87e2a): Section mismatch: reference to > .init.text: (between 'process_zones' and 'setup_per_cpu_pageset') > > and how to resolve it? I had a quick look and it was not obvious to me. Please send the .config. > Ingo cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Mon, Jan 14, 2008 at 03:58:54PM +0100, Ingo Molnar wrote: >... > a good way i see is to: >... > - quickly reach a close-to-100%-perfect stage, brute-force. Drop >__init* annotations en masse if they are not perfectly layered. >Whoever reintroduces them will then have to do it perfectly. >... First of all, this would really hurt space limited systems. And "whoever reintroduces them" will most likely mean in practice that some janitors will go through the code and you as a maintainer will anyway be busy with getting them in shape. Let's get them rightonce and we have a reasonable status quo. > Ingo cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Mon, Jan 14, 2008 at 02:52:40PM +0100, Ingo Molnar wrote: >... > but longer-term, shouldnt these annotations be automated? We'll see a > constant stream of them, all around the clock as people regularly get it > wrong (because it's not intuitive). Definitely. Even more when you looking at what Maciej Rozycki suggested regarding discardable strings. [1] But _how_ can we get this automated at all? > Ingo cu Adrian [1] http://lkml.org/lkml/2007/10/12/297 -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
* Ingo Molnar <[EMAIL PROTECTED]> wrote:
> also, in theory we've got a pretty reliable set of the following
> information:
>
> function X references symbol Y
>
> and we know what type of sections they are in, right?
>
> could -ffunction-sections be used to delay the categorization of
> functions to the link stage, and in essence remove the need to mark
> functions via any of the __init markers?
find below the current set of warnings on -git. There are 62.
for example, instead of the rather cryptic:
WARNING: vmlinux.o(.text+0x1815e): Section mismatch: reference to
.init.text:calibrate_delay (between 'smp_callin' and '__cpu_up')
the following output would be more informative:
->
| function:
|
| ./init/calibrate.c:void __devinit calibrate_delay(void)
|
| calls:
|
| ./arch/x86/kernel/smpboot_32.c:static void __cpuinit smp_callin(void)
|
| but calibrate_delay() is __devinit while smp_callin() is __cpuinit.
| Change smp_callin() to __devinit to resolve this warning.
|>
would result in a lot faster cycles of fixing this.
do we have all the info to print this?
Ingo
.init.text: calibrate_delay ('smp_callin' <-> '__cpu_up')
.init.text: register_cpu ('arch_register_cpu' <-> 'in_gate_area_no_task')
.init.text: idle_regs ('fork_idle' <-> 'get_task_mm')
.init.text: ('process_zones' <-> 'pageset_cpuup_callback')
.init.text: pcibios_fixup_bus ('pci_scan_child_bus' <-> 'pci_scan_bus_parented')
.init.data: mtrr ('param_set_scroll' <-> 'uvesafb_cn_callback')
.init.data: mtrr ('param_set_scroll' <-> 'uvesafb_cn_callback')
.init.data: pmi_setpal ('param_set_scroll' <-> 'uvesafb_cn_callback')
.init.data: pmi_setpal ('param_set_scroll' <-> 'uvesafb_cn_callback')
.init.text: pci_acpi_scan_root ('acpi_pci_root_add' <->
'acpi_pci_unregister_driver')
.init.text: ('olympic_open' <-> 'olympic_interrupt')
.init.text: setup_teles3 ('checkcard' <-> 'hisax_sched_event')
.init.text: setup_s0box ('checkcard' <-> 'hisax_sched_event')
.init.text: setup_telespci ('checkcard' <-> 'hisax_sched_event')
.init.text: setup_avm_pcipnp ('checkcard' <-> 'hisax_sched_event')
.init.text: setup_elsa ('checkcard' <-> 'hisax_sched_event')
.init.text: setup_diva ('checkcard' <-> 'hisax_sched_event')
.init.text: setup_sedlbauer ('checkcard' <-> 'hisax_sched_event')
.init.text: setup_netjet_s ('checkcard' <-> 'hisax_sched_event')
.init.text: setup_hfcpci ('checkcard' <-> 'hisax_sched_event')
.init.text: setup_hfcsx ('checkcard' <-> 'hisax_sched_event')
.init.text: setup_niccy ('checkcard' <-> 'hisax_sched_event')
.init.text: setup_bkm_a4t ('checkcard' <-> 'hisax_sched_event')
.init.text: setup_sct_quadro ('checkcard' <-> 'hisax_sched_event')
.init.text: setup_gazel ('checkcard' <-> 'hisax_sched_event')
.init.text: setup_w6692 ('checkcard' <-> 'hisax_sched_event')
.init.text: setup_netjet_u ('checkcard' <-> 'hisax_sched_event')
.init.text: setup_enternow_pci ('checkcard' <-> 'hisax_sched_event')
.init.data: ISACVer ('ISACVersion' <-> 'DC_Close_isac')
.init.text: clear_pending_isac_ints ('inithscxisac' <-> 'HscxVersion')
.init.text: initisac ('inithscxisac' <-> 'HscxVersion')
.init.text: clear_pending_isac_ints ('AVM_card_msg' <-> 'setup_avm_a1_pcmcia')
.init.text: setup_isac ('setup_avm_a1_pcmcia' <-> 'avm_a1p_interrupt')
.init.text: clear_pending_isac_ints ('AVM_card_msg' <-> 'ReadISACfifo_IPAC')
.init.text: initisac ('AVM_card_msg' <-> 'ReadISACfifo_IPAC')
.init.text: clear_pending_isac_ints ('Sedl_card_msg' <-> 'WriteISACfifo')
.init.text: initisac ('Sedl_card_msg' <-> 'WriteISACfifo')
.init.text: initisar ('Sedl_card_msg' <-> 'WriteISACfifo')
.init.text: clear_pending_isac_ints ('NETjet_S_card_msg' <->
'netjet_s_interrupt')
.init.text: initisac ('NETjet_S_card_msg' <-> 'netjet_s_interrupt')
.init.text: clear_pending_isac_ints ('BKM_card_msg' <-> 'ReadISAC')
.init.text: initisac ('BKM_card_msg' <-> 'ReadISAC')
.init.text: Amd7930_init ('enpci_card_msg' <-> 'ReadWordAmd7930')
.init.text: Amd7930_init ('enpci_card_msg' <-> 'ReadWordAmd7930')
.init.text: snd_usb_caiaq_audio_init ('snd_probe' <->
'usb_ep1_command_reply_dispatch')
.init.text: snd_usb_caiaq_midi_init ('snd_probe' <->
'usb_ep1_command_reply_dispatch')
.init.data: _asc_def_iop_base ('advansys_isa_remove' <-> 'advansys_exit')
.init.text: suni_init ('__ksymtab_suni_init' <-> '__ksymtab_scsi_device_lookup')
.init.text: profile_cpu_callback ('profile_cpu_callback_nb.19579' <->
'prof_cpu_mask')
.init.text: workqueue_cpu_callback ('workqueue_cpu_callback_nb.12756' <->
'workqueue_mutex')
.init.text: cpu_callback ('cpu_callback_nb.23833' <-> 'shrinker_rwsem')
.init.text: tpm_inf_pnp_probe ('tpm_inf_pnp' <-> 'inf_attr_grp')
.init.data: prism2_plx_id_table ('prism2_plx_drv_id' <-> 'prism2_plx_funcs')
.init.text: asd_aic9410_setup ('asd_pcidev_data' <-> 'dev_attr_revision')
.init.text: asd_aic9410_setup ('asd_pcidev_data' <-> 'dev_attr_revision')
.init.text: asd_aic9405_setup ('asd_pcidev_data' <->
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
* Ingo Molnar <[EMAIL PROTECTED]> wrote: > > Would be great to have them automated - just dunno how to do it. Do > > you see a feasible way to do it? > > a good starting point would be to make the warnings a lot more > self-explanatory. Right now it's often non-obvious trying to figure > out how the dependencies are structured and which one should be > changed to get rid of the bug. also, in theory we've got a pretty reliable set of the following information: function X references symbol Y and we know what type of sections they are in, right? could -ffunction-sections be used to delay the categorization of functions to the link stage, and in essence remove the need to mark functions via any of the __init markers? Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
* Ingo Molnar <[EMAIL PROTECTED]> wrote: > > Would be great to have them automated - just dunno how to do it. Do > > you see a feasible way to do it? > > a good starting point would be to make the warnings a lot more > self-explanatory. Right now it's often non-obvious trying to figure > out how the dependencies are structured and which one should be > changed to get rid of the bug. for example, in current -git, could you tell me why this triggers: WARNING: vmlinux.o(.text+0x87e2a): Section mismatch: reference to .init.text: (between 'process_zones' and 'setup_per_cpu_pageset') and how to resolve it? I had a quick look and it was not obvious to me. Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
* Sam Ravnborg <[EMAIL PROTECTED]> wrote: > > ok, i've dropped this patch from x86.git for now: > > > > Subject: x86: force __cpuinit on for CONFIG_PM without HOTPLUG_CPU > > From: Andi Kleen <[EMAIL PROTECTED]> > > > > but longer-term, shouldnt these annotations be automated? We'll see > > a constant stream of them, all around the clock as people regularly > > get it wrong (because it's not intuitive). > > Would be great to have them automated - just dunno how to do it. Do > you see a feasible way to do it? a good starting point would be to make the warnings a lot more self-explanatory. Right now it's often non-obvious trying to figure out how the dependencies are structured and which one should be changed to get rid of the bug. > Short term modpost etc. will be enhanced to be less dependent on the > actual configuration when performing the checks. It should not matter > if CONFIG_HOTPLUG_CPU is enabled or not when we check the __cpuint > annotations but this is how we see it today so far too many faults > slip through. a good way i see is to: - improve the debug output so that it's obvious at a glance what the problem is. - quickly reach a close-to-100%-perfect stage, brute-force. Drop __init* annotations en masse if they are not perfectly layered. Whoever reintroduces them will then have to do it perfectly. - then turn these into hard failures (which they _are_ in a significant percentage of the situations). once it's a hard build failure, people will fix them quickly and automated tests will pick them up as well. Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Mon, Jan 14, 2008 at 02:52:40PM +0100, Ingo Molnar wrote: > > * Adrian Bunk <[EMAIL PROTECTED]> wrote: > > > On Thu, Jan 10, 2008 at 02:12:59PM +0100, Andi Kleen wrote: > > > Adrian Bunk <[EMAIL PROTECTED]> writes: > > > > > > > > Technically you are the one who has to deal with problems in your > > > > patches, not the people pointing at the problems. > > > > > > If you believe that my patch adds a new problem then please describe > > > it clearly so that I can understand it. > > > > Description: > > - there are already __cpuinit* annotations in the kernel > > - on UP kernels supporting suspend/resume, such annotated code > > currently gets freed after booting (and this works) > > - with your patch applied, this code no longer gets freed > > ok, i've dropped this patch from x86.git for now: > > Subject: x86: force __cpuinit on for CONFIG_PM without HOTPLUG_CPU > From: Andi Kleen <[EMAIL PROTECTED]> > > but longer-term, shouldnt these annotations be automated? We'll see a > constant stream of them, all around the clock as people regularly get it > wrong (because it's not intuitive). Would be great to have them automated - just dunno how to do it. Do you see a feasible way to do it? Short term modpost etc. will be enhanced to be less dependent on the actual configuration when performing the checks. It should not matter if CONFIG_HOTPLUG_CPU is enabled or not when we check the __cpuint annotations but this is how we see it today so far too many faults slip through. Sam -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
* Adrian Bunk <[EMAIL PROTECTED]> wrote: > On Thu, Jan 10, 2008 at 02:12:59PM +0100, Andi Kleen wrote: > > Adrian Bunk <[EMAIL PROTECTED]> writes: > > > > > > Technically you are the one who has to deal with problems in your > > > patches, not the people pointing at the problems. > > > > If you believe that my patch adds a new problem then please describe > > it clearly so that I can understand it. > > Description: > - there are already __cpuinit* annotations in the kernel > - on UP kernels supporting suspend/resume, such annotated code > currently gets freed after booting (and this works) > - with your patch applied, this code no longer gets freed ok, i've dropped this patch from x86.git for now: Subject: x86: force __cpuinit on for CONFIG_PM without HOTPLUG_CPU From: Andi Kleen <[EMAIL PROTECTED]> but longer-term, shouldnt these annotations be automated? We'll see a constant stream of them, all around the clock as people regularly get it wrong (because it's not intuitive). Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
* Adrian Bunk [EMAIL PROTECTED] wrote: On Thu, Jan 10, 2008 at 02:12:59PM +0100, Andi Kleen wrote: Adrian Bunk [EMAIL PROTECTED] writes: Technically you are the one who has to deal with problems in your patches, not the people pointing at the problems. If you believe that my patch adds a new problem then please describe it clearly so that I can understand it. Description: - there are already __cpuinit* annotations in the kernel - on UP kernels supporting suspend/resume, such annotated code currently gets freed after booting (and this works) - with your patch applied, this code no longer gets freed ok, i've dropped this patch from x86.git for now: Subject: x86: force __cpuinit on for CONFIG_PM without HOTPLUG_CPU From: Andi Kleen [EMAIL PROTECTED] but longer-term, shouldnt these annotations be automated? We'll see a constant stream of them, all around the clock as people regularly get it wrong (because it's not intuitive). Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Mon, Jan 14, 2008 at 02:52:40PM +0100, Ingo Molnar wrote: * Adrian Bunk [EMAIL PROTECTED] wrote: On Thu, Jan 10, 2008 at 02:12:59PM +0100, Andi Kleen wrote: Adrian Bunk [EMAIL PROTECTED] writes: Technically you are the one who has to deal with problems in your patches, not the people pointing at the problems. If you believe that my patch adds a new problem then please describe it clearly so that I can understand it. Description: - there are already __cpuinit* annotations in the kernel - on UP kernels supporting suspend/resume, such annotated code currently gets freed after booting (and this works) - with your patch applied, this code no longer gets freed ok, i've dropped this patch from x86.git for now: Subject: x86: force __cpuinit on for CONFIG_PM without HOTPLUG_CPU From: Andi Kleen [EMAIL PROTECTED] but longer-term, shouldnt these annotations be automated? We'll see a constant stream of them, all around the clock as people regularly get it wrong (because it's not intuitive). Would be great to have them automated - just dunno how to do it. Do you see a feasible way to do it? Short term modpost etc. will be enhanced to be less dependent on the actual configuration when performing the checks. It should not matter if CONFIG_HOTPLUG_CPU is enabled or not when we check the __cpuint annotations but this is how we see it today so far too many faults slip through. Sam -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
* Ingo Molnar [EMAIL PROTECTED] wrote: Would be great to have them automated - just dunno how to do it. Do you see a feasible way to do it? a good starting point would be to make the warnings a lot more self-explanatory. Right now it's often non-obvious trying to figure out how the dependencies are structured and which one should be changed to get rid of the bug. for example, in current -git, could you tell me why this triggers: WARNING: vmlinux.o(.text+0x87e2a): Section mismatch: reference to .init.text: (between 'process_zones' and 'setup_per_cpu_pageset') and how to resolve it? I had a quick look and it was not obvious to me. Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
* Ingo Molnar [EMAIL PROTECTED] wrote:
also, in theory we've got a pretty reliable set of the following
information:
function X references symbol Y
and we know what type of sections they are in, right?
could -ffunction-sections be used to delay the categorization of
functions to the link stage, and in essence remove the need to mark
functions via any of the __init markers?
find below the current set of warnings on -git. There are 62.
for example, instead of the rather cryptic:
WARNING: vmlinux.o(.text+0x1815e): Section mismatch: reference to
.init.text:calibrate_delay (between 'smp_callin' and '__cpu_up')
the following output would be more informative:
-
| function:
|
| ./init/calibrate.c:void __devinit calibrate_delay(void)
|
| calls:
|
| ./arch/x86/kernel/smpboot_32.c:static void __cpuinit smp_callin(void)
|
| but calibrate_delay() is __devinit while smp_callin() is __cpuinit.
| Change smp_callin() to __devinit to resolve this warning.
|
would result in a lot faster cycles of fixing this.
do we have all the info to print this?
Ingo
.init.text: calibrate_delay ('smp_callin' - '__cpu_up')
.init.text: register_cpu ('arch_register_cpu' - 'in_gate_area_no_task')
.init.text: idle_regs ('fork_idle' - 'get_task_mm')
.init.text: ('process_zones' - 'pageset_cpuup_callback')
.init.text: pcibios_fixup_bus ('pci_scan_child_bus' - 'pci_scan_bus_parented')
.init.data: mtrr ('param_set_scroll' - 'uvesafb_cn_callback')
.init.data: mtrr ('param_set_scroll' - 'uvesafb_cn_callback')
.init.data: pmi_setpal ('param_set_scroll' - 'uvesafb_cn_callback')
.init.data: pmi_setpal ('param_set_scroll' - 'uvesafb_cn_callback')
.init.text: pci_acpi_scan_root ('acpi_pci_root_add' -
'acpi_pci_unregister_driver')
.init.text: ('olympic_open' - 'olympic_interrupt')
.init.text: setup_teles3 ('checkcard' - 'hisax_sched_event')
.init.text: setup_s0box ('checkcard' - 'hisax_sched_event')
.init.text: setup_telespci ('checkcard' - 'hisax_sched_event')
.init.text: setup_avm_pcipnp ('checkcard' - 'hisax_sched_event')
.init.text: setup_elsa ('checkcard' - 'hisax_sched_event')
.init.text: setup_diva ('checkcard' - 'hisax_sched_event')
.init.text: setup_sedlbauer ('checkcard' - 'hisax_sched_event')
.init.text: setup_netjet_s ('checkcard' - 'hisax_sched_event')
.init.text: setup_hfcpci ('checkcard' - 'hisax_sched_event')
.init.text: setup_hfcsx ('checkcard' - 'hisax_sched_event')
.init.text: setup_niccy ('checkcard' - 'hisax_sched_event')
.init.text: setup_bkm_a4t ('checkcard' - 'hisax_sched_event')
.init.text: setup_sct_quadro ('checkcard' - 'hisax_sched_event')
.init.text: setup_gazel ('checkcard' - 'hisax_sched_event')
.init.text: setup_w6692 ('checkcard' - 'hisax_sched_event')
.init.text: setup_netjet_u ('checkcard' - 'hisax_sched_event')
.init.text: setup_enternow_pci ('checkcard' - 'hisax_sched_event')
.init.data: ISACVer ('ISACVersion' - 'DC_Close_isac')
.init.text: clear_pending_isac_ints ('inithscxisac' - 'HscxVersion')
.init.text: initisac ('inithscxisac' - 'HscxVersion')
.init.text: clear_pending_isac_ints ('AVM_card_msg' - 'setup_avm_a1_pcmcia')
.init.text: setup_isac ('setup_avm_a1_pcmcia' - 'avm_a1p_interrupt')
.init.text: clear_pending_isac_ints ('AVM_card_msg' - 'ReadISACfifo_IPAC')
.init.text: initisac ('AVM_card_msg' - 'ReadISACfifo_IPAC')
.init.text: clear_pending_isac_ints ('Sedl_card_msg' - 'WriteISACfifo')
.init.text: initisac ('Sedl_card_msg' - 'WriteISACfifo')
.init.text: initisar ('Sedl_card_msg' - 'WriteISACfifo')
.init.text: clear_pending_isac_ints ('NETjet_S_card_msg' -
'netjet_s_interrupt')
.init.text: initisac ('NETjet_S_card_msg' - 'netjet_s_interrupt')
.init.text: clear_pending_isac_ints ('BKM_card_msg' - 'ReadISAC')
.init.text: initisac ('BKM_card_msg' - 'ReadISAC')
.init.text: Amd7930_init ('enpci_card_msg' - 'ReadWordAmd7930')
.init.text: Amd7930_init ('enpci_card_msg' - 'ReadWordAmd7930')
.init.text: snd_usb_caiaq_audio_init ('snd_probe' -
'usb_ep1_command_reply_dispatch')
.init.text: snd_usb_caiaq_midi_init ('snd_probe' -
'usb_ep1_command_reply_dispatch')
.init.data: _asc_def_iop_base ('advansys_isa_remove' - 'advansys_exit')
.init.text: suni_init ('__ksymtab_suni_init' - '__ksymtab_scsi_device_lookup')
.init.text: profile_cpu_callback ('profile_cpu_callback_nb.19579' -
'prof_cpu_mask')
.init.text: workqueue_cpu_callback ('workqueue_cpu_callback_nb.12756' -
'workqueue_mutex')
.init.text: cpu_callback ('cpu_callback_nb.23833' - 'shrinker_rwsem')
.init.text: tpm_inf_pnp_probe ('tpm_inf_pnp' - 'inf_attr_grp')
.init.data: prism2_plx_id_table ('prism2_plx_drv_id' - 'prism2_plx_funcs')
.init.text: asd_aic9410_setup ('asd_pcidev_data' - 'dev_attr_revision')
.init.text: asd_aic9410_setup ('asd_pcidev_data' - 'dev_attr_revision')
.init.text: asd_aic9405_setup ('asd_pcidev_data' - 'dev_attr_revision')
.init.text: megaraid_probe_one ('megaraid_pci_driver_g' -
'megaraid_template_g')
.init.text: snd_ad1889_probe
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Mon, Jan 14, 2008 at 02:52:40PM +0100, Ingo Molnar wrote: ... but longer-term, shouldnt these annotations be automated? We'll see a constant stream of them, all around the clock as people regularly get it wrong (because it's not intuitive). Definitely. Even more when you looking at what Maciej Rozycki suggested regarding discardable strings. [1] But _how_ can we get this automated at all? Ingo cu Adrian [1] http://lkml.org/lkml/2007/10/12/297 -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
* Sam Ravnborg [EMAIL PROTECTED] wrote: ok, i've dropped this patch from x86.git for now: Subject: x86: force __cpuinit on for CONFIG_PM without HOTPLUG_CPU From: Andi Kleen [EMAIL PROTECTED] but longer-term, shouldnt these annotations be automated? We'll see a constant stream of them, all around the clock as people regularly get it wrong (because it's not intuitive). Would be great to have them automated - just dunno how to do it. Do you see a feasible way to do it? a good starting point would be to make the warnings a lot more self-explanatory. Right now it's often non-obvious trying to figure out how the dependencies are structured and which one should be changed to get rid of the bug. Short term modpost etc. will be enhanced to be less dependent on the actual configuration when performing the checks. It should not matter if CONFIG_HOTPLUG_CPU is enabled or not when we check the __cpuint annotations but this is how we see it today so far too many faults slip through. a good way i see is to: - improve the debug output so that it's obvious at a glance what the problem is. - quickly reach a close-to-100%-perfect stage, brute-force. Drop __init* annotations en masse if they are not perfectly layered. Whoever reintroduces them will then have to do it perfectly. - then turn these into hard failures (which they _are_ in a significant percentage of the situations). once it's a hard build failure, people will fix them quickly and automated tests will pick them up as well. Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
* Adrian Bunk [EMAIL PROTECTED] wrote: for example, in current -git, could you tell me why this triggers: WARNING: vmlinux.o(.text+0x87e2a): Section mismatch: reference to .init.text: (between 'process_zones' and 'setup_per_cpu_pageset') and how to resolve it? I had a quick look and it was not obvious to me. Please send the .config. 'make allyesconfig' and disable CONFIG_HOTPLUG=y. Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Mon, Jan 14, 2008 at 08:10:09PM +0100, Ingo Molnar wrote: * Adrian Bunk [EMAIL PROTECTED] wrote: for example, in current -git, could you tell me why this triggers: WARNING: vmlinux.o(.text+0x87e2a): Section mismatch: reference to .init.text: (between 'process_zones' and 'setup_per_cpu_pageset') and how to resolve it? I had a quick look and it was not obvious to me. Please send the .config. 'make allyesconfig' and disable CONFIG_HOTPLUG=y. Works for me without the warning. Wait, I remember something @[EMAIL PROTECTED]: 'make allyesconfig' on x86 became ambiguously. If you have by chance a 64bit CPU that would explain why I didn't get it. @Sam: Can you fix the bug that menuconfig does not let me enable CONFIG_64BIT? Ingo cu Adrian -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Mon, Jan 14, 2008 at 04:24:40PM +0100, Ingo Molnar wrote: * Ingo Molnar [EMAIL PROTECTED] wrote: also, in theory we've got a pretty reliable set of the following information: function X references symbol Y and we know what type of sections they are in, right? could -ffunction-sections be used to delay the categorization of functions to the link stage, and in essence remove the need to mark functions via any of the __init markers? find below the current set of warnings on -git. There are 62. The correct figure is 112. You need to do a: make KCFLAGS=-fno-unit-at-a-time build to see them all. for example, instead of the rather cryptic: WARNING: vmlinux.o(.text+0x1815e): Section mismatch: reference to .init.text:calibrate_delay (between 'smp_callin' and '__cpu_up') the following output would be more informative: - | function: | | ./init/calibrate.c:void __devinit calibrate_delay(void) | | calls: | | ./arch/x86/kernel/smpboot_32.c:static void __cpuinit smp_callin(void) | | but calibrate_delay() is __devinit while smp_callin() is __cpuinit. | Change smp_callin() to __devinit to resolve this warning. | would result in a lot faster cycles of fixing this. do we have all the info to print this? Not yet. The patch I posted on lkml bring us one step further. At modpost time today we only see .init.text and .text but with the suggested patch we will be able to see what section a function belong to and we are several steps closer to better error messages. So let me get that into shape and I will revisit the messages. In this case I would not know if calibrate_delay() should be __cpuinit or smp_callin() should be __devinit looking at the information modpost has available without additional digging. But a quick grep tells me that the right fix is to annotate calibrate_delay() with __cpuinit because is is used from either __cpuinit annotated or __init annotated functions. Sam -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Mon, Jan 14, 2008 at 09:01:26PM +0100, Sam Ravnborg wrote: On Mon, Jan 14, 2008 at 09:52:58PM +0200, Adrian Bunk wrote: On Mon, Jan 14, 2008 at 08:10:09PM +0100, Ingo Molnar wrote: * Adrian Bunk [EMAIL PROTECTED] wrote: for example, in current -git, could you tell me why this triggers: WARNING: vmlinux.o(.text+0x87e2a): Section mismatch: reference to .init.text: (between 'process_zones' and 'setup_per_cpu_pageset') and how to resolve it? I had a quick look and it was not obvious to me. Please send the .config. 'make allyesconfig' and disable CONFIG_HOTPLUG=y. Works for me without the warning. Wait, I remember something @[EMAIL PROTECTED]: 'make allyesconfig' on x86 became ambiguously. If you have by chance a 64bit CPU that would explain why I didn't get it. @Sam: Can you fix the bug that menuconfig does not let me enable CONFIG_64BIT? make ARCH=x86_64 allyesconfig will set CONFIG_64BIT for you - no? Yes. But this still leaves the fact that when someone says 'allyesconfig' it's no longer clear which configuration he has. And no, I do not consider it funny that this now implies two hours of compilation twice just for seeing one bug. And the fact that I have to set ARCH only for seeing the 32/64bit Kconfig choice is clearly bullshit. Sam cu Adrian -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
Can you fix the bug that menuconfig does not let me enable CONFIG_64BIT? make ARCH=x86_64 allyesconfig will set CONFIG_64BIT for you - no? Yes. But this still leaves the fact that when someone says 'allyesconfig' it's no longer clear which configuration he has. Assuming you are on an x86 box... make allyesconfig will give you a config for same OS bit-size as you run. make ARCH=i386 allyesconfig gives you a 32 bit config make ARCH=x86_64 allyesconfig gives you a 64 bit config make ARCH=x86 allyesconfig gives you a 32 bit config where you can select 64 bit. It was seen as the best solution when unifying 32 and 64 bit stuff and introducing support for ARCH=x86. No-one has complained until now so most people seems to get it or maybe they are just silent. Sam -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Mon, Jan 14, 2008 at 04:01:03PM +0100, Ingo Molnar wrote:
* Ingo Molnar [EMAIL PROTECTED] wrote:
Would be great to have them automated - just dunno how to do it. Do
you see a feasible way to do it?
a good starting point would be to make the warnings a lot more
self-explanatory. Right now it's often non-obvious trying to figure
out how the dependencies are structured and which one should be
changed to get rid of the bug.
for example, in current -git, could you tell me why this triggers:
WARNING: vmlinux.o(.text+0x87e2a): Section mismatch: reference to
.init.text: (between 'process_zones' and 'setup_per_cpu_pageset')
and how to resolve it? I had a quick look and it was not obvious to me.
I was confused by your error message - it looked all wrong.
process_zones is .text but setup_per_cpu_pageset is __init. I expect you
had some local changes.
So I tried myself and got this warning:
WARNING: mm/built-in.o(.text+0x6864): Section mismatch: reference to
.init.text: (between 'process_zones' and 'pageset_cpuup_callback')
This made much more sense.
So I looked closely at process_zones() and the first
thing I always do is to check all the local functions.
I noticed that we use the function zone_batchsize() which is marked __devinit.
A function marked __cpuinit may use other functions marked __cpuinit, data
marked
__cpuinitdata and .text/.data. But references to __devinit is not ok.
I furthermore noticed that zone_batchsize() were used in anohter
function marked __meminit.
So the simple fix for this warning is to remove the annotation of
zone_batchsize.
It looks like a real opps candidate to me..
Why modpost did not pick up the zone_batchsize symbol is anohter matter.
It is present in the file:
$ objdump --syms vmlinux.o | grep zone_batchsize
00016929 l F .init.text 0053 zone_batchsize
Debugging modpost I could see that we had an addend value of 695,
but the addr of the symbol is 699. So somehow we point 4 bytes wrong.
Strange...
Anyway - here follows the patch.
Sam
[PATCH] mm: fix section mismatch warning in page_alloc.c
With CONFIG_HOTPLUG=n and CONFIG_HOTPLUG_CPU=y we saw
following warning:
WARNING: mm/built-in.o(.text+0x6864): Section mismatch: reference to
.init.text: (between 'process_zones' and 'pageset_cpuup_callback')
The culprint was zone_batchsize() which were annotated
__devinit but used from process_zones() which is annotated __cpuinit.
zone_batchsize() are used from another function annotated __meminit
so the only valid option is to drop the annotation of
zone_batchsize() so we know it is always valid to use it.
Signed-off-by: Sam Ravnborg [EMAIL PROTECTED]
---
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index e1028fa..b2838c2 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2566,7 +2566,7 @@ static void __meminit zone_init_free_lists(struct
pglist_data *pgdat,
memmap_init_zone((size), (nid), (zone), (start_pfn), MEMMAP_EARLY)
#endif
-static int __devinit zone_batchsize(struct zone *zone)
+static int zone_batchsize(struct zone *zone)
{
int batch;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Mon, Jan 14, 2008 at 09:27:40PM +0100, Sam Ravnborg wrote: Can you fix the bug that menuconfig does not let me enable CONFIG_64BIT? make ARCH=x86_64 allyesconfig will set CONFIG_64BIT for you - no? Yes. But this still leaves the fact that when someone says 'allyesconfig' it's no longer clear which configuration he has. Assuming you are on an x86 box... make allyesconfig will give you a config for same OS bit-size as you run. make ARCH=i386 allyesconfig gives you a 32 bit config make ARCH=x86_64 allyesconfig gives you a 64 bit config make ARCH=x86 allyesconfig gives you a 32 bit config where you can select 64 bit. It was seen as the best solution when unifying 32 and 64 bit stuff and introducing support for ARCH=x86. No-one has complained until now so most people seems to get it or maybe they are just silent. Or I'm the only person doing kernel development on a 32bit x86 machine? Even oldconfig is busted since the Kconfig choice is not available without explicitely setting ARCH. Sam cu Adrian -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Thursday, 10 of January 2008, Andi Kleen wrote: > On Thursday 10 January 2008 12:26:07 Adrian Bunk wrote: > > On Thu, Jan 10, 2008 at 12:15:15PM +0100, Andi Kleen wrote: > > > > But your patch does: > > > > > > > > +config PM_CPUINIT > > > > + bool > > > > + depends on PM > > > > > > That is because arch/x86/power/cpu.c where this happens is currently > > > > > > obj-$(CONFIG_PM)+= cpu.o > > > > > > If it was changed to CONFIG_something else then yes that dependency > > > should be changed too. > > > > > > Then fix this first. > > Rafael indicated he would do that, Yes, and I'm still going to do that. :-) Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
[Sorry for not replying earlier, I missed your message.] On Friday, 4 of January 2008, Ingo Molnar wrote: > > * Rafael J. Wysocki <[EMAIL PROTECTED]> wrote: > > > > So you would need to fix that first. Would be fine for me, but is > > > out of scope for my patch. > > > > OK, I'll fix that up later. > > i'll apply Andi's patch to x86.git - or would like to maintain that > patch? x86.git would be fine. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Thu, Jan 10, 2008 at 02:12:59PM +0100, Andi Kleen wrote: > Adrian Bunk <[EMAIL PROTECTED]> writes: > > > > Technically you are the one who has to deal with problems in your > > patches, not the people pointing at the problems. > > If you believe that my patch adds a new problem then please describe > it clearly so that I can understand it. Description: - there are already __cpuinit* annotations in the kernel - on UP kernels supporting suspend/resume, such annotated code currently gets freed after booting (and this works) - with your patch applied, this code no longer gets freed Whether or not this is a problem depends on whether you care about the memory used by the kernel or not... My other issues with your patch were: - whatever warning this patch was supposed to fixed was not shown (the patch description didn't mention any warning at all) - and understanding a fix gets easier when you know the problem it's supposed to fix - this patch shouldn't be x86 specific since the issue how to annotate suspend and CPU hotplug code isn't x86 specific > -Andi cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
Adrian Bunk <[EMAIL PROTECTED]> writes: > > Technically you are the one who has to deal with problems in your > patches, not the people pointing at the problems. If you believe that my patch adds a new problem then please describe it clearly so that I can understand it. -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Thu, Jan 10, 2008 at 12:42:53PM +0100, Andi Kleen wrote: > On Thursday 10 January 2008 12:26:07 Adrian Bunk wrote: > > On Thu, Jan 10, 2008 at 12:15:15PM +0100, Andi Kleen wrote: > > > > But your patch does: > > > > > > > > +config PM_CPUINIT > > > > + bool > > > > + depends on PM > > > > > > That is because arch/x86/power/cpu.c where this happens is currently > > > > > > obj-$(CONFIG_PM)+= cpu.o > > > > > > If it was changed to CONFIG_something else then yes that dependency > > > should be changed too. > > > > > > Then fix this first. > > Rafael indicated he would do that, but it is really outside the scope > of my patch. I was just interested in fixing a linker warning. Your patch description doesn't mention any linker warning. Can you send the linker warning so that we can see the problem and not only the patch you wrote for fixing the undisclosed problem? > > And the following other points you didn't bother to reply to also still > > stand even after this fix: > > - already __cpuinit code will waste memory with CONFIG_PM_SLEEP=y > > Don't know what your point is. Anyways if you think there is a problem > somewhere please feel free to write patches. Technically you are the one who has to deal with problems in your patches, not the people pointing at the problems. > > - change shouldn't be x86 specific > > CPU initialization is deeply architecture specific. I don't see much use > in generalizing that. That the code is architecture specific is clear. But how to best annotate suspend and CPU hotplug code is a problem that is shared between many architectures and whose solution should not be architecture specific. > -Andi cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Thursday 10 January 2008 12:26:07 Adrian Bunk wrote: > On Thu, Jan 10, 2008 at 12:15:15PM +0100, Andi Kleen wrote: > > > But your patch does: > > > > > > +config PM_CPUINIT > > > + bool > > > + depends on PM > > > > That is because arch/x86/power/cpu.c where this happens is currently > > > > obj-$(CONFIG_PM)+= cpu.o > > > > If it was changed to CONFIG_something else then yes that dependency > > should be changed too. > > > Then fix this first. Rafael indicated he would do that, but it is really outside the scope of my patch. I was just interested in fixing a linker warning. > > And the following other points you didn't bother to reply to also still > stand even after this fix: > - already __cpuinit code will waste memory with CONFIG_PM_SLEEP=y Don't know what your point is. Anyways if you think there is a problem somewhere please feel free to write patches. > - change shouldn't be x86 specific CPU initialization is deeply architecture specific. I don't see much use in generalizing that. -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Thu, Jan 10, 2008 at 12:15:15PM +0100, Andi Kleen wrote: > > But your patch does: > > > > +config PM_CPUINIT > > + bool > > + depends on PM > > That is because arch/x86/power/cpu.c where this happens is currently > > obj-$(CONFIG_PM)+= cpu.o > > If it was changed to CONFIG_something else then yes that dependency > should be changed too. Then fix this first. And the following other points you didn't bother to reply to also still stand even after this fix: - already __cpuinit code will waste memory with CONFIG_PM_SLEEP=y - change shouldn't be x86 specific > -Andi cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
> But your patch does: > > +config PM_CPUINIT > + bool > + depends on PM That is because arch/x86/power/cpu.c where this happens is currently obj-$(CONFIG_PM)+= cpu.o If it was changed to CONFIG_something else then yes that dependency should be changed too. -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Thu, Jan 10, 2008 at 10:58:57AM +0100, Andi Kleen wrote: > > It seems the correct solution would be not to hijack __cpuinit > > (as your patch does), but to create a new annotation. > > The rationale is that after suspend the CPU has to be reinitialized. > That is because it is essentially like a reboot. All the previous > CPU state is gone. >... But your patch does: +config PM_CPUINIT + bool + depends on PM + default y As an example, even plain ACPI support without any suspend support in the kernel at all requires CONFIG_PM and therefore forces all __cpuinit code to be non-__init after your patch. And if the dependency was corrected to PM_SLEEP it will still make the UP kernel use more memory since we currently have __cpuinit code that gets discarded after boot but suspend/resume is apparently working. Plus my other point that it seems to be wrong to do whatever change only for x86. > -Andi cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
> It seems the correct solution would be not to hijack __cpuinit > (as your patch does), but to create a new annotation. The rationale is that after suspend the CPU has to be reinitialized. That is because it is essentially like a reboot. All the previous CPU state is gone. It doesn't need to touch state in memory only -- if you want you could create a new annotation for functions that only touch memory; but I'm not sure it would buy all that much. -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Thu, Jan 03, 2008 at 07:43:43PM +0100, Andi Kleen wrote: > On Thursday 03 January 2008 19:14:38 Adrian Bunk wrote: > > On Thu, Jan 03, 2008 at 04:42:29PM +0100, Andi Kleen wrote: > > > > > > This avoids the requirement to mark a lot of initialization functions not > > > __cpuinit just for resume from RAM. > > > > > > More functions could be converted now, didn't do all. > > >... > > > > Shouldn't this aready be handled by the following? > > > > config PM_SLEEP_SMP > > bool > > depends on SUSPEND_SMP_POSSIBLE || HIBERNATION_SMP_POSSIBLE > > depends on PM_SLEEP > > select HOTPLUG_CPU > > default y > > Won't help for UP at least. I know that it's not popular to care about the kernel size, but your patch will cost precious memory in the common case of UP embedded systems with CONFIG_PM=y. It seems the correct solution would be not to hijack __cpuinit (as your patch does), but to create a new annotation. > -Andi cu Adrian BTW: Is there any good reason why your patch is x86 only? No matter how this gets handled, it should be an architecture independent issue. -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
It seems the correct solution would be not to hijack __cpuinit (as your patch does), but to create a new annotation. The rationale is that after suspend the CPU has to be reinitialized. That is because it is essentially like a reboot. All the previous CPU state is gone. It doesn't need to touch state in memory only -- if you want you could create a new annotation for functions that only touch memory; but I'm not sure it would buy all that much. -Andi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Thu, Jan 03, 2008 at 07:43:43PM +0100, Andi Kleen wrote: On Thursday 03 January 2008 19:14:38 Adrian Bunk wrote: On Thu, Jan 03, 2008 at 04:42:29PM +0100, Andi Kleen wrote: This avoids the requirement to mark a lot of initialization functions not __cpuinit just for resume from RAM. More functions could be converted now, didn't do all. ... Shouldn't this aready be handled by the following? config PM_SLEEP_SMP bool depends on SUSPEND_SMP_POSSIBLE || HIBERNATION_SMP_POSSIBLE depends on PM_SLEEP select HOTPLUG_CPU default y Won't help for UP at least. I know that it's not popular to care about the kernel size, but your patch will cost precious memory in the common case of UP embedded systems with CONFIG_PM=y. It seems the correct solution would be not to hijack __cpuinit (as your patch does), but to create a new annotation. -Andi cu Adrian BTW: Is there any good reason why your patch is x86 only? No matter how this gets handled, it should be an architecture independent issue. -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Thu, Jan 10, 2008 at 10:58:57AM +0100, Andi Kleen wrote: It seems the correct solution would be not to hijack __cpuinit (as your patch does), but to create a new annotation. The rationale is that after suspend the CPU has to be reinitialized. That is because it is essentially like a reboot. All the previous CPU state is gone. ... But your patch does: +config PM_CPUINIT + bool + depends on PM + default y As an example, even plain ACPI support without any suspend support in the kernel at all requires CONFIG_PM and therefore forces all __cpuinit code to be non-__init after your patch. And if the dependency was corrected to PM_SLEEP it will still make the UP kernel use more memory since we currently have __cpuinit code that gets discarded after boot but suspend/resume is apparently working. Plus my other point that it seems to be wrong to do whatever change only for x86. -Andi cu Adrian -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
But your patch does: +config PM_CPUINIT + bool + depends on PM That is because arch/x86/power/cpu.c where this happens is currently obj-$(CONFIG_PM)+= cpu.o If it was changed to CONFIG_something else then yes that dependency should be changed too. -Andi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Thu, Jan 10, 2008 at 12:15:15PM +0100, Andi Kleen wrote: But your patch does: +config PM_CPUINIT + bool + depends on PM That is because arch/x86/power/cpu.c where this happens is currently obj-$(CONFIG_PM)+= cpu.o If it was changed to CONFIG_something else then yes that dependency should be changed too. Then fix this first. And the following other points you didn't bother to reply to also still stand even after this fix: - already __cpuinit code will waste memory with CONFIG_PM_SLEEP=y - change shouldn't be x86 specific -Andi cu Adrian -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Thursday 10 January 2008 12:26:07 Adrian Bunk wrote: On Thu, Jan 10, 2008 at 12:15:15PM +0100, Andi Kleen wrote: But your patch does: +config PM_CPUINIT + bool + depends on PM That is because arch/x86/power/cpu.c where this happens is currently obj-$(CONFIG_PM)+= cpu.o If it was changed to CONFIG_something else then yes that dependency should be changed too. Then fix this first. Rafael indicated he would do that, but it is really outside the scope of my patch. I was just interested in fixing a linker warning. And the following other points you didn't bother to reply to also still stand even after this fix: - already __cpuinit code will waste memory with CONFIG_PM_SLEEP=y Don't know what your point is. Anyways if you think there is a problem somewhere please feel free to write patches. - change shouldn't be x86 specific CPU initialization is deeply architecture specific. I don't see much use in generalizing that. -Andi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Thu, Jan 10, 2008 at 12:42:53PM +0100, Andi Kleen wrote: On Thursday 10 January 2008 12:26:07 Adrian Bunk wrote: On Thu, Jan 10, 2008 at 12:15:15PM +0100, Andi Kleen wrote: But your patch does: +config PM_CPUINIT + bool + depends on PM That is because arch/x86/power/cpu.c where this happens is currently obj-$(CONFIG_PM)+= cpu.o If it was changed to CONFIG_something else then yes that dependency should be changed too. Then fix this first. Rafael indicated he would do that, but it is really outside the scope of my patch. I was just interested in fixing a linker warning. Your patch description doesn't mention any linker warning. Can you send the linker warning so that we can see the problem and not only the patch you wrote for fixing the undisclosed problem? And the following other points you didn't bother to reply to also still stand even after this fix: - already __cpuinit code will waste memory with CONFIG_PM_SLEEP=y Don't know what your point is. Anyways if you think there is a problem somewhere please feel free to write patches. Technically you are the one who has to deal with problems in your patches, not the people pointing at the problems. - change shouldn't be x86 specific CPU initialization is deeply architecture specific. I don't see much use in generalizing that. That the code is architecture specific is clear. But how to best annotate suspend and CPU hotplug code is a problem that is shared between many architectures and whose solution should not be architecture specific. -Andi cu Adrian -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
Adrian Bunk [EMAIL PROTECTED] writes: Technically you are the one who has to deal with problems in your patches, not the people pointing at the problems. If you believe that my patch adds a new problem then please describe it clearly so that I can understand it. -Andi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Thu, Jan 10, 2008 at 02:12:59PM +0100, Andi Kleen wrote: Adrian Bunk [EMAIL PROTECTED] writes: Technically you are the one who has to deal with problems in your patches, not the people pointing at the problems. If you believe that my patch adds a new problem then please describe it clearly so that I can understand it. Description: - there are already __cpuinit* annotations in the kernel - on UP kernels supporting suspend/resume, such annotated code currently gets freed after booting (and this works) - with your patch applied, this code no longer gets freed Whether or not this is a problem depends on whether you care about the memory used by the kernel or not... My other issues with your patch were: - whatever warning this patch was supposed to fixed was not shown (the patch description didn't mention any warning at all) - and understanding a fix gets easier when you know the problem it's supposed to fix - this patch shouldn't be x86 specific since the issue how to annotate suspend and CPU hotplug code isn't x86 specific -Andi cu Adrian -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
[Sorry for not replying earlier, I missed your message.] On Friday, 4 of January 2008, Ingo Molnar wrote: * Rafael J. Wysocki [EMAIL PROTECTED] wrote: So you would need to fix that first. Would be fine for me, but is out of scope for my patch. OK, I'll fix that up later. i'll apply Andi's patch to x86.git - or would like to maintain that patch? x86.git would be fine. Thanks, Rafael -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Thursday, 10 of January 2008, Andi Kleen wrote: On Thursday 10 January 2008 12:26:07 Adrian Bunk wrote: On Thu, Jan 10, 2008 at 12:15:15PM +0100, Andi Kleen wrote: But your patch does: +config PM_CPUINIT + bool + depends on PM That is because arch/x86/power/cpu.c where this happens is currently obj-$(CONFIG_PM)+= cpu.o If it was changed to CONFIG_something else then yes that dependency should be changed too. Then fix this first. Rafael indicated he would do that, Yes, and I'm still going to do that. :-) Thanks, Rafael -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
* Rafael J. Wysocki <[EMAIL PROTECTED]> wrote: > > So you would need to fix that first. Would be fine for me, but is > > out of scope for my patch. > > OK, I'll fix that up later. i'll apply Andi's patch to x86.git - or would like to maintain that patch? Ingo -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
* Rafael J. Wysocki [EMAIL PROTECTED] wrote: So you would need to fix that first. Would be fine for me, but is out of scope for my patch. OK, I'll fix that up later. i'll apply Andi's patch to x86.git - or would like to maintain that patch? Ingo -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Thursday, 3 of January 2008, Andi Kleen wrote: > > > > +config PM_CPUINIT > > > + bool > > > + depends on PM > > > > Please make it PM_SLEEP (PM is more than suspend/hibernation). > > That was something that irritated me too while writing the patch, but the > functions I > am interested in with this are referenced from arch/x86/power/cpu.c and that > is > > obj-$(CONFIG_PM)+= cpu.o > > So you would need to fix that first. Would be fine for me, but is out of > scope for > my patch. OK, I'll fix that up later. Thanks, Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Thursday 03 January 2008 19:14:38 Adrian Bunk wrote: > On Thu, Jan 03, 2008 at 04:42:29PM +0100, Andi Kleen wrote: > > > > This avoids the requirement to mark a lot of initialization functions not > > __cpuinit just for resume from RAM. > > > > More functions could be converted now, didn't do all. > >... > > Shouldn't this aready be handled by the following? > > config PM_SLEEP_SMP > bool > depends on SUSPEND_SMP_POSSIBLE || HIBERNATION_SMP_POSSIBLE > depends on PM_SLEEP > select HOTPLUG_CPU > default y Won't help for UP at least. -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Thu, Jan 03, 2008 at 04:42:29PM +0100, Andi Kleen wrote: > > This avoids the requirement to mark a lot of initialization functions not > __cpuinit just for resume from RAM. > > More functions could be converted now, didn't do all. >... Shouldn't this aready be handled by the following? config PM_SLEEP_SMP bool depends on SUSPEND_SMP_POSSIBLE || HIBERNATION_SMP_POSSIBLE depends on PM_SLEEP select HOTPLUG_CPU default y cu Adrian -- "Is there not promise of rain?" Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. "Only a promise," Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
> > +config PM_CPUINIT > > + bool > > + depends on PM > > Please make it PM_SLEEP (PM is more than suspend/hibernation). That was something that irritated me too while writing the patch, but the functions I am interested in with this are referenced from arch/x86/power/cpu.c and that is obj-$(CONFIG_PM)+= cpu.o So you would need to fix that first. Would be fine for me, but is out of scope for my patch. -Andi -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Thursday, 3 of January 2008, Andi Kleen wrote:
>
> This avoids the requirement to mark a lot of initialization functions not
> __cpuinit just for resume from RAM.
>
> More functions could be converted now, didn't do all.
>
> Cc: [EMAIL PROTECTED]
> Cc: [EMAIL PROTECTED]
>
> Signed-off-by: Andi Kleen <[EMAIL PROTECTED]>
>
> ---
> arch/x86/Kconfig|5 +
> arch/x86/kernel/cpu/mtrr/main.c |2 +-
> arch/x86/kernel/trampoline_64.S |2 +-
> include/linux/init.h|2 +-
> 4 files changed, 8 insertions(+), 3 deletions(-)
>
> Index: linux/arch/x86/kernel/trampoline_64.S
> ===
> --- linux.orig/arch/x86/kernel/trampoline_64.S
> +++ linux/arch/x86/kernel/trampoline_64.S
> @@ -34,7 +34,7 @@
> #include
>
> /* We can free up trampoline after bootup if cpu hotplug is not supported. */
> -#ifndef CONFIG_HOTPLUG_CPU
> +#if !defined(CONFIG_HOTPLUG_CPU) && !defined(CONFIG_PM_CPUINIT)
> .section .init.data, "aw", @progbits
> #else
> .section .rodata, "a", @progbits
> Index: linux/include/linux/init.h
> ===
> --- linux.orig/include/linux/init.h
> +++ linux/include/linux/init.h
> @@ -266,7 +266,7 @@ void __init parse_early_param(void);
> #define __devexitdata __exitdata
> #endif
>
> -#ifdef CONFIG_HOTPLUG_CPU
> +#if defined(CONFIG_HOTPLUG_CPU) || defined(CONFIG_PM_CPUINIT)
> #define __cpuinit
> #define __cpuinitdata
> #define __cpuexit
> Index: linux/arch/x86/Kconfig
> ===
> --- linux.orig/arch/x86/Kconfig
> +++ linux/arch/x86/Kconfig
> @@ -130,6 +130,11 @@ config GENERIC_PENDING_IRQ
> depends on GENERIC_HARDIRQS && SMP
> default y
>
> +config PM_CPUINIT
> + bool
> + depends on PM
Please make it PM_SLEEP (PM is more than suspend/hibernation).
> + default y
> +
> config X86_SMP
> bool
> depends on SMP && ((X86_32 && !X86_VOYAGER) || X86_64)
> Index: linux/arch/x86/kernel/cpu/mtrr/main.c
> ===
> --- linux.orig/arch/x86/kernel/cpu/mtrr/main.c
> +++ linux/arch/x86/kernel/cpu/mtrr/main.c
> @@ -712,7 +712,7 @@ void __init mtrr_bp_init(void)
> }
> }
>
> -void mtrr_ap_init(void)
> +void __cpuinit mtrr_ap_init(void)
> {
> unsigned long flags;
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Thursday, 3 of January 2008, Andi Kleen wrote:
This avoids the requirement to mark a lot of initialization functions not
__cpuinit just for resume from RAM.
More functions could be converted now, didn't do all.
Cc: [EMAIL PROTECTED]
Cc: [EMAIL PROTECTED]
Signed-off-by: Andi Kleen [EMAIL PROTECTED]
---
arch/x86/Kconfig|5 +
arch/x86/kernel/cpu/mtrr/main.c |2 +-
arch/x86/kernel/trampoline_64.S |2 +-
include/linux/init.h|2 +-
4 files changed, 8 insertions(+), 3 deletions(-)
Index: linux/arch/x86/kernel/trampoline_64.S
===
--- linux.orig/arch/x86/kernel/trampoline_64.S
+++ linux/arch/x86/kernel/trampoline_64.S
@@ -34,7 +34,7 @@
#include asm/segment.h
/* We can free up trampoline after bootup if cpu hotplug is not supported. */
-#ifndef CONFIG_HOTPLUG_CPU
+#if !defined(CONFIG_HOTPLUG_CPU) !defined(CONFIG_PM_CPUINIT)
.section .init.data, aw, @progbits
#else
.section .rodata, a, @progbits
Index: linux/include/linux/init.h
===
--- linux.orig/include/linux/init.h
+++ linux/include/linux/init.h
@@ -266,7 +266,7 @@ void __init parse_early_param(void);
#define __devexitdata __exitdata
#endif
-#ifdef CONFIG_HOTPLUG_CPU
+#if defined(CONFIG_HOTPLUG_CPU) || defined(CONFIG_PM_CPUINIT)
#define __cpuinit
#define __cpuinitdata
#define __cpuexit
Index: linux/arch/x86/Kconfig
===
--- linux.orig/arch/x86/Kconfig
+++ linux/arch/x86/Kconfig
@@ -130,6 +130,11 @@ config GENERIC_PENDING_IRQ
depends on GENERIC_HARDIRQS SMP
default y
+config PM_CPUINIT
+ bool
+ depends on PM
Please make it PM_SLEEP (PM is more than suspend/hibernation).
+ default y
+
config X86_SMP
bool
depends on SMP ((X86_32 !X86_VOYAGER) || X86_64)
Index: linux/arch/x86/kernel/cpu/mtrr/main.c
===
--- linux.orig/arch/x86/kernel/cpu/mtrr/main.c
+++ linux/arch/x86/kernel/cpu/mtrr/main.c
@@ -712,7 +712,7 @@ void __init mtrr_bp_init(void)
}
}
-void mtrr_ap_init(void)
+void __cpuinit mtrr_ap_init(void)
{
unsigned long flags;
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
+config PM_CPUINIT + bool + depends on PM Please make it PM_SLEEP (PM is more than suspend/hibernation). That was something that irritated me too while writing the patch, but the functions I am interested in with this are referenced from arch/x86/power/cpu.c and that is obj-$(CONFIG_PM)+= cpu.o So you would need to fix that first. Would be fine for me, but is out of scope for my patch. -Andi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Thu, Jan 03, 2008 at 04:42:29PM +0100, Andi Kleen wrote: This avoids the requirement to mark a lot of initialization functions not __cpuinit just for resume from RAM. More functions could be converted now, didn't do all. ... Shouldn't this aready be handled by the following? config PM_SLEEP_SMP bool depends on SUSPEND_SMP_POSSIBLE || HIBERNATION_SMP_POSSIBLE depends on PM_SLEEP select HOTPLUG_CPU default y cu Adrian -- Is there not promise of rain? Ling Tan asked suddenly out of the darkness. There had been need of rain for many days. Only a promise, Lao Er said. Pearl S. Buck - Dragon Seed -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Thursday 03 January 2008 19:14:38 Adrian Bunk wrote: On Thu, Jan 03, 2008 at 04:42:29PM +0100, Andi Kleen wrote: This avoids the requirement to mark a lot of initialization functions not __cpuinit just for resume from RAM. More functions could be converted now, didn't do all. ... Shouldn't this aready be handled by the following? config PM_SLEEP_SMP bool depends on SUSPEND_SMP_POSSIBLE || HIBERNATION_SMP_POSSIBLE depends on PM_SLEEP select HOTPLUG_CPU default y Won't help for UP at least. -Andi -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH x86] [15/16] Force __cpuinit on for CONFIG_PM without HOTPLUG_CPU
On Thursday, 3 of January 2008, Andi Kleen wrote: +config PM_CPUINIT + bool + depends on PM Please make it PM_SLEEP (PM is more than suspend/hibernation). That was something that irritated me too while writing the patch, but the functions I am interested in with this are referenced from arch/x86/power/cpu.c and that is obj-$(CONFIG_PM)+= cpu.o So you would need to fix that first. Would be fine for me, but is out of scope for my patch. OK, I'll fix that up later. Thanks, Rafael -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
