When CONFIG_HOTPLUG_CPU on i386 there are places where __init[data] is
referenced from normal code.
On startup:
arch/i386/kernel/cpu/amd.c::amd_init_cpu():
cpu_devs[X86_VENDOR_AMD] = &amd_cpu_dev;
amd_cpu_dev is declared __initdata and is freed
On CPU hotplug:
arch/i386/kernel/cpu/common.c::get_cpu_vendor():
for (i = 0; i < X86_VENDOR_NUM; i++) {
if (cpu_devs[i]) {
if (!strcmp(v,cpu_devs[i]->c_ident[0]) ||
To fix this, change every instance of __init that seems suspicious
into __cpuinit. When !CONFIG_HOTPLUG_CPU there is no change in .text
or .data size. When enabled, .text += 3248 bytes; .data += 2148 bytes.
This should be safe in every case; the only drawback is the extra code and
data when CPU hotplug is enabled.
Signed-off-by: Chuck Ebbert <[email protected]>
---
arch/i386/kernel/cpu/amd.c | 4 +--
arch/i386/kernel/cpu/centaur.c | 22 +++++++++----------
arch/i386/kernel/cpu/cyrix.c | 37 ++++++++++++++++-----------------
arch/i386/kernel/cpu/intel_cacheinfo.c | 1
arch/i386/kernel/cpu/nexgen.c | 8 +++----
arch/i386/kernel/cpu/rise.c | 4 +--
6 files changed, 39 insertions(+), 37 deletions(-)
--- 2.6.16-rc1-mm4-386.orig/arch/i386/kernel/cpu/amd.c
+++ 2.6.16-rc1-mm4-386/arch/i386/kernel/cpu/amd.c
@@ -22,7 +22,7 @@
extern void vide(void);
__asm__(".align 4\nvide: ret");
-static void __init init_amd(struct cpuinfo_x86 *c)
+static void __cpuinit init_amd(struct cpuinfo_x86 *c)
{
u32 l, h;
int mbytes = num_physpages >> (20-PAGE_SHIFT);
@@ -255,7 +255,7 @@ static unsigned int amd_size_cache(struc
return size;
}
-static struct cpu_dev amd_cpu_dev __initdata = {
+static struct cpu_dev amd_cpu_dev __cpuinitdata = {
.c_vendor = "AMD",
.c_ident = { "AuthenticAMD" },
.c_models = {
--- 2.6.16-rc1-mm4-386.orig/arch/i386/kernel/cpu/centaur.c
+++ 2.6.16-rc1-mm4-386/arch/i386/kernel/cpu/centaur.c
@@ -8,7 +8,7 @@
#ifdef CONFIG_X86_OOSTORE
-static u32 __init power2(u32 x)
+static u32 __cpuinit power2(u32 x)
{
u32 s=1;
while(s<=x)
@@ -21,7 +21,7 @@ static u32 __init power2(u32 x)
* Set up an actual MCR
*/
-static void __init centaur_mcr_insert(int reg, u32 base, u32 size, int key)
+static void __cpuinit centaur_mcr_insert(int reg, u32 base, u32 size, int key)
{
u32 lo, hi;
@@ -39,7 +39,7 @@ static void __init centaur_mcr_insert(in
* Shortcut: We know you can't put 4Gig of RAM on a winchip
*/
-static u32 __init ramtop(void) /* 16388 */
+static u32 __cpuinit ramtop(void) /* 16388 */
{
int i;
u32 top = 0;
@@ -90,7 +90,7 @@ static u32 __init ramtop(void) /* 16388
* Compute a set of MCR's to give maximum coverage
*/
-static int __init centaur_mcr_compute(int nr, int key)
+static int __cpuinit centaur_mcr_compute(int nr, int key)
{
u32 mem = ramtop();
u32 root = power2(mem);
@@ -165,7 +165,7 @@ static int __init centaur_mcr_compute(in
return ct;
}
-static void __init centaur_create_optimal_mcr(void)
+static void __cpuinit centaur_create_optimal_mcr(void)
{
int i;
/*
@@ -188,7 +188,7 @@ static void __init centaur_create_optima
wrmsr(MSR_IDT_MCR0+i, 0, 0);
}
-static void __init winchip2_create_optimal_mcr(void)
+static void __cpuinit winchip2_create_optimal_mcr(void)
{
u32 lo, hi;
int i;
@@ -226,7 +226,7 @@ static void __init winchip2_create_optim
* Handle the MCR key on the Winchip 2.
*/
-static void __init winchip2_unprotect_mcr(void)
+static void __cpuinit winchip2_unprotect_mcr(void)
{
u32 lo, hi;
u32 key;
@@ -238,7 +238,7 @@ static void __init winchip2_unprotect_mc
wrmsr(MSR_IDT_MCR_CTRL, lo, hi);
}
-static void __init winchip2_protect_mcr(void)
+static void __cpuinit winchip2_protect_mcr(void)
{
u32 lo, hi;
@@ -256,7 +256,7 @@ static void __init winchip2_protect_mcr(
#define RNG_ENABLED (1 << 3)
#define RNG_ENABLE (1 << 6) /* MSR_VIA_RNG */
-static void __init init_c3(struct cpuinfo_x86 *c)
+static void __cpuinit init_c3(struct cpuinfo_x86 *c)
{
u32 lo, hi;
@@ -302,7 +302,7 @@ static void __init init_c3(struct cpuinf
display_cacheinfo(c);
}
-static void __init init_centaur(struct cpuinfo_x86 *c)
+static void __cpuinit init_centaur(struct cpuinfo_x86 *c)
{
enum {
ECX8=1<<1,
@@ -456,7 +456,7 @@ static unsigned int centaur_size_cache(s
return size;
}
-static struct cpu_dev centaur_cpu_dev __initdata = {
+static struct cpu_dev centaur_cpu_dev __cpuinitdata = {
.c_vendor = "Centaur",
.c_ident = { "CentaurHauls" },
.c_init = init_centaur,
--- 2.6.16-rc1-mm4-386.orig/arch/i386/kernel/cpu/cyrix.c
+++ 2.6.16-rc1-mm4-386/arch/i386/kernel/cpu/cyrix.c
@@ -12,7 +12,7 @@
/*
* Read NSC/Cyrix DEVID registers (DIR) to get more detailed info. about the CPU
*/
-static void __init do_cyrix_devid(unsigned char *dir0, unsigned char *dir1)
+static void __cpuinit do_cyrix_devid(unsigned char *dir0, unsigned char *dir1)
{
unsigned char ccr2, ccr3;
unsigned long flags;
@@ -52,25 +52,25 @@ static void __init do_cyrix_devid(unsign
* Actually since bugs.h doesn't even reference this perhaps someone should
* fix the documentation ???
*/
-static unsigned char Cx86_dir0_msb __initdata = 0;
+static unsigned char Cx86_dir0_msb __cpuinitdata = 0;
-static char Cx86_model[][9] __initdata = {
+static char Cx86_model[][9] __cpuinitdata = {
"Cx486", "Cx486", "5x86 ", "6x86", "MediaGX ", "6x86MX ",
"M II ", "Unknown"
};
-static char Cx486_name[][5] __initdata = {
+static char Cx486_name[][5] __cpuinitdata = {
"SLC", "DLC", "SLC2", "DLC2", "SRx", "DRx",
"SRx2", "DRx2"
};
-static char Cx486S_name[][4] __initdata = {
+static char Cx486S_name[][4] __cpuinitdata = {
"S", "S2", "Se", "S2e"
};
-static char Cx486D_name[][4] __initdata = {
+static char Cx486D_name[][4] __cpuinitdata = {
"DX", "DX2", "?", "?", "?", "DX4"
};
-static char Cx86_cb[] __initdata = "?.5x Core/Bus Clock";
-static char cyrix_model_mult1[] __initdata = "12??43";
-static char cyrix_model_mult2[] __initdata = "12233445";
+static char Cx86_cb[] __cpuinitdata = "?.5x Core/Bus Clock";
+static char cyrix_model_mult1[] __cpuinitdata = "12??43";
+static char cyrix_model_mult2[] __cpuinitdata = "12233445";
/*
* Reset the slow-loop (SLOP) bit on the 686(L) which is set by some old
@@ -80,9 +80,10 @@ static char cyrix_model_mult2[] __initda
* FIXME: our newer udelay uses the tsc. We don't need to frob with SLOP
*/
-extern void calibrate_delay(void) __init;
+/* is __devinit, should be __cpuinit in init/calibrate.c ??? */
+extern void calibrate_delay(void) __devinit;
-static void __init check_cx686_slop(struct cpuinfo_x86 *c)
+static void __cpuinit check_cx686_slop(struct cpuinfo_x86 *c)
{
unsigned long flags;
@@ -107,7 +108,7 @@ static void __init check_cx686_slop(stru
}
-static void __init set_cx86_reorder(void)
+static void __cpuinit set_cx86_reorder(void)
{
u8 ccr3;
@@ -122,7 +123,7 @@ static void __init set_cx86_reorder(void
setCx86(CX86_CCR3, ccr3);
}
-static void __init set_cx86_memwb(void)
+static void __cpuinit set_cx86_memwb(void)
{
u32 cr0;
@@ -137,7 +138,7 @@ static void __init set_cx86_memwb(void)
setCx86(CX86_CCR2, getCx86(CX86_CCR2) | 0x14 );
}
-static void __init set_cx86_inc(void)
+static void __cpuinit set_cx86_inc(void)
{
unsigned char ccr3;
@@ -158,7 +159,7 @@ static void __init set_cx86_inc(void)
* Configure later MediaGX and/or Geode processor.
*/
-static void __init geode_configure(void)
+static void __cpuinit geode_configure(void)
{
unsigned long flags;
u8 ccr3, ccr4;
@@ -191,7 +192,7 @@ static struct pci_device_id cyrix_55x0[]
};
#endif
-static void __init init_cyrix(struct cpuinfo_x86 *c)
+static void __cpuinit init_cyrix(struct cpuinfo_x86 *c)
{
unsigned char dir0, dir0_msn, dir0_lsn, dir1 = 0;
char *buf = c->x86_model_id;
@@ -429,7 +430,7 @@ static void cyrix_identify(struct cpuinf
generic_identify(c);
}
-static struct cpu_dev cyrix_cpu_dev __initdata = {
+static struct cpu_dev cyrix_cpu_dev __cpuinitdata = {
.c_vendor = "Cyrix",
.c_ident = { "CyrixInstead" },
.c_init = init_cyrix,
@@ -444,7 +445,7 @@ int __init cyrix_init_cpu(void)
//early_arch_initcall(cyrix_init_cpu);
-static struct cpu_dev nsc_cpu_dev __initdata = {
+static struct cpu_dev nsc_cpu_dev __cpuinitdata = {
.c_vendor = "NSC",
.c_ident = { "Geode by NSC" },
.c_init = init_nsc,
--- 2.6.16-rc1-mm4-386.orig/arch/i386/kernel/cpu/nexgen.c
+++ 2.6.16-rc1-mm4-386/arch/i386/kernel/cpu/nexgen.c
@@ -10,7 +10,7 @@
* to have CPUID. (Thanks to Herbert Oppmann)
*/
-static int __init deep_magic_nexgen_probe(void)
+static int __cpuinit deep_magic_nexgen_probe(void)
{
int ret;
@@ -27,12 +27,12 @@ static int __init deep_magic_nexgen_prob
return ret;
}
-static void __init init_nexgen(struct cpuinfo_x86 * c)
+static void __cpuinit init_nexgen(struct cpuinfo_x86 * c)
{
c->x86_cache_size = 256; /* A few had 1 MB... */
}
-static void __init nexgen_identify(struct cpuinfo_x86 * c)
+static void __cpuinit nexgen_identify(struct cpuinfo_x86 * c)
{
/* Detect NexGen with old hypercode */
if ( deep_magic_nexgen_probe() ) {
@@ -41,7 +41,7 @@ static void __init nexgen_identify(struc
generic_identify(c);
}
-static struct cpu_dev nexgen_cpu_dev __initdata = {
+static struct cpu_dev nexgen_cpu_dev __cpuinitdata = {
.c_vendor = "Nexgen",
.c_ident = { "NexGenDriven" },
.c_models = {
--- 2.6.16-rc1-mm4-386.orig/arch/i386/kernel/cpu/rise.c
+++ 2.6.16-rc1-mm4-386/arch/i386/kernel/cpu/rise.c
@@ -5,7 +5,7 @@
#include "cpu.h"
-static void __init init_rise(struct cpuinfo_x86 *c)
+static void __cpuinit init_rise(struct cpuinfo_x86 *c)
{
printk("CPU: Rise iDragon");
if (c->x86_model > 2)
@@ -28,7 +28,7 @@ static void __init init_rise(struct cpui
set_bit(X86_FEATURE_CX8, c->x86_capability);
}
-static struct cpu_dev rise_cpu_dev __initdata = {
+static struct cpu_dev rise_cpu_dev __cpuinitdata = {
.c_vendor = "Rise",
.c_ident = { "RiseRiseRise" },
.c_models = {
--- 2.6.16-rc1-mm4-386.orig/arch/i386/kernel/cpu/intel_cacheinfo.c
+++ 2.6.16-rc1-mm4-386/arch/i386/kernel/cpu/intel_cacheinfo.c
@@ -152,6 +152,7 @@ static int __cpuinit cpuid4_cache_lookup
return 0;
}
+/* __init is safe here */
static int __init find_num_cache_leaves(void)
{
unsigned int eax, ebx, ecx, edx;
--
Chuck
On Tue, Jan 31, 2006 at 11:49:43PM -0500, Chuck Ebbert wrote:
> When CONFIG_HOTPLUG_CPU on i386 there are places where __init[data] is
> referenced from normal code.
>
> On startup:
> arch/i386/kernel/cpu/amd.c::amd_init_cpu():
> cpu_devs[X86_VENDOR_AMD] = &amd_cpu_dev;
> amd_cpu_dev is declared __initdata and is freed
>
> On CPU hotplug:
> arch/i386/kernel/cpu/common.c::get_cpu_vendor():
> for (i = 0; i < X86_VENDOR_NUM; i++) {
> if (cpu_devs[i]) {
> if (!strcmp(v,cpu_devs[i]->c_ident[0]) ||
>
> To fix this, change every instance of __init that seems suspicious
> into __cpuinit. When !CONFIG_HOTPLUG_CPU there is no change in .text
> or .data size. When enabled, .text += 3248 bytes; .data += 2148 bytes.
>
> This should be safe in every case; the only drawback is the extra code and
> data when CPU hotplug is enabled.
Especially as for the bulk of them, those CPUs aren't hotplug capable.
(I seriously doubt we'll ever see a hotplugable cyrix for eg, which
takes up the bulk of your diff).
How about leaving it __init on non-hotplug systems, and somehow removing
those from cpu_devs, so get_cpu_vendor() just skips them ?
NULL'ing those entries should be just a few bytes, instead of adding 5KB.
Dave
Dave Jones <[email protected]> wrote:
> Especially as for the bulk of them, those CPUs aren't hotplug capable.
> (I seriously doubt we'll ever see a hotplugable cyrix for eg, which
> takes up the bulk of your diff).
For SMP systems, suspend/resume "unplugs" all non-boot CPUs before
executing the suspend code. I don't recall any SMP cyrix systems, but
it's potentially something to consider.
--
Matthew Garrett | [email protected]
In-Reply-To: <[email protected]>
On Wed, 1 Feb 2006 at 00:33:57 -0500, Dave Jones wrote:
> On Tue, Jan 31, 2006 at 11:49:43PM -0500, Chuck Ebbert wrote:
> > To fix this, change every instance of __init that seems suspicious
> > into __cpuinit. When !CONFIG_HOTPLUG_CPU there is no change in .text
> > or .data size. When enabled, .text += 3248 bytes; .data += 2148 bytes.
> >
> > This should be safe in every case; the only drawback is the extra code and
> > data when CPU hotplug is enabled.
>
> How about leaving it __init on non-hotplug systems, and somehow removing
> those from cpu_devs, so get_cpu_vendor() just skips them ?
> NULL'ing those entries should be just a few bytes, instead of adding 5KB.
That's what I wanted to do but wasn't sure how. Maybe e.g. like this?
--- 2.6.16-rc1-mm4-386.orig/arch/i386/kernel/cpu/umc.c
+++ 2.6.16-rc1-mm4-386/arch/i386/kernel/cpu/umc.c
@@ -5,12 +5,12 @@
/* UMC chips appear to be only either 386 or 486, so no special init takes place.
*/
-static void __cpuinit init_umc(struct cpuinfo_x86 * c)
+static void __init init_umc(struct cpuinfo_x86 * c)
{
}
-static struct cpu_dev umc_cpu_dev __cpuinitdata = {
+static struct cpu_dev umc_cpu_dev __initdata = {
.c_vendor = "UMC",
.c_ident = { "UMC UMC UMC" },
.c_models = {
@@ -31,3 +31,11 @@ int __init umc_init_cpu(void)
}
//early_arch_initcall(umc_init_cpu);
+
+int __init umc_exit_cpu(void)
+{
+ cpu_devs[X86_VENDOR_UMC] = NULL;
+ return 0;
+}
+
+late_initcall(umc_exit_cpu);
--
Chuck
On Wed, Feb 01, 2006 at 08:05:51AM +0000, Matthew Garrett wrote:
> Dave Jones <[email protected]> wrote:
>
> > Especially as for the bulk of them, those CPUs aren't hotplug capable.
> > (I seriously doubt we'll ever see a hotplugable cyrix for eg, which
> > takes up the bulk of your diff).
>
> For SMP systems, suspend/resume "unplugs" all non-boot CPUs before
> executing the suspend code. I don't recall any SMP cyrix systems, but
> it's potentially something to consider.
There weren't any. Until AMD's Athlon MPs, Intel had the only
SMP x86 afair.
Dave
On Mer, 2006-02-01 at 11:03 -0500, Dave Jones wrote:
> > For SMP systems, suspend/resume "unplugs" all non-boot CPUs before
> > executing the suspend code. I don't recall any SMP cyrix systems, but
> > it's potentially something to consider.
>
> There weren't any. Until AMD's Athlon MPs, Intel had the only
> SMP x86 afair.
Several vendors demonstrated OpenMP designs including Cyrix. Nothing
production and nothing we support.
On Wed, Feb 01, 2006 at 06:56:22PM +0000, Alan Cox wrote:
> On Mer, 2006-02-01 at 11:03 -0500, Dave Jones wrote:
> > > For SMP systems, suspend/resume "unplugs" all non-boot CPUs before
> > > executing the suspend code. I don't recall any SMP cyrix systems, but
> > > it's potentially something to consider.
> >
> > There weren't any. Until AMD's Athlon MPs, Intel had the only
> > SMP x86 afair.
>
> Several vendors demonstrated OpenMP designs including Cyrix. Nothing
> production and nothing we support.
In order to support logical cpu hotplug, you dont need any special hw, as long
as you has some SMP box. Sometimes its just a little bit more than
moving __init to __cpuinit. A lot of these references were not changed
just speculatively to ensure when the need is there, someone will change and
test those as well.
Chuck, would it be possible for you also test cpu hotplug for the ones you
are interested in and accompany the related changes if there are more
as separate patches? (Documentation now has a howto on cpu hotplug)
That would make it more useful for this exersise.
--
Cheers,
Ashok Raj
- Open Source Technology Center
Hi!
> > When CONFIG_HOTPLUG_CPU on i386 there are places where __init[data] is
> > referenced from normal code.
> >
> > On startup:
> > arch/i386/kernel/cpu/amd.c::amd_init_cpu():
> > cpu_devs[X86_VENDOR_AMD] = &amd_cpu_dev;
> > amd_cpu_dev is declared __initdata and is freed
> >
> > On CPU hotplug:
> > arch/i386/kernel/cpu/common.c::get_cpu_vendor():
> > for (i = 0; i < X86_VENDOR_NUM; i++) {
> > if (cpu_devs[i]) {
> > if (!strcmp(v,cpu_devs[i]->c_ident[0]) ||
> >
> > To fix this, change every instance of __init that seems suspicious
> > into __cpuinit. When !CONFIG_HOTPLUG_CPU there is no change in .text
> > or .data size. When enabled, .text += 3248 bytes; .data += 2148 bytes.
> >
> > This should be safe in every case; the only drawback is the extra code and
> > data when CPU hotplug is enabled.
>
> Especially as for the bulk of them, those CPUs aren't hotplug capable.
> (I seriously doubt we'll ever see a hotplugable cyrix for eg, which
> takes up the bulk of your diff).
>
> How about leaving it __init on non-hotplug systems, and somehow removing
> those from cpu_devs, so get_cpu_vendor() just skips them ?
> NULL'ing those entries should be just a few bytes, instead of adding 5KB.
We use cpu hotplug system for swsusp; but unless someone makes
cyrix/SMP machine and tries to suspend it, we are ok.
Pavel
--
Thanks, Sharp!
On Thu, Feb 02, 2006 at 10:34:50PM +0100, Pavel Machek wrote:
> Hi!
>
> > > When CONFIG_HOTPLUG_CPU on i386 there are places where __init[data] is
> > > referenced from normal code.
> > >
> > > On startup:
> > > arch/i386/kernel/cpu/amd.c::amd_init_cpu():
> > > cpu_devs[X86_VENDOR_AMD] = &amd_cpu_dev;
> > > amd_cpu_dev is declared __initdata and is freed
> > >
> > > On CPU hotplug:
> > > arch/i386/kernel/cpu/common.c::get_cpu_vendor():
> > > for (i = 0; i < X86_VENDOR_NUM; i++) {
> > > if (cpu_devs[i]) {
> > > if (!strcmp(v,cpu_devs[i]->c_ident[0]) ||
> > >
> > > To fix this, change every instance of __init that seems suspicious
> > > into __cpuinit. When !CONFIG_HOTPLUG_CPU there is no change in .text
> > > or .data size. When enabled, .text += 3248 bytes; .data += 2148 bytes.
> > >
> > > This should be safe in every case; the only drawback is the extra code and
> > > data when CPU hotplug is enabled.
> >
> > Especially as for the bulk of them, those CPUs aren't hotplug capable.
> > (I seriously doubt we'll ever see a hotplugable cyrix for eg, which
> > takes up the bulk of your diff).
> >
> > How about leaving it __init on non-hotplug systems, and somehow removing
> > those from cpu_devs, so get_cpu_vendor() just skips them ?
> > NULL'ing those entries should be just a few bytes, instead of adding 5KB.
>
> We use cpu hotplug system for swsusp; but unless someone makes
> cyrix/SMP machine and tries to suspend it, we are ok.
As Alan mentioned, there were never any production SMP cyrix machines,
and the prototypes never worked on Linux anyway. With Cyrix no longer
around this code is safe to assume 'UP only'.
Dave